MDEV-35658 Assertion `commit_trx' failed in test galera_as_master #3753
Conversation
This file contains 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
|
|
|
I don’t know the replication code at all. Perhaps @andrelkin or @knielsen would be better reviewers? |
andrelkin
requested changes
Jan 14, 2025
sql/log.cc
Outdated
| @@ -7761,7 +7761,7 @@ MYSQL_BIN_LOG::write_transaction_to_binlog(THD *thd, | |||
| { | |||
| DBUG_RETURN(0); | |||
| } | |||
| else if (!(thd->variables.option_bits & OPTION_BIN_LOG)) | |||
| else if (!WSREP(thd) && !(thd->variables.option_bits & OPTION_BIN_LOG)) | |||
There was a problem hiding this comment.
I'd prefer to isolate !WSREP(thd) && with
#ifdef WITH_WSREP
!WSREP(thd) &&
#endif
which would be also consistent with l.8671.
There was a problem hiding this comment.
Should be fixed now.
The test issues a simple INSERT statement, while sql_log_bin = 0. This option disables writes to binlog. However, since MDEV-7205, the option does not affect Galera, so changes are still replicated. So sql_log_bin=off, "partially" disabled the binlog and the INSERT will involve both binlog and innodb, thus requiring internal 2 phase commit (2PC). In 2PC INSERT is first prepared, which will make it transition to PREPARED state in innodb, and later committed which causes the new assertion from MDEV-24035 to fail. Running the same test with sql_log_bin enabled also results in 2PC, but the execution has one more step for ordered commit, between prepare and commit. Ordered commit causes the transaction state to transition back to TRX_STATE_NOT_STARTED. Thus avoiding the assertion. This patch makes sure that when sql_log_bin=off, the ordered commit step is not skipped, thus going through the expected state transitions in the storage engine.
sciascid
force-pushed
the
10.6-MDEV-35658
branch
from
ab148fc to
c0abe2e
Compare
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.
Description
Fix assertion
commit_trxin innobase_commit() during MTR test galera.galera_as_master.The test issues a simple INSERT statement, while sql_log_bin = 0. This option disables writes to binlog. However, since MDEV-7205, the option does not affect Galera, so changes are still replicated. So sql_log_bin=off, "partially" disabled the binlog and the INSERT will involve both binlog and innodb, thus requiring internal 2 phase commit (2PC). In 2PC INSERT is first prepared, which will make it transition to PREPARED state in innodb, and later committed which causes the new assertion from MDEV-24035 to fail.
Running the same test with sql_log_bin enabled also results in 2PC, but the execution has one more step for ordered commit, between prepare and commit. Ordered commit causes the transaction state to transition back to TRX_STATE_NOT_STARTED. Thus avoiding the assertion. This patch makes sure that when sql_log_bin=off, the ordered commit step is not skipped, thus going through the expected state transitions in the storage engine.
Release Notes
Nothing.
How can this PR be tested?
Should fix MTR test galera.galera_as_master.
Basing the PR against the correct MariaDB version
mainbranch.PR quality check