Skip to content

MDEV-40378 heap: roll back key changes when a blob write fails in heap_update()#5407

Open
arcivanov wants to merge 1 commit into
MariaDB:preview-13.1-previewfrom
arcivanov:MDEV-40378
Open

MDEV-40378 heap: roll back key changes when a blob write fails in heap_update()#5407
arcivanov wants to merge 1 commit into
MariaDB:preview-13.1-previewfrom
arcivanov:MDEV-40378

Conversation

@arcivanov

Copy link
Copy Markdown
Contributor

Problem

heap_update() moves all changed key entries to the new key values before writing the new blob chains. When a blob chain write then fails (e.g. HA_ERR_RECORD_FILE_FULL on a full HEAP table), the rollback restores the record bytes and blob chain pointers, but the err: label only undoes key changes for HA_ERR_FOUND_DUPP_KEY — historically the only possible failure once the key loop had run.

The hash/btree entries are left keyed on the new values while pointing at a record holding the old values, corrupting the index:

  1. index lookups by the old key value miss the row;
  2. CHECK TABLE reports the table corrupt;
  3. on debug builds the heap consistency check in ha_heap::external_lock() raises a second error into an already-set diagnostics area, firing a Diagnostics_area assertion on the next statement.

Fix

The blob-write failure gets its own recovery label err_blob: that moves every changed key back to the old value (delete_key(new) + write_key(old)), mirroring the existing HA_ERR_FOUND_DUPP_KEY recovery. All keys are fully moved before the blob section runs, so a plain sweep over all changed keys suffices, and the record at pos has already been restored to the old image — exactly the state the DUPP_KEY recovery path operates in.

The blob failure's errno is captured before the rollback loop and restored after, so a rollback write_key failure (which hp_rb_write_key() reports as HA_ERR_FOUND_DUPP_KEY with a stale errkey) cannot mask the original error. err_blob: then falls through into err: to share the record-count/blength epilogue, which is safe because the restored errno is never HA_ERR_FOUND_DUPP_KEY.

Test

New heap.blob_update_key_rollback exercises:

  • hash index, single-row UPDATE;
  • BTREE index, single-row UPDATE;
  • two changed indexes (both rolled back);
  • an index on an unchanged column (rollback must leave it untouched);
  • a partial multi-row UPDATE (invariant-only assertions, no platform-dependent fill point).

Each asserts index consistency after the failure via CHECK TABLE and index lookups. Verified to crash the unfixed server (DA assertion / index-corrupt) and pass with the fix; full heap suite passes (28/28). gcov confirms every executable line of the new recovery block is exercised.

…ap_update()`

`heap_update()` moves all changed key entries to the new key values
**before** writing the new blob chains. When a blob chain write then
failed (e.g. with `HA_ERR_RECORD_FILE_FULL`), the rollback restored the
record bytes and blob chain pointers, but the `err:` label only undid
key changes for `HA_ERR_FOUND_DUPP_KEY` -- historically the only
possible failure once the key loop had run. The hash/btree entries were
left keyed on the new values while pointing at a record holding the old
values, corrupting the index:

1. index lookups by the old key value missed the row
2. `CHECK TABLE` reported the table corrupt
3. on debug builds the heap consistency check in
   `ha_heap::external_lock()` raised a second error into an already-set
   diagnostics area, firing a `Diagnostics_area` assertion on the next
   statement

Fix: give the blob-write failure its own recovery label (`err_blob:`)
that moves every changed key back to the old value
(`delete_key(new)` + `write_key(old)`), mirroring the existing
`HA_ERR_FOUND_DUPP_KEY` recovery. All keys were fully moved before the
blob section runs, so a plain sweep over all changed keys suffices, and
the record at `pos` has already been restored to the old image --
exactly the state the `DUPP_KEY` recovery path operates in.

The blob failure's errno is captured before the rollback loop and
restored after it, so that a rollback `write_key` failure (which
`hp_rb_write_key()` reports as `HA_ERR_FOUND_DUPP_KEY` with a stale
`errkey`) cannot mask the original error. `err_blob:` then falls through
into `err:` to share the record-count/blength epilogue, which is safe
because the restored errno is never `HA_ERR_FOUND_DUPP_KEY`.

The new test `heap.blob_update_key_rollback` exercises hash, BTREE, two
changed indexes, an index on an unchanged column (which the rollback
must leave untouched), and a partial multi-row UPDATE; each asserts the
index stays consistent after the failure via `CHECK TABLE` and index
lookups.
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant