Wait 500ms for the index instead of scanning 100k commits - #421
Merged
Conversation
The PreToolUse hook fires per edit, so an agent touching several files runs several `commitlore inject` processes at once and each one calls `ensureIndex`, which writes. No `busy_timeout` was set, and SQLite's default is 0: a contended connection fails immediately. Eight concurrent cold starts put four of them on the full-scan fallback. What was never wrong is worth stating, because it is why this is a cost bug and not a correctness one. All eight payloads were byte-identical to a serial warm-index run, and `doctor` reported the index healthy afterwards -- the `disk I/O error` in that sample was transient contention, not damage. The fallback is the fail-safe design working. The cost is the problem. At 100,000 commits an indexed `context` is 496ms p50 against 86,673ms for the scan, so losing the index to a lock that would have cleared in milliseconds costs eighty-six seconds inside a hook the agent is blocked on -- and prints an alarming line on a path whose contract is silence when there is nothing to say. 500ms is chosen to separate two cases rather than to be generous. An incremental update is milliseconds, so this absorbs it; a full rebuild on a large repository is seconds, and waiting one out inside the hook would be worse than scanning, so this expires first on purpose and the existing fallback answers. Readers get it too: WAL lets a reader and one writer run together, but a reader still meets SQLITE_BUSY while the writer checkpoints. Limit: a full rebuild on a large repository takes longer than any timeout a hook can afford to wait, so the scan fallback stays reachable by design Ruled-out: a longer timeout that waits out a rebuild | the agent is blocked on this hook, and at 100k commits the rebuild is the same order as the 86s scan it would be waiting to avoid Ruled-out: serialising injections behind a lock file | it makes every edit wait for the slowest neighbour, where SQLite already lets readers run concurrently under WAL and only writers contend Warn: the scan fallback is not dead code — it answers whenever the timeout expires, and its output must stay byte-identical to the indexed path Blast: module Undo: easy Certainty: firm Verified: eight concurrent cold-start injections go from four fallbacks to zero, with every payload equal to a serial warm-index reference; the regression test was re-run with the pragma removed and fails there with four fallbacks; index-perf gate and full suite green Provenance: authored Record-Id: r-busy420
CommitLore — record lintTrailers: clean — 1 commit in Active constraints for the paths this PR touchesLimits (93)
Ruled out (215)
Warnings (52)
Truncated: 41 lines omitted — the comment hit GitHub's 65000 character limit. Trailer violations fail this check. Active constraints are informational — they are what the repository already decided, not a verdict on this PR. |
This was referenced Aug 7, 2026
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 #420.
What happens
The PreToolUse hook fires per edit, so an agent touching several files runs several
commitlore injectprocesses at once, and each callsensureIndex— which writes. Nobusy_timeoutwas set, and SQLite's default is 0: a contended connection fails immediately.Eight concurrent cold starts on a 40-commit repository:
What was never wrong
This is a cost bug, not a correctness one, and the difference is worth stating:
doctorafterwards:ok — 160 trailers over 40 commits, current with HEAD. Thedisk I/O errorwas transient contention, not damage.The scan fallback is the fail-safe design working.
Why the cost is not small
docs/evidence.md: at 100,000 commits, indexedcontextis 496 ms p50 and the--no-indexfallback is 86,673 ms. Losing the index to a lock that would have cleared in milliseconds costs eighty-six seconds inside a hook the agent is blocked on — and prints an alarming line on a path whose contract is silence when there is nothing to say.The number, and why it is that number
PRAGMA busy_timeout = 500, on readers too — WAL lets a reader and one writer run together, but a reader still meetsSQLITE_BUSYwhile the writer checkpoints.500 ms separates two cases rather than being generous:
Result
Fallbacks go from 4/8 to 0/8, with every payload still equal to the serial reference.
The test can fail
Removing the pragma and re-running:
It asserts two things that break for different reasons: that concurrent answers equal a serial warm-index reference (true before this change too, and kept so a future locking change cannot trade the answer for speed), and that no run falls back (what this change fixes).
Verification
test/index-concurrency.test.ts— 2 cases, red without the pragma.test/index-perf.test.ts(the PRD-F2 100k-commit gate) green.test/dogfood.test.tsre-run after committing: 9 passed.dist/committed,npm run typecheckclean.