Repository navigation
[2026.10.7] Backport: SSI write-skew fixes (both misses) — supersedes #247 - #258
Merged
Merged
Conversation
…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.
(cherry picked from commit cced5c4)
…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).
(cherry picked from commit 697c965)
This was referenced Nov 3, 2026
S1: SSI production-readiness — fix cross-leaf write-skew miss, add Adya checker, bindings, docs
#246
Merged
Contributor
ABI diff vs
|
Contributor
Coccinelle convention checksNo new violations. ✅ Resolved since baseline (3) -- update dist/cocci/baseline.txt to lock these in. |
Collaborator
Author
|
Lead: HOLD, same livelock as #257 (see that PR); the 2-thread test_ssi_cycle hits its watchdog on this branch too. |
… 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. (cherry picked from commit 2097393)
Collaborator
Author
|
Lead: the same fix, cherry-picked with -x (71dac9c). Merge-ready for v2026.10.7.2. PR-level LTS checks, against the base lts/2026.10.7, same host and same configure:
|
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)
The cherry-pick of the second-miss fix (b803b8c) resolved test/MANIFEST by taking master's copy. That added entries for seven test/db runners that exist only on master (run_foreign_disassoc_leak, run_foreign_reassoc, run_r1_revive, run_typed_footprint, run_typed_old_reader, run_typed_util, run_dd_stripes, run_dtrace_probes) and dropped one LTS entry. The manifest gate then failed on every CI platform, because it promised tests that cannot run here. Restore lts/2026.10.7's MANIFEST and add only the isolation cells this backport introduces: ssi_cycle_btree, ssi_cycle_hash, ssi_multiread_skew and ssi_livelock. test/check_manifest_test.sh passes.
gburd
marked this pull request as ready for review
November 3, 2026 22:58
This was referenced Nov 3, 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.
[2026.10.7] Backport: SSI write-skew fixes (both misses) — supersedes #247
One backport branch carrying all three SSI write-skew fixes together, each cherry-picked
-x:5bca388b0←cced5c481— the S1: SSI production-readiness — fix cross-leaf write-skew miss, add Adya checker, bindings, docs #246 cross-leaf fix (keep the writer's own SIREAD marker).b803b8cd1←697c965eb— the second-miss fix: lazy-read_lsnboundary (>=/<) + the SIREAD rw-conflict scan on all four WRITE grant paths (fresh, re-grant, waited/promote, upgrade), factored into static__lock_si_wconflict.Supersedes #247 (which backported only the cross-leaf fix). Close #247 and track the full fix here.
Gates (dev box,
--enable-diagnostic, vslts/2026.10.7basedefea6df0)0xf1574878== base (no format change).libdb-2026.0.so==; exported symbols 1947 == 1947;__lock_si_wconflictis static (not exported); handle sizesDB=1744 DBC=552 DB_ENV=2088 DBT=40 DB_TXN=336 DB_LOCK=24== base. No ABI change.test_ssi_cycle,CYC_VALPAD=200): base btree 2 threads 6/20, 16 threads 11/20 non-serializable → backport 0/40 and 0/40; SI anti-vacuity still finds cycles.-xcherry-pick trailers on both commits.test/dbtier: 36 PASS / 0 FAIL (the non-zero exit is a pre-existing LTS manifest missing-runner mismatch, identical on the base; this change adds nodb/runners).No on-disk, log, region or ABI change — eligible for the LTS line.
Draft.