Skip to content

fs: Coalesce queued rescans after watcher overflow - #60098

Merged
Anthony-Eid merged 1 commit into
mainfrom
fs-coalesce-rescan-batch
Jun 29, 2026
Merged

fs: Coalesce queued rescans after watcher overflow#60098
Anthony-Eid merged 1 commit into
mainfrom
fs-coalesce-rescan-batch

Conversation

@Anthony-Eid

@Anthony-Eid Anthony-Eid commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

After a filesystem watcher loses sync (e.g. a git pull that changes many files overflows the backend's event queue), the backend can enqueue many Rescan markers in quick succession. The dispatch thread processed each one separately, and each rescan invokes every registration for that watcher mode, so a single burst could kick off several full worktree scans at once — leading to sustained CPU and an unresponsive project panel.

This adds dispatch_batch, which handles the first event and drains the events already waiting in the channel, forwarding at most one Rescan per WatcherMode per drained batch while letting ordinary filesystem events and errors pass through untouched. A later watcher overflow can still trigger another recovery scan.

Reported with a reproduction and patch in #59610.

Release Notes:

  • Batch file watcher rescan events to improve Zed's responsiveness under heavy FS usage

@cla-bot cla-bot Bot added the cla-signed The user has signed the Contributor License Agreement label Jun 29, 2026
@zed-community-bot zed-community-bot Bot added the staff Pull requests authored by a current member of Zed staff label Jun 29, 2026
@Anthony-Eid
Anthony-Eid added this pull request to the merge queue Jun 29, 2026
Merged via the queue into main with commit f8e1ab7 Jun 29, 2026
49 checks passed
@Anthony-Eid
Anthony-Eid deleted the fs-coalesce-rescan-batch branch June 29, 2026 16:22
pull Bot pushed a commit to Jaleel-zhu/zed that referenced this pull request Jul 24, 2026
…ed-industries#61541)

# Objective

When the OS file watcher loses sync (e.g. its event queue overflows
under heavy fs churn), it drops pending events and reports a single
`Rescan` event for the watched root. Git changes hidden behind such a
rescan were silently lost, leaving the git panel and branch indicator
stale until something else touched `.git`.

Contributes to zed-industries#13176. May fix zed-industries#60102, though that report predates
zed-industries#60660 and may already be addressed by it on nightly (see Related PRs
below).

Two bugs caused this, and they masked each other:

1. **A rescan never triggered a git reload.** The `Rescan` event's path
is the worktree root, not something inside `.git`, so it never populated
`dot_git_abs_paths` in `process_events` — and since `.git` is excluded
from entry scanning, the rescan produced no `.git` events either. The
dropped git changes were simply never picked up.
2. **Re-scanning reset `git_dir_scan_id` to 0.** The snapshot diff
detects git changes by comparing scan ids, so re-inserting the
repository with a fresh id could wipe out a bump made earlier in the
same scan cycle, swallowing the corresponding `UpdatedGitRepositories`
signal.

The masking is why these fixes land together rather than as two PRs: the
root-rescan case in `test_dot_git_dir_event_does_not_suppress_children`
only passed on `main` because the buggy scan-id reset made the snapshot
diff fire spuriously. Fixing either bug alone turns that (currently
green) test red.

## Solution

- `process_events`: a `Rescan` event now schedules a git state reload
for every repository whose git directory (`dot_git`, `common_dir`, or
`repository_dir`) lies under the rescanned path, covering linked
worktrees and gitfile repositories, including git dirs watched outside
the worktree root.
- `insert_git_repository_for_path`: carry the existing `git_dir_scan_id`
forward when re-inserting a repository instead of resetting it to 0.
Deliberately *not* bumped either: re-insertion is snapshot bookkeeping,
not evidence of a git change — bumping would trigger spurious full
reloads on non-lossy paths (explicit refreshes, path-prefix scans). Only
`update_git_repositories` claims that git state changed.
- `changed_repos`: a `debug_assert` enforcing that `git_dir_scan_id`
never regresses, so future violations of this invariant fail loudly in
tests instead of manifesting as a stale git panel.

## Testing

New fault-injection infrastructure and tests:

- `FakeFs::simulate_watcher_overflow` models the kernel's watch queue
overflowing: buffered (undelivered) events are discarded and replaced by
a single `Rescan` for the given root, mirroring FSEvents
`kFSEventStreamEventFlagMustScanSubDirs`, inotify `IN_Q_OVERFLOW`, and
Windows `ERROR_NOTIFY_ENUM_DIR`.
- `test_watcher_overflow_rescan_reloads_git_state`: a git change whose
events are lost to an overflow must still be picked up via the rescan
(reproduces bug 1; fails on `main`).
- `test_git_update_in_same_batch_as_rescan_is_not_lost`: a git event
processed in the same batch as a rescan must not lose its scan-id bump
to the repository re-insertion (reproduces bug 2; fails on `main`).
- `test_random_git_updates_with_watcher_overflows`: randomized property
test (100 iterations) asserting that every git state change is
eventually signaled via `UpdatedGitRepositories` under random event
batching, delays, and overflows. Fails on `main` within the first few
seeds.
- `test_random_worktree_changes` now also injects watcher overflows,
extending its existing convergence property to rescan reconciliation of
worktree entries (this already passed; the injection guards it going
forward).

Full `worktree` and `fs` suites pass. Verified the directed tests
exercise the intended code paths via trace logging (the same-batch test
hits `update_git_repositories` stamping followed by re-insertion,
distinct from the overflow test where no `.git` event arrives at all).

## Related PRs

The recent stale-git-state reports trace back to three distinct
mechanisms that share one symptom. This PR addresses the third:

- **Events never generated** — zed-industries#60660 (merged, in this PR's base):
Linux's non-recursive watcher missed nested `refs/` directories, so
external commits/fetches produced no events at all. That PR (together
with zed-industries#60590, which explicitly rescans after Zed-initiated reset/fetch
and touches only `git_store.rs`) fixed zed-industries#60348. No overlap with this PR;
a merge against current `main` is clean, and the refs-watching tests are
disjoint from the overflow/rescan tests added here.
- **Events coalesced** — zed-industries#59876 (open, complementary): FSEvents can
merge `.git` child events into a bare `.git` `Changed` event; the signal
arrives, in a shape Zed ignores. @RemcoSmitsDev's review comment there
describes this PR's failure mode and calls the two fixes complementary;
this is effectively the follow-up promised in that comment. Both PRs
touch the same region of `process_events`, so whichever lands second
needs a trivial rebase, and zed-industries#59876 flips the bare-`.git` expectation in
`test_dot_git_dir_event_does_not_suppress_children` Case 2, which this
PR preserves.
- **Events dropped** — this PR: the watcher generated events but lost
them to a queue overflow, and the resulting `Rescan` did not reach the
git reload path. zed-industries#59976 and zed-industries#60098 (merged) reduced how often this
happens; this PR makes git state recover correctly when it does.

## Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content adheres to Zed's UI standards
([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
and
[icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md)
guidelines)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Release Notes:

- Fixed the git panel and branch indicator showing stale state after
heavy file-system activity caused the file watcher to lose events
0arm pushed a commit to 0arm/zed that referenced this pull request Jul 26, 2026
…ed-industries#61541)

# Objective

When the OS file watcher loses sync (e.g. its event queue overflows
under heavy fs churn), it drops pending events and reports a single
`Rescan` event for the watched root. Git changes hidden behind such a
rescan were silently lost, leaving the git panel and branch indicator
stale until something else touched `.git`.

Contributes to zed-industries#13176. May fix zed-industries#60102, though that report predates
zed-industries#60660 and may already be addressed by it on nightly (see Related PRs
below).

Two bugs caused this, and they masked each other:

1. **A rescan never triggered a git reload.** The `Rescan` event's path
is the worktree root, not something inside `.git`, so it never populated
`dot_git_abs_paths` in `process_events` — and since `.git` is excluded
from entry scanning, the rescan produced no `.git` events either. The
dropped git changes were simply never picked up.
2. **Re-scanning reset `git_dir_scan_id` to 0.** The snapshot diff
detects git changes by comparing scan ids, so re-inserting the
repository with a fresh id could wipe out a bump made earlier in the
same scan cycle, swallowing the corresponding `UpdatedGitRepositories`
signal.

The masking is why these fixes land together rather than as two PRs: the
root-rescan case in `test_dot_git_dir_event_does_not_suppress_children`
only passed on `main` because the buggy scan-id reset made the snapshot
diff fire spuriously. Fixing either bug alone turns that (currently
green) test red.

## Solution

- `process_events`: a `Rescan` event now schedules a git state reload
for every repository whose git directory (`dot_git`, `common_dir`, or
`repository_dir`) lies under the rescanned path, covering linked
worktrees and gitfile repositories, including git dirs watched outside
the worktree root.
- `insert_git_repository_for_path`: carry the existing `git_dir_scan_id`
forward when re-inserting a repository instead of resetting it to 0.
Deliberately *not* bumped either: re-insertion is snapshot bookkeeping,
not evidence of a git change — bumping would trigger spurious full
reloads on non-lossy paths (explicit refreshes, path-prefix scans). Only
`update_git_repositories` claims that git state changed.
- `changed_repos`: a `debug_assert` enforcing that `git_dir_scan_id`
never regresses, so future violations of this invariant fail loudly in
tests instead of manifesting as a stale git panel.

## Testing

New fault-injection infrastructure and tests:

- `FakeFs::simulate_watcher_overflow` models the kernel's watch queue
overflowing: buffered (undelivered) events are discarded and replaced by
a single `Rescan` for the given root, mirroring FSEvents
`kFSEventStreamEventFlagMustScanSubDirs`, inotify `IN_Q_OVERFLOW`, and
Windows `ERROR_NOTIFY_ENUM_DIR`.
- `test_watcher_overflow_rescan_reloads_git_state`: a git change whose
events are lost to an overflow must still be picked up via the rescan
(reproduces bug 1; fails on `main`).
- `test_git_update_in_same_batch_as_rescan_is_not_lost`: a git event
processed in the same batch as a rescan must not lose its scan-id bump
to the repository re-insertion (reproduces bug 2; fails on `main`).
- `test_random_git_updates_with_watcher_overflows`: randomized property
test (100 iterations) asserting that every git state change is
eventually signaled via `UpdatedGitRepositories` under random event
batching, delays, and overflows. Fails on `main` within the first few
seeds.
- `test_random_worktree_changes` now also injects watcher overflows,
extending its existing convergence property to rescan reconciliation of
worktree entries (this already passed; the injection guards it going
forward).

Full `worktree` and `fs` suites pass. Verified the directed tests
exercise the intended code paths via trace logging (the same-batch test
hits `update_git_repositories` stamping followed by re-insertion,
distinct from the overflow test where no `.git` event arrives at all).

## Related PRs

The recent stale-git-state reports trace back to three distinct
mechanisms that share one symptom. This PR addresses the third:

- **Events never generated** — zed-industries#60660 (merged, in this PR's base):
Linux's non-recursive watcher missed nested `refs/` directories, so
external commits/fetches produced no events at all. That PR (together
with zed-industries#60590, which explicitly rescans after Zed-initiated reset/fetch
and touches only `git_store.rs`) fixed zed-industries#60348. No overlap with this PR;
a merge against current `main` is clean, and the refs-watching tests are
disjoint from the overflow/rescan tests added here.
- **Events coalesced** — zed-industries#59876 (open, complementary): FSEvents can
merge `.git` child events into a bare `.git` `Changed` event; the signal
arrives, in a shape Zed ignores. @RemcoSmitsDev's review comment there
describes this PR's failure mode and calls the two fixes complementary;
this is effectively the follow-up promised in that comment. Both PRs
touch the same region of `process_events`, so whichever lands second
needs a trivial rebase, and zed-industries#59876 flips the bare-`.git` expectation in
`test_dot_git_dir_event_does_not_suppress_children` Case 2, which this
PR preserves.
- **Events dropped** — this PR: the watcher generated events but lost
them to a queue overflow, and the resulting `Rescan` did not reach the
git reload path. zed-industries#59976 and zed-industries#60098 (merged) reduced how often this
happens; this PR makes git state recover correctly when it does.

## Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content adheres to Zed's UI standards
([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
and
[icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md)
guidelines)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Release Notes:

- Fixed the git panel and branch indicator showing stale state after
heavy file-system activity caused the file watcher to lose events
jolutz pushed a commit to jolutz/zed that referenced this pull request Aug 8, 2026
)

After a filesystem watcher loses sync (e.g. a `git pull` that changes
many files overflows the backend's event queue), the backend can enqueue
many `Rescan` markers in quick succession. The dispatch thread processed
each one separately, and each rescan invokes every registration for that
watcher mode, so a single burst could kick off several full worktree
scans at once — leading to sustained CPU and an unresponsive project
panel.

This adds `dispatch_batch`, which handles the first event and drains the
events already waiting in the channel, forwarding at most one `Rescan`
per `WatcherMode` per drained batch while letting ordinary filesystem
events and errors pass through untouched. A later watcher overflow can
still trigger another recovery scan.

Reported with a reproduction and patch in zed-industries#59610.

Release Notes:

- Batch file watcher rescan events to improve Zed's responsiveness under
heavy FS usage
jolutz pushed a commit to jolutz/zed that referenced this pull request Aug 8, 2026
…ed-industries#61541)

# Objective

When the OS file watcher loses sync (e.g. its event queue overflows
under heavy fs churn), it drops pending events and reports a single
`Rescan` event for the watched root. Git changes hidden behind such a
rescan were silently lost, leaving the git panel and branch indicator
stale until something else touched `.git`.

Contributes to zed-industries#13176. May fix zed-industries#60102, though that report predates
zed-industries#60660 and may already be addressed by it on nightly (see Related PRs
below).

Two bugs caused this, and they masked each other:

1. **A rescan never triggered a git reload.** The `Rescan` event's path
is the worktree root, not something inside `.git`, so it never populated
`dot_git_abs_paths` in `process_events` — and since `.git` is excluded
from entry scanning, the rescan produced no `.git` events either. The
dropped git changes were simply never picked up.
2. **Re-scanning reset `git_dir_scan_id` to 0.** The snapshot diff
detects git changes by comparing scan ids, so re-inserting the
repository with a fresh id could wipe out a bump made earlier in the
same scan cycle, swallowing the corresponding `UpdatedGitRepositories`
signal.

The masking is why these fixes land together rather than as two PRs: the
root-rescan case in `test_dot_git_dir_event_does_not_suppress_children`
only passed on `main` because the buggy scan-id reset made the snapshot
diff fire spuriously. Fixing either bug alone turns that (currently
green) test red.

## Solution

- `process_events`: a `Rescan` event now schedules a git state reload
for every repository whose git directory (`dot_git`, `common_dir`, or
`repository_dir`) lies under the rescanned path, covering linked
worktrees and gitfile repositories, including git dirs watched outside
the worktree root.
- `insert_git_repository_for_path`: carry the existing `git_dir_scan_id`
forward when re-inserting a repository instead of resetting it to 0.
Deliberately *not* bumped either: re-insertion is snapshot bookkeeping,
not evidence of a git change — bumping would trigger spurious full
reloads on non-lossy paths (explicit refreshes, path-prefix scans). Only
`update_git_repositories` claims that git state changed.
- `changed_repos`: a `debug_assert` enforcing that `git_dir_scan_id`
never regresses, so future violations of this invariant fail loudly in
tests instead of manifesting as a stale git panel.

## Testing

New fault-injection infrastructure and tests:

- `FakeFs::simulate_watcher_overflow` models the kernel's watch queue
overflowing: buffered (undelivered) events are discarded and replaced by
a single `Rescan` for the given root, mirroring FSEvents
`kFSEventStreamEventFlagMustScanSubDirs`, inotify `IN_Q_OVERFLOW`, and
Windows `ERROR_NOTIFY_ENUM_DIR`.
- `test_watcher_overflow_rescan_reloads_git_state`: a git change whose
events are lost to an overflow must still be picked up via the rescan
(reproduces bug 1; fails on `main`).
- `test_git_update_in_same_batch_as_rescan_is_not_lost`: a git event
processed in the same batch as a rescan must not lose its scan-id bump
to the repository re-insertion (reproduces bug 2; fails on `main`).
- `test_random_git_updates_with_watcher_overflows`: randomized property
test (100 iterations) asserting that every git state change is
eventually signaled via `UpdatedGitRepositories` under random event
batching, delays, and overflows. Fails on `main` within the first few
seeds.
- `test_random_worktree_changes` now also injects watcher overflows,
extending its existing convergence property to rescan reconciliation of
worktree entries (this already passed; the injection guards it going
forward).

Full `worktree` and `fs` suites pass. Verified the directed tests
exercise the intended code paths via trace logging (the same-batch test
hits `update_git_repositories` stamping followed by re-insertion,
distinct from the overflow test where no `.git` event arrives at all).

## Related PRs

The recent stale-git-state reports trace back to three distinct
mechanisms that share one symptom. This PR addresses the third:

- **Events never generated** — zed-industries#60660 (merged, in this PR's base):
Linux's non-recursive watcher missed nested `refs/` directories, so
external commits/fetches produced no events at all. That PR (together
with zed-industries#60590, which explicitly rescans after Zed-initiated reset/fetch
and touches only `git_store.rs`) fixed zed-industries#60348. No overlap with this PR;
a merge against current `main` is clean, and the refs-watching tests are
disjoint from the overflow/rescan tests added here.
- **Events coalesced** — zed-industries#59876 (open, complementary): FSEvents can
merge `.git` child events into a bare `.git` `Changed` event; the signal
arrives, in a shape Zed ignores. @RemcoSmitsDev's review comment there
describes this PR's failure mode and calls the two fixes complementary;
this is effectively the follow-up promised in that comment. Both PRs
touch the same region of `process_events`, so whichever lands second
needs a trivial rebase, and zed-industries#59876 flips the bare-`.git` expectation in
`test_dot_git_dir_event_does_not_suppress_children` Case 2, which this
PR preserves.
- **Events dropped** — this PR: the watcher generated events but lost
them to a queue overflow, and the resulting `Rescan` did not reach the
git reload path. zed-industries#59976 and zed-industries#60098 (merged) reduced how often this
happens; this PR makes git state recover correctly when it does.

## Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content adheres to Zed's UI standards
([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
and
[icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md)
guidelines)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Release Notes:

- Fixed the git panel and branch indicator showing stale state after
heavy file-system activity caused the file watcher to lose events
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed The user has signed the Contributor License Agreement staff Pull requests authored by a current member of Zed staff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants