test(ssi): qualification suite + abort-rate benchmark, and a btree crash fix#38
Open
gburd wants to merge 3 commits into
Open
test(ssi): qualification suite + abort-rate benchmark, and a btree crash fix#38gburd wants to merge 3 commits into
gburd wants to merge 3 commits into
Conversation
…et_root Under MVCC write contention, __bam_get_root fetches the root read-only then calls __memp_dirty(mpf, &h, ...) to get it dirty. __memp_dirty releases the read-only page and re-fetches it dirty, setting *addrp (h) to NULL as it does so; if the re-fetch fails (e.g. DB_LOCK_DEADLOCK), it returns nonzero with h NULL. The error branch then called __memp_fput(mpf, ..., h, ...) on that NULL page (crash at mp_fput.c:105), and even guarded, fell through to BT_STK_ENTER with a NULL page the caller dereferences. Release only a page we still hold and return the error immediately instead of falling through to the stack-enter with a NULL page. Matches the sibling "someone split the root" error branch just above, which also returns directly. Pre-existing bug on master (not introduced by the SSI work); surfaced by a new high-contention SSI read/write benchmark that drove many concurrent MVCC writes to a small hot page set. Verified: the benchmark no longer crashes, and the standard btree/txn/lock regression (test001, txn001-003, lock001/002) passes.
Broadens SSI qualification beyond the correctness unit tests, per the review's call for a HA/recovery/exhaustion/abort-rate matrix and ROADMAP #17's "measured, not asserted". - ssi007: lock-region pressure. A long run of committed readers (far more than the region has objects or lockers) must not exhaust the region -- proving the bounded incremental reclaim; and a genuinely exhausted region must return a clean resource error with the environment still usable (no panic). This test drove the read-only-reader marker/locker reclaim fix. - ssi008: state across recovery. SIREAD markers and conflict flags are in-memory lock-region state, not logged. Runs snapshot-safe txns (one left in-flight), reopens with -recover, and verifies recovery succeeds and SSI still works (a fresh write-skew is caught; non-conflicting txns still commit) with no stale marker state causing spurious conflicts. - lab/bench/ssi_abort_bench.c: measures the SSI abort rate as a function of contention (hot-set size x thread count), so the page-granularity abort cost (review 5.7) is measured rather than asserted. Under a tiny hot set most aborts are deadlocks on page write-locks that conflict before an SSI edge even forms -- a concrete characterization of the page-granularity behaviour. Benchmark binaries are gitignored.
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
1 participant
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.
What
Tier 2 of the SSI work: qualification tests, an abort-rate benchmark, and a
pre-existing btree crash fix surfaced while benchmarking. Stacked on #37
(SSI hardening) — review/merge that first; this PR shows only its own two
commits once #37 lands.
test(ssi): qualification suite + abort-rate benchmarkssi007— lock-region pressure: a long run of committed readers (far morethan the region has objects/lockers) must not exhaust the region; a
genuinely exhausted region returns a clean error with the env still usable.
ssi008— state across recovery: crash with an in-flight SSI txn, reopenwith
-recover, verify SSI still catches a fresh write-skew and allowsnon-conflicting txns.
lab/bench/ssi_abort_bench.c— measures the SSI abort rate vs. contentionso the page-granularity cost is measured (ROADMAP ci(windows): build VS2010 solution retargeted to v143 #17).
fix(btree): NULL page in__bam_get_rooton__memp_dirtyfailure —a pre-existing crash on master (not from the SSI work), found by the new
benchmark: under MVCC write contention
__memp_dirtynulls the page on afailed dirty re-fetch (e.g.
DB_LOCK_DEADLOCK), and the error path thenfput'd / stack-entered that NULL page (segfault at
mp_fput.c:105). Nowreleases only a page still held and returns the error.
Why
The review flagged that one positive-path test is a demo, not qualification,
and asked for HA/recovery/exhaustion coverage plus measured abort rates before
the next feature.
ssi007also drove a real reclaim fix in #37(committed read-only readers leaked lockers). The benchmark then exposed the
long-standing btree NULL-deref.
Testing
Nix dev shell, Linux x86_64, clang, TCL 8.6:
ssi007/ssi008proven to exercise reclaim andrecovery respectively.
ssi_abort_benchruns a thread sweep and reports abort rates (e.g. hot=8:13–31% under contention, mostly deadlock-driven page conflicts; hot=512:
0–2%).
test001,txn001–003,lock001/002) passes withthe btree fix; no crash on the previously-crashing high-contention run.
Note on process
The
fix(btree)commit is engine-independent of SSI and could be cherry-pickedto its own PR against master if preferred; it lives here because the benchmark
that found it lives here.