Skip to content

MDEV-37541 Race of rolling back and committing transaction to binlog #4301

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 29, 2025
+101 −2
Merged

MDEV-37541 Race of rolling back and committing transaction to binlog #4301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions mysql-test/suite/binlog/r/binlog_mdev37541.result
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
create table t1 (a int primary key, b text) engine=innodb;
connect trx1_rollback,localhost,root,,;
CREATE TABLE t_x (a int) engine=MEMORY;
SET binlog_format=row;
CREATE TEMPORARY TABLE tt_tmp ( id INT ) ENGINE = Memory;
BEGIN;
insert into t_x values (1);
drop temporary table tt_tmp;
insert into t1 values (99, "gotta log first");
SET DEBUG_SYNC= 'reset';
SET DEBUG_SYNC= "before_group_commit_queue SIGNAL trx1_at_log WAIT_FOR trx1_go";
ROLLBACK;
connect trx2_commit,localhost,root,,;
insert into t1 values (99, "second best in binlog");
connection default;
SET DEBUG_SYNC= "now WAIT_FOR trx1_at_log";
SET DEBUG_SYNC= "now SIGNAL trx1_go";
connection trx2_commit;
select * from t1;
a b
99 second best in binlog
# Prove the logging order is Trx1, Trx2
include/show_binlog_events.inc
Log_name Pos Event_type Server_id End_log_pos Info
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
master-bin.000001 # Annotate_rows # # insert into t_x values (1)
master-bin.000001 # Table_map # # table_id: # (test.t_x)
master-bin.000001 # Write_rows_v1 # # table_id: # flags: STMT_END_F
master-bin.000001 # Query # # COMMIT
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
master-bin.000001 # Annotate_rows # # insert into t1 values (99, "gotta log first")
master-bin.000001 # Table_map # # table_id: # (test.t1)
master-bin.000001 # Write_rows_v1 # # table_id: # flags: STMT_END_F
master-bin.000001 # Query # # ROLLBACK
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
master-bin.000001 # Query # # use `test`; insert into t1 values (99, "second best in binlog")
master-bin.000001 # Xid # # COMMIT /* XID */
drop table t_x, t1;
disconnect trx1_rollback;
disconnect trx2_commit;
# end of the tests
50 changes: 50 additions & 0 deletions mysql-test/suite/binlog/t/binlog_mdev37541.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
--source include/have_debug_sync.inc
--source include/have_innodb.inc
--source include/have_binlog_format_mixed.inc

create table t1 (a int primary key, b text) engine=innodb;

connect(trx1_rollback,localhost,root,,);
CREATE TABLE t_x (a int) engine=MEMORY;

--let $master_file= query_get_value(SHOW MASTER STATUS, File, 1)
--let $binlog_start=query_get_value(SHOW MASTER STATUS, Position, 1)
SET binlog_format=row;
CREATE TEMPORARY TABLE tt_tmp ( id INT ) ENGINE = Memory;
BEGIN;
insert into t_x values (1);
drop temporary table tt_tmp;
insert into t1 values (99, "gotta log first");

SET DEBUG_SYNC= 'reset';
SET DEBUG_SYNC= "before_group_commit_queue SIGNAL trx1_at_log WAIT_FOR trx1_go";

--send ROLLBACK

connect(trx2_commit,localhost,root,,);
--send insert into t1 values (99, "second best in binlog")

connection default;

# Make sure ROLLBACK is in the binlogging phase ..
SET DEBUG_SYNC= "now WAIT_FOR trx1_at_log";
# .. and the contender trx2 in the locking phase ..
let $wait_condition=
select count(*) = 1 from information_schema.innodb_trx
where trx_state = "LOCK WAIT" and trx_query like "%insert into t1 values%";
source include/wait_condition.inc;
# .. when both provided unfreeze the trx:s ..
SET DEBUG_SYNC= "now SIGNAL trx1_go";

connection trx2_commit;
reap;
select * from t1;

# .. to observe proper binlogging.
--echo # Prove the logging order is Trx1, Trx2
--source include/show_binlog_events.inc

drop table t_x, t1;
disconnect trx1_rollback;
disconnect trx2_commit;
--echo # end of the tests
11 changes: 9 additions & 2 deletions sql/handler.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2308,11 +2308,18 @@ int ha_rollback_trans(THD *thd, bool all)
if (is_real_trans) /* not a statement commit */
thd->stmt_map.close_transient_cursors();

int err;
if (has_binlog_hton(ha_info) &&
(err= binlog_hton->rollback(binlog_hton, thd, all)))
Copy link
Contributor

@bnestere bnestere Sep 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't had time to fully investigate some of my questions/concerns before I leave for vacation, but I figured I'd at least voice them before I leave.

@knielsen mentions on JIRA

Hm. Normally this kind of wrong order is prevented by ensuring that binlogging always happens before releasing table and row locks.
Apparently, this is violated here for ROLLBACK. Not sure why or how the ROLLBACK is executed before binlogging happens.

Which I interpret as supporting your solution. But to me it looks a bit unconventional:

  • Why does the 2pc coordinator/orderer not consider rollback (or does it and I just misunderstand)?
  • Would it be better to integrate the rollback case with the 2pc coordinator?

Otherwise, it seems we may run other opposite binlog-order problems when rollback is BINLOG->ENGINE and commit can be ENGINE->BINLOG for direct writes.

Copy link
Contributor Author

@andrelkin andrelkin Sep 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the 2pc coordinator/orderer not consider rollback

I am somewhat lost with the question.. Let me make it fully clear, the matter at hand is about the engine rollback
runs ahead of its order in the 2pc protocol. The engine completes (a similar would be with COMMIT, but it does not exist that thanks to the same measure as in the patch) the trx before binlog (as coordinator) gets informed.

{
my_error(ER_ERROR_DURING_COMMIT, MYF(0), err);
error= 1;
}
for (; ha_info; ha_info= ha_info_next)
{
int err;
handlerton *ht= ha_info->ht();
if ((err= ht->rollback(ht, thd, all)))

if (ht != binlog_hton && (err= ht->rollback(ht, thd, all)))
{
// cannot happen
my_error(ER_ERROR_DURING_ROLLBACK, MYF(0), err);
Expand Down
1 change: 1 addition & 0 deletions sql/log.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8235,6 +8235,7 @@ MYSQL_BIN_LOG::queue_for_group_commit(group_commit_entry *orig_entry)
bool
MYSQL_BIN_LOG::write_transaction_to_binlog_events(group_commit_entry *entry)
{
DEBUG_SYNC(entry->thd, "before_group_commit_queue");
int is_leader= queue_for_group_commit(entry);
#ifdef WITH_WSREP
/* commit order was released in queue_for_group_commit() call,
Expand Down