MDEV-40378 heap: roll back key changes when a blob write fails in heap_update()#5407
Open
arcivanov wants to merge 1 commit into
Open
MDEV-40378 heap: roll back key changes when a blob write fails in heap_update()#5407arcivanov wants to merge 1 commit into
heap_update()#5407arcivanov wants to merge 1 commit into
Conversation
…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.
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
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.
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_FULLon a full HEAP table), the rollback restores the record bytes and blob chain pointers, but theerr:label only undoes key changes forHA_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:
CHECK TABLEreports the table corrupt;ha_heap::external_lock()raises a second error into an already-set diagnostics area, firing aDiagnostics_areaassertion 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 existingHA_ERR_FOUND_DUPP_KEYrecovery. All keys are fully moved before the blob section runs, so a plain sweep over all changed keys suffices, and the record atposhas already been restored to the old image — exactly the state theDUPP_KEYrecovery path operates in.The blob failure's
errnois captured before the rollback loop and restored after, so a rollbackwrite_keyfailure (whichhp_rb_write_key()reports asHA_ERR_FOUND_DUPP_KEYwith a staleerrkey) cannot mask the original error.err_blob:then falls through intoerr:to share the record-count/blength epilogue, which is safe because the restored errno is neverHA_ERR_FOUND_DUPP_KEY.Test
New
heap.blob_update_key_rollbackexercises:Each asserts index consistency after the failure via
CHECK TABLEand index lookups. Verified to crash the unfixed server (DA assertion / index-corrupt) and pass with the fix; fullheapsuite passes (28/28). gcov confirms every executable line of the new recovery block is exercised.