Check if file has disappeared sooner in Log reader #13907
Conversation
ycombinator
added
bug
Filebeat
filebeat/input/log/log.go
Outdated
| return nil | ||
| } | ||
|
|
||
| func (f *Log) checkFileErrors() error { |
There was a problem hiding this comment.
Unfortunately github doesn't allow me to comment in code.
With this change every single read is preceded by an extra syscall, to check the file state. Maybe we can reduce the check to be only run if CloseRemoved or CloseRenamed is set.
The CloseInactive check MUST NOT be executed here. This function is run before the next line is read. If the harvester was blocked by the outputs for some amount of time, we might trigger an ErrInactive here, although the file was still written to. The CloseInactive check is supposed to trigger only after we did reach EOF.
The truncate case is somewhat tricky. I think checking truncate after EOF should be ok, as you should get a return of 0bytes read + EOF if you read on a truncated file. The check (and any synchronous check alike) is always subject to false-negatives, if the output was blocked for such a long time that the file was allowed to grow past offset.
By moving truncate and CloseInactive checks back to errorCheck, we don't need to execute the stat syscall for everyone line we want to read if CloseRemoved or CloseRenamed are not set.
| @@ -111,16 +116,22 @@ func (f *Log) errorChecks(err error) error { | |||
| return err | |||
| } | |||
|
|
|||
| // At this point we have hit EOF! | |||
There was a problem hiding this comment.
Consider adding this detail to the godoc: errorChecks determines the cause for EOF errors, and how the EOF event should be handled based on the config options.
ycombinator
force-pushed
the
close_removed+close_renamed_before_eof
branch
from
92bf942
to
5b4505e
Compare
|
@urso Thanks for the review. I've addressed your feedback and this PR is ready for another round of review, when you get a chance. |
ycombinator
changed the title
ycombinator
force-pushed
the
close_removed+close_renamed_before_eof
branch
from
44921d6
to
1fefbe4
Compare
…ttings are enabled
ycombinator
force-pushed
the
close_removed+close_renamed_before_eof
branch
from
1fefbe4
to
71804f0
Compare
|
Failing CI job is unrelated to this PR. Merging. |
ycombinator
removed
the
needs_backport
* Adding comment about EOF and removing redundant logic * Adding debug logging when close_remove and close_renamed situations are reached * Check file stats related errors before adding to buffer * Reordering checks to be same as before * Add CHANGELOG entry * Fixing issue # in CHANGELOG entry * Move CloseInactive and truncate checks back to being after EOF check * Don't perform stat call for CloseRemoved/CloseRenamed unless those settings are enabled * Better comments / function godoc * Renaming method to be more descriptive * Fixing up CHANGELOG
* Adding comment about EOF and removing redundant logic * Adding debug logging when close_remove and close_renamed situations are reached * Check file stats related errors before adding to buffer * Reordering checks to be same as before * Add CHANGELOG entry * Fixing issue # in CHANGELOG entry * Move CloseInactive and truncate checks back to being after EOF check * Don't perform stat call for CloseRemoved/CloseRenamed unless those settings are enabled * Better comments / function godoc * Renaming method to be more descriptive * Fixing up CHANGELOG
* Adding comment about EOF and removing redundant logic * Adding debug logging when close_remove and close_renamed situations are reached * Check file stats related errors before adding to buffer * Reordering checks to be same as before * Add CHANGELOG entry * Fixing issue # in CHANGELOG entry * Move CloseInactive and truncate checks back to being after EOF check * Don't perform stat call for CloseRemoved/CloseRenamed unless those settings are enabled * Better comments / function godoc * Renaming method to be more descriptive * Fixing up CHANGELOG
…3960) * Check if file has disappeared sooner in Log reader (#13907) * Adding comment about EOF and removing redundant logic * Adding debug logging when close_remove and close_renamed situations are reached * Check file stats related errors before adding to buffer * Reordering checks to be same as before * Add CHANGELOG entry * Fixing issue # in CHANGELOG entry * Move CloseInactive and truncate checks back to being after EOF check * Don't perform stat call for CloseRemoved/CloseRenamed unless those settings are enabled * Better comments / function godoc * Renaming method to be more descriptive * Fixing up CHANGELOG * Fix CHANGELOG * Fixed compile error * [WIP] Trying to check if file is removed * Adding Removed() to Source interface * Initialize procGetFileInformationByHandleEx * Adding missed import
…lastic#13959) * Adding comment about EOF and removing redundant logic * Adding debug logging when close_remove and close_renamed situations are reached * Check file stats related errors before adding to buffer * Reordering checks to be same as before * Add CHANGELOG entry * Fixing issue # in CHANGELOG entry * Move CloseInactive and truncate checks back to being after EOF check * Don't perform stat call for CloseRemoved/CloseRenamed unless those settings are enabled * Better comments / function godoc * Renaming method to be more descriptive * Fixing up CHANGELOG
Resolves #13889.
This PR teach the Log reader to check file stat related changes, e.g. a file being moved or renamed, more aggressively than before. As a result a slow or flaky output can no longer hold up these checks from being performed.