[MDEV-32020] XA transaction replicates incorrectly, must be applied at XA COMMIT, not XA PREPARE Created: 2023-08-26  Updated: 2024-01-21

Status: In Progress
Project: MariaDB Server
Component/s: Replication, XA
Affects Version/s: 10.5.2
Fix Version/s: 10.5

Type: Bug Priority: Critical
Reporter: Kristian Nielsen Assignee: Kristian Nielsen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File rpl_mdev32020.test     File rpl_xa_provision.test    
Issue Links:
Blocks
blocks MDEV-32014 When binlogging enabled, committing a... Confirmed
Relates
relates to MDEV-742 LP:803649 - Xa recovery failed on cli... Closed

 Description   

XA changes done in 10.5 introduces a regression that breaks replication.

The problem is that the slave now applies XA transactions while replicating
an XA_prepare_log_event binlogged by "XA PREPARE" on the master. This is
wrong, the transaction must not be applied on the slave until "XA COMMIT",
as it is done correctly in 10.4.

Applying the XA PREPARE on the slave leaves dangling InnoDB row locks that
can conflict with the replication of later transactions and cause
replication to break. The below test case (also attached) demonstrates one
simple instance of this.

Another problem is that splitting a transaction in this way in the binlog
means there is no longer a unique binlog position corresponding to the
database state. This is demonstrated by the attached testcase
rpl_xa_provision.test .

This test case takes a mysqldump while an XA PREPARED transaction is active
on the master, and uses it to provision a new slave. The new slave's GTID
position cannot be set correctly. Setting it after the XA PREPARED
transaction means the XA COMMIT will fail. But setting it before the XA
PREPARE would also not be correct, as it would duplicate transactions
binlogged after the XA PREPARE. Thus, in 10.5, the provisioned slave breaks
its replication.

The fix is to revert the change so that XA transactions are applied on the
slave only as part of the XA COMMIT event. When the XA PREPARE event is
received by the slave, it must not be applied. Instead it can be saved
somewhere (there are several possible designs). In case of a master crash
and the slave is promoted as the new master, those saved XA PREPAREd events
can then be used to recover the XA transaction into the prepared state for
the application to XA COMMIT or XA ROLLBACK.

--source include/have_innodb.inc
 
--source include/have_binlog_format_row.inc
 
--source include/master-slave.inc
 
 
 
--connection master
 
 
 
CREATE TABLE t1 (a int, b int, c int,
 
  INDEX i1(a),
 
  INDEX i2(b))
 
  ENGINE=InnoDB;
 
 
 
INSERT INTO t1 VALUES
 
  (1,1,0), (1,2,0),
 
  (2,1,0), (2,2,0);
 
--sync_slave_with_master
 
 
 
--source include/stop_slave.inc
 
SET @old_timeout= @@GLOBAL.innodb_lock_wait_timeout;
 
SET @old_retries= @@GLOBAL.slave_transaction_retries;
 
SET GLOBAL innodb_lock_wait_timeout= 2;
 
SET GLOBAL slave_transaction_retries= 3;
 
--source include/start_slave.inc
 
 
 
--connection master
 
XA START "t1";
 
UPDATE t1 FORCE INDEX (i2) SET c=c+1 WHERE a=1 AND b=1;
 
XA END "t1";
 
XA PREPARE "t1";
 
 
 
--connection master1
 
XA START "t2";
 
UPDATE t1 FORCE INDEX (i2) SET c=c+1 WHERE a=1 AND b=2;
 
XA END "t2";
 
XA PREPARE "t2";
 
 
 
--connection master
 
XA COMMIT "t1";
 
 
 
--connection master1
 
XA COMMIT "t2";
 
 
 
--connection master
 
SELECT * FROM t1 ORDER BY a,b,c;
 
 
 
--sync_slave_with_master
 
SELECT * FROM t1 ORDER BY a,b,c;
 
 
 
# Cleanup
 
--connection master
 
DROP TABLE t1;
 
 
 
--connection slave
 
SET GLOBAL innodb_lock_wait_timeout= @old_timeout;
 
SET GLOBAL slave_transaction_retries= @old_retries;
 
 
 
--source include/rpl_end.inc



 Comments   
Comment by Andrei Elkin [ 2023-08-28 ]

knielsen, indeed employing a non-default non-unique index is problematic. Thanks for revealing it!

I will not argue how critical this issue is. I doubt it's critical, knowing as you are the use of non-unique index in replication is generally fragile, not always leading to hangs but prone to data inconsistency.
Nevertheless this specific hang can be fixed by a number of ways, including

  • add the forced index(es) into the Rows-log-event context and force using them on slave too (should be the easiest);
  • noticing dangerous (XA unsafe 2pl) properties of the master DML query, retreat to one-phase-logging into binary log (a lighter version of your proposal);
  • improve the engine to skip a locked 2ndry index record to try next one, to succeed or to give up to
    waiting only when all such records have been processed and none of unlocked satisfy the ultimate search criteria.

To a separate mysqldump issue, it obviously needs proper integration with XA binlogging.
With ROW format binlogging, all prepared XA:s could be appended to the dump sql script (in the form of BINLOG '...'). Otherwise there are few options, including to list xids/gtids of the prepared XA:s and requests them at connecting to master as sort of initialization step (think of init_slave to executed them.
Sure if none of that is good for a user, she could opt to (a patch exists already) the old one-phase bin-logged XA, possibly merely for time of taking the dump.

Comment by Vladislav Lesin [ 2023-08-28 ]

The issue is in the locking order. After the initial INSERT we have the following indexes:

# Clustered:                                                                              
# (20, 1, 1, 0)                                                                           
# (21, 1, 2, 0)                                                                           
# (22, 2, 1, 0)                                                                           
# (23, 2, 2, 0)                                                                           
#                                                                                         
# i1:                                                                                     
# (1, 20)                                                                                 
# (1, 21)                                                                                 
# (2, 22)                                                                                 
# (2, 23)                                                                                 
#                                                                                         
# i2:                                                                                     
# (1, 20)
 
# (1, 22)                                                
# (2, 21)                                                                                                                                                                  
# (2, 23)

I set RC isolation level for both master and slave to simplify debugging. All set locks are non-gap X-locks. The locking order is the following:

Locking order on master for XA "t1" (UPDATE t1 FORCE INDEX (i2) SET c=c+1 WHERE a=1 AND b=1):

# i2: (1, 20)                                                                   
# clust: (20, 1, 1, 0)                                                          
# i2: (1, 22)                                                                   
# clust: (22, 2, 1, 0)

Locking order on master for XA "t2" (UPDATE t1 FORCE INDEX (i2) SET c=c+1 WHERE a=1 AND b=2):

# i2: (2, 21)                                                                   
# clust: (21, 1, 2, 0)                                                          
# i2: (2, 23)                                                                   
# clust: (23, 2, 2, 0)

As we can see the transactions don't conflict each other, as index i2 is forced. Let's consider slave.

Locking order on slave for XA "t1", executes row events for "UPDATE t1 SET c=c+1 WHERE a=1 AND b=1", index i1 is used:

# i1: (1, 20)                                                                   
# clust: (20, 1, 1, 0)                                                          
# i1: (1, 21)                                                                   
# clust: (21, 1, 2, 0)

Locking order on slave for XA "t2", executes row events for "UPDATE t1 SET c=c+1 WHERE a=1 AND b=2", index i1 is used:

# i1: (1, 20) - already locked by t1, lock-wait-timeout

As index i1 is used on slave ("force index" is not passed for row-based event), we have lock conflict, which does not happen on master.

Note, that for non-XA's we would not have such issue, because after the transaction is committed, its locks are released, and conflicting transaction can be retried. For XA's 'XA PREPARE "t1"' holds locks while XA "t2" is executing, and, as I understood it correctly, Elkin, correct me if I am wrong, XA "t1" can't be committed before XA "t2" is prepared, as for XA's not only commit, but also preparing order is preserved.

Comment by Andrei Elkin [ 2023-08-28 ]

> XA "t1" can't be committed before XA "t2" is prepared
Correct. That follows from the test flow.

Comment by Marko Mäkelä [ 2023-09-21 ]

How would this proposal affect MDEV-31038 or MDEV-31999?

Comment by Marko Mäkelä [ 2023-09-21 ]

The only potential problem that I (as someone who does not know replication well) would foresee with this proposal is that the primary server would have to retain all binlog between XA START and XA PREPARE until an XA ROLLBACK has been executed or an XA COMMIT replicated. Hopefully all binlog writing is handling out-of-space issues gracefully; I vaguely remember that otto pointed out a problematic case in the past, but I can’t remember any details.

Comment by Marko Mäkelä [ 2023-09-21 ]

knielsen, a drawback of not replicating anything before XA COMMIT would be that when a replica is promoted to a master, it would be missing all transactions that had successfully reached the XA PREPARE state. But, what if we change the logic at the replica, and not the primary server?

  • Replicas would retain all binlog for any received XA PREPARE transactions until an XA ROLLBACK or XA COMMIT is received.
  • Applying the transaction would start upon parsing XA COMMIT. Any resolution of locking conflicts would be similar to normal (non-distributed) transactions.
Comment by Kristian Nielsen [ 2023-09-21 ]

marko, MDEV-31038 would become a no-issue. The XA PREPARE would only result in binlogging, and the normal two-phase commit between binlog and InnoDB (mysql.gtid_slave_pos table) would apply. And the XA COMMIT would just be a normal transaction commit using the same mechanism.

I'm not sure about MDEV-31999 ("Can not XA COMMIT a recovered prepared XA transaction when autocommit off"), I think this is unrelated to replication or this proposal.

Agree that primary server would need to retain binlog of XA PREPARE until XA COMMIT or XA ROLLBACK. (Not XA START I believe, which is not binlogged). The existing binlog checkpoint mechanism could be used to ensure this, and also ensure that the replication state of XA PREPARE will be recoved from the binlog after a crash. I don't see any problems retaining the binlog between XA PREPARE and XA COMMIT. This period will (hopefully) be short compared to the normal period that binlogs are normally preserved, which is usually long (eg. days) since it needs to be retained for the maximum time any slave can be off-line and still able to re-connect and catch up.

Agree with the idea to require slaves/replicas to write and retain binlogs for any replicated XA PREPARE that needs to be recoverable on a slave promoted to master. I think this is a reasonable requirement to support XA PREPARE on one server and XA COMMIT on another across async replication. And exactly as you wrote, applying the transaction would start upon XA COMMIT, which is how it used to work, and which solves a lot of fundamental issues and regressions with the current approach.

The replication of XA PREPARE would just involve updating some in-memory state that can be used to recover the transaction later, if the slave is promoted as master and an XA COMMIT of that XID is attempted by the user. This in-memory state would be recovered from the slave's binlog in case of slave crash and restart, again re-using the binlog checkpoint mechanism.

Comment by Andrei Elkin [ 2023-09-21 ]

knielsen, just to let you know MDEV-31038 actually caused by MDEV-31185 (vlad.lesin has just confirmed that, and I belatedly accorded their relation).

Comment by Marko Mäkelä [ 2023-09-22 ]

There is a concern that an application could create arbitrarily many transactions in XA PREPARE state, and the architecture would have to deal with that. Long-running transactions have a well known impact on InnoDB: they would block the purge of committed transaction history, and any undo logs would keep growing. This is limited by the file system, not any RAM data structure. I would claim that a well-behaving application would not leave behind any XA PREPARE transactions.

To my understanding, replication events are primarily buffered in RAM, sometimes backed by temporary files. Without using temporary files, the maximum size of a transaction would be limited by the size of available RAM. Compared to non-distributed transactions, which are written to the binlog at COMMIT time, distributed transactions add an extra complexity that the number of transactions that need buffering is not limited by the number of active client connections; multiple transactions may be ”stashed” (XA PREPARE), and this must be persistent: the transactions must be retained across any server restarts until an explicit XA ROLLBACK or XA COMMIT is received.

Sorry, I should have been more careful when looking up possibly related tickets. For this discussion, MDEV-31998 and MDEV-21469 look relevant to me.

Comment by Kristian Nielsen [ 2023-09-22 ]

marko, the binlog events for active transactions are stored in IO_CACHE, which is always disk-based AFAIK, RAM usage is limited to what's needed for the buffered I/O.

By using the binlog file as the recovery source of (replication events for) XA PREPARED transactions, there would be no need to hold event data in ram for active transactions, so that should be fine.

Generated at Thu Feb 08 10:28:12 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.