Repository navigation
fix: rotate polling sweep across source directories within per-tick budget - #1130
Open
rootsiemen wants to merge 1 commit into
Open
rootsiemen wants to merge 1 commit into
rootsiemen wants to merge 1 commit into
Conversation
…udget Each tick restarted at the first source dir, so a first dir that fills the per-tick budget on its own was re-scanned on every tick and later dirs starved: their parked iterators never got another turn and changes there were never detected. The sweep now resumes at the dir the previous tick stopped in and wraps around, resetting to the first dir only once every dir has been visited. Closes crazy-goat#1125
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
PollingMonitorWatcherrestarted every tick at the first source directory. When that directory filled the per-tick budget on its own, it was re-scanned on every tick, and a later directory starved: its parked iterator never got another turn, so a change in the later directory was never detected.Repro (from the issue)
Source dirs A and B, per-tick budget 2, two matching files in A and a newer matching file in B: after six polling ticks the change in B was never detected, while with B alone it was detected in one tick.
Fix
The sweep now visits source directories in rotated order, starting each tick at the directory the previous tick stopped in (
sweepStart), and wraps around. The start resets to the first directory only once every directory has been visited, andresetSweep()(on change detection) also resets it so the next tick starts a fresh sweep from the root of every dir. The per-tick budget is unchanged: every entry still counts, including the first one after a resume. The start is stored as a position into the source-dir list rather than a dir key, because the parent constructor filters out non-existent dirs and the keys may not be sequential. Single-directory behaviour is untouched (rotation over one dir is a no-op).Tests
PollingMonitorWatcherTest::testLaterSourceDirIsReachedWhenFirstDirFillsBudget: dirs A (2 files) + B (1 file), budget 2. Tick 1 leaves B parked; the sweep converges (all iterators cleared) within 3 ticks. Verified the test fails on the unfixed code (B stays parked forever) and passes with the fix.tests/PollingMonitorWatcherTest.php: 21 tests, OK (PHP 8.3, PHPUnit 10.5).tests/Reboot/FileMonitorWatcher/FileMonitorWatcherTest.php+tests/PollingMonitorWatcherE2ETest.php: 19 tests, OK (existing single-directory, resume, deletion and budget tests all pass).[Unreleased]/ Fixed.Note: the full
composer testdaemon run andbin/lint.sh(needs shellcheck/hadolint) were not run locally; maintainer CI covers them.Closes #1125