dmtrKovalenko / dmtrKovalenko/fff

[Bug]: tons of full rescans while tracked repo files are barely changed

Open
#690 2 comments 0 reactions 0 assignees View on GitHub
bug triaged
Dominant language
Rust
Stars
10.7k
Forks
446
Avg merge
1d 21h
Merged PRs (30d)
39

Description

Continuation of https://github.com/dmtrKovalenko/fff/issues/616

The issue is still present, so I've ran another deeper investigation. Here is the full report (kudos to gpt-5.6-sol-xhigh):

# macOS watcher rescan storms: investigation and fix direction

## Scope

This report investigates two warnings observed when `fff-search` watches an
active Rust workspace on macOS:

```text
Received rescan event for paths [".../.git"], triggering full rescan
Too many affected paths in a single batch, triggering full rescan
```

The findings were checked against:

- `fff` current `main` at `073698c` (`fff-search` 0.10.0).
- `fff-search` 0.9.6, which shows the same core behavior.
- Resolved watcher dependencies `notify` 9.0.0-rc.4 and
`fff-notify-debouncer-full` 0.9.4.
- A real workload with timestamp-correlated watcher logs and filesystem
metadata.
- Focused unit reproductions against current `main`.

## Executive summary

There are three separate problems.

1. **Ignored paths count toward an index-overflow limit.** On macOS, fff
recursively receives events from ignored trees such as `target/`. The
handler filters those paths out of the index, but then counts the original
raw paths against `MAX_OVERFLOW_FILES = 1024`. A batch of 1,025 ignored
events therefore triggers a full rescan even though it requires zero index
additions.

2. **The unconditional recursive strategy exposes fff to avoidable event
storms on macOS (and currently Windows).** An earlier adaptive macOS
strategy used non-recursive watches for indexed directories below a
directory-count threshold and recursive watching above it. PR #431 changed
macOS and Windows to always recursively watch the workspace root. This
means `.gitignore` affects indexing but does not reduce the event volume
delivered by the OS watcher.

3. **The `Flag::Rescan` bypass is a correctness bug.** For a small set of
non-directory, non-ignored paths, the handler responds to an OS
“events were lost” marker with a plain `break`. It neither rescans nor
processes later events in the same debounced batch. On Linux, an inotify
queue overflow has no paths, so the empty path list also takes this bypass
due to vacuous `all()`.

The `.git` path in the first warning is not evidence that `.git` caused the
event storm. fff registers `.git` as an overlapping watch root even when the
recursive base watch already covers it, and the debouncer retains only one
rescan marker. In the measured incident, thousands of `target/doc` writes and
the warnings occurred at the same timestamps, while sampled `.git` mtimes did
not show activity in that window.

## End-to-end event flow

```text
build / generator writes ignored files under the workspace
-> macOS recursive FSEvents stream receives those events
-> notify translates raw flags to Events
-> fff-notify-debouncer-full batches for 50 ms
-> handle_debounced_events()
-> ignored paths are excluded from index mutations
-> raw path count still exceeds 1024
-> full rescan

At higher event rates:
-> FSEvents may report MustScanSubDirs (commonly with dropped-event flags)
-> notify emits EventKind::Other + Flag::Rescan
-> fff logs "Received rescan event..." and requests a full rescan
```

The two warnings are sibling consequences of the same event flood. The
user-space `> 1024` branch does not manufacture a later OS `Flag::Rescan`
event.

## Confirmed issue 1: ignored paths trigger the raw batch limit

The macOS watcher is recursive:

```rust
let use_recursive = cfg!(any(target_os = "macos", target_os = "windows"));

if use_recursive {
debouncer.watch(base_path.as_path(), RecursiveMode::Recursive)?;
}
```

Source:
`crates/fff-core/src/watcher/background_watcher.rs`, `BackgroundWatcher::new`
and `create_debouncer`.

The handler correctly filters ignored creates and modifications:

```rust
let (is_dir, is_ignored) = if is_removed {
(false, true)
} else {
(path.is_dir(), filter.is_ignored(path))
};

// ...
} else if !is_ignored {
paths_to_add_or_modify.push(path.as_path());
}
```

It then increments the limit with the unfiltered event paths:

```rust
affected_paths_count += debounced_event.event.paths.len();
if affected_paths_count > MAX_OVERFLOW_FILES {
warn!(
?affected_paths_count,
max = MAX_OVERFLOW_FILES,
"Too many affected paths in a single batch, triggering full rescan",
);
need_full_rescan = true;
break;
}
```

`MAX_OVERFLOW_FILES` is documented as storage reserved for files discovered
after the initial scan:

```rust
/// Capacity reserved for files the watcher discovers after the initial scan;
/// exceeding it forces a full rescan.
pub const MAX_OVERFLOW_FILES: usize = 1024;
```

Source: `crates/fff-core/src/constants.rs`.

Raw event count and index overflow capacity are different concepts:

- An ignored create consumes no index capacity.
- A modification of an existing indexed file consumes no new capacity.
- A removal consumes no overflow capacity.
- A `.git` event is skipped as an index entry.
- Duplicate paths are deduplicated only after the raw limit check.

The current guard can therefore force a rescan without protecting the
resource named by the constant.

### Focused reproduction

A temporary unit test was run against current `main`:

1. Initialize a git repository with `/target` in `.gitignore`.
2. Build a picker whose initial index excludes `target/`.
3. Construct one debounced batch containing 1,025 `Create(File)` events for
existing files under `target/`.
4. Call `handle_debounced_events`.
5. Observe a dispatched `WatchEventKind::Rescan`.

The test passed, confirming that 1,025 ignored paths trigger a full rescan.
The temporary test was removed after validation.

## Confirmed issue 2: macOS and Windows always watch ignored subtrees

Commit `1bcbce2` from PR #431 changed the strategy from adaptive to
unconditional recursion.

Before that commit:

```rust
let use_recursive =
cfg!(target_os = "macos") && watch_dirs.len() > MAX_MACOS_NONRECURSIVE_WATCHES;
```

Current behavior:

```rust
let use_recursive = cfg!(any(target_os = "macos", target_os = "windows"));
```

For a repository with at most 4,096 discovered indexed directories, the old
macOS strategy watched the base plus each discovered indexed directory
non-recursively. Ignored subtrees were not recursively observed. Windows also
used the non-recursive branch. The current strategy observes every descendant
on both platforms and relies on filtering after delivery.

This matters for build systems and package managers. A repository can have a
few hundred indexed source files while its ignored build directory contains
hundreds of thousands of files.

### The current FSEvents rationale is stale

The source comment says each non-recursive `watch()` call creates a separate
FSEvent stream. That is not how the resolved `notify` 9.0.0-rc.4 backend
currently behaves:

- `FsEventWatcher::watch_inner` stops the current stream, appends the new
path, and calls `run`.
- `run` creates one `FSEventStream` from the complete `self.paths` array.
- `update_paths` can apply many path operations with one stop/start cycle.

There may still be a practical limit or performance cost for a large
`pathsToWatch` array, so a recursive fallback remains reasonable. However,
“one stream per directory” is not a valid reason to force recursive watching
for a repository with only a small number of indexed directories.

## Confirmed issue 3: the rescan bypass can leave the index stale

Current code:

```rust
if debounced_event.event.need_rescan() {
if debounced_event.event.paths.len() < 16
&& debounced_event
.paths
.iter()
.all(|p| !p.is_dir() && !filter.is_ignored(p))
{
break;
}

warn!(/* ... */);
need_full_rescan = true;
break;
}
```

`need_rescan()` means the backend reports that individual events may have
been lost. The bypass:

- Does not set `need_full_rescan`.
- Does not reconcile the reported path.
- Exits the entire event loop.
- Drops every event later in the same debounced batch.

`fff-notify-debouncer-full` stores the rescan marker separately, then
chronologically sorts it together with expired normal events. A rescan marker
can therefore appear before or after ordinary create/modify events:

- If it appears first, the bypass skips later events.
- If it appears later, earlier changes may be applied, but the handler still
ignores the “events were lost” condition and can leave a partially updated
index.

On Linux, `notify` emits an inotify `Q_OVERFLOW` marker with no paths:

```rust
Event::new(EventKind::Other).set_flag(Flag::Rescan)
```

For an empty list, `len() < 16` is true and `Iterator::all` is vacuously true,
so the overflow is silently ignored and the rest of the batch is abandoned.

### Focused reproduction

A second temporary unit test was run against current `main`. It intentionally
exercises a valid rescan-first handler input rather than claiming this is the
only debouncer ordering:

1. Build a picker containing `existing.rs`.
2. Create `new.rs` after the scan.
3. Pass a batch containing:
- `Flag::Rescan` for `existing.rs`.
- `Create(File)` for `new.rs`.
4. Observe that `new.rs` is absent from the picker after
`handle_debounced_events` returns.

The test passed, confirming that the bypass discards following events without
recovering the index.

## Strongly supported issue 4: overlapping `.git` roots obscure attribution

fff always calls `watch_git_status_paths`, including after installing a
recursive base watch:

```rust
if use_recursive {
debouncer.watch(base_path.as_path(), RecursiveMode::Recursive)?;
}

watch_git_status_paths(&mut debouncer, git_workdir.as_ref());
```

That function adds `.git` and `.git/logs` as non-recursive roots. On macOS,
the recursive base already covers both paths.

With resolved `notify` 9.0.0-rc.4:

- All roots are passed to one FSEvent stream.
- The callback chooses the longest matching registered root.
- The debouncer has one `rescan_event` slot; a later rescan marker replaces
the earlier one.

This makes a surviving `.git` label possible even when the pressure that
caused event loss came from another root in the same stream. The existing
warning logs only `paths`; it omits `event.info()`, which could distinguish
`"rescan: user dropped"` from `"rescan: kernel dropped"`.

The exact FSEvents root-selection sequence cannot be reconstructed from the
existing logs. Nevertheless, the sampled mtimes are inconsistent with `.git`
being the high-volume source during the measured burst and strongly support
the root-attribution explanation.

## Runtime case study

The affected application used `fff-search` 0.9.6 with `watch: true` on a Rust
workspace:

- Normal project tree: about 163 files and 15 directories outside `.git` and
`target/`.
- Ignored `target/`: 17 GiB, about 140,858 files and 16,325 directories.
- Recorded between July 14 and July 17:
- 3,744 `Received rescan event .../.git` warnings.
- 43 `Too many affected paths` warnings.
- Every structured batch-overflow warning reported exactly
`affected_paths_count = 1025`, matching the strict `> 1024` branch.

One especially clear burst occurred at `2026-07-17T22:00:12Z`:

- 5,429 existing entries under `target/` had that second as their mtime.
- 3,146 more had `22:00:13`.
- 388 more had `22:00:14`.
- 8,719 of the 8,963 entries were rustdoc-generated `.js` files under
`target/doc`.
- No sampled `.git` entry had an mtime in this window; its next sampled writes
were at `22:01:28`.
- `.git` rescan warnings appeared at `22:00:12.254`,
`22:00:12.511`, `22:00:13.120`, and `22:00:23.417`.

This is a timestamp-level correlation between ignored build output and
rescan warnings whose displayed path was `.git`.

Static size alone is not the trigger. The trigger is high-rate creation,
replacement, and deletion inside a recursively watched ignored subtree.
Deleted temporary files and reused directory mtimes also mean a
post-incident mtime count underestimates the original event volume.

## Related API contract mismatch

The public watch documentation states:

```text
Gitignored and other ignored files are never triggering watcher.
```

This appears in `crates/fff-core/src/shared.rs` and is repeated by language
bindings.

Ignored files are excluded from normal subscriber events, but an ignored
event storm can trigger `watch_registry.dispatch_rescan(base_path)`, which is
broadcast to every subscription. The documented guarantee is therefore not
true for rescan events.

## What is not causing the warnings

- A normal low-volume `.git/index`, `HEAD`, or `logs/HEAD` change requests the
separate `GitStatusWorker`; it does not directly request a full filesystem
rescan. Raw `.git` paths still count toward the flawed 1,024-path guard, so
an unusually large git-internal batch can contribute to that branch.
- A user-space “Too many affected paths” rescan does not create a synthetic
`notify::Flag::Rescan`. Both warnings can follow the same underlying flood,
but there is no direct code path from one warning to the other.
- Full rescans do not reinstall the watcher
(`install_watcher: false`). Repeated warning lines do not imply an equal
number of concurrent scans: `trigger_full_rescan_async` coalesces requests
through `rescan_pending`.
- Application databases stored outside the watched workspace are unrelated
to this event stream.

## Recommended fix direction

### Phase 1: correctness and accounting

1. **Always recover from `Flag::Rescan`.**

Remove the `< 16` bypass. Until fff has a correct path-scoped subtree
reconciliation operation, any backend “events were lost” marker should
schedule a full rescan.

Minimal direction:

```rust
if debounced_event.event.need_rescan() {
warn!(
paths = ?debounced_event.event.paths,
info = ?debounced_event.event.info(),
"Filesystem events were lost; scheduling full rescan",
);
need_full_rescan = true;
break;
}
```

2. **Decouple raw batch size from overflow index capacity.**

Do not compare `sum(event.paths.len())` with `MAX_OVERFLOW_FILES`.
First classify and deduplicate paths, then reason about actual index
mutations.

In particular:

- Ignored creates/modifications should not count.
- Existing-file modifications should not count as new overflow files.
- Removes can be applied without consuming overflow capacity.
- `.git` events should not count.

A proactive capacity check should use the number of genuinely new,
indexable files and the remaining overflow capacity. The existing
`handle_create_or_modify(path).is_none()` and post-apply overflow check
already provide a final safety net.

If a separate CPU/memory guard for exceptionally large callback batches is
still needed, give it a separate name and threshold. It should be applied
after cheap ignore filtering and deduplication rather than reuse
`MAX_OVERFLOW_FILES`.

3. **Improve rescan observability and warning coalescing.**

Log:

- `event.info()` (`user dropped` versus `kernel dropped`).
- Raw path count.
- Ignored path count.
- Deduplicated actionable count.
- Whether a scan was started or only marked pending.

Avoid emitting one warning for every drop marker while a rescan is already
active or pending. This changes noise, not correctness.

### Phase 2: reduce ignored traffic at the OS watcher boundary

Restore an adaptive macOS strategy:

- When the discovered indexed-directory count is below a benchmarked
threshold, watch the base and each indexed directory non-recursively. The
old threshold was 4,096.
- Register the path set with one `update_paths` operation so `notify` does not
repeatedly stop and recreate the stream during initialization.
- Fall back to one recursive base root when the indexed directory count or
measured initialization cost exceeds a benchmarked threshold.
- Keep Windows recursive unless separately validated; its backend and failure
modes differ from FSEvents.

The previous threshold of 4,096 is a starting point, not necessarily the
correct final value. Benchmark stream initialization, event latency, and path
limits with representative repositories.

When using the recursive base strategy, do not additionally register `.git`
and `.git/logs` on macOS/Windows. The recursive root already delivers these
events, and the handler checks status-affecting git paths before skipping
`.git` entries. Keep the explicit git roots for non-recursive strategies.

An optional public watcher strategy (`auto`, `recursive`, `indexed-dirs`)
would also let SDK consumers choose the right trade-off for build-heavy
workspaces without disabling live updates entirely.

## Regression tests to add

1. **Ignored batch accounting**
- Git repo with `/target` ignored.
- Feed 1,025 create events under `target/`.
- Assert no full rescan and no index additions.

2. **Large legitimate modification batch**
- Feed more than 1,024 modifications of already indexed files.
- Assert incremental updates do not fail due to overflow-file capacity.

3. **Actual new-file overflow**
- Feed enough non-ignored new files to exhaust remaining overflow capacity.
- Assert exactly one rescan is scheduled.

4. **Rescan marker followed by ordinary event**
- Feed `[Flag::Rescan(existing_file), Create(new_file)]`.
- Assert a rescan is scheduled; never silently return with `new_file`
absent.

5. **Linux empty-path overflow**
- Feed `EventKind::Other + Flag::Rescan` with no paths.
- Assert a rescan is scheduled.

6. **macOS ignored-tree integration**
- Start a real watcher in a git repo with an ignored build directory.
- Generate a high-rate ignored burst.
- Assert ignored events do not cause the user-space 1,024 overflow branch.
- Record raw FSEvents drop information separately; OS event loss can still
require a rescan when recursive fallback is used.

7. **Watcher-root strategy**
- For a small macOS repo, assert registered roots are non-recursive indexed
directories plus required git roots.
- For a large repo above the selected threshold, assert one recursive root.

## Suggested implementation order

1. Remove the `Flag::Rescan` bypass and add its regression tests.
2. Replace raw path accounting with actionable/capacity accounting.
3. Add structured diagnostic fields and warning coalescing.
4. Gate explicit `.git` roots on the selected watcher strategy.
5. Restore and benchmark adaptive macOS root registration using
`update_paths`.
6. Update the public ignored-event guarantee to explicitly describe rescan
semantics.

The first two changes address correctness and false rescans without requiring
an immediate watcher architecture redesign. The adaptive macOS strategy then
prevents ignored build traffic from reaching the handler in the common
small-repository case and reduces the chance of FSEvents buffer loss.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in crates/fff-core/src/watcher/background_watcher.rs at BackgroundWatcher::new, create_debouncer, and handle_debounced_events; also read MAX_OVERFLOW_FILES in crates/fff-core/src/constants.rs. Reproduce the watcher batches described in the report, then verify ignored event floods do not cause unnecessary rescans and rescan markers do not discard later events.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
devtools, performance
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.