Re: [Maria-developers] 9ea85a70a75: MDEV-24654 GTID event falsely marked transactional

2022-01-11 Thread Sergei Golubchik
Hi, Kristian!

On Jan 11, Kristian Nielsen wrote:
> Sergei Golubchik  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 secur...@mariadb.org

___
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help   : https://help.launchpad.net/ListHelp


Re: [Maria-developers] 9ea85a70a75: MDEV-24654 GTID event falsely marked transactional

2022-01-11 Thread Sergei Golubchik
Hi, Andrei!

On Jan 11, Andrei Elkin wrote:
> On Tue, Jan 11, 2022 at 1:06 PM Sergei Golubchik  wrote:
> 
> > 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.
> 
> Got your very specific point is about a pure myisam can be trx-cached.
> This is OPTION_BINLOG_GTID
>commit 5426facdcbfba2d78dab3c709cbf278073383b7c 's decision
>  >> - All tables are regarded as transactional tables in the binary log
> (to ensure things are executed exactly as on the master)

Okay, thanks, that answers my question

Regards,
Sergei
VP of MariaDB Server Engineering
and secur...@mariadb.org

___
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help   : https://help.launchpad.net/ListHelp


Re: [Maria-developers] 9ea85a70a75: MDEV-24654 GTID event falsely marked transactional

2022-01-11 Thread Kristian Nielsen
Sergei Golubchik  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.

___
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help   : https://help.launchpad.net/ListHelp


Re: [Maria-developers] 9ea85a70a75: MDEV-24654 GTID event falsely marked transactional

2022-01-11 Thread Sergei Golubchik
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,  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 secur...@mariadb.org
> >
Regards,
Sergei
VP of MariaDB Server Engineering
and secur...@mariadb.org

___
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help   : https://help.launchpad.net/ListHelp


Re: [Maria-developers] 9ea85a70a75: MDEV-24654 GTID event falsely marked transactional

2022-01-10 Thread Sergei Golubchik
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 through 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 secur...@mariadb.org

___
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help   : https://help.launchpad.net/ListHelp