fix(qam): P7 -- DB_INORDER + DB_CONSUME spins forever on a deleted record - #205
Merged
Merged
Conversation
…record A queue opened with DB_INORDER, drained with DB_CONSUME, hangs at 100% CPU the moment the record at the head has been DELETED rather than consumed. No concurrency is required: a single thread, 20 appends, one del, then consume. Root cause is a disagreement between two notions of "the head": * the LOCAL `first, which advances past records confirmed gone
Contributor
Coccinelle convention checksNo new violations. ✅ Resolved since baseline (3) -- update dist/cocci/baseline.txt to lock these in. |
Contributor
ABI diff vs
|
The first version of this runner called timeout(1) directly and hard-coded a `ls libdb.a .libs/libdb-*.so | head -1` library search. That broke four macOS CI jobs with rc=127: stock macOS has no timeout(1), so the driver never ran, and the runner reported "no VERDICT line" -- correctly refusing to call it a pass, but for the wrong reason. Rewritten to follow test/db/run_qam_readpath_bound.sh instead of reinventing it: library discovery with .dylib candidates, static-lib preference, and the pkg-config liburing dependency list (a static libdb.a otherwise fails to link with undefined io_uring_* symbols -- hit locally in the nix shell). The timeout handling cannot be copied wholesale, though. The other runners fall back to running UNTIMED when neither timeout nor gtimeout exists, which is safe for a test that merely takes a while. It is not safe here: the defect under test is an infinite loop, so an unbounded run would wedge CI instead of reporting. run_limited() therefore falls back to backgrounding the driver and polling with kill -0, returning 124 like timeout(1), using only shell builtins. Both paths verified with teeth in both directions, on rebuilt-and-checked libraries: timeout path, fix present -> PASS (both arms, consumed=19) timeout path, fix reverted -> inorder=1 HUNG, inorder=0 control PASS -> FAIL poll fallback, fix present -> PASS (both arms) poll fallback, fix reverted -> inorder=1 HUNG, inorder=0 control PASS -> FAIL The fallback mattering is the point: without proving the poll path can still SEE the hang, macOS would have had a gate that could only ever pass. Also added an explicit `test -x` on the freshly compiled binary, since a stale one from an earlier build would otherwise give a false pass.
gburd
added a commit
that referenced
this pull request
Sep 21, 2026
The tracker is the index a reader checks first, so a fix that lands in code but leaves the tracker saying 'open' is only two-thirds done.
gburd
added a commit
that referenced
this pull request
Sep 21, 2026
…tion The flagapi teeth step asserts that FLAGAPI_STRICT=1 FAILS, and requires each refused XFAIL to appear BY NAME so that a build break cannot be misread as teeth. P7 is now fixed (PR #205), so inorder@inorder reports PASS and no longer appears in that list -- which made the step fail. That is the gate working correctly: it refuses to let a fixed defect stay recorded as broken, and the step's own error text names this exact remedy ("either a fix landed -- remove the XFAIL branch, this step's expectation and the MANIFEST note"). Changes: the expected refused-XFAIL list drops inorder@inorder (P6 and P8 remain), and the step name and messages say P6/P8. The MANIFEST note records P7 as fixed. The DRIVER needs no edit, which was the design intent: the in-driver watchdog emits XFAIL only if the drain does not return, so when the defect goes away the mode reports PASS by itself. Verified against a build carrying the fix: --- backup_nologs: FAIL (XFAIL refused) <- P6, still open --- inorder@default: PASS --- inorder@inorder: PASS <- retired itself, no edit --- noflush@default: PASS --- noflush@noflush: FAIL (XFAIL refused) <- P8, still open Worth recording how nearly this went wrong: the first two strict runs showed inorder@inorder STILL refusing its XFAIL, which looked like the P7 fix being incomplete. It was a stale build -- flagapi-run.sh takes the build directory as $1, not from $BUILDDIR, so both runs silently measured build_unix from before the fix. Same shape as the false pass caught earlier in this session, and the reason the rule is to verify the artifact rather than the invocation.
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
A queue opened with
DB_INORDERand drained withDB_CONSUMEhangs at 100% CPU as soon as the record at the head was deleted rather than consumed. Single thread, no concurrency: 20 appends, onedel, then consume.Root cause
Two notions of "the head" disagree, and the
is_firstguard demanded they match:first, which advances past records confirmed gonemeta->first_recno, which only advances when a consume succeedsSkipping a deleted record advances the first but not the second, so they legitimately differ. The guard compared only against the persistent value and jumped to
retry, which re-entersDB_FIRSTand resetscp->recnoto the hole just skipped:DB_INORDERis needed to reach it: the non-inorder path leaves the switch before the test.Fix
Accept either notion of the head. When nothing has been skipped the two are equal and this is exactly the original test, so the restart-on-moved-head behaviour the guard exists for is preserved.
Test
test/db/run_qam_inorder_consume.sh, registered intest/MANIFEST.The defect is a hang, so
rccannot judge it — the driver emits exactly oneVERDICTline and the runner treats its absence as failure.Teeth verified both ways on rebuilt-and-verified libraries (the first attempt was a false pass from a stale build, caught and redone):
The control staying green in the must-fail arm is what shows the test isolates this defect rather than the whole consume path.
run_all.sh: 18 PASS / 0 FAIL.run_qam_extent_vrfyandrun_qam_readpath_boundstill pass.