Re: [Maria-developers] 9ea85a70a75: MDEV-24654 GTID event falsely marked transactional
Hi, Andrei! On Jan 11, Andrei Elkin wrote:
Howdy, Sergei!
To the question of the usage of trX cache by non-trX let me answer broadly to mention @@binlog_direct_non_transactional_update = false leads to aggregation of mixed, say innodb + myisam, events in trx cache.
That's different. The bug summary is "GTID event falsely marked transactional", meaning, the group of events is not transactional and it's falsely marked as transactional. If you have an innodb/myisam mix, it is correctly marked transactional. You need to have only non-transactional events in the transactional cache. And I'm trying to understand how can that happen.
I may need to process deeper the referred comments though.
Next to the commit's idea, it actually covers your concern. I wonder on the 'Wrong' conclusion. There is no intent to xidify anything extra. Th commit assumes xid eVent is created, before Gtid one, accordingly. And that fact is propagated to Gtid ctor.
Your commit, if I'm not mistaken, assumes that "last event is Xid if and only if the cache contains a transaction". Which is incorrect, a transaction does not necesarily have to end with a Xid event. /Sergei
On Mon, 10 Jan 2022, 20:56 Sergei Golubchik, <serg@mariadb.org> wrote:
Hi, Andrei!
On Jan 10, Andrei Elkin wrote:
revision-id: 9ea85a70a75 (mariadb-10.2.40-4-g9ea85a70a75) parent(s): 160d97a4aaa author: Andrei Elkin committer: Andrei Elkin timestamp: 2021-08-08 14:20:57 +0300 message:
MDEV-24654 GTID event falsely marked transactional
GTID event can be falsely marked transactional in few cases including binary-logging on the slave side upon execution. In some execution branches bin-logging of non-transactional events can be done through transactional cache. E.g see comments in THD::binlog_write_table_map().
I saw that and still couldn't understand why bin-logging of non-transactional events can be done thr-ough transactional cache.
In order to not create the false 'trans' tag the fact of Xid event logging is checked to compute correct argument to Gtid_log_event ctor.
This is wrong. Transactions only end with a Xid event if all participating engines support XA. Try to create an InnoDB+FederatedX transaction (I just did) and it'll end with Query_log_event("COMMIT")
diff --git a/sql/log.cc b/sql/log.cc index 1d9b4645421..7fef2e0d739 100644 --- a/sql/log.cc +++ b/sql/log.cc @@ -8118,9 +8118,11 @@ MYSQL_BIN_LOG::write_transaction_or_stmt(group_commit_entry *entry, uint64 commit_id) { binlog_cache_mngr *mngr= entry->cache_mngr; + bool has_xid= entry->end_event->get_type_code() == XID_EVENT; DBUG_ENTER("MYSQL_BIN_LOG::write_transaction_or_stmt");
- if (write_gtid_event(entry->thd, false, entry->using_trx_cache, commit_id)) + if (write_gtid_event(entry->thd, false, + entry->using_trx_cache && has_xid, commit_id)) DBUG_RETURN(ER_ERROR_ON_WRITE);
if (entry->using_stmt_cache && !mngr->stmt_cache.empty() &&
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
Sergei Golubchik <serg@mariadb.org> writes:
To the question of the usage of trX cache by non-trX let me answer broadly to mention @@binlog_direct_non_transactional_update = false leads to aggregation of mixed, say innodb + myisam, events in trx cache.
That's different. The bug summary is "GTID event falsely marked transactional", meaning, the group of events is not transactional and it's falsely marked as transactional. If you have an innodb/myisam mix, it is correctly marked transactional.
Jumping into a disucssion here, so apologies if I misunderstood, but... The point of the "transactional" mark in GTID is to inform parallel replication that the entire event group is safe for optimistic parallel replication - basically that it can be rolled back. So if a event group contains innodb/myisam mix, and is marked as "transactional", that is not correct. This would allow parallel replication to speculatively execute and roll back the mix, and the myisam changes would remain and cause replication to break. Hope this helps, - Kristian.
Hi, Kristian! On Jan 11, Kristian Nielsen wrote:
Sergei Golubchik <serg@mariadb.org> writes:
To the question of the usage of trX cache by non-trX let me answer broadly to mention @@binlog_direct_non_transactional_update = false leads to aggregation of mixed, say innodb + myisam, events in trx cache.
That's different. The bug summary is "GTID event falsely marked transactional", meaning, the group of events is not transactional and it's falsely marked as transactional. If you have an innodb/myisam mix, it is correctly marked transactional.
Jumping into a disucssion here, so apologies if I misunderstood, but...
In a way :) It doesn't answer any of my questions, but it's an important insight - I didn't know that and how I have even more questions. Thanks for this clarification, it was very helpful!
The point of the "transactional" mark in GTID is to inform parallel replication that the entire event group is safe for optimistic parallel replication - basically that it can be rolled back.
So if a event group contains innodb/myisam mix, and is marked as "transactional", that is not correct. This would allow parallel replication to speculatively execute and roll back the mix, and the myisam changes would remain and cause replication to break.
Andrei, so now I have not one but two use cases: 1. InnoDB and FederatedX (or any other transactional but not XA-capable engine) in one transaction - this transaction can be speculatively executed and rolled back if needed. But it doesn't end with Xid_log_event, and after your patch it won't have the trans flag. 2. InnoDB and MyISAM. This "transaction" cannot be rolled back, so must not be executed speculatively. But it does end with a Xid_log_event, so it seems that you'll put a trans flag on it. The first case is a performance issue, but not a problem of correctness. Some some transactions might not be executed speculatively, but everything will still work. The second case can break replication when a speculatively executed InnoDB+MyISAM "transaction" will fail to roll back. Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
participants (2)
-
Kristian Nielsen -
Sergei Golubchik