Stop verification-table slot collisions from answering FRESH for another key

[kriszyp]

⊙ Problem

The verification table can answer FRESH for a key whose cached copy is stale.

Slots are addressed by hash(dbEpoch, cfId, key) & mask, so unrelated keys share slots, and a slot stored the plain version. Keys written in one transaction carry the same version. Suppose A and B share a slot and were written together at version V, and A is then rewritten. A later read that re-caches B publishes V into the shared slot, and verifyVersion(A, V) or getSync(A, …, expectedVersion = V) answers FRESH for A's old copy. That is a stale read. With the default 128K slots, the chance per key is roughly the number of keys sharing its version divided by 131,072, so large transactions make it routine. The README and header both claimed collisions "never" cause a stale value to be treated as fresh.

It surfaced as an intermittent CI failure in test/verification-table.test.ts ("a version cached before close never leaks into a later incarnation"), first seen on Make optimistic commit lock buckets and validation policy configurable (#897). That test opens 40 fresh databases and seeds the same key and version in each. Every incarnation has a unique epoch, but each new slot can still collide with one an earlier incarnation seeded, which is a birthday collision (≈0.6% per run of that test alone).

❓ Your call: as specified, yes. A FRESH answer for a stale copy is a correctness bug, so collisions must cost only misses.

💡 Solution

VerificationTable::slotRefFor() returns a VtSlotRef: the slot plus a 63-bit tag taken from the same hash (h >> 1). Keys that share a slot agree only on the index bits, so at 128K slots two colliding keys still get the same tag with probability 2^-47. Every path that compares a version with a slot or publishes one now goes through vtEncodeVersion(version, tag) = version ^ tag. Bit 63 stays clear, so tagged lock and settled-empty values are unaffected, and an encoding of 0 is never published or matched. A colliding key with an equal version therefore stores a different value, so collisions are back to costing only misses. Lock and settle paths keep using slotFor()'s bare pointer because they never compare versions. The bare-pointer primitives are renamed verifyEncoded / populateEncoded / populateEncodedIfUnchanged, so a plain version is not passed to them by mistake.

⚖️ Alternatives

• Wider slots carrying a key fingerprint (16 bytes per slot). This needs a double-width CAS on every populate and doubles memory. The XOR tag gets the same collision safety inside the existing 8-byte lock-free word.
• A larger table. It makes collisions rarer but cannot remove them, and it costs memory process-wide.
• Retrying or skipping the test. That would hide a real stale-read path.

🔧 Changes

• src/binding/core/verification_table.{h,cpp}:
  • vtEncodeVersion, VtSlotRef (with holds()), slotRefFor(), and VtSlotRef overloads of verifyVersion / populateVersion / populateVersionIfUnchanged;
  • the bare-pointer forms are renamed *Encoded;
  • slotFor() and slotRefFor() share one hashFor();
  • the header's collision contract is rewritten.
• src/binding/database/database.{h,cpp}, src/binding/transaction/transaction{,_handle}.{h,cpp}: vtSlotFor() returns a VtSlotRef. The sync FRESH fast paths use holds(), and vtPopulateIfSettled, the async-get state, verifyVersion() and populateVersion() go through the ref.
• README.md, src/binding/core/DESIGN.md, DESIGN.md: state the encoding and why keys that share a version need it. The README now states the residual as a probability, 2^-47 per lookup at the default 131,072 slots, doubling with each doubling of verificationTableEntries, rather than "never".

✅ Verification

• test/verification-table-collision.test.ts (child-process fixture test/fixtures/fork-vt-shared-version-collision.mts):

  • the table is configured with 16 slots;
  • 256 keys are written in one transaction and cached;
  • one key is rewritten and the others are re-cached;
  • the rewritten key must not verify at its old version, getSync with that expected version must return the new value, and the last re-cached key must still verify.
  • Fails on base: 3 of 3 runs, with verifyVersion answering true for the stale version.
  • CI follow-up: the fixture's "last re-cached key still verifies" check failed about 1 run in 16 (first seen on Node 26 / ubuntu). The expected-version getSync of the rewritten key takes the soft-miss path, which publishes that key's new version and evicts the last re-cached key whenever the two share a slot under the random per-process seed. The check now runs before those reads. Local Linux, Node 26: 6 of 60 runs failed before, 0 of 200 after.

• Native: CollidingKeysWithSameVersionDoNotVouchForEachOther, VersionEqualToKeyTagIsNeverCached.

• Standalone repro of the CI test's loop (open a fresh DB, check the key, seed it, close; 2,000 iterations). Birthday math predicts about 15 hits.

  build	stale FRESH answers
  origin/main (macOS Node 24 / Node 22, Linux Node 22)	13–20
  this branch	0 (two runs of 2,000)

  The hit rate does not change with the optimistic lock-bucket count (2^20, 65,536 or 16), so Make optimistic commit lock buckets and validation policy configurable #897 did not cause this.

• Cost on the FRESH fast path: a microbenchmark of 2M getSync(key, 0, undefined, version) hits per round (7 rounds, 3 alternating runs, macOS arm64) measured 93–95 ns per hit on origin/main and 97–102 ns with this change. That is about +3 to +8 ns, or 3–8%, and it held after shifting main's code layout. I could not attribute it to the extra XOR and compare, which should cost about a nanosecond, so treat it as an upper bound. ❓ Your call: whether that cost is acceptable for closing the stale read.

• The cross-model pre-push review (Codex, Gemini, Cursor Composer, plus Harper-domain adjudication) has run. Its findings are addressed: the cheaper tag, the *Encoded rename, and the bounded README claim. The hot-path cost above is the remaining open item. A follow-up full round on the CI fix (Codex, Gemini, Cursor Composer, Harper-domain) found no code defects and produced the README wording fix; its delta round converged. Declined: a nit that the fixture only exercises the collision when the rewritten key shares a slot with another key, which fails to happen with probability ≈7×10⁻⁸; the native test forces the collision deterministically.

• macOS: pnpm check, pnpm test (75 files, 1,104 passed, 10 skipped; then the VT, collision and transaction files again after the review fixes) and pnpm test:native (301 passed). Linux, Windows, Bun and Deno are left to CI.

Make optimistic commit lock buckets and validation policy configurable (#897) surfaced this in its CI and should merge after this PR. Add a native RocksDB storage lease for derived indexes (#842) and Run a database's async commits on up to four concurrent commit threads (#902) touch read/write paths: any verify or populate site they add must go through vtSlotFor() / slotRefFor(), since a raw slotFor() comparison still compiles.

Rebased onto main past Read every entry of a transaction-log segment that one transaction pushed past transactionLogMaxSize (#890) (unrelated transaction-log work). The only conflict was an add/add on the root DESIGN.md index — both branches independently added the same one-line pointer file — resolved by keeping both entries under main's heading. Because the rebase moves every commit SHA, the pre-push review re-ran as a full round (Codex, Gemini, Cursor Composer, Harper-domain); it found no new defects. Every finding mapped to a decision already recorded above or adjudicator-dismissed: the hot-path-cost ❓ restated as a "major" (no committed benchmark/ case for the tagged cache-hit path; the domain reviewer's own code trace calls the added cost "likely negligible" and unattributable, matching the microbenchmark above), the fixture-seed-dependence nit already declined, and a comment-narration nit already declined. Gemini's coverage-gap nit (no verifyVersion check after an async populate) was dismissed by the domain adjudicator — already covered by test/verification-table.test.ts:209-212. Cursor's claim that the README's 2^-47 is wrong was dismissed as factually wrong (it has the mod-N/tag math backwards).

Review follow-up (2026-10-06): a reviewer asked for a doc comment on VerificationTable::slotRefFor; added in the file's tab style, also stating that a disabled table returns an empty ref. The earlier fixture-ordering thread was already fixed by the CI follow-up above. Delta pre-push review (Codex, Gemini, Harper-domain) on the doc comment found no new defects; it re-raised the items already recorded above (hot-path cost ❓, fixture seed nit, comment-narration nit, which now also names this doc comment — kept, since a human reviewer asked for it and every sibling method has one). Gemini's new claim of a dead holds() check after the read in GetSync was dismissed as factually wrong: holds() appears only on the fast path, and the post-read block publishes through vtPopulateIfSettled's conditional CAS.

Related PRs: #897 overlaps, #842 overlaps, #902 overlaps, #742 independent, #767 independent, #890 independent, #900 independent
Complexity: low

— Claude Opus 5.5 (CI fixture fix and README wording, 2026-10-05)
— Claude Sonnet 5 (rebase onto main past #890, 2026-10-06)
— Claude Opus 5.5 (review follow-up: slotRefFor doc comment, 2026-10-06)

🤖 Generated with Claude Code

Closes #903.

_{Review-Coverage: authored=claude; ran=gemini,codex; adjudicated=domain; declined=cursor-grok,cursor-composer,cursor-kimi,cursor-muse; rounds=4; full=2 @ e82deef}

_{Review-Attention: study ~12m (critical: transaction.cpp, transaction_handle.cpp +1; decisions: xor-tag-in-slot, tag-from-index-hash, probabilistic-contract, do-less-alternative) @ e82deef}

[gemini-code-assist]

Code Review

This pull request introduces key-tag encoding for verification table slots to prevent stale reads when colliding keys share the same version. It replaces bare slot pointers with a VtSlotRef struct that encapsulates both the slot pointer and a 63-bit key tag, and updates the verification table primitives to encode versions using XOR with the key tag. Feedback highlights a compatibility issue in the new integration test where spawning a .mts file directly via process.execPath will fail on Node.js versions prior to 22.6.0, suggesting instead to resolve and spawn the tsx CLI entry point.

[github-actions]

📊 Benchmark Results

get-sync.bench.ts

getSync() > random keys - small key size (100 records)

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 lmdb	1	24.54K ops/sec	40.75	39.30	685.469	0.112	122,704
🥈 rocksdb	2	10.61K ops/sec	94.27	90.83	24,588.874	0.999	53,038

getSync() > sequential keys - small key size (100 records)

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 lmdb	1	28.68K ops/sec	34.86	33.84	567.121	0.101	143,411
🥈 rocksdb	2	11.34K ops/sec	88.17	85.27	573.138	0.049	56,712

ranges.bench.ts

getRange() > small range (100 records, 50 range)

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 lmdb	1	25.52K ops/sec	39.19	35.91	1,836.193	0.286	127,581
🥈 rocksdb	2	14.97K ops/sec	66.78	57.04	1,061.161	0.121	74,873

realistic-load.bench.ts

Realistic write load with workers > write variable records with transaction log

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 rocksdb	1	373.70 ops/sec	2,675.967	65.07	56,246.739	13.64	748
🥈 lmdb	2	26.15 ops/sec	38,236.279	410.863	1,214,034.851	137.283	64.00

transaction-log.bench.ts

Transaction log > read 100 iterators while write log with 100 byte records

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 rocksdb	1	35.34K ops/sec	28.30	12.78	20,836.52	0.852	176,705
🥈 lmdb	2	435.35 ops/sec	2,296.998	113.475	28,898.839	1.64	2,180

Transaction log > read one entry from random position from log with 1000 100 byte records

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 rocksdb	1	660.61K ops/sec	1.51	1.31	4,780.329	0.199	3,303,060
🥈 lmdb	2	450.80K ops/sec	2.22	1.11	8,702.28	0.529	2,253,997

worker-put-sync.bench.ts

putSync() > random keys - small key size (100 records, 10 workers)

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 rocksdb	1	829.27 ops/sec	1,205.88	1,032.098	3,885.504	0.432	1,659
🥈 lmdb	2	1.16 ops/sec	865,666.843	800,356.005	933,073.917	3.69	10.00

worker-transaction-log.bench.ts

Transaction log with workers > write log with 100 byte records

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 rocksdb	1	23.17K ops/sec	43.15	29.44	25,992.34	2.61	46,350
🥈 lmdb	2	825.37 ops/sec	1,211.578	314.729	17,078.95	5.31	1,655

---

Results from commit 3561821