Check if file has disappeared sooner in Log reader#13907
Check if file has disappeared sooner in Log reader#13907ycombinator merged 11 commits intoelastic:masterfrom ycombinator:close_removed+close_renamed_before_eof
Conversation
filebeat/input/log/log.go
Outdated
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.
filebeat/input/log/log.go
Outdated
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.
|
@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. |
…ttings are enabled
|
Failing CI job is unrelated to this PR. Merging. |
* 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.