fix(watcher): stop ignored batches and small Flag::Rescan markers from breaking the index (#690)#693
Open
gustav-fff wants to merge 1 commit into
Open
fix(watcher): stop ignored batches and small Flag::Rescan markers from breaking the index (#690)#693gustav-fff wants to merge 1 commit into
gustav-fff wants to merge 1 commit into
Conversation
…m breaking the index (#690) Two independent correctness bugs in handle_debounced_events: 1. affected_paths_count summed raw event.paths.len() before ignore filtering. A burst of >1024 events under an ignored subtree (e.g. target/ churn from rustdoc/cargo build) forced a full rescan even though every event was filtered out. Now only actionable paths (non-ignored, non-.git, added to the remove/add/dir buckets) count toward the guard. 2. The `debounced_event.event.paths.len() < 16 && all(!is_dir && !is_ignored)` bypass for Flag::Rescan silently returned without scheduling a rescan and without processing subsequent events in the batch. On Linux, inotify Q_OVERFLOW arrives with no paths at all, making the `all()` vacuously true and dropping every overflow marker on the floor. Always schedule a full rescan on need_rescan() and log event.info() so `user dropped` vs `kernel dropped` is distinguishable. Refs #690
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.
Closes #690
Root cause
Two independent bugs in
handle_debounced_events(crates/fff-core/src/watcher/background_watcher.rs):affected_paths_countsummed rawdebounced_event.event.paths.len()atbackground_watcher.rs:457before ignore filtering. A burst of >1024 events under an ignored subtree (e.g.target/churn from rustdoc / cargo build on macOS FSEvents recursive stream) forced a full rescan even though every path was filtered out atbackground_watcher.rs:451-453.paths.len() < 16 && all(!is_dir && !is_ignored)bypass atbackground_watcher.rs:365-374silentlybreaked out of the event loop onFlag::Rescanwithout scheduling a rescan and without processing later events in the batch. On Linux, inotifyQ_OVERFLOWarrives with no paths at all, soall()was vacuously true and every overflow marker was dropped on the floor. On macOS, any batch of <16 small non-ignored paths hit the same bypass.Fix
paths_to_remove/dirs_to_remove/paths_to_add_or_modify) againstMAX_OVERFLOW_FILES. Ignored churn no longer trips the guard.need_rescan(). Logevent.info()alongside the paths souser droppedvskernel droppedis distinguishable.Architectural fix #2 from the issue (adaptive vs unconditional recursive FSEvents strategy) is intentionally out of scope — it needs benchmarking and a public strategy knob.
Steps to reproduce
Trigger 1 (ignored batch → false rescan):
```sh
Any recursively watched macOS repo with .gitignore containing /target
cargo doc --workspace # or any rustdoc generator producing many .js under target/doc
fff-search emits: "Too many affected paths in a single batch, triggering full rescan"
affected_paths_count = 1025, exactly matching the > 1024 branch,
even though every path is under /target and filtered out of the index.
```
Trigger 2 (silent
Flag::Rescanbypass): unit-testable directly. On pre-fixmain:```sh
cd crates/fff-core
Feed a batch containing a single non-ignored, non-dir path with Flag::Rescan set.
Expected: watch_registry dispatches a Rescan event.
Actual: handler
breaks out early; subscribers never see the rescan.```
Both triggers are covered by unit tests added in this PR:
ignored_batch_does_not_trigger_full_rescan— 2000Create(File)events under.gitignoredtarget/must not dispatch aRescan.rescan_flag_still_triggers_rescan_when_batch_is_small— a single-pathFlag::Rescanmust dispatch aRescan.empty_path_rescan_flag_triggers_rescan— the LinuxQ_OVERFLOWshape (empty path list) must dispatch aRescan.All three tests fail on
origin/main(bug 1 dispatches a spurious rescan; bugs 2/3 silently return with no dispatch).How verified
```sh
cargo test -p fff-search --lib background_watcher
5 passed; 0 failed — includes 3 new regression tests.
cargo test -p fff-search --lib
127 passed; 0 failed.
cargo test -p fff-search --test watch_subscription_test
11 passed; 0 failed.
cargo test -p fff-search --test new_directory_watcher_test
5 passed; 0 failed.
cargo fmt --package fff-search --check
clean
```
Automated triage via Gustav. Honk-Honk 🪿