Skip to content

Conversation

@denis-protivensky
Copy link

@denis-protivensky denis-protivensky commented Jul 14, 2025

Apparently, applier thread performing IST would be left with REPEATABLE_READ isolation level.

Every applier thread in Galera should run with READ_COMMITTED transaction isolation level to prevent applying issues caused by InnoDB gap locks.

The exception is statement-based replication, where REPEATABLE_READ is required by the server code.

@denis-protivensky denis-protivensky marked this pull request as ready for review July 14, 2025 16:25
Copy link

@janlindstrom janlindstrom left a comment

Choose a reason for hiding this comment

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

LGTM.

}
}

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

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.

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?

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

@mariadb-DenisProtivensky

Closing in favor of MariaDB#4422

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants