Skip to content
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

Add close_removed and close_renamed, deprecate force_close_files #1909

Merged
merged 2 commits into from
Jun 28, 2016

Conversation

ruflin
Copy link
Contributor

@ruflin ruflin commented Jun 27, 2016

force_close_files is replaced by the two option close_removed and close_renamed. Force_close_files is deprecated. In case it is enabled, it sets close_removed and close_renamed to true.

This is part of #1600

force_close_files is replaced by the two option close_removed and close_renamed. Force_close_files is deprecated. In case it is enabled, it sets close_removed and close_renamed to true.

This is part of elastic#1600
# file is skipped, as the reading starts at the end. We recommend to leave this option on false
# but lower the ignore_older value to release files faster.
#force_close_files: false
# Close renamed means a file handler is close when a file is renamed / rotated. In case the harvester was
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'Close ensures a file handler is closed when the file is renamed or rotated'.

'Note: Potential data loss if renamed file is not picked up by prospector'.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added accordingly, and move the change to beat.yml :-)

if r.config.ForceClose {
// Check if the file name exists (see #93)
if r.config.CloseRenamed {
if !file.IsSameFile(r.fs.Name(), info) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's keep the code comment Check if the file name still exists.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be: check if file has been renamed, but still exists ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added Check if the file has been renamed. It does not check if the file still exists, it only checks if the file with the same name is this file or an other file. In case no file under the same name exists, it just assumes rename (it could also be removed). But that is checked in the next check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, changed it again to: // Check if the file can still be found under the same path

@ruflin
Copy link
Contributor Author

ruflin commented Jun 28, 2016

@urso review applied and new version pushed.

@urso urso merged commit 1a5be3c into elastic:master Jun 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants