-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
[tail/logparser] resume from last known offset when reloading #6074
Conversation
I'm going to merge the cleanup code from #6089 first, it's the same as you have here, since I need to cherry pick it to the 1.11.2 release. |
gotcha; i'll rebase |
* Fixes influxdata#3522 - Not able to read rotated log file without missing lines
for _, tailer := range t.tailers { | ||
if !t.Pipe && !t.FromBeginning { | ||
// store offset for resume | ||
offset, err := tailer.Tell() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it safe to call this after the tailer has Stopped?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, but if you stop first Tell returns an offset of 0.
when tailFileSync() finishes, it calls tail.close() which calls tail.closeFile() which sets tailer.file = nil, which causes Tell() to return an zero value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slightly unfortunate, but worse case we reprocess some lines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An improvement to the tail lib could resolve this
I think we just need to add in a spot where the package level offset map is cleared to prevent it from growing without bounds. We could place it in Start after the call to tailNewFiles. It probably also makes sense to clear the structs copy of the offset map there too, since we only need to restore offsets on the initial startup and wouldn't want to reapply them to files with the same name after they are rotated. |
Fixes Logparser plugin don't process new lines after telegraf configuration reload. #3573 - logparser/tail plugin stop after reload (removes unnecessary tailer.Cleanup() calls)Required for all PRs:
I did not write a unit test for this, but I did test externally:
telegraf.conf
genmetrics.sh
telegraf.out
telegraf.out (vanilla)
stops reading log after SIGHUP
telegraf.out (removing tailer.Close())
misses line that would have been
temperature=75