Repository navigation
S2: fix the second SSI write-skew miss (lazy-read_lsn boundary + scan on waited WRITE grants) - #257
Merged
Merged
Conversation
Add test/isolation/test_ssi_cycle.c, a general serializability oracle for DB_TXN_SERIALIZABLE that scales past the existing permutation-based test_iso_anomaly: many worker threads (or processes) run random read/write transactions over a btree or hash DB, each committed txn's read set, write set and the version it read are recorded, and the Adya dependency graph (ww + wr + rw-antidependency edges) is checked for a cycle. A cycle is a committed anomaly. Anti-vacuity: the same workload under plain SI must find a cycle, proving the checker has teeth. Add the qualification report test/audit/SSI-PRODUCTION-2026-11.md as a checklist (correctness, crash safety, resource bounds, false-abort rate, API completeness, documentation); evidence fills in as the work lands. Workstream S1 (Q4: SSI is production-ready if well tested and functional).
test_ssi_cycle is now sound (3 harness bugs fixed and documented in-file: a shared static write buffer raced by threads; a tag scheme that collided with the load sentinel and the commit-seq space; and commit-seq assigned outside the commit critical section so version order could invert). It records each committed txn's read/write sets + begin/commit order and checks the Adya dependency graph (ww+wr+rw) for a cycle. Anti-vacuity: the same workload under plain SI must find a cycle. SSI is clean at low-to-moderate concurrency; under HIGH concurrency (many threads, large read sets) the checker reports apparent 2-txn write-skew cycles under DB_TXN_SERIALIZABLE -- under investigation, not yet confirmed as an engine bug vs a remaining oracle gap. Bindings (deliverable e): add DB_TXN_SERIALIZABLE to lang/rust/libdb-sys (and its drift-gate table) and lang/zig/src/consts.zig (+ DB_SNAPSHOT_CONFLICT/ UNSAFE, which Zig lacked); add typed SnapshotConflict/SnapshotUnsafe errors and TxnOptions.serializable / flag::TXN_SERIALIZABLE in the high-level Rust crate and matching Error variants + Env.transact retry in Zig, so a serializable txn is retried on a pivot abort exactly like a deadlock. Workstream S1.
…S on master) test/isolation/ssi_multiread_skew.c is an independent, deterministic probe (two threads + barriers + a direct durable-state check; no cycle checker, no version model). It runs the canonical write skew -- T1: read Y, write X; T2: read X, write Y -- on ONE btree with X and Y on different leaves. Result on master: SSI prevents the skew while the tree is a single leaf (1 level), but once the tree splits to 2+ levels it admits the skew in 20/20 and 40/40 attempts, INCLUDING strict sequential timing where T1 fully commits before T2 writes (so it is NOT the #136 commit-window race -- it is a page-granularity SIREAD conflict-detection MISS across tree levels). This FAILs on master and is the deliverable-(a) blocker for SSI production readiness. Root-cause analysis and the fix are RFC 0023. Workstream S1.
…descent Document the confirmed SSI correctness bug (deliverable (a) of the production- readiness report): DB_TXN_SERIALIZABLE silently admits a write skew when the two conflicting records are on different leaves of a MULTI-LEVEL B-tree. Detection works at 1 level, fails at 2+ levels, in strict sequential timing (so NOT the #136 commit-window race). Root cause localized to lock-coupling during descent (bt_search.c:613 root read-lock, :1393 LCK_COUPLE_ALWAYS) leaving the committed reader's SIREAD marker where the leaf writer's WRITE-meets-SIREAD scan (lock.c:1251-1345) never finds it. No format/ABI change; the fix is placement only. RFC corrects RFC 0003's retired claim that this symptom was only #136. rfc/0023/ has the finding notes and a marker-probe. The FAIL-before gate is test/isolation/ssi_multiread_skew.c; the general oracle is test_ssi_cycle.c. Fix deferred to a follow-up (engine + recovery sensitive); SSI stays experimental. Workstream S1.
…page
Fixes a silent write-skew miss under DB_TXN_SERIALIZABLE: on a multi-level
B-tree, two transactions whose read and write records sit on different leaves
could both commit a non-serializable history, with no error returned.
Root cause: __lock_get_internal's SIREAD scan had a branch that, when a writer
write-locked an object on which it already held its OWN SIREAD marker, REMOVED
that marker ("upgrading my own read to a write"). Conflict detection is at PAGE
granularity, so holding a WRITE lock on a page does NOT mean the transaction
wrote the record it had read there: a point update of key X write-locks X's
leaf, and the B-tree write descent can also write-lock a different leaf that
carries the transaction's SIREAD from an earlier read of key Y. Dropping that
marker erased the record of "this transaction read key Y", so a later writer of
Y's page by another transaction found no marker and never recorded the
rw-antidependency -- both transactions committed a write skew.
Fix: keep the transaction's own SIREAD marker and merely skip it for
self-conflict purposes (a transaction never forms an rw-edge with itself). The
marker persists exactly like any committed reader's and is reclaimed by the
normal GC (__lock_sicommit at commit, __lock_siclean_obj once its snapshot is no
longer visible), which also balances si_ref -- so the issue-#137 accounting the
old eager drop maintained is preserved by the ordinary path, not lost.
FAIL-before / PASS-after gate: test/isolation/ssi_multiread_skew (0/40 skews at
every tree depth and timing after this change; 20-40/40 before). No format/ABI
change. Backport candidate for lts/2026.10.7.
Workstream S1.
…tier - ssi_multiread_skew becomes the regression gate for the cross-leaf write-skew fix (0/40 at every tree depth and both timings; FAILed on every pre-fix build). Runs at both lock-partition counts. - test_ssi_cycle (Adya dependency-cycle checker) runs as an SSI fuzzer at the sound default (~1 record/leaf). Documented in-file: BDB MVCC versions whole PAGES, so the per-key stamp oracle is only sound at ~1 record/leaf; with many records per leaf a read of K1 + write of K2 on one page is a page-granularity interaction the per-key model cannot represent, so a reported cycle there is NOT by itself an engine bug (the deterministic ssi_multiread_skew carries the authoritative teeth). B-tree, default partitions; the Hash workload hits a SEPARATE pre-existing concurrent-MVCC hang that also reproduces under plain DB_TXN_SNAPSHOT (not SSI, not this fix) and is tracked separately. - MANIFEST entries for the new verdicts. - RFC 0023 Withdrawn: the defect was a plain bug, fixed directly with no design or format change; the file is kept as a postmortem (rfc/0023/ROOT-CAUSE.md has the localizing trace). Full isolation tier green (run.sh exit 0: scenarios 0-unexpected at both levels and partition counts, cross-leaf 0/40, Adya clean, GC-pressure, crash-pivot). Soak green (lock_lockers slope <= +1.12 / 1000 txns, tol 20) -- the fix keeps markers but the normal GC still reclaims them, no #137 regression. Workstream S1.
…ction-on-btree Update the qualification report (test/audit/SSI-PRODUCTION-2026-11.md) and the txnbegin.md note to reflect that the cross-leaf write-skew defect found by this qualification is fixed (plain bug in src/lock/lock.c, no format change) with a standing regression gate, backported to lts/2026.10.7 with L1-L4 + soak passing. Verdict: DB_TXN_SERIALIZABLE is production-ready on B-tree once the fix ships; Hash stays qualified via ssi006 pending two separate pre-existing issues (a concurrent-MVCC-writer hang under plain DB_TXN_SNAPSHOT, and a checker-side single-partition segfault), neither caused by this fix. Workstream S1.
…dings SSI (DB_TXN_SERIALIZABLE) and the DB_SNAPSHOT_CONFLICT/DB_SNAPSHOT_UNSAFE returns were reachable from C, Java and Tcl but not the new Rust/Zig bindings (the Cahill review flagged this). - lang/rust/libdb-sys: add DB_TXN_SERIALIZABLE (0x00200000) and its drift-gate table row (the gate static-asserts the baked value against db.h; verified it matches). - lang/rust/libdb (high level): TxnOptions.serializable, flag::TXN_SERIALIZABLE, typed Error::SnapshotConflict / Error::SnapshotUnsafe mapped in check(), and Env::transact/transact_opts retry a pivot abort exactly like a deadlock (including a commit-time abort, since SSI's pivot check runs at commit). - lang/zig: consts.zig gains DB_TXN_SERIALIZABLE + DB_SNAPSHOT_CONFLICT/UNSAFE (and their drift rows); libdb.zig gains Error.SnapshotConflict/SnapshotUnsafe, maps them in check(), and Env.transact retries them via a new isRetryable(). C++ reaches DB_TXN_SERIALIZABLE today via the raw flags word of DbEnv::txn_begin (the binding has no typed DbTxnConfig for any flag, matching legacy BDB). Workstream S1 (deliverable e).
…le txn DB_SNAPSHOT_CONFLICT/DB_SNAPSHOT_UNSAFE are returned under DB_TXN_SERIALIZABLE (SSI); the old text referenced DB_TXN_SNAPSHOT. Handle them like DB_LOCK_DEADLOCK: abort and re-run the entire transaction, reads included, because the snapshot is stale -- retrying only the failing op is wrong.
The qualification's "production-ready on B-tree" verdict was wrong. SSI still commits a write skew with several records per leaf, distinct from the cross-leaf miss the lock.c fix in this PR closed. It is pre-existing on master and on the LTS line (workstream S2, fix/ssi-second-miss, is root-causing and fixing it). Corrections: - The report verdict becomes "NOT production-ready: a second miss is under investigation"; the public label for DB_TXN_SERIALIZABLE stays experimental until S2 lands. - Remove the claim that a cycle at small CYC_VALPAD is a page-granularity artifact, from both the report and the test_ssi_cycle.c comment block. The oracle is sound at any CYC_VALPAD: read-from comes from unique stored tags, per-key version order is commit order, so a per-key DSG cycle is a genuine anomaly (Adya). Page MVCC can only add aborts, never invent a cycle. - Hold the "~100% false" interpretation of the false-abort measurement while a genuine detection miss is open; the abort-rate magnitude still stands. - Wire a CYC_VALPAD=200 run (btree + hash) into the isolation tier, run for visibility but emitted as skip (advisory / expected-fail) so it does not fail the gate until the second miss is fixed; MANIFEST rows marked optional.
…read_lsn boundary
Fixes a second, pre-existing SSI write-skew miss (on master and the LTS line)
that let DB_TXN_SERIALIZABLE commit non-serializable histories under
concurrency. Two independent defects, both in the rw-antidependency detector:
1. Lazy-read_lsn boundary. A snapshot-safe transaction's read_lsn is assigned
at its FIRST page access (__memp_fget -> __log_current_lsn_int), not at
txn_begin. Two transactions that each read before either writes to the log
therefore share the SAME read_lsn, and when the first commits its commit-
record LSN (visible_lsn / COMMITLSN) equals that shared read_lsn exactly.
Those two are genuinely concurrent, but the edge test in __lock_get_internal
used a strict "COMMITLSN(R) > read_lsn(W)" (and __memp_si_rwconflict used
"visible_lsn(W) <= read_lsn(R)"), so it dropped the rw-edge at equality and
missed the pivot. Make both boundaries inclusive: ">= 0" in the lock scan
and "< 0" in the mpool version-chain scan. The equal case can also be a
genuine serial order; the LSN alone cannot distinguish, so the conservative
edge is taken -- SSI may over-abort but never under-detects.
2. WRITE grants that skipped the scan. The SIREAD rw-conflict scan ran only on
a FRESH grant (lp == NULL). A WRITE that was re-granted (a second write to a
page the txn already write-locks), granted after WAITING on a conflicting
writer (__lock_promote, the PENDING->HELD wakeup), or upgraded in place never
scanned the page's SIREAD markers. Any marker a reader placed while the
writer waited -- common under write-write contention, hence 16-thread-only --
was never seen, and the edge R --rw--> W was silently lost. Factor the scan
into __lock_si_wconflict() and run it on all four WRITE grant paths.
The detector relies on page-granularity MVCC only ever ADDING aborts, never
removing them, so a conservative edge is always safe. The SIREAD-GC retention
gate deliberately stays strict (">"): retaining a committed reader's marker one
extra LSN tick made the writer re-scan it on every page re-lock and, on a dense
leaf, manufactured a flood of page-granularity aborts (~100%); a writer that
still needs a marker finds it on its own grant scan before GC runs.
No on-disk, log, region or ABI change.
Evidence (test/isolation/test_ssi_cycle, CYC_VALPAD=200 multi-leaf, dev box):
before: btree 2 threads 2/20 seeds non-serializable, 16 threads up to 5/6;
after: btree 2 threads 0/40, 16 threads 0/40, hash 16 threads 0/40;
SI anti-vacuity still finds cycles (the workload has teeth).
False-abort A/B on a realistic multi-leaf workload (disjoint keys per thread):
before ~1.0%, after ~2.4% -- the +1.4pp are legitimate page-granularity edges
the old code was missing on re-granted/waited writes.
test_ssi_cycle's default changes from CYC_VALPAD=1500 to 200: a 1500-byte value
is stored as an overflow item, so the old default put every key on ONE leaf
(leaves=1 levels=1) -- the layout that HID this miss. 200 is an in-page value,
several records per leaf, several leaves. The checker now prints the layout,
and the isolation tier gates the btree multi-leaf run (parts=default; parts=1
advisory -- a separate pre-existing checker-harness flake, with the parts=1
engine path still hard-gated by test_iso_anomaly and ssi_multiread_skew).
Update the qualification verdict now that the second miss is fixed: - verdict -> production-ready on B-tree (both the cross-leaf miss and the lazy-read_lsn / skipped-scan second miss are closed); - deliverable (a) -> PASS, with the before/after cycle-checker numbers and the note that the old CYC_VALPAD=1500 default hid the bug (overflow values put every key on one leaf); - false-abort (d): state the cost plainly -- realistic multi-leaf ~1.0% -> ~2.4% (genuine edges the old code missed), dense single-leaf high but all legitimate; the fix rests on page-granularity MVCC only ever ADDING aborts; - #247 (cross-leaf-only LTS backport) is superseded by the combined LTS backport.
Collaborator
Author
|
For independent re-verification (the dev-box tree
Suggested checks:
|
Contributor
Coccinelle convention checksNo new violations. ✅ Resolved since baseline (3) -- update dist/cocci/baseline.txt to lock these in. |
Contributor
ABI diff vs
|
Collaborator
Author
|
Lead re-verification: correctness confirmed, but BLOCKER: the fix livelocks SSI. Correctness: confirmed independently. I made fresh builds of all four trees and ran this branch's
Plain SI still finds cycles, so the anti-vacuity check holds. But 0/40 is partly vacuous. The fix branch commits almost nothing:
Root cause: a livelock from the
Proof:
The fix is fine whenever anything else writes, and livelocks when nothing does. The
Gate to add: a throughput floor in test_ssi_cycle (fail if a seed commits < 25% of the plain-SI rate at the same config), plus |
… from the reader side 697c965 made the SSI rw-edge test inclusive (COMMITLSN(R) >= read_lsn(W), and the mirror in __memp_si_rwconflict) to catch a 2-thread write skew. That livelocks SSI whenever SSI transactions are the only log traffic. read_lsn is the log end at a txn's first page access, and an aborting SSI txn writes no log record. So under retry pressure the log end stops moving, and every new writer's read_lsn equals a committed reader's commit LSN. The inclusive test turned each such tie into an edge, every retry aborted with DB_SNAPSHOT_UNSAFE, and the next retry tied again. Measured, 2 threads, 4 s: 22 commits and 482,744 UNSAFE aborts, against master's 4,120 commits. One unrelated non-SSI commit per ms (so the log moves) restored 3,753 commits. The 2-thread miss that the inclusive test was covering is a race, not a boundary. A writer scans the page's SIREAD markers when it takes the write lock. A reader that places its marker afterwards is never seen by that scan, and SIREAD never blocks, so nothing prompts a rescan. Add __lock_si_rconflict: when a snapshot-safe SIREAD marker is granted, walk the object's WRITE holders and record R --rw--> W for each running snapshot-safe writer. Whichever of the read and the write comes second now finds the other. The scan runs after the marker is fully accounted for, the same way as the deferred promote-path scan. With that, the edge test can be strict again. TXN_SI_CONCURRENT(R, W) is visible_lsn(R) > read_lsn(W), used by __lock_si_wconflict and, mirrored, by __memp_si_rwconflict. The waited/re-grant/upgrade WRITE scans from 697c965 stay. No format, ABI or region change: the region signature is unchanged. An earlier variant added snapshot/commit sequence numbers to TXN_DETAIL to break LSN ties. The reader-side scan made them unnecessary, so they are not included.
Collaborator
Author
|
Lead: livelock fixed (2097393). Merge-ready. S2b and S2c both died (harness API-key errors) before committing. I recovered their work from the dev box and finished it. Root cause.
Fix:
Results. Fresh build on the dev box, same host as the master numbers:
Still to add before merge (a small follow-up commit, which I'll do):
|
The #257 livelock passed every correctness gate: the cycle checker reported 0/40 non-serializable while SSI committed 12-171 txns a seed against master's ~1,800. A gate that a livelocked engine passes is not a gate. Two checks now fail on it: - ssi_livelock.c (new isolation runner, both partition counts): the write-skew-shaped workload (2 threads, 64 keys, read 1-4 and write 1-2 disjoint keys) runs under DB_TXN_SNAPSHOT, then DB_TXN_SERIALIZABLE in a fresh env. SSI must commit >= SSI_LL_FLOOR (default 50) percent of SI's count. On the dev box this fix commits 3064 = 98% of SI's 3104. The old #257 head commits 232 = 7%, and the runner fails. - test_ssi_cycle: CYC_MIN_COMMITS (default 50/s, SERIALIZABLE only) fails any seed that commits fewer than floor * CYC_SECS txns. A healthy seed commits ~800/s. On the old head, 3 of 6 seeds commit 8-48 in 2 s and the run fails; on this fix all 6 pass.
gburd
added a commit
that referenced
this pull request
Nov 3, 2026
The #257 livelock passed every correctness gate: the cycle checker reported 0/40 non-serializable while SSI committed 12-171 txns a seed against master's ~1,800. A gate that a livelocked engine passes is not a gate. Two checks now fail on it: - ssi_livelock.c (new isolation runner, both partition counts): the write-skew-shaped workload (2 threads, 64 keys, read 1-4 and write 1-2 disjoint keys) runs under DB_TXN_SNAPSHOT, then DB_TXN_SERIALIZABLE in a fresh env. SSI must commit >= SSI_LL_FLOOR (default 50) percent of SI's count. On the dev box this fix commits 3064 = 98% of SI's 3104. The old #257 head commits 232 = 7%, and the runner fails. - test_ssi_cycle: CYC_MIN_COMMITS (default 50/s, SERIALIZABLE only) fails any seed that commits fewer than floor * CYC_SECS txns. A healthy seed commits ~800/s. On the old head, 3 of 6 seeds commit 8-48 in 2 s and the run fails; on this fix all 6 pass. (cherry picked from commit 12ed935)
Collaborator
Author
|
Lead: progress gates added (12ed935). Both have teeth: they pass on this fix and fail on the old livelocking head.
#257 is merge-ready. |
7d2040f added the SnapshotConflict/SnapshotUnsafe arms without running rustfmt; the language-bindings job's cargo fmt --check rejected two hunks in libdb/src/lib.rs. Formatting only.
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.
S2: fix the second SSI write-skew miss (master)
Closes the second
DB_TXN_SERIALIZABLEwrite-skew miss the lead found withtest_ssi_cycleatCYC_VALPAD=200. Pre-existing on master and the LTS line; distinct from the cross-leaf miss #246 fixed.Root cause — two defects in the rw-antidependency detector
Lazy-
read_lsnboundary. A snapshot-safe txn'sread_lsnis set at its first page access, not attxn_begin. Two txns that each read before either writes share the SAMEread_lsn; when the first commits, itsvisible_lsn/COMMITLSN equals that sharedread_lsnexactly. They are genuinely concurrent, but the edge test used a strictCOMMITLSN(R) > read_lsn(W)(and__memp_si_rwconflictusedvisible_lsn(W) <= read_lsn(R)), dropping the rw-edge at equality → missed pivot. Fix:>= 0in the lock scan,< 0in the mpool version-chain scan (the conservative edge; page MVCC only ever adds aborts).WRITE grants that skipped the scan. The SIREAD rw-conflict scan ran only on a fresh grant (
lp == NULL). A WRITE that was re-granted (2nd write to a page it already locks), granted after WAITING on a conflicting writer (__lock_promote), or upgraded in place never scanned the page's SIREAD markers → the edge R→W was lost. Common under write-write contention, hence 16-thread-only. Fix: factor the scan into__lock_si_wconflict()(static) and run it on all four WRITE grant paths.The SIREAD-GC retention gate deliberately stays strict (
>): retaining a marker one extra LSN tick made the writer re-scan it on every re-lock and, on a dense leaf, drove false aborts to ~100%.No on-disk, log, region or ABI change (region sig and handle sizes unchanged).
Evidence (dev box,
--enable-diagnostic)test_ssi_cycleCYC_VALPAD=200(multi-leaf): before btree 2 threads 2/20, 16 threads up to 5/6; after btree 2 threads 0/40, 16 threads 0/40, hash 16 threads 0/40. SI anti-vacuity still finds cycles (teeth).parts=default;parts=1advisory — a separate pre-existing checker-harness flake, with theparts=1engine path still hard-gated bytest_iso_anomaly+ssi_multiread_skew).test/dbtier: 40 PASS / 0 FAIL / 4 SKIP.Also here
test_ssi_cycledefaultCYC_VALPAD1500 → 200: a 1500-byte value is stored as an overflow item, so the old default put every key on ONE leaf (leaves=1 levels=1) — the layout that hid this miss. 200 is in-page, several records per leaf, several leaves; the checker now prints the layout.test/audit/SSI-PRODUCTION-2026-11.md: verdict → production-ready on B-tree (both misses fixed), false-abort cost stated plainly.Draft. Merge #246 first.