You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
A storage-admission fix for transient catalog WAL bytes.
The guiding principle is performance first, with eventual consistency: managed v2 views may serve slightly stale results while reconciliation catches up.
Query planning: dropped lookarounds are replaced with an empty group. Without this, the surrounding pieces could fuse into a different repetition when the index query is relaxed.
-M 0 now means no column limit, matching ripgrep.
Explicit file arguments now bypass -g/-t/-T filters, matching ripgrep.
Shared daemons tolerate transient accept() failures with a bounded retry.
Index builder:index.bin and lookup.bin are fsynced before publication.
Scheduler stall: re-enqueued pending operations now count as progress, so RetrySchedule no longer backs off the scan and stalls attaches. Idle catalog ticks are paced.
Eventual serving: ready managed views keep answering from the last published snapshot while later edits are reconciled. Status distinguishes serving from ready.
Watcher churn: metadata-only events for unchanged files are filtered. watch_events counters are exposed in status.
Query cliffs: these are removed, large results are paged instead of failing, and JSON stats are fixed.
Cold readiness:
parallel base-generation build
larger verified-read proof batches
batched catalog commits
retry backoff and a housekeeping precheck
a Windows parent-handle gate of 8192
binary-extension files no longer split verified-read batches
4. Faster first attach (97079b3, fa6eefe)
On the first attachment of an eventual (managed v2) view, the pinned base generation is published provisionally right after the metadata walk. Paths whose content is not from the base are tombstoned.
The view reports serving: true, ready: false, and the scheduler then runs full content verification.
FileVersion evidence is recorded only when it was validated around an actual read, so provisional paths can never be skipped by later stat-based refreshes.
provisional_files is reported in reconcile stats.
Legacy v1 shared attach keeps its ready-on-attach contract and is never provisional.
Integration tests that search immediately after a first attach now wait for ready.
5. Storage admission reclaims the catalog WAL (c5792dd, a69d804)
Storage admission charges catalog WAL bytes, but the WAL is checkpointed only once it reaches a quarter of the metadata budget. A retryable namespace-storage-allocation refusal could therefore persist until unrelated writes happened to cross that threshold.
reserve now truncates the WAL once and re-evaluates before refusing.
The collection admission test derives its limit from the checkpointed footprint, so on every platform the retained stages alone close admission and their reclamation alone reopens it. The test previously failed intermittently on Ubuntu and Windows; on macOS its refusal depended on WAL bytes.
Validation
cargo fmt --check and cargo clippy --workspace --all-targets -D warnings are clean.
cargo test -p tgrep-core --lib --features managed-test-hooks: 487 passed.
cargo test -p tgrep-cli: all suites pass, including shared_daemon (64 passed, 1 ignored).
Python harness tests: 15 passed.
Chromium (Windows), earlier runs on this branch, before provisional attach. Host speed varied by about 4–5× between days.
Run
Base attach
WT1
WT2
Notes
Fast day
838.8 s
686.7 s
1647.6 s
—
Query run (slow day)
4095.6 s
2371.3 s
2883.7 s
Query p50 was equal for base and worktree views
Full Chromium validation on this branch with provisional attach (--worktrees 2 --dirty-files 16 --repeat 2 --query-limit 12). The host was fast, comparable with the fast-day run above. All parity, backend, generation-reuse and overlay-isolation checks passed, and every timed query completed.
View
Attach to serving
Attach to ready
Fast-day attach to ready
Base (cold generation build)
412.8 s
834.7 s
838.8 s
Worktree 1
37.0 s
744.5 s
686.7 s
Worktree 2 (16 dirty)
50.4 s
1,061.6 s
1,647.6 s
Query p50 sums over 12 queries were equal across the base and worktree views: about 79 s client and 53 s server. Details are in SHARED_INDEX_BENCHMARKS.md.
Review findings not fixed in this PR
These came up in the review and are left for follow-ups:
- query: replace dropped lookarounds with an empty group so neighbours
cannot fuse into a different repetition during index relaxation
- cli: -M 0 means no column limit, matching ripgrep
- cli: explicit file arguments bypass -g/-t/-T filters, matching ripgrep
- shared daemons: tolerate transient accept() failures with bounded retry
- builder: fsync index.bin/lookup.bin before publication
- scripts: benchmark_worktrees.py measures ordinary index, cold managed
base build, linked worktree attach, and base vs worktree query times
- ci: manual Chromium multi-worktree benchmark workflow
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
An eventual (stale_reads) view's first attachment now publishes the pinned
generation after the metadata walk, tombstoning paths whose content is not
from the base. The view reports serving:true, ready:false and the scheduler
runs full content verification before it becomes ready. FileVersion evidence
is recorded only when validated around an actual read.
The Chromium harness records time-to-serving separately from time-to-ready
and reports verification counters from the reconcile that made the view ready.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Legacy v1 attachments keep their ready-on-attach contract. Managed integration tests that search immediately after a first attach now wait for the provisional view to finish content verification.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Rename the readiness helper to verified_attach so it no longer collides with local 'attached' bindings in managed-test-hooks modules, and make the remaining hook-gated tests wait for (or assert) provisional first-attach verification.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Admit results in path order to prevent prefix budget starvation
tgrep-cli/src/serve/shared/server.rs:1423
Later sibling groups can claim the shared response budget before earlier groups finish. The assembler keeps only the path-ordered prefix, so it discards those later results when an earlier group cannot fit. Under this completion order, a page can return just the first file via the serial retry while repeatedly reading and serializing later files on subsequent pages. Admit results against the page budget in path order, with bounded buffering, so later groups cannot consume the space needed by the prefix. Cover out-of-order completion in a paging regression test.
Drain prefetch before serial extraction to avoid memory exhaustion
tgrep-core/src/generations/mod.rs:1193
A one-blob batch is extracted serially, leaving results empty. The next batch's prefetch starts before that extraction, but retry is None, so this branch does not drain it. Retained prefetch results and readers can then exhaust the private reservation and make source.extract(...)? abort a build that would fit serially. Avoid overlapping prefetch with an unadmitted serial blob, or handle private-memory pressure from the serial call by stopping and draining prefetch before retrying. Add a regression with a one-blob batch followed by a parallel batch under a tight reservation.
Storage admission charges catalog WAL bytes, which are only checkpointed once the WAL reaches a quarter of the metadata budget. A retryable namespace-storage-allocation refusal could therefore persist until unrelated writes crossed that threshold. Truncate the WAL once and re-evaluate before refusing, and include the usage breakdown in the collection test's admission panic.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
If a worktree attachment fails or times out, managed_phase has already appended its entry but has not assigned attach. main catches that error and saves the partial JSON report, but this lookup then raises KeyError('attach'), preventing the Markdown report and intended exit code 2. Handle unfinished attachments here; the replacement below renders that partial-report case successfully.
Complete publication checks before swapping live state
tgrep-core/src/worktrees.rs:547
Once a view has published successfully, this gate also permits queries after a failed refresh. publish_prepared replaces the live index, evidence and visibility before calling the fallible measure_overlay_cost and final permit check (lines 1137–1147). Cancellation or deadline expiry there leaves the replacement visible under the previous published epoch, potentially with the previous overlay cost. Marking the view not ready no longer hides it. Prepare the replacement and its cost and complete all fallible checks before swapping the state and published epoch, so a failed pass keeps serving the previous publication.
Admission now reclaims catalog WAL frames before refusing, so the macOS layout's expected refusal can no longer rely on WAL bytes. Size the storage limit from the truncated catalog footprint so retained stages alone close admission and their reclamation alone reopens it on every platform.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
A concurrent scheduler pass can accept a reconciliation while the
testing.reconcile-timer hook is between invalidating and recording its
own pending operation. The duplicate then stays queued or deferred after
the view becomes ready, so the next timer call returned early without
invalidating. The hook now waits for the view's running and queued work
to settle before evaluating the injected elapsed time.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve work counters across pressure fallback and discarded results
tgrep-core/src/generations/mod.rs:1208
These counters now measure consumed results, not actual blob reads and extractions. When private-memory pressure triggers the serial fallback, results.clear() and the discarded prefetch results can drop completed work that is then repeated serially. Those discarded attempts never reach these increments, so successful builds underreport their work in attach telemetry, contrary to the documented BuildStats contract. Record reads, bytes and extraction calls where the work occurs, and retain those counts even when results are discarded. Update the pressure-fallback test to compare generated bytes separately from work counters.
…n replaced parent
- GroupSearch now checks an optional page deadline at every file boundary,
so a slow window ends the page with the searched prefix and a next_page
cursor instead of overrunning the operation deadline. The single-file
progress retry stays deadline-free so every page makes progress.
- When an eventual sibling batch fails final parent verification, the
group's rows are dropped, their response budget is refunded, and the
paths stay requeued, matching how vanished candidates are skipped.
- Extract a managed query test fixture and cover both behaviours.
- Limit the Chromium benchmark workflow token to contents: read.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Verbatim-prefix stripping is Windows-only: on Unix a name beginning with \\?\ is ordinary and must be shown as is. The Windows display tests are gated accordingly, with a Unix test for backslash names.
A server now removes serve.json on SIGTERM before exiting, while it still holds the inherited start.lock. The agent runtime waited for neither and fell back to a scan; it now waits briefly for the lock, or for a server to publish.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Cross-cutting consistency and persistence changes require human validation, including the unresolved post-commit query-admission issue.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Reject queries when publication fails after catalog commit
tgrep-core/src/managed/views.rs:2245
If a migration commits but fails before the in-memory swap, publish() retains the old publication, advances the slot epoch, and sets state.error. Eventual query admission now ignores that epoch and checks only status.serving, so a client requesting the old version can still query it after the catalog has committed the new version. Reject queries while state.error is set, without blocking ordinary freshness lag. Add a stale_reads: true regression test at ViewAfterCommit to cover this boundary.
…tion
On Windows the shared daemon holds a byte-range lock on .git/tgrep-daemon-v1.lock, so a --no-index --hidden scan reports it unreadable and exits 2 like ripgrep. Indexed queries never include .git, so the parity case excludes it explicitly.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Chromium multi-worktree validation (release build of this PR)
--worktrees 3 --worktree-rev HEAD --worktree-rev HEAD~1 --dirty-files 16 --repeat 2 --query-limit 12 --query-timeout 900 (the third worktree is at HEAD), same host,
release build of the provisional-attach sources. Heavy Rust test suites ran
on the same machine during verification and the timed query rounds, so treat
the later rows and the query table as a loaded-host measurement.
Step
Serving (s)
Ready (s)
Notes
Ordinary tgrep index / server
-
273.2 / 6.5
2.76 GiB index
Managed base (cold generation build)
398.1
908.7
4.49 GiB storage
Worktree 1 (HEAD)
32.1
781.8
generation reused, blobs_read=0
Worktree 2 (HEAD~1)
772.5
1,788.0
new generation (731.8 s build)
Worktree 3 (HEAD, 16 dirty, 1 untracked)
93.2
4,577.7
reused; 17-file private overlay
git worktree add took 444-454 s per worktree; storage after all attachments
was 8.99 GiB. Worktree 3 served after 93 s but needed 4,409 s of content
verification, with 7,219 s of namespace catalog time. Its verification
competed with the test load and with the periodic full verification of the
three views already ready: each rereads about 466,000 files every --poll-interval, about two cores of steady daemon work at this scale.
Parity, generation-reuse and overlay-isolation checks passed. One ordinary
round-0 query hit a connect timeout (os error 10060) under that load and
fell back to the on-disk index, so the backend check failed.
p50 (ms)
Ordinary base
Managed base
Worktree 1
Worktree 2
Worktree 3
blink::Document (498 matches), client
484.1
2,843.6
2,979.2
2,730.1
2,834.3
blink::Document, server
188
593
582
579
600
content::RenderFrameHost (10,694), client
1,110.5
4,197.5
4,429.2
4,405.3
5,061.8
Sum of 12 queries, client
78,843.5
86,567.0
87,766.5
89,648.0
90,503.4
Sum of 12 queries, server
68,817.5
55,406.9
55,630.6
58,478.1
57,383.2
Worktree views again match the managed base. Against the ordinary server,
managed views add about 2.2 s of client time per query beyond the server time
(ordinary: about 0.3 s), and selective queries spend 2-3x longer in the
server. Broad queries narrow that gap; total server time is lower for
managed views. The fixed managed client and per-query server costs are the
main remaining query-latency targets.
Full write-up: SHARED_INDEX_BENCHMARKS.md → Full validation for PR #211. The backend-check failure is the single 10060 connect timeout under host load; all parity, reuse and isolation checks passed.
The warm-admission operation spans the whole collection loop, which can exceed the fixture's 30 s operation_timeout_ms under a parallel test run and fail reserve with operation-deadline.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Cross-layer publication, pagination, and storage changes require human validation, with a publication-recovery issue still unresolved.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Reject queries after commit when publication recovery fails
tgrep-core/src/managed/views.rs:2243
If a migration commits but fails before the in-memory publication swap (for example, at ViewAfterCommit), publish leaves the old state.current and sets state.error. That old view still reports serving, so this branch accepts a new query for version 1 even though the authoritative pin is already version 2. Allowing stale filesystem results must not bypass publication recovery. Reject queries while state.error is set, and add an eventual-view regression test for this commit boundary.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Coupled publication, paging, and storage changes require human validation, and a JSON source-byte fidelity issue remains unresolved.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Preserve bare trailing CR at EOF instead of converting it to LF
tgrep-cli/src/output.rs:974
For an unterminated final line such as needle\r, JSON emits needle\n instead of preserving the source bytes. LineIndex::line_end strips the trailing CR even without an LF, so line_terminator_len returns 1 and this branch reconstructs it as LF. Invalid-UTF-8 lines have the same corruption in lines.bytes. Keep a bare trailing CR in the content (strip CR only when an LF was consumed), and add JSON regression cases for both valid and invalid UTF-8 at EOF.
A daemon catalog write can outlast the 250 ms SQLite writer wait, so a refusal probe may legally see retryable, uncommitted busy/catalog-io first. Rerun the exited helper with the same claim, as the client contract and retry_catalog_busy already do.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ontention
retained_checkpoint now waits for the eventual first attachment's verification and collects the superseded provisional checkpoint before the refresh, so the refresh retires the only collectible checkpoint. Previously a collection pass could reach member-before-io on the provisional checkpoint before skipping the reader-guarded target (reproduced deterministically by delaying the refresh).
The missing-evicted exact-commit test retries its final attach on documented retryable, not-committed catalog-io contention with a fresh token.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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
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.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Summary
This PR contains five pieces of work:
The guiding principle is performance first, with eventual consistency: managed v2 views may serve slightly stale results while reconciliation catches up.
Addresses #210.
1. Review fixes (
765d0f4)-M 0now means no column limit, matching ripgrep.-g/-t/-Tfilters, matching ripgrep.accept()failures with a bounded retry.index.binandlookup.binare fsynced before publication.2. Chromium multi-worktree benchmark (
765d0f4,9fce03c,97079b3)scripts/benchmark_worktrees.py, with tests inscripts/test_benchmark_worktrees.py, plus a manual CI workflow. It measures:Results are in
SHARED_INDEX_BENCHMARKS.md.3. Issue #210 (
d9bcc1f)RetryScheduleno longer backs off the scan and stalls attaches. Idle catalog ticks are paced.servingfromready.watch_eventscounters are exposed in status.4. Faster first attach (
97079b3,fa6eefe)serving: true, ready: false, and the scheduler then runs full content verification.FileVersionevidence is recorded only when it was validated around an actual read, so provisional paths can never be skipped by later stat-based refreshes.provisional_filesis reported in reconcile stats.shared attachkeeps its ready-on-attach contract and is never provisional.ready.5. Storage admission reclaims the catalog WAL (
c5792dd,a69d804)namespace-storage-allocationrefusal could therefore persist until unrelated writes happened to cross that threshold.reservenow truncates the WAL once and re-evaluates before refusing.Validation
cargo fmt --checkandcargo clippy --workspace --all-targets -D warningsare clean.cargo test -p tgrep-core --lib --features managed-test-hooks: 487 passed.cargo test -p tgrep-cli: all suites pass, includingshared_daemon(64 passed, 1 ignored).Python harness tests: 15 passed.
Chromium (Windows), earlier runs on this branch, before provisional attach. Host speed varied by about 4–5× between days.
Full Chromium validation on this branch with provisional attach (
--worktrees 2 --dirty-files 16 --repeat 2 --query-limit 12). The host was fast, comparable with the fast-day run above. All parity, backend, generation-reuse and overlay-isolation checks passed, and every timed query completed.Query p50 sums over 12 queries were equal across the base and worktree views: about 79 s client and 53 s server. Details are in
SHARED_INDEX_BENCHMARKS.md.Review findings not fixed in this PR
These came up in the review and are left for follow-ups:
serve.rspublication is not fsynced.\in paths is converted lossily on Unix..gitignorefiles.linkextension is ignored.serve.jsonis trusted.-0is ignored outside--files/-l.--trimstrips trailing whitespace.statushas no read timeout.