Filebeat: Fix leak in log harvester (#6797) #6829
Conversation
filebeat/input/log/harvester.go
Outdated
| @@ -52,6 +52,10 @@ var ( | |||
| ErrClosed = errors.New("reader closed") | |||
| ) | |||
|
|
|||
| type OutletFactory interface { | |||
There was a problem hiding this comment.
exported type OutletFactory should have comment or be unexported
filebeat/input/log/harvester.go
Outdated
| // OutletFactory provides an outlet for the harvester | ||
| type OutletFactory interface { | ||
| GetOutlet() channel.Outleter | ||
| } |
There was a problem hiding this comment.
This type can be changed to type OutletFactory func() channel.Outleter.
By just passing a function/closure, we don't need to add another public method to each Input type, but still get the lazy initialization we want.
|
The lazy init approach looks safe. All filebeat tests have been green as well. We only must be sure the factory is called only once. The SubOutlet created by the log file prospector must not be created multiple times from within the same harvester. I don't think that's the case. Buhaving a local function/closure, we can add a counter/check, checking the factory is indeed not being called twice. Alternatively remember the current SubOutlet. |
|
Could you add a Changelog? |
This patch reorganizes a little bit how the log harvester works, so that suboutlets are only created when the harvester is ready to use them (inside Run()), instead of being passed during constructor. This prevents a memory leak caused by some internal goroutines not stopping if the harvester Setup() fails, for example when files cannot be read. Fixes elastic#6797
|
@adriansr I think we forgot to backport it to 6.2 is that possible? |
|
@ph so it seems :/ |
This patch reorganizes a little bit how the log harvester works, so that suboutlets are only created when the harvester is ready to use them (inside Run()), instead of being passed during constructor.
This prevents a goroutine leak when the harvester's Setup() call fails and Run() is never invoked.
Fixes #6797