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

Fix for spurious rebuilds with watchdog>=2.3.0 #1117

Merged
merged 5 commits into from Apr 2, 2023

Conversation

dairiki
Copy link
Contributor

@dairiki dairiki commented Mar 10, 2023

In release 2.3.0, watchdog enabled tracking of inotify IN_OPEN events. These fire whenever a file is opened (even when opened for reading). This can happen for any number of reasons, many of which are outside of our control. Currently, this can lead to frequent spurious rebuilds in some cases.

The PR fixes things so as to ignore the new file-opened events from watchdog. Actually, here, we change strategy from ignoring events that we don't think we're interested in to specifically tracking only those types of events that we're interested in.

This PR also refactors our watchdog tests, to clean them up as well as to enable testing for the unwanted file-opened events.

(An alternative fix might be to adjust the event_mask passed to the Inotify observer constructor so as to exclude the IN_OPEN bit. Doing so would reduce the number of events processed by watchdog, and so be more efficient — but it takes a lot of special-casing code on our part to apply the proper modifications to just the Inotify observer — and only in certain versions of watchdog.)

Issue(s) Resolved

Related Issues / Links

Also see PR #1118 which builds on this one, further refactoring and cleaning up the Watcher code (while changing the API in a backward-incompatible manner.)

Description of Changes

  • Wrote at least one-line docstrings (for any new functions)
  • Added unit test(s) covering the changes (if testable)

@dairiki dairiki force-pushed the fix.watcher-fileopened-events branch 3 times, most recently from 2276354 to db85427 Compare March 13, 2023 03:36
Under macOS, the FSEventObserver (at least) seems to return events for
filesystem changes that happened a bit for the Observer was started.
We had a short wait in place to let these stale events go by. It
mostly worked, but there have been sporadic spurious failures on the
macOS GitHub test runner.  Here we increase the wait time, but — so as
not to slow tests on other platforms — only perform the wait when
running on macOS.
@dairiki dairiki force-pushed the fix.watcher-fileopened-events branch from db85427 to 9ced7ef Compare March 13, 2023 03:50
@dairiki dairiki marked this pull request as ready for review March 13, 2023 21:30
@dairiki dairiki added this to the 3.4 milestone Mar 14, 2023
@dairiki dairiki added the backport 3.3-maintenance PR should be backported label Mar 14, 2023
@dairiki dairiki merged commit 6267964 into master Apr 2, 2023
dairiki added a commit that referenced this pull request Apr 2, 2023
@dairiki dairiki deleted the fix.watcher-fileopened-events branch April 22, 2023 02:25
dairiki added a commit to dairiki/lektor that referenced this pull request Sep 11, 2023
* fix(watcher): ignore FileOpenedEvents

Since watchdog==2.3.0, its InotifyObserver emits events whenever a
file is opened. This was causing spurious Lektor rebuilds.

* refactor(watcher): optimization: we only care about file events

* fix(tests): increase wait for stale events on macOS

Under macOS, the FSEventObserver (at least) seems to return events for
filesystem changes that happened a bit for the Observer was started.
We had a short wait in place to let these stale events go by. It
mostly worked, but there have been sporadic spurious failures on the
macOS GitHub test runner.  Here we increase the wait time, but — so as
not to slow tests on other platforms — only perform the wait when
running on macOS.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 3.3-maintenance PR should be backported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant