Skip to content

MDEV-37229: Set proper trx isolation level before every applied event #512

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

Closed
wants to merge 1 commit into from
Closed

MDEV-37229: Set proper trx isolation level before every applied event #512

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
26 changes: 26 additions & 0 deletions mysql-test/suite/galera/r/galera_binlog_stmt_autoinc.result
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,28 @@ i c
3 dummy_text
5 dummy_text
7 dummy_text
insert into t1(i) values(null);
Warnings:
Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. Statement is unsafe because it uses a system variable that may have a different value on the slave
insert into t1(i) values(null);
Warnings:
Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. Statement is unsafe because it uses a system variable that may have a different value on the slave
insert into t1(i) values(null);
Warnings:
Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. Statement is unsafe because it uses a system variable that may have a different value on the slave
insert into t1(i) values(null);
Warnings:
Note 1592 Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT. Statement is unsafe because it uses a system variable that may have a different value on the slave
select * from t1 order by i;
i c
1 dummy_text
3 dummy_text
5 dummy_text
7 dummy_text
9 dummy_text
11 dummy_text
13 dummy_text
15 dummy_text
connection node_2;
show variables like 'auto_increment%';
Variable_name Value
Expand All @@ -44,6 +66,10 @@ i c
3 dummy_text
5 dummy_text
7 dummy_text
9 dummy_text
11 dummy_text
13 dummy_text
15 dummy_text
SET GLOBAL wsrep_forced_binlog_format='none';
connection node_1;
SET GLOBAL wsrep_forced_binlog_format='none';
Expand Down
1 change: 1 addition & 0 deletions mysql-test/suite/galera/t/galera_binlog_stmt_autoinc.cnf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ auto_increment_offset=1
auto_increment_increment=1

[mysqld.2]
wsrep_slave_threads=2
auto_increment_offset=2
auto_increment_increment=1
11 changes: 10 additions & 1 deletion mysql-test/suite/galera/t/galera_binlog_stmt_autoinc.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,17 @@ insert into t1(i) values(null), (null), (null);

select * from t1 order by i;

# Ensure both appliers have some work to perform in parallel.
# This way we can see they both have proper isolation level set.
insert into t1(i) values(null);
insert into t1(i) values(null);
insert into t1(i) values(null);
insert into t1(i) values(null);

select * from t1 order by i;

--connection node_2
--let $wait_condition = SELECT COUNT(*) = 4 FROM t1;
--let $wait_condition = SELECT COUNT(*) = 8 FROM t1;
--source include/wait_condition.inc
show variables like 'auto_increment%';
select * from t1 order by i;
Expand Down
13 changes: 13 additions & 0 deletions sql/wsrep_applier.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,19 @@ int wsrep_apply_events(THD* thd,
}
}

/* Statement-based replication requires InnoDB repeatable read
Copy link
Collaborator

@mariadb-TeemuOllakka mariadb-TeemuOllakka Oct 30, 2025

Choose a reason for hiding this comment

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

I wonder if this is a correct fix. It may make the test pass for sure, but the purpose of the wsrep_forced_binlog_format is to force given binlog format on master node. Here it is used in applier context. There is no guarantee that this variable would have the same value on all of the nodes.

Would there be a better way check whether the isolation level should be changed, e.g. by looking at event type? If the type is QUERY_EVENT, use repeatable read, otherwise read committed.

This fix also does not address the fact that the isolation level is left to repeatable read after IST (any idea why is that?), maybe that should be at least mentioned in commit message.

Copy link

@mariadb-DenisProtivensky mariadb-DenisProtivensky Oct 30, 2025

Choose a reason for hiding this comment

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

I don't remember the details of IST, I'll have to recall that. Concerning the QUERY_EVENT, is it only used for statement-based replication?

Copy link

@janlindstrom janlindstrom Oct 30, 2025

Choose a reason for hiding this comment

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

In my understanding Query event is also used in TOI

transaction isolation level or higher. */
if (wsrep_forced_binlog_format == BINLOG_FORMAT_STMT)
{
thd->variables.tx_isolation= ISO_REPEATABLE_READ;
thd->tx_isolation = ISO_REPEATABLE_READ;
}
else
{
thd->variables.tx_isolation= ISO_READ_COMMITTED;
thd->tx_isolation = ISO_READ_COMMITTED;
}

if (LOG_EVENT_IS_WRITE_ROW(typ) ||
LOG_EVENT_IS_UPDATE_ROW(typ) ||
LOG_EVENT_IS_DELETE_ROW(typ))
Expand Down