loki.source.windowsevent: save bookmark every 10s #2558
Conversation
| @@ -226,5 +252,6 @@ func (t *Target) Stop() error { | |||
| close(t.done) | |||
| t.wg.Wait() | |||
| t.handler.Stop() | |||
| t.saveBookmarkPosition() | |||
There was a problem hiding this comment.
As we're triggering the <-t.done case in updateBookmark's loop already in the Stop function, should we keep the save behavior in that function and just put it in the t.done case before returning from the function? Or is there potential changes to the bookmark in the handler before handler.Stop() returns?
There was a problem hiding this comment.
makes sense, no potential changes because the "loop" goroutine is guaranteed to be stopped. I will make the change
There was a problem hiding this comment.
ah wait actually no, because both goroutines (loop and updateBookmark) are stopped in parallel, so to get the last position you need to make sure that the loop goroutine exited
|
Looks reasonable, should we consider this a breaking change? Its a big change in functionality that I think is warranted but we may want a bigger callout. |
For the user there should not be any noticeable change because the position is guaranteed to be updated when alloy exits. I believe that the only thing noticeable for the user will be a massive CPU improvement. I sent a custom installer to community users who run it at scale (see #2539). |
|
The only risk is when Alloy does not exit properly (a panic?). Then you may repeat up to the last 10s of events that were processed and get some duplication |
|
That's reasonable then, ignore more statement :), we never guarantee signals on unexpected shutdowns. |
|
Tests at scale from community users confirm the CPU improvements without regression:
The conversation in the linked issue mentions an issue that is unrelated to this PR and which will be treated separately (#2539 (comment)) @dehaansa @mattdurham the PR is ready for another review |
PR Description
The investigation carried in #2539 shows that more than 60% of the CPU usage of this component is spent updating the bookmark file (position file). This is because it is done after every events.
With this PR, the position is updated every 10s via a goroutine instead, just like it is done in the loki.source.file component.
The position is also updated when the component exits.
PR Checklist