test: add isolation, resource-accounting and lock-mode test tiers - #143
Open
gburd wants to merge 5 commits into
Open
test: add isolation, resource-accounting and lock-mode test tiers#143gburd wants to merge 5 commits into
gburd wants to merge 5 commits into
Conversation
The suite was strong on crash/durability and on memory safety against malformed input, but blind to isolation semantics as a checkable property: a write skew that COMMITS produces no crash, no corrupt page and no sanitizer report, just a database state no serial execution could reach. Issue #136 shipped in v5.3.34 through that gap. test/isolation/test_iso_anomaly.c runs concurrent schedules under DB_TXN_SNAPSHOT and computes the verdict rather than hard-coding it: each transaction declares its semantics as a pure function over an abstract state vector, and after the run the harness reads the durable state back and enumerates every permutation of the COMMITTED transactions looking for one that reproduces it. No serial order means the history is not serializable, and the schedule is printed. The state vector carries observation slots as well as record slots, so the read-only anomaly is checkable -- there the stored state is fine and only the read-only transaction's observation lacks a serial explanation. Nine scenarios: write skew in trigger/control/late timings, the same skew with the two records on different pages of one B-tree, G2-item on a predicate read, Fekete's three-transaction read-only anomaly, lost update, and a read-your-writes sanity case that fails if the harness ever stops driving the engine. The #136 interleaving -- T2's write landing while T1 is INSIDE DB_TXN->commit -- is reached from the application side only, with barriers plus an atomic flag T1 sets before entering commit, as the reporter did. No engine change, no HAVE_DST site, zero production overhead. That window is racy, so those scenarios run several attempts under an asymmetric rule: one violation is a reproduction, a pass needs every attempt clean. On master both #136 shapes violate on the first attempt; the other seven scenarios pass. Note that the "separate defect" the reporter suspected -- different pages of one B-tree detecting no conflict even in the control -- does NOT reproduce: with the shape verified via DB->stat, the control correctly returns DB_SNAPSHOT_CONFLICT, so on this construction the different-pages case has the same root cause as #136 proper. Expectations are self-correcting: the tier also fails when a scenario marked as reproducing a known issue STOPS violating, so a fix cannot land without updating the table.
…138) A slot/mutex/locker leak produces no crash, no corrupt page and no sanitizer report. It only shows up after thousands of sequential transactions, when some later call returns ENOMEM because the region filled. Neither the crash/durability tier nor the memory-safety tier can see that shape; issues #137 and #138 are both of it. test/soak/test_soak_resources.c is a general soak harness parameterised by workload: one long-lived environment with DEFAULT region sizes, N (>= 2000) sequential transactions, sampling seven counters through the PUBLIC stat APIs (mutex_stat, lock_stat, txn_stat, memp_stat). The verdict is a least-squares slope over the post-warmup samples in units per 1000 transactions against a documented per-counter tolerance -- least squares so one noisy endpoint cannot decide it, and post-warmup so the tier does not fight normal lazy region allocation. The full growth curve is always printed, so a CI log alone diagnoses a regression. ENOMEM/DB_RUNRECOVERY is recorded with the call name and transaction number and fails the workload without aborting the run: "ENOMEM at transaction 1400" beats a stack trace. Five workloads: read-only snapshot txns (#137), snapshot txns that read and write so their details are MVCC-retained then reaped (#138), plus plain read-write, all-abort and cursor-churn controls. On master, 2000 transactions: ro_snapshot leaks +1000.00 mutex slots and +1000.00 lockers per 1000 transactions -- exactly one of each per transaction, never returned. mvcc_retained leaks +1345 mutex slots against +673 lockers (the extra mvcc_mtx of #138 on top of #137) and hits ENOMEM from txn_begin at transaction 1400. All three controls are flat, which is what makes the leak signal credible rather than a measurement artefact. cursor_churn deliberately uses a plain transaction: as a snapshot reader it tripped the #137 leak and was just a second copy of ro_snapshot. Lives in test/soak/ to stay clear of the targeted leak tests going into test/c/ with the #137/#138 fix. Those prove a specific path frees a specific resource; this proves the aggregate accounting is stable over a long run, which is the property that would have caught both before release.
Issue #140 is a heap out-of-bounds WRITE in __lock_vec, but the root-cause class is what matters: the SSI work added a lock mode (DB_LOCK_SIREAD = 9) without auditing pre-existing loops that enumerate modes exhaustively. __lock_vec's DB_LOCK_PUT_READ / DB_LOCK_UPGRADE_WRITE path sizes an objlist from sh_locker->nwrites while its release loop tests only DB_LOCK_READ and DB_LOCK_READ_UNCOMMITTED; SIREAD matches neither those nor IS_WRITELOCK, so it is never counted yet still consumes a slot. The DB_ASSERT that would catch it is diagnostic-only and compiled out of release builds. test/lockmatrix/test_lock_matrix.c asserts the invariant instead of the symptom -- the locks the release loop SKIPS must all be counted in nwrites, because nwrites sizes the allocation -- so the NEXT mode added is covered too. Three sections, all through the public DB_ENV lock API: modes every db_lockmode_t acquired and released via both lock_get/lock_put and lock_vec GET/PUT_ALL. conflicts every (held, wanted) cell. The verdict is not hard-coded; the engine's table is the specification. What is asserted are the properties a table must have regardless of policy: totality (a definite granted/NOTGRANTED answer, never an internal error or a hang) and symmetry of conflict, plus st_nmodes against the number of db_lockmode_t values -- which catches "a mode was added without widening the table" directly. list PUT_READ and UPGRADE_WRITE with an objlist, driven by a locker holding a MIX of write and SIREAD locks over a sweep of (nwrite, nsiread) shapes, with pure-write and write+read controls. The overflow is (nsiread - nwrite) DBTs, so the sweep walks nsiread past nwrite. Objects are genuine DB_LOCK_ILOCKs, since __lock_fix_list only coalesces objects of exactly that size -- opaque blobs would skip that code. run.sh builds an ASan-instrumented libdb under build_asan_gate/, reusing test/fuzz/check-crashes.sh's mechanism and directory, because the OOB write is inside libdb's own allocation and a harness-only ASan build cannot see it. Each shape is printed BEFORE it is attempted, so the last line of output names the shape that faulted when ASan aborts. On master this reproduces #140 on the first mixed shape: PUT_READ nwrite=0 nsiread=1 nread=0 ... ERROR: AddressSanitizer: heap-buffer-overflow, WRITE of size 8 #0 __lock_vec src/lock/lock.c:457:15 nwrite=0/nsiread=1 means a zero-sized allocation written with one DBT -- the minimal form. All pure-write and write+read controls pass first, which confirms SIREAD specifically is the trigger. The corruption is severe enough that an UNinstrumented build also aborts, via glibc's own heap checks. Expectations are written for the FIXED engine: no expect_fail flag, so once #140 lands the matrix must complete and exit 0.
…uilds
New workflow .github/workflows/test-tiers.yml, following the house style of
ci.yml/fuzz.yml -- hard gates for the fast deterministic part, advisory plus
scheduled for the slow or currently-failing parts:
B1 isolation HARD GATE per push/PR. ~6s, deterministic, and its
expectations are self-correcting (it fails both on a new
violation and when a known-issue scenario stops violating),
so a fix cannot land without updating the table.
B3 lock matrix advisory (continue-on-error), because on master it
legitimately aborts under ASan -- that abort IS the #140
reproduction. A summary step surfaces the faulting shape
as a warning annotation. Flip to a hard gate in the PR
that fixes #140.
B2 soak scheduled (04:41 UTC, offset from ci.yml's 03:17) plus
workflow_dispatch with a configurable transaction count.
Advisory until #137/#138 land.
Build wiring, neither of which needs a configure option or library-side
hooks since all three tiers drive the public API only:
autoconf `make tier_tests` builds the three drivers (dist/Makefile.in).
meson test/tiers/meson.build, entered from the root shim like
test/pbt. The drivers are build_by_default:false so a plain
ninja is unaffected. Three suites: `tiers` (B1, gating),
`tiers-xfail` (B3, reproduces #140), `tiers-slow` (B2).
Each tier keeps its own run.sh for local use and a README.md documenting
what it checks, how to run it, what it currently reproduces, and which flag
to clear when the corresponding issue is fixed.
.gitignore covers the per-tier build dirs and the scratch environment dirs
the drivers create (ISODIR.*, SOAKDIR.*, LOCKDIR).
Coccinelle convention checksNo new violations. ✅ Resolved since baseline (2) -- update dist/cocci/baseline.txt to lock these in. |
…ures
Two wiring bugs that made the first CI run lie:
- the three run.sh scripts were committed mode 100644, so B1 failed with
exit 126 (not executable) rather than running, and B3 "passed" in 24s
without ever building the ASan libdb or exercising a single shape.
- `./run.sh | tee log` takes tee's exit status, so an ASan abort in B3 or a
leak verdict in B2 would have been swallowed. set -o pipefail on both.
Verified against test/fuzz/run.sh, which is 100755.
ABI diff vs
|
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.
Why
Five external bug reports (#136-#140) landed on v5.3.34 despite an extensive
test suite. The suite is strong on crash/durability (
test/sim: 41 DSTscenarios, write-back crash model, 9 planted bugs) and on memory safety
against malformed input (
test/fuzz: 9 crash seeds, ASan-instrumented gate).It was blind to two things:
commits produces no crash, no corrupt page and no sanitizer report, just a
database state no serial execution could reach.
manifests after thousands of sequential transactions, when some later call
returns
ENOMEM.This PR builds the missing tiers. Each one provably reproduces the issue it
was designed to catch, which is the proof it has teeth.
What
test/isolation/test/soak/test/lockmatrix/No
src/changes. All three tiers drive the public API only, so no enginehook was needed — see below.
B1 — serializability checker
Runs concurrent schedules under
DB_TXN_SNAPSHOTand computes the verdict:each transaction declares its semantics as a pure function over an abstract
state vector; after the run the harness reads the durable state back and
enumerates every permutation of the committed transactions looking for one
that reproduces it. No serial order → not serializable, schedule printed.
The vector carries observation slots as well as record slots, which is what
makes the read-only anomaly checkable (there the stored state is fine and only
the read-only transaction's observation lacks a serial explanation).
Nine scenarios: write skew (trigger/control/late), same skew with the records on
different pages of one B-tree, G2-item on a predicate read, Fekete's 3-txn
read-only anomaly, lost update, and a read-your-writes sanity case that fails if
the harness ever stops driving the engine.
B2 — resource-accounting soak
One long-lived environment, default region sizes, N ≥ 2000 sequential
transactions, sampling seven counters through the public stat APIs
(
mutex_stat,lock_stat,txn_stat,memp_stat). Verdict = least-squaresslope over post-warmup samples, in units per 1000 transactions, against a
documented per-counter tolerance. The full growth curve is always printed so a
CI log alone diagnoses a regression.
Kept general (a harness parameterised by workload) and in
test/soak/tostay clear of the targeted leak tests going into
test/c/with the #137/#138fix.
B3 — lock-mode matrix
Asserts the invariant rather than the symptom: the locks the release loop
skips must all be counted in
nwrites, becausenwritessizes the objlistallocation. So the next mode added is covered too. Three sections: every
mode acquired/released; the whole conflict matrix checked for totality and
symmetry (plus
st_nmodesvs the number ofdb_lockmode_tvalues); andPUT_READ/UPGRADE_WRITEwith an objlist over a sweep of mixedwrite/SIREAD shapes.
run.shbuilds an ASan libdb underbuild_asan_gate/, reusingtest/fuzz/check-crashes.sh's mechanism, because the OOB write is insidelibdb's own allocation.
Evidence — what each tier reproduces
B1 → #136. Both trigger shapes violate on the first attempt:
B2 → #137 and #138. 2000 transactions:
ro_snapshotleaks exactly one mutex + one locker per transaction.mvcc_retainedleaks mutex slots at 2x the locker rate — the extramvcc_mtxof #138 on top of #137 — and exhausts the region at txn 1400. Thethree flat controls are what make the signal credible rather than a measurement
artefact.
B3 → #140. First mixed shape faults, after all pure-write controls pass:
nwrite=0, nsiread=1= a zero-sized allocation written with one DBT, theminimal form. Also aborts an uninstrumented build via glibc's own heap
checks, so it is reachable in a stock build.
Notable finding for the #136 fix
The #136 reporter suspected a separate defect: two records on different
pages of one B-tree detecting no conflict even in the control. That does not
reproduce. With the shape built explicitly and verified via
DB->stat→bt_leaf_pg(512-byte pages + filler keys → 33 leaf pages,alicethe min key and
bobthe max), the control correctly returnsDB_SNAPSHOT_CONFLICT; only the trigger timing commits both. On thisconstruction the different-pages case has the same root cause as #136 proper
(the commit-window race), not an extra page-granularity hole — so fixing the
commit-window atomicity should suffice. The reporter did not publish their
same-btree variant, so their shape may differ; the control is kept as a live
PASS expectation precisely so a real page-granularity regression would surface.
No engine hook was needed
The #136 interleaving — landing T2's write while T1 is inside
DB_TXN->commit— is reached entirely from the application side: barriers plusan atomic flag T1 sets immediately before entering commit, as the reporter did.
No
src/change, noHAVE_DSTsite, zero production overhead.git diff --stat mastertouches no file undersrc/. The--enable-dstOFF build stillhas 0
__db_sim_*symbols (nm build_unix/libdb.a), unchanged.That window is genuinely racy, so those scenarios run multiple attempts under an
asymmetric rule: one violation is a reproduction, a pass needs every
attempt clean. A violation is a real counterexample; one clean run of a racy
schedule proves nothing.
Self-correcting expectations
Every tier fails both ways: on a new violation/leak, and when a scenario
marked as reproducing a known issue stops doing so. The message names the flag
to clear. So a fix cannot land without updating the expectation, and the tier
then gates the fix against regression.
CI wiring
New
.github/workflows/test-tiers.yml, following the house style ofci.yml/fuzz.yml:continue-on-error), because on master it legitimatelyaborts; a summary step surfaces the faulting shape as a warning annotation.
Flip to a hard gate in the PR that fixes libdb 5.3.34: incomplete replication commit lock lists can violate client transaction isolation #140.
ci.yml's 03:17) plusworkflow_dispatchwith a configurable transaction count.Both build systems:
make tier_tests(autoconf) andtest/tiers/meson.buildentered from the root shim like
test/pbt, with suitestiers(gating),tiers-xfail(B3),tiers-slow(B2). Drivers arebuild_by_default: false, soa plain
ninjais unaffected.Validation
test/fuzz/check-crashes.sh— 9/9 PASStest_sim_rng+test_sim_crash_recover— PASS (--enable-dstbuild)--enable-dstOFF build — 0__db_sim_*symbolsssi001,ssi009— PASS (--enable-testbuild)--enable-debug— buildsmeson setup build && ninja -C build— builds;meson test --suite tiersgreenRunning locally
Each directory has a
README.mdwith the full detail, including which flag toclear when the corresponding issue is fixed.