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.

Sign up for GitHub

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

Jump to bottom

Eliminate unnecessary delay with inotify events #477

Merged
merged 1 commit into from
Jan 12, 2019
Merged

Eliminate unnecessary delay with inotify events #477

merged 1 commit into from
Jan 12, 2019

Conversation

mbakiev
Copy link
Contributor

@mbakiev mbakiev commented Oct 15, 2018

Proposing a change to the way inotify events are processed to avoid (almost) completely all delays for the clients.

  1. As far as I know most Linux filesystems implement moves as atomic operations, so we should expect to always have IN_MOVE_FROM and IN_MOVE_TO events in the buffer. By doing the move operation matching on the event list from the buffer without going through the DelayQueue we can avoid introducing delays when IN_MOVE_FROM and IN_MOVE_TO are readily available.
  2. Do not add a delay for any non-move events. There is no matching to be done, so these events can be dispatched to clients immediately. Move events will stall the queue, so we don't end up with out of order events.

Sorry for resubmitting.

@mbakiev
Remove delay from non-move events
7efc123 
Only add delay to IN_MOVE_* events if they do not have a matching
event in the event buffer already. In practice, most filesystems
implement moves as atomic operations, so we don't expect to see
any delays in processing of inode events.
@mbakiev
Copy link
Contributor Author

mbakiev commented Oct 15, 2018

Related to #390

@BoboTiG
Copy link
Collaborator

BoboTiG commented Oct 21, 2018

Thanks @mbakiev!

This is good for me. I will test your PR next week in a real world product to ensure this does not perturb the current behavior elsewhere.

Thank you for the good patch :)

BoboTiG
BoboTiG approved these changes Oct 23, 2018
View reviewed changes
Copy link
Collaborator

@BoboTiG BoboTiG left a comment

Choose a reason for hiding this comment

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

LGTM 👍
Ping @danilobellini.

@BoboTiG
Copy link
Collaborator

BoboTiG commented Dec 22, 2018 •
edited
Loading

@danilobellini, I am going to merge it the next week or juste after. LMK if you are against :)

@BoboTiG BoboTiG merged commit 2b2091a into gorakhargosh:master Jan 12, 2019
@BoboTiG
Copy link
Collaborator

BoboTiG commented Jan 12, 2019

Thank you very much @mbakiev :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BoboTiG BoboTiG BoboTiG approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mbakiev @BoboTiG