MDEV-38839 Assertion (thd->state_flags & Open_tables_state::BACKUPS_A…#5027
MDEV-38839 Assertion (thd->state_flags & Open_tables_state::BACKUPS_A…#5027pranavktiwari wants to merge 3 commits into12.3from
Conversation
pranavktiwari
force-pushed
the
12.3-MDEV-38839
branch
4 times, most recently
from
58ff35c to
afea455
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes binlog table-access classification for CREATE TABLE ... SELECT when the source is a temporary table, aiming to avoid incorrectly treating read-only temp-table access as a write and thereby prevent MIXED-mode binlog cache state from getting stuck and triggering the reported assertion.
Changes:
- Add a dedicated
stmt_writes_to_non_trans_temp_table()helper and use it when marking non-transactional temp-table modifications in binlog write paths. - Adjust
THD::decide_logging_format()soCREATE TABLE ... SELECTcan classify temporary source-table access as reads instead of writes. - Add an MTR regression test covering several temp-table engine combinations around
CREATE TABLE ... SELECT ... FOR UPDATE.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
sql/sql_lex.h |
Adds a helper to distinguish non-transactional temp-table writes from general access. |
sql/sql_class.cc |
Changes table-access flag assignment during logging-format analysis for CREATE TABLE. |
sql/log.cc |
Narrows the condition that marks a non-transactional temp table as modified in transactional cache paths. |
mysql-test/main/binlog_mdev38839.test |
Adds a regression test for the reported assertion scenario. |
mysql-test/main/binlog_mdev38839.result |
Records the expected output for the new regression test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pranavktiwari
force-pushed
the
12.3-MDEV-38839
branch
from
afea455 to
9bd571d
Compare
| !lex->tmp_table() && !found_first_not_own_table) | ||
| lex->set_stmt_accessed_table(trans ? | ||
| LEX::STMT_READS_TEMP_TRANS_TABLE : | ||
| LEX::STMT_READS_TEMP_NON_TRANS_TABLE); |
There was a problem hiding this comment.
Maybe it is better to set READS flag in another if() below vvvv. This if (tbl->lock_type >= TL_FIRST_WRITE) should be completely excluded for such tables? But really I'm not sure if not setting WRITES is a good idea. How SELECT .. FOR UPDATE behaves now on slave? Does it lose FOR UPDATE locking? This at least should be explained in commit message if it is not important for slave.
There was a problem hiding this comment.
@pranavktiwari I would still like to get these answers:
How SELECT .. FOR UPDATE behaves now on slave? Does it lose FOR UPDATE locking?
If it is temporary table, does it make sense at all? Worth mentioning in commit message.
There was a problem hiding this comment.
In MIXED mode, CREATE TABLE ... SELECT * FROM t FOR UPDATE switches to row-based logging because of the FOR UPDATE on MyISAM. What gets written to the binlog is:
CREATE TABLE t ... — the DDL
Row events (WRITE_ROWS_EVENT) — the actual row data selected
There will not be any direct effect of for update.
There was a problem hiding this comment.
But there is no check for MIXED mode in your patch, so it works for any mode, isn't it?
pranavktiwari
force-pushed
the
12.3-MDEV-38839
branch
from
f380b5e to
10c48d3
Compare
| !lex->tmp_table() && !found_first_not_own_table) | ||
| lex->set_stmt_accessed_table(trans ? | ||
| LEX::STMT_READS_TEMP_TRANS_TABLE : | ||
| LEX::STMT_READS_TEMP_NON_TRANS_TABLE); |
There was a problem hiding this comment.
@pranavktiwari I would still like to get these answers:
How SELECT .. FOR UPDATE behaves now on slave? Does it lose FOR UPDATE locking?
If it is temporary table, does it make sense at all? Worth mentioning in commit message.
pranavktiwari
force-pushed
the
12.3-MDEV-38839
branch
3 times, most recently
from
2393bb5 to
f3b5b86
Compare
| binlog_truncate_trx_cache() in MIXED mode and cause an assertion | ||
| in close_thread_tables(). | ||
| */ | ||
| if (tbl->updating || tbl->sequence) |
There was a problem hiding this comment.
Shouldn't this condition be in place of if (tbl->lock_type >= TL_FIRST_WRITE) above? Why tbl->updating cannot be set for tbl->sequence? That would make your comment shorter.
pranavktiwari
force-pushed
the
12.3-MDEV-38839
branch
from
f3b5b86 to
76b8145
Compare
…LECT FOR UPDATE with MyISAM temp table in MIXED binlog mode FOR UPDATE on a MyISAM table acquires TL_WRITE due to lack of row-level locking. In decide_logging_format(), this caused STMT_WRITES_TEMP_NON_TRANS_TABLE to be set for a temp table that was only being read, not written to. This incorrectly set MODIFIED_NON_TRANS_TABLE via mark_modified_non_trans_temp_table() in MYSQL_BIN_LOG::write(), which blocked binlog_truncate_trx_cache() in MIXED mode, leaving row events stranded in the cache and triggering the assertion: Fix: In decide_logging_format(), gate the write flag assignment on tbl->updating, which is false for FOR UPDATE (read-only access with write lock) and true for genuine writes (INSERT/UPDATE/DELETE). Sequences are handled separately via tbl->sequence since SELECT NEXT VALUE genuinely modifies the sequence table internally but has tbl->updating=false. Note: SELECT...FOR UPDATE locking is not replicated to the slave. In MIXED mode the statement is logged as row events representing the resulting data changes. The slave replays only those row events so FOR UPDATE locking semantics are irrelevant on the slave and are not affected by this fix.
pranavktiwari
force-pushed
the
12.3-MDEV-38839
branch
from
76b8145 to
fa8cf40
Compare
MDEV-38839: Fix assertion in close_thread_tables on CREATE TABLE...SELECT FOR UPDATE with MyISAM temp table in MIXED binlog mode
Cause: FOR UPDATE on MyISAM temp tables uses a write lock (no row-level locking), incorrectly marking a read as a write. This blocks binlog_truncate_trx_cache() and triggers an assertion.
Fix: In decide_logging_format(), check tbl->updating to detect real writes. For read-only access, mark as STMT_READS_TEMP_NON_TRANS_TABLE instead of write, preventing incorrect behavior.