Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Antivirus workers #10383

Merged
merged 4 commits into from
Oct 23, 2024
Merged

Antivirus workers #10383

merged 4 commits into from
Oct 23, 2024

Conversation

butonic
Copy link
Member

@butonic butonic commented Oct 22, 2024

We made the number of go routines that pull events from the queue configurable.

related: #10259

@kobergj I set the default to 10 to make configuring the amount of required memory easier. In kubernetes we can then set the memory request lower than ANTIVIRUS_MAX_SCAN_SIZE (memory limit to ANTIVIRUS_WORKERS*ANTIVIRUS_MAX_SCAN_SIZE)

@butonic butonic self-assigned this Oct 22, 2024
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@mmattel
Copy link
Contributor

mmattel commented Oct 22, 2024

Will this go into 7.0 ?


Service Service `yaml:"-"`

Tracing *Tracing `yaml:"tracing"`

InfectedFileHandling string `yaml:"infected-file-handling" env:"ANTIVIRUS_INFECTED_FILE_HANDLING" desc:"Defines the behaviour when a virus has been found. Supported options are: 'delete', 'continue' and 'abort '. Delete will delete the file. Continue will mark the file as infected but continues further processing. Abort will keep the file in the uploads folder for further admin inspection and will not move it to its final destination." introductionVersion:"pre5.0"`
Events Events
Scanner Scanner
MaxScanSize string `yaml:"max-scan-size" env:"ANTIVIRUS_MAX_SCAN_SIZE" desc:"The maximum scan size the virus scanner can handle. Only this many bytes of a file will be scanned. 0 means unlimited and is the default. Usable common abbreviations: [KB, KiB, MB, MiB, GB, GiB, TB, TiB, PB, PiB, EB, EiB], example: 2GB." introductionVersion:"pre5.0"`
Workers int `yaml:"workers" env:"ANTIVIRUS_WORKERS" desc:"The number of concurrent go routines that fetch events from the event queue." introductionVersion:"6.7"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we build a 6.7, this is ok,
else we need to set this to 7.0.0
or %%NEXT%% if that goes into a release after 7.0

wg.Add(1)
go func() {
defer wg.Done()
for e := range ch {
Copy link
Contributor

@2403905 2403905 Oct 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This loop never breaks because we are not closing a channel ch

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be like this, but graceful shutdown anyway not working

	wg := sync.WaitGroup{}
	for i := 0; i < av.c.Workers; i++ {
		wg.Add(1)
		go func() {
			defer wg.Done()
			for {
				select {
				case e, ok := <-ch:
					if !ok {
						return
					}
					err := av.processEvent(e, natsStream)
					if err != nil {
						switch {
						case errors.Is(err, ErrFatal):
							av.l.Fatal().Err(err).Msg("fatal error - exiting")
						case errors.Is(err, ErrEvent):
							av.l.Error().Err(err).Msg("continuing")
						default:
							av.l.Fatal().Err(err).Msg("unknown error - exiting")
						}
					}
				case <-av.c.Context.Done():
					av.l.Debug().Msg("antivirus worker stopped")
					return
				}
			}
		}()
	}
	wg.Wait()

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go for it as is. We can later rework for graceful shutdowns.

Copy link

@2403905 2403905 merged commit 1eef762 into master Oct 23, 2024
3 checks passed
ownclouders pushed a commit that referenced this pull request Oct 23, 2024
@mmattel mmattel deleted the antivirus-workers branch October 23, 2024 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants