Fall back to polling when native watch capacity is exhausted by shengyfu · Pull Request #142 · microsoft/tgrep

Shengyu Fu (shengyfu) · 2026-09-08T01:53:44Z

Summary

Fixes #141.

This is a standalone branch from main at 10c73887b6326f395afdf2853188dd398ca9bdfc, not a continuation or stack of the earlier watcher PRs.

Default --watch-mode auto retains native notifications while complete coverage fits a conservative 8,192-subscription process budget. Budget preflight or native registration/constructor failures retire all of this server's native watches and enter sticky, whole-process polling. No repeated native registration attempts or partial native/polling coverage.
--watch-mode poll creates no native watcher. --poll-interval SECONDS defaults to 120 seconds after reconciliation completes; --watch-budget N controls the native ceiling. Explicit conflicting options are rejected. --no-watch remains the opt-out from all automatic refresh.
Polling reuses the authoritative ignore-aware metadata walk and streaming delta merge, catches up after setup/handoff/build/reload, and is not deferred by search activity. Registration snapshots remain serialized; busy work and failed startup attempts cannot create competing polling loops. Queue overflow retains native reconciliation recovery.
Full-resolution, read-bound file versions include Linux mtime/ctime and device/inode identity. Optional, validated v fields extend the existing filestamp JSON compatibly; the public FileStamp and index format remain unchanged. Unavailable/raced evidence fails open rather than acquiring a newer scan baseline. Existing content IDs, overlay deduplication and publication safeguards remain intact.
Status/logs expose requested/active modes, fallback reason, last successful reconciliation, duration, failure, running/pending/overdue state. Obsolete read failures do not permanently poison status.

Validation

Validated head: fed7dd58a57e6b30eee283dff80901f49c1370b4.

Windows local validation:

cargo fmt --all --check: passed.
cargo clippy --workspace --all-targets -- -D warnings: passed.
cargo build --workspace --all-targets: passed.
cargo test --workspace: 825 passed, 0 failed, 1 ignored (the opt-in measurement).
Explicit ripgrep_compat and ripgrep_parity targets: 218/218 and 102/102 passed.
Production scheduler/lifecycle regressions cover tiny budgets, constructor and partial-registration capacity failures, watcher retirement/sticky fallback, zero-watch polling, overflow separation, file/ignore changes, no-change persistence, busy/concurrent refreshes, failure recovery, and build/publication races.
Mutation controls: temporarily disabling precise polling classification or budget preflight each produced the expected regression failure; both mutations were restored and the passing suite rerun.

The existing platform CI run passed all build, bench-build, test, Format and Clippy jobs:

Platform	Passed	Ignored
Linux	857	1
macOS	852	1
Windows	825	1

Linux CI explicitly passed the real server's one-watch-budget fallback/bootstrap test, zero remaining native-watch assertions, serialized registration snapshot regression, and restored-mtime/same-second polling/read-race regressions. The Unix timestamp regressions also passed on macOS. Linux was exercised by CI, not locally; no host quota/sysctl was changed or exhausted.

A bounded Windows debug fixture of 1,000 small files measured 50.05 ms for a no-change poll (15 ms metadata walk) and 131.47 ms for a 20-file changed batch (16 ms metadata walk). This is not a measurement of the customer's 414k-file corpus, which is unavailable. Small deltas still stream a replacement of the on-disk index; unchanged polls do not rewrite it.

Operational limits

The budget is not free shared inotify capacity: same-user processes may exhaust the OS quota earlier. Polling cannot repair unreadable files or process-wide resource failures. Freshness includes the interval plus scan/update time and in-progress work; it is not a hard two-minute bound. Metadata-preserving changes beyond the available OS/filesystem evidence may still be missed, and reconciliation is not an atomic filesystem snapshot. README documents these semantics.

Add bounded native-first watching, explicit polling, serialized catch-up and read-bound precise metadata evidence for automatic refresh. Fixes #141 Co-authored-by: Copilot App <[email protected]> Copilot-Session: bcc85b55-e821-447e-b9a9-144b78829ae4

Copilot

Copilot review overview

🟡 Changes recommended

The subscription recovery path can miss same-size updates with unchanged or restored timestamps, leaving stale postings.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1

New issues introduced by this change (1)

Severity	Finding
	`tgrep-cli/src/serve.rs` — The precise version is only retained for `force` calls, but subscription recovery scans call…

What changed in this PR

Adds polling fallback when native watcher capacity is exhausted, with precise metadata reconciliation and operational status reporting.

Changes:

Adds configurable watch modes, polling intervals, and watch budgets.
Introduces read-bound file-version evidence.
Adds fallback lifecycle, status, documentation, and regression coverage.
Contains an unresolved recovery-scan correctness issue for same-size, same-timestamp updates.

File	Description
`tgrep-core/src/walker.rs`	Collects precise metadata during filesystem walks.
`tgrep-core/src/meta.rs`	Persists validated file-version evidence.
`tgrep-core/src/builder.rs`	Binds metadata evidence to indexed reads.
`tgrep-cli/tests/watcher_watch_registration.rs`	Tests polling and budget fallback end to end.
`tgrep-cli/src/status.rs`	Displays reconciliation health and timing.
`tgrep-cli/src/serve/poll_tests.rs`	Covers polling and fallback lifecycle behavior.
`tgrep-cli/src/serve.rs`	Implements polling and native fallback; recovery scans can retain stale postings until later reconciliation.
`tgrep-cli/src/main.rs`	Adds and validates refresh CLI options.
`README.md`	Documents refresh modes, limits, and status.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Compare trusted read-bound file versions during subscription recovery, verify changed and legacy reads before publication, and preserve unchanged-file and exact-content deduplication fast paths. Add regressions for timestamp collisions, legacy evidence, and read/publication races. Co-authored-by: Copilot App <[email protected]>

Copilot

Copilot review overview

🟡 Changes recommended

Three moderate reconciliation issues could cause redundant scans or full index rewrites.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 2

New issues introduced by this change (2)

Severity	Finding
	`tgrep-cli/src/serve.rs` — A successfully verified content-binary file still takes the branch below that calls…
	`tgrep-cli/src/serve.rs` — This read-bound version is discarded when decoding later classifies the file as binary, because…

Issues resolved since last review (1)

Severity	Finding
	`tgrep-cli/src/serve.rs` — The precise version is only retained for `force` calls, but subscription recovery scans call… View resolved comment

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

tgrep-cli/src/serve.rs:977

The startup refresh is launched independently of the polling scheduler. In poll mode (or when auto mode falls back before this thread runs), the periodic thread can consume catch_up before this call marks the reconciliation as running, so both threads execute full-tree reconciliations back-to-back. Mark the startup attempt as running before spawning it so the scheduler cannot claim the same bootstrap work; this avoids doubling startup I/O on the large repositories this fallback targets.

Keep validated versions when content-binary files move to filename-only membership, and carry binary classifications through resumed build batches and final evidence publication without adding postings. Cover unchanged reconciliation, content transitions, batching, flushes, and publication races. Co-authored-by: Copilot App <[email protected]>

Copilot

Copilot review overview

🔵 Needs a closer look

Startup and periodic polling need an atomic claim/recheck to prevent duplicate full-tree reconciliations.

Review tier: Balanced
Findings: None

Issues resolved since last review (2)

Severity	Finding
	`tgrep-cli/src/serve.rs` — This read-bound version is discarded when decoding later classifies the file as binary, because… View resolved comment
	`tgrep-cli/src/serve.rs` — A successfully verified content-binary file still takes the branch below that calls… View resolved comment

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

tgrep-cli/src/serve.rs:5358

running is not set until background_refresh_stale acquires stale_refresh_lock, after this status guard is released. At startup, the polling loop can consume catch_up and begin a scan while the independently spawned startup_refresh_stale queues another one (or vice versa), causing two full-tree reconciliations back-to-back on a large repository. Make startup and periodic polling share an atomic claim/recheck after acquiring the serialization lock so an already-satisfied initial poll is skipped.

Recheck the polling schedule under the stale refresh lock and retain the status guard through the shared scan claim. Skip already-handled startup work without resetting cadence or hiding failures, while preserving forced refreshes and new catch-up requests. Co-authored-by: Copilot App <[email protected]>

Copilot

Copilot review overview

🟡 Changes recommended

The critical memoized-failure handling issue can leave stale or missing indexed content indefinitely.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1

New issues introduced by this change (1)

Severity	Finding
	`tgrep-cli/src/serve.rs` — The native branch still suppresses memoized read failures using only `(mtime, size)` in…

Co-authored-by: Copilot App <[email protected]>

Copilot

Copilot review overview

🟡 Changes recommended

Resolve the SemVer break and avoid repeated full-index rewrites for unreadable files.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 2

New issues introduced by this change (1)

Severity	Finding
	`Cargo.toml` — This patch release adds public fields to `FileMeta`, `FileEvidence`, `BuildOutcome`, and…

Pre-existing issues (1)

Severity	Finding
	`tgrep-cli/src/serve.rs` — The native branch still suppresses memoized read failures using only `(mtime, size)` in… View comment

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

tgrep-cli/src/serve.rs:6440

Polling deliberately bypasses the unreadable-file memo here, so a permanently unreadable but still-present path becomes a candidate every interval. When the delta build rejects that sole candidate, stream_merge_stale_changes removes it from candidates but still calls merge_index_with_delta, which streams and republishes the entire reader unchanged (tgrep-core/src/builder.rs:1386-1459). On the reported 2.4 GB index this can cause a full rewrite every two minutes. Add a no-content-candidate path that preserves the unhealthy/retry state and updates filename membership if needed without rebuilding the content index.

Keep failed-read scan versions separate from indexed evidence and suppress native retries only when precise versions match. Leave unknown versions retryable and clear failure entries after successful auto-save delta reads. Co-authored-by: Copilot App <[email protected]>

Copilot

Copilot review overview

🔵 Needs a closer look

Required public struct fields break semver compatibility; preserve existing APIs or publish version 2.0.0 and update both lockfiles.

Review tier: Balanced
Findings: 1

Pre-existing issues (1)

Severity	Finding
	`Cargo.toml` — This patch release adds public fields to `FileMeta`, `FileEvidence`, `BuildOutcome`, and… View comment

Issues resolved since last review (1)

Severity	Finding
	`tgrep-cli/src/serve.rs` — The native branch still suppresses memoized read failures using only `(mtime, size)` in… View resolved comment

Suppressed comments (1)

Cargo.toml:6

1.0.5 is not semver-compatible with 1.0.4: this PR adds required public fields to constructible public structs (FileMeta::version, FileEvidence::versions, BuildOutcome::versions, and FileDeltaOutcome::versions). Downstream struct literals and exhaustive destructuring can therefore stop compiling even though Cargo may select this patch release for ^1.0.4. Preserve the existing public shapes via new APIs/types, or publish the breaking API as 2.0.0 and update both lockfiles.

version = "1.0.5"

Copilot AI balanced review requested due to automatic review settings September 8, 2026 01:53

Copilot started reviewing on behalf of Shengyu Fu (shengyfu) September 8, 2026 01:54 View session