MDEV-26057 Assertion `!vcol->v_indexes.empty() in trx_undo_log_v_idx - #5631
MDEV-26057 Assertion `!vcol->v_indexes.empty() in trx_undo_log_v_idx#5631Thirunarayanan wants to merge 1 commit into
Conversation
Problem: ======== - Rollback of an INPLACE ALTER TABLE is executed while holding only a shared metadata lock on the table, so DML can run concurrently. rollback_inplace_alter_table() resets dict_col_t::ord_part in a critical section of its own, after row_merge_drop_indexes() already removed the aborted indexes from the dictionary cache and emptied dict_v_col_t::v_indexes. During this time, DML statement can see a virtual column with ord_part set and an empty v_indexes, which makes assert failure in trx_undo_report_insert_virtual(). Solution: ======== row_merge_reset_ord_part(): Added a function to reset dict_col_t::ord_part for the columns that are no longer a field of any index remaining in the dictionary cache. For virtual columns the decision is based on dict_v_col_t::v_indexes being empty, and no element is ever removed from that list. row_merge_drop_indexes(): Added a call to row_merge_reset_ord_part() in the branch that removes the indexes from the cache, in the same dict_sys.latch critical section. That branch is taken only when MDL_EXCLUSIVE is held or when this is the only handle to the table, so no concurrent DML can observe the intermediate state. In the lazy drop branch the indexes and their v_indexes entries stay in the cache and nothing is reset; that is done later, when the indexes are dropped while holding MDL_EXCLUSIVE. check_col_exists_in_indexes(): Removed the only_committed parameter, which no longer has any caller.
|
|
iMineLink
left a comment
iMineLink
left a comment
There was a problem hiding this comment.
I failed in having the MTR test reproduce the assertion on a build with the storage/ hunks reverted: I get a timeout instead. Code change looks sensible but I hope the test could reproduce the issue un an unpatched build.
| send set STATEMENT DEBUG_DBUG="d,innodb_OOM_inplace_alter" FOR alter table t1 add index(f3), algorithm=inplace; | ||
|
|
||
| connect(con1,localhost,root,,,); | ||
| SET DEBUG_SYNC="now WAIT_FOR dml_start"; |
There was a problem hiding this comment.
This times out with the storage/ hunks reverted, and does not reproduce the assertion.
There was a problem hiding this comment.
🟢 Approval recommended
The race is addressed within the dictionary-latch critical section and covered by a targeted regression test, with only a minor annotation typo remaining.
Pull request overview
Fixes MDEV-26057 by atomically resetting stale index metadata during failed INPLACE ALTER rollback.
Changes:
- Resets
ord_partwhile holdingdict_sys.latch. - Removes obsolete committed-index filtering.
- Adds a synchronized concurrency regression test.
File summaries
| File | Description |
|---|---|
storage/innobase/row/row0merge.cc |
Performs atomic metadata cleanup. |
storage/innobase/handler/handler0alter.cc |
Removes delayed cleanup and obsolete parameter. |
storage/innobase/trx/trx0rec.cc |
Adds test synchronization. |
mysql-test/suite/innodb/t/innodb-virtual-columns-debug.test |
Reproduces concurrent rollback and DML. |
mysql-test/suite/innodb/r/innodb-virtual-columns-debug.result |
Records expected regression-test output. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| DROP TABLE t1; | ||
|
|
||
| --echo # | ||
| --echo # MDEV-26057 Assertion `!vcol->v_indexes.empty() in trx_undo_log_v_idx |
Problem:
Solution:
row_merge_reset_ord_part(): Added a function to reset dict_col_t::ord_part for the columns that are no longer a field of any index remaining in the dictionary cache.
For virtual columns the decision is based on dict_v_col_t::v_indexes being empty, and no element is ever removed from that list.
row_merge_drop_indexes(): Added a call to row_merge_reset_ord_part() in the branch that removes the indexes from the cache, in the same dict_sys.latch critical section. That branch is taken only when MDL_EXCLUSIVE is held or when this is the only handle to the table, so no concurrent DML can observe the intermediate state. In the lazy drop branch the indexes and their v_indexes entries stay in the cache and nothing is reset; that is done later, when the indexes are dropped while holding MDL_EXCLUSIVE.
check_col_exists_in_indexes(): Removed the only_committed parameter, which no longer has any caller.