Skip to content

Extend the disjoint-slot merge to plain record deletes (follow-up to #5279) #5569

Description

@lvca

Summary

Follow-up to #5279 (PR #5566). A plain single-record delete is the last common false-conflict class left on a bucket page: deleting record A still makes every concurrent transaction that merely updated record B on the same page fail with a ConcurrentModificationException, even though the two writes commute.

Where it stands today

TX_PAGE_SLOT_MERGE now replays, on top of the newer committed page:

Deletes are excluded wholesale. LocalBucket.deleteRecordInternal poisons the page as its very first action, before it has even read the record:

// DISJOINT-SLOT MERGE (#5381): a delete frees a slot and can relink placeholder/chunk records elsewhere, so
// it is never a pure single-slot change - keep the page out of the slot merge.
final TransactionContext slotTx = database.getTransactionIfExists();
if (slotTx != null && slotTx.isSlotMergeEnabled())
  slotTx.poisonSlotRebasePage(fileId, pageId);

That is correct but coarse. The reasoning holds for a placeholder pointer (its content record lives on another page) and for a FIRST_CHUNK chain, but a plain in-place record is deleted by writing 0 into its slot-table entry (plus an optional content wipe-out when BUCKET_WIPEOUT_ONDELETE is on) - a single-slot change that commutes with writes to every other slot, exactly like the shapes the merge already covers.

Proposed change

  1. Move the poison from the top of deleteRecordInternal to after the record's marker shape is known. Keep poisoning for: placeholder pointer, placeholder content, FIRST_CHUNK / NEXT_CHUNK, and the corrupted-slot branch.
  2. For a plain positive size marker, track the delete as rebasable: trackRebasableDelete(fileId, pageNumber, slot, baseBody) where baseBody is the pre-image, captured before the wipe-out.
  3. Add a third kind to TransactionContext.SlotRebaseBuffer. A null value in finalBody is the natural marker for "this slot is deleted" (HashMap permits null values), so rebaseSlots keeps iterating one map.
  4. LocalBucket.rebaseRecordOnPage gains a delete mode: verify the committed slot still holds the pre-image byte for byte (so a concurrent write to the same record is still a true conflict), then write 0 into the slot-table entry, wipe the content if configured, and update the page statistics.

Cases to get right

  • Insert-then-delete in the same transaction. The slot never existed on the committed page, so a pre-image check would fail and turn a perfectly mergeable transaction into a conflict. The replay must recognise the slot as insertedSlots + deleted and skip it entirely (the net effect of the transaction on that slot is "nothing").
  • Delete-then-insert on the same slot in one transaction cannot happen, so it needs no handling: LocalBucket.getFreeSpaceInPage deliberately never re-uses a slot freed by the current transaction before it commits (guarded by Issue5279ConcurrentInsertTest.deletedSlotsAreRecycledByTheNextInsert). Worth asserting explicitly so a future change to that policy cannot silently break the replay.
  • A delete that frees the last records of a page must leave recordCountInPage consistent with what compressPage expects, since the commit path compresses a rebased page right after the replay.
  • The record-count delta, deletedRecordsInTx and the index changes are all transaction-level and unaffected by which page image is committed, so they need no special handling - worth confirming with a test that a merged delete still removes the index entries.

Tests

  • Concurrent delete of record A + update of record B on one page, attempts=1: must not conflict, A gone, B exact.
  • Concurrent deletes of different records on one page: must not conflict.
  • Delete of a record another transaction is updating: must still raise ConcurrentModificationException, and the surviving value must be the winner's.
  • Delete of a placeholder / multi-page record co-located with a tracked write: must still fall back cleanly (this is what Issue5381FalseConflictTest.deleteOnSharedPageFallsBackAndStaysCorrect asserts today - its rationale will need rewording, since a plain delete would no longer fall back).
  • check database clean after a contended mix of inserts, updates and deletes on a single-bucket type.

Why it is worth doing

The workload behind #5279 (many concurrent users on a type with few buckets) deletes as well as inserts and updates, and a delete on a hot page currently poisons it for every co-located writer. This is the last shape where the engine reports a conflict that provably is not one.

Metadata

Metadata

Assignees

Labels

concurrencyThreading / concurrency / MVCCenhancementNew feature or requeststorageStorage/paging/WAL engine layer

Type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions