Repository navigation
[2026.10.7] btree: equal-key put over a fully-deleted duplicate set (R2) - #245
Merged
Merged
Conversation
added 4 commits
November 2, 2026 18:14
The R1 fix (50ef9c7) left a gap (KNOWN-ISSUES R2): a duplicate set whose members are all deleted while a cursor pins one, revived by an equal-key put under a non-identity comparator, kept the deleted key's bytes. This note records the four affected paths and the chosen fix (redirect to a fresh insert when the whole set is deleted), with the weighed alternative. (cherry picked from commit 7616931)
…ld key (R2) The R1 fix (50ef9c7) left a gap recorded as KNOWN-ISSUES R2: in a btree whose bt_compare equates distinct byte strings, a duplicate set under key "abc" whose members are ALL deleted while a cursor still pins one, followed by a put of an equal-comparing key "ABC", revived the set under the DELETED key's bytes. R1 rewrites the key item in place for a non-duplicate slot, but deliberately skipped a key item shared by an on-page duplicate set, and did not cover off-page duplicate trees at all. The residual is every duplicate shape: on-page DB_DUP (the KEYLAST append copies the shared key), on-page DB_DUPSORT (the DB_CURRENT overwrite keeps the shared key), and the off-page tree (the data goes into the dead tree whose key is the unchanged parent-leaf slot). Measured identical on master 2026.10.8 and LTS 2026.10.7.1. The rule: while any member of the set is live the set's key stays (the documented shared-key semantics); when every member is deleted the set is logically gone, so an equal-key put must store the caller's key bytes, exactly as it does when no cursor pins the set -- the set is then reaped before the search, so the put already inserts a fresh pair. __bamc_put now detects, on an exact key match in a DB_AM_DUP database whose stored key bytes differ from the caller's, a duplicate set all of whose members are deleted (__bam_dup_alldeleted walks the on-page shared-key run or the off-page tree, short-circuiting on the first live member), and redirects to a fresh insert at that slot (iiop = DB_KEYFIRST). Case 1 of __bam_iitem completes it with the caller's key, reusing its overflow-key, split/retry and cursor-adjustment handling; the dead set stays pinned until it is reaped. No shared-key surgery, no off-page key rewrite, no new or changed log record, no on-disk/log/region format change. (cherry picked from commit 9d6d005)
r2_dup_revive.c drives the full matrix: {DB_DUP, DB_DUPSORT} x {on-page,
off-page} x {pin, no pin} x {no env, txn env}, deleting every member of the
set then putting an equal-comparing key, and requires the new record's stored
key to be the caller's bytes with the dead set gone (one live record). It
adds overflow keys (asserted real overflow items), a near-full-leaf split
(asserted through bt_leaf_pg), recovery from the log alone (DB_RECOVER_FATAL
with the .db removed), DB->verify before and after, and the live-member
control (delete 1 of 3, 2 remain) which must keep the set's key "abc".
run_r2_dup_revive.sh runs two arms: the build's library (must PASS) and
bt_cursor.o rebuilt with -DR2_REVIVE_SABOTAGE, the R2 redirect disabled (must
FAIL -- the teeth check). FAILs on master 2026.10.8, LTS 2026.10.7.1 and the
LTS+R1 build (28 cases); PASSes with the fix. MANIFEST row added so the db
CI tier runs it; the sabotage guard is a one-line #ifdef in __bamc_put.
(cherry picked from commit b9064d5)
R2 (the equal-key revive for duplicate sets) is now fixed in __bamc_put, so the shared-key bail in __bam_revive_chk only ever sees a set with a live member whose key must not move. The KNOWN-ISSUES R2 row records the full scope (every duplicate shape, not just on-page DUPSORT) and the fix. (cherry picked from commit ca66e9f)
__bam_opd_alldel descended an off-page duplicate tree's internal pages with GET_BINTERNAL for both P_IBTREE and P_IRECNO. Both structs begin with the child pgno so it read the right bytes, but a recno dup tree (unsorted DB_DUP off-page) is P_IRECNO/RINTERNAL, so read it with GET_RINTERNAL. The test now adds a tall off-page tree (3000 members, several dup pages) for DB_DUP and DB_DUPSORT so the internal-page descent is actually exercised, and gives each member a unique 7-byte value so DB_DUPSORT does not reject it. (cherry picked from commit 2e6ae3f)
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
|
Do not merge (lead review). With this change, two equal-key puts over an all-deleted, pinned DUPSORT set create two separate key items instead of one duplicate set:
The reproducer is |
… v2)
The first R2 fix redirected an equal-key put over a fully-deleted duplicate
set to a fresh DB_KEYFIRST insert. That inserted a NEW standalone key item
next to the still-pinned dead set; a second equal-key put then found the dead
set again (an exact match, still all-deleted) and redirected again, producing
a SECOND lone key item. Equal-comparing keys then formed two separate non-dup
key items, which DB->verify rejects ("non-dup dup key"), DBC->count reported 1
instead of 2, and a reverse walk dropped a trailing record. The tree was
structurally corrupt. (Lead-reproduced with r2_walk_corruption.c.)
Fix: the members of a duplicate set share ONE physical key item -- every
member slot's inp[] points at it -- so a single in-place rewrite of that item
changes the key for the whole set at once, and the new member then joins by
the ordinary duplicate path, keeping the set a single set. A second equal-key
put now finds the set already carrying the caller's key bytes, so it simply
joins; no second key item is created.
- on-page, non-overflow key: __bam_ritem replaces the shared item and
re-aims every inp[] offset that referenced it (its __bam_repl log record
replays the same way, so recovery is correct with no new log record);
a growing key that will not fit returns DB_NEEDSPLIT, which __bamc_put's
existing retry splits and re-drives, exactly as for __bam_iitem;
- off-page duplicate tree: the key is a lone (unshared) slot in the parent
leaf, rewritten by delete-and-re-add as R1 does for a non-dup slot, which
also covers overflow keys and type changes on that lone slot.
A deleted set whose SHARED on-page key is an overflow item is left under its
old key bytes (there is no logged in-place rewrite that keeps the shared
on-page offset for an overflow item). That is still one valid duplicate set
-- DB->verify clean, count correct -- identical to master, differing only in
the key bytes; it is not corruption. KNOWN-ISSUES records the residual.
The pinning cursor still sees the dead members as deleted: a key item is never
B_DISSET (only its data slots are), and __bam_ritem preserves that state.
(cherry picked from commit 07a240a)
… fix
Rework the R2 regression for the corruption the first fix caused. Every case
now does TWO equal-comparing puts (data "NEW" then "NE2") -- the second put is
what exposed the first fix's bug, where the redirect made each put a fresh
standalone key item so two "equal" keys ended up as two non-dup key items --
and then asserts, for every case:
- DB->verify is clean (the first fix produced "non-dup dup key");
- DBC->count on the key equals the live set size (the first fix reported 1);
- a forward DB_NEXT walk and a reverse DB_LAST+DB_PREV walk return the same
complete, mirrored set of live records (the first fix dropped a record on
the reverse walk);
- the two NEW records share ONE key item, not two;
- the key bytes are the caller's "ABC" for the shapes the fix rewrites, and
stay "abc" (a valid single set, not corruption) for the one documented
residual: an on-page duplicate set whose SHARED key is an overflow item.
DB->verify runs after every case (txn and no-env), and again after recovery
from the log alone (DB_RECOVER_FATAL, the .db removed) for every txn case.
Coverage: DB_DUP and DB_DUPSORT; on-page and off-page; pin and no pin; a tall
off-page tree; overflow keys on-page (residual) and off-page (rewritten); a
growing key that forces the in-place rewrite to split; the live-member control
(delete 1 of 3 -- the key must not move).
Teeth confirmed on three libraries: the first R2 fix FAILs (DB_VERIFY_BAD +
wrong count), master FAILs (wrong key bytes), the new fix PASSes. The sabotage
arm (bt_cursor.c -DR2_REVIVE_SABOTAGE, rekey disabled) reverts to the master
behaviour and FAILs the key-bytes check.
(cherry picked from commit 2d24d21)
The first R2 fix (redirect an equal-key put over a fully-deleted dup set to a fresh DB_KEYFIRST insert) corrupted the tree: a second equal-key put made a second standalone key item, so two equal-comparing keys became two non-dup key items and DB->verify failed. Update the design note (rfc/0014/R2-DUP-REVIVE.md) and KNOWN-ISSUES to describe the replacement -- rewrite the set's one shared key item in place so the whole set adopts the caller's bytes and the new member joins the single set -- and the narrow documented residual (an on-page set whose shared key is an overflow item keeps its old bytes, still a valid single set). Refresh the __bam_revive_chk comment, which referred to the redirect. (cherry picked from commit 1711687)
A case-insensitive DB_DUP/DB_DUPSORT btree driven with random pins, puts of equal-comparing spellings, and deletes. After every op it requires DB->verify to pass, the live record count per case-folded key to match a model, and every live record of one folded key to carry a single spelling (one duplicate set). This directly catches the first R2 fix, whose redirect made two equal-comparing keys into two separate non-dup key items: 5 of 60 seeds (x300 ops) fail with DB_VERIFY_BAD "non-dup dup key" or "two spellings in one walk". The v2 fix passes 60/60; master passes too (it keeps a valid single set, only the key bytes differ, which the case-folding model tolerates), so the fuzz isolates the structural corruption rather than the benign key-bytes difference. run_r2_fuzz.sh drives 60 seeds x 300 ops (~30 s); MANIFEST entry added. (cherry picked from commit d816880)
gburd
changed the title
Collaborator
Author
|
Ready for re-review. Cherry-picks ( LTS gates:
R1's |
A leftover from an earlier shape of the helper; it was always 1 and COMPQUIET'd. This is the version built and tested on the diagnostic + TCL builds. (cherry picked from commit ab11f6f)
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.
Backport of R2 (equal-key duplicate-set revive) to
lts/2026.10.7Cherry-picks (with
-x) the master R2 fix and its tests. Master PR: #244.This supersedes the earlier version of this backport. The first R2 fix (a
redirect to a fresh
DB_KEYFIRSTinsert) corrupted the btree: a secondequal-key put produced a second standalone key item, so two equal-comparing
keys became two non-dup key items that
DB->verifyrejects. The new commitson this branch replace that approach with an in-place rewrite of the set's
shared key item; the superseded commits are left in history (never rewritten),
and the final state builds the correct fix.
What changed vs master
The master fix touches only
src/btree/bt_cursor.c(self-contained; the threenew helpers are
static). R1's__bam_revive_chkis not on the LTS line,so the master docs commit's cosmetic comment edit to
bt_put.cwas droppedduring the cherry-pick; the design note and
KNOWN-ISSUESchanges were kept.No functional difference.
The fix
The members of a duplicate set share one physical key item, so rewriting it in
place changes the key for the whole set and the new member joins the single
set. On-page non-overflow keys use
__bam_ritem(re-aims all sharinginp[]offsets; its
__bam_repllog record replays identically, so recovery needs nonew log record); a growing key splits and retries. The off-page key is a lone
parent slot, rewritten by delete-and-re-add. An on-page set whose shared key is
an overflow item is left under its old bytes — still a single valid set, not
corruption — documented in
KNOWN-ISSUESR2.LTS gates
0xf1574878, byte-identical to the LTS base(
lts/2026.10.7). No format change.libdb-2026.0.sounchanged; 1947 exported symbols,identical set (helpers are
static); handle sizes unchanged(
DB=1744,DBC=552,DB_ENV=2088,DBT=40).run_r2_dup_reviveFAILs on the LTS base(26 cases wrong — the R2 bug) and PASSes on this branch;
r2_fuzz0/60 FAILon this branch.
(cherry picked from commit …)-xtrailer.
Tests
test/db/r2_dup_revive.c(full matrix, two equal-key puts per case,DB->verify/count/forward+reverse walk after every case, recovery from the logalone, a sabotage arm) and
test/db/r2_fuzz.c(randomised, 60 × 300). Bothrun in the
dbtier viarun_all.sh; MANIFEST entries added.Built and verified
--enable-diagnosticon Debian 12.