Skip to content

Fix behaviour when the destination file is deleted #21

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

Merged
merged 2 commits into from
Nov 19, 2015
Merged

Fix behaviour when the destination file is deleted #21

merged 2 commits into from
Nov 19, 2015

Conversation

purbon
Copy link

@purbon purbon commented Nov 17, 2015

Context

As commented out in #20, if during the time LS is running the destination file is deleted by another agent in the machine, the plugin takes the event without any kind of feedback. This makes the plugin behaviour awkward and confusing, including data loss.

Reason

This happen because the plugin caches the file objects without doing any maintenance in case the file got deleted. There is a check for something similar at close_stale_files function, but this might not make it on time.

Fix

Add a new variable (create_if_deleted) to control the behaviour and make the open function react to it. Also include an option to forward events to the failure file in case this new variable is set to false. This enhancements make feedback always there, either by adding the files again to the destination file or using the failure file, there is also logging added.

Fixes #20

…th this change the deleted file can either be recreated or the events appended to the errorlog

private
def open(path)
if !deleted?(path) && cached?(path)
Copy link
Contributor

Choose a reason for hiding this comment

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

This introduces up to 2 File.stat calls per event sent to this output where previously there were zero File.stat calls per event.

I wonder about the performance implications of this.

Maybe we should only bother checking if the file is deleted if @create_if_deleted is true, that way the performance penalty is isolated to those wishing to use this 'create-if-deleted' behavior?

Copy link
Author

Choose a reason for hiding this comment

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

this is a fair concern, however I added another "feature" here, if the file is deleted but @create_if_deleted is false, it appends the data to the failure file, like this there is no chance stuff is "lost". What do you think?

@jordansissel
Copy link
Contributor

Tests pass. Code LGTM. One comment about the performance impact of doing 2 stat syscalls per event.

This reverts commit e2ce341.
@purbon
Copy link
Author

purbon commented Nov 19, 2015

Amended my last commit, because this will make the situation where the error file is deleted not being recreated again, I think this might be a bug. Lets release the plugin like this and improve the situation if required.

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.

Behavior when output file is deleted while LS is running
3 participants