Bug #20920851 ASSERTION `IS_STARTED()' IN HA_TRX_INFO::NEXT() AT TRANSACTION_INFO.H:138

The assert is hit when XA transaction updated only a non-transactional table and
went to prepare stage.
At that time MYSQL_BIN_LOG::write_binlog_and_commit_engine() is invoked where
the trx should not attempt any committing. But that's happened.
The binlog "engine" managed to commit_low() in conditions of the reported case
which led to the assert few instructions later

#7  0x00007fad151dbe42 in __GI___assert_fail (assertion=0x1e37d16 "is_started()", file=0x1e37c68
#8  0x0000000000fafcf7 in Ha_trx_info::next (this=0x7fac8c002028) at
#9  0x0000000000f9dfa9 in ha_prepare (thd=0x7fac8c000bb0)

More analysis proved there's an "inverted" related issue in the rollback branch.
When the pure non-transactional engine xa transaction is rolled back, this time, it
misses to execute rollback_low() method to leave a screwed state which
the following query discovers hiting against an assert.

   bool trans_commit_stmt(THD*): Assertion
   `thd->in_active_multi_stmt_transaction() || thd->m_transaction_psi
   == __null' failed.

And finally, testing revealed a case of no test coverage so far in combination
of XA ROLLBACK, no transactional tables involved and no XA PREPARE.
In such case logging was just incorrect mixing XA START and ROLLBACK.

The original issue is fixed with making
MYSQL_BIN_LOG::write_binlog_and_commit_engine() to compute a local boolean flag `skip-commit'
correctly based on the value of the XA state.
Commit is disallowed when the state is Prepared.
To satisfy to the ONE-phase XA, the committing XA is also made
to receive XA_PREPARED status, as intermediate, right after the prepare phase is done.
in a general commit handler of ha_commit_trans().

The second issue of the rollback part is fixed with relocating an existing explicit
rollback_low() for xa-rollback to a safer point.

And the final third issue is fixed with augmentment of
ending_trans()'s the trans_cannot_safely_rollback(thd) branch to
compose an appropriate Query-log-event.
Logics of preventing second time do_binlog_xa_commit_rollback()
invocation that is actual to the "externally" committing XA is
simplified.
The former idea was in that the first invocation of
do_binlog_xa_commit_rollback() in the "external" XA commit branch
would necessarily turn the cache from empty, asserted, to not empty.
On the other hand, at running do_binlog_xa_commit_rollback() for the
local xa the cache must be empty (because it should've been flushed at
prepare), asserted in the rollback case too.
Hence the state of the cache checking was correct: (the local xa go
through, the external xa goes through once which is first time).  Now
instead of the above deduction the 1st invocation is just gets
explicitly flagged. And because we would like to preserve signature of
MYSQL_BIN_LOG class methods the flag is made to pass as a new member
of `bool binlog_cache_mngr::has_logged_xid'

Mixing transactional and non-transactional tables in
rpl.rpl_xa_survive_disconnect_mixed_engines reveal one issue in MTS
grouping. An XA transaction "prepare" group can be closed
with XA-ROLLBACK query which was previously missed to capture.
A use case for that is mixed transactional and non-transactional updates.
It's been corrected now.

As a side effect is_loggable_xa_prepare() had to be refined to satisfy
@c simulate_commit_failure.

Author: Andrei Elkin

mysql-test/extra/rpl_tests/rpl_xa_mixed_engines.inc

@@ -0,0 +1,73 @@
+#
+# The test file is invoked from rpl.rpl_xa_survive_disconnect_mixed_engines
+#
+# The test file is orginized as three sections: setup, run and cleanup.
+# The main logics is resided in the run section which generates
+# three types of XA transaction: two kinds of mixed and one on non-transactional
+# table.
+#
+# param $command          one of three of: 'setup', 'run' or 'cleanup'
+# param $xa_terminate     how to conclude: 'XA COMMIT' or 'XA ROLLBACK'
+# param $one_phase        'one_phase' can be opted with XA COMMIT above
+# param $xa_prepare_opt   '1' or empty can be opted to test with and without XA PREPARE
+# param $xid              arbitrary name for xa trx, defaults to 'xa_trx'
+# Note '' is merely to underline, not a part of the value.
+#
+
+if ($command == setup)
+{
+  # Test randomizes the following variable's value:
+  SET @@session.binlog_direct_non_transactional_updates := if(floor(rand()*10)%2,'ON','OFF');
+  CREATE TABLE t (a INT) ENGINE=innodb;
+  CREATE TABLE tm (a INT) ENGINE=myisam;
+}
+if (!$xid)
+{
+  --let $xid=xa_trx
+}
+if ($command == run)
+{
+  # Non transactional table goes first
+
+  --eval XA START '$xid'
+  INSERT INTO tm VALUES (1);
+  INSERT INTO t VALUES (1);
+  --eval XA END '$xid'
+  if ($xa_prepare_opt)
+  {
+   --eval XA PREPARE '$xid'
+  }
+  --eval $xa_terminate '$xid' $one_phase
+
+  # Transactional table goes first
+
+  --eval XA START '$xid'
+  INSERT INTO t VALUES (2);
+  --disable_warnings
+  INSERT INTO tm VALUES (2);
+  --enable_warnings
+  --eval XA END '$xid'
+  if ($xa_prepare_opt)
+  {
+   --eval XA PREPARE '$xid'
+  }
+  --eval $xa_terminate '$xid' $one_phase
+
+  # The pure non-transactional table
+
+  --eval XA START '$xid'
+  --disable_warnings
+  INSERT INTO tm VALUES (3);
+  --enable_warnings
+  --eval XA END '$xid'
+  if ($xa_prepare_opt)
+  {
+   --eval XA PREPARE '$xid'
+  }
+  --eval $xa_terminate '$xid' $one_phase
+}
+
+if ($command == cleanup)
+{
+  DROP TABLE t, tm;
+}

mysql-test/suite/rpl/r/rpl_xa_survive_disconnect_mixed_engines.result

@@ -3,43 +3,101 @@ Warnings:
 Note	####	Sending passwords in plain text without SSL/TLS is extremely insecure.
 Note	####	Storing MySQL user name or password information in the master info repository is not secure and is therefore not recommended. Please consider using the USER and PASSWORD connection options for START SLAVE; see the 'START SLAVE Syntax' in the MySQL Manual for more information.
 [connection master]
+[connection slave]
+CALL mtr.add_suppression("An unexpected event sequence was detected by the IO thread while queuing the event");
+CALL mtr.add_suppression("GTID_LOG_EVENT or ANONYMOUS_GTID_LOG_EVENT is not expected in an event stream in the middle of a DML");
+[connection master]
 CALL mtr.add_suppression("Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT");
 SET @@session.binlog_direct_non_transactional_updates := if(floor(rand()*10)%2,'ON','OFF');
 CREATE TABLE t (a INT) ENGINE=innodb;
 CREATE TABLE tm (a INT) ENGINE=myisam;
-XA START '1';
+=== COMMIT ===
+XA START 'xa_trx';
+INSERT INTO tm VALUES (1);
+INSERT INTO t VALUES (1);
+XA END 'xa_trx';
+XA PREPARE 'xa_trx';
+XA COMMIT 'xa_trx' ;
+XA START 'xa_trx';
+INSERT INTO t VALUES (2);
+INSERT INTO tm VALUES (2);
+XA END 'xa_trx';
+XA PREPARE 'xa_trx';
+XA COMMIT 'xa_trx' ;
+XA START 'xa_trx';
+INSERT INTO tm VALUES (3);
+XA END 'xa_trx';
+XA PREPARE 'xa_trx';
+XA COMMIT 'xa_trx' ;
+include/sync_slave_sql_with_master.inc
+[connection master]
+=== COMMIT ONE PHASE ===
+XA START 'xa_trx';
 INSERT INTO tm VALUES (1);
 INSERT INTO t VALUES (1);
-XA END '1';
-XA PREPARE '1';
-XA COMMIT '1';
-XA START '2';
+XA END 'xa_trx';
+XA COMMIT 'xa_trx' ONE PHASE;
+XA START 'xa_trx';
 INSERT INTO t VALUES (2);
 INSERT INTO tm VALUES (2);
+XA END 'xa_trx';
+XA COMMIT 'xa_trx' ONE PHASE;
+XA START 'xa_trx';
+INSERT INTO tm VALUES (3);
+XA END 'xa_trx';
+XA COMMIT 'xa_trx' ONE PHASE;
+include/sync_slave_sql_with_master.inc
+[connection master]
+=== ROLLBACK with PREPARE ===
+XA START 'xa_trx';
+INSERT INTO tm VALUES (1);
+INSERT INTO t VALUES (1);
+XA END 'xa_trx';
+XA PREPARE 'xa_trx';
+xa rollback 'xa_trx' ;
+Warnings:
+Warning	1196	Some non-transactional changed tables couldn't be rolled back
+XA START 'xa_trx';
 INSERT INTO t VALUES (2);
-XA END '2';
-XA PREPARE '2';
-XA COMMIT '2';
-XA START '3';
+INSERT INTO tm VALUES (2);
+XA END 'xa_trx';
+XA PREPARE 'xa_trx';
+xa rollback 'xa_trx' ;
+Warnings:
+Warning	1196	Some non-transactional changed tables couldn't be rolled back
+XA START 'xa_trx';
 INSERT INTO tm VALUES (3);
-INSERT INTO t VALUES (3);
-XA END '3';
-XA PREPARE '3';
-XA ROLLBACK '3';
+XA END 'xa_trx';
+XA PREPARE 'xa_trx';
+xa rollback 'xa_trx' ;
 Warnings:
 Warning	1196	Some non-transactional changed tables couldn't be rolled back
-XA START '4';
-INSERT INTO t VALUES (4);
-INSERT INTO tm VALUES (4);
-INSERT INTO t VALUES (4);
-XA END '4';
-XA PREPARE '4';
-XA ROLLBACK '4';
+include/sync_slave_sql_with_master.inc
+[connection master]
+=== ROLLBACK with no PREPARE ===
+XA START 'xa_trx';
+INSERT INTO tm VALUES (1);
+INSERT INTO t VALUES (1);
+XA END 'xa_trx';
+xa rollback 'xa_trx' ;
+Warnings:
+Warning	1196	Some non-transactional changed tables couldn't be rolled back
+XA START 'xa_trx';
+INSERT INTO t VALUES (2);
+INSERT INTO tm VALUES (2);
+XA END 'xa_trx';
+xa rollback 'xa_trx' ;
+Warnings:
+Warning	1196	Some non-transactional changed tables couldn't be rolled back
+XA START 'xa_trx';
+INSERT INTO tm VALUES (3);
+XA END 'xa_trx';
+xa rollback 'xa_trx' ;
 Warnings:
 Warning	1196	Some non-transactional changed tables couldn't be rolled back
 include/sync_slave_sql_with_master.inc
 include/diff_tables.inc [master:tm, slave:tm]
-DELETE FROM t;
+[connection master]
 DROP TABLE t, tm;
 include/sync_slave_sql_with_master.inc
 include/rpl_end.inc

mysql-test/suite/rpl/t/rpl_xa_survive_disconnect_mixed_engines.test

@@ -2,78 +2,71 @@
 #
 # The test verifies correct XA transaction two phase logging and its applying
 # in a case the transaction updates transactional and non-transactional tables.
+# Transactions are terminated according to specfied parameters to
+# a sourced inc-file.
 
 --source include/not_gtid_enabled.inc
 --source include/master-slave.inc
 
---connection master
+--source include/rpl_connection_slave.inc
+#Bug #21273010 	SLAVE GROUP EVENT PARSER DOES NOT RECOGNIZE XA ROLLBACK AS A BOUNDARY
+CALL mtr.add_suppression("An unexpected event sequence was detected by the IO thread while queuing the event");
+CALL mtr.add_suppression("GTID_LOG_EVENT or ANONYMOUS_GTID_LOG_EVENT is not expected in an event stream in the middle of a DML");
+
+--source include/rpl_connection_master.inc
 CALL mtr.add_suppression("Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT");
 
-# Test randomizes the following variable's value:
-SET @@session.binlog_direct_non_transactional_updates := if(floor(rand()*10)%2,'ON','OFF');
-CREATE TABLE t (a INT) ENGINE=innodb;
-
-# memorize for the following show
---let $binlog_start= query_get_value(SHOW MASTER STATUS, Position, 1)
---let $binlog_file= query_get_value(SHOW MASTER STATUS, File, 1)
-
-CREATE TABLE tm (a INT) ENGINE=myisam;
-
-# case A: COMMIT
-
-# Non transactional table goes first
-XA START '1';
-INSERT INTO tm VALUES (1);
-INSERT INTO t VALUES (1);
-XA END '1';
-XA PREPARE '1';
-XA COMMIT '1';
-
-# Transactional table goes first
-XA START '2';
-INSERT INTO t VALUES (2);
---disable_warnings
-INSERT INTO tm VALUES (2);
---enable_warnings
-INSERT INTO t VALUES (2);
-XA END '2';
-XA PREPARE '2';
-XA COMMIT '2';
-
-# case B: ROLLBACK
-
-# Non transactional table goes first
-XA START '3';
---disable_warnings
-INSERT INTO tm VALUES (3);
---enable_warnings
-INSERT INTO t VALUES (3);
-XA END '3';
-XA PREPARE '3';
-XA ROLLBACK '3';
-
-# Transactional table goes first
-XA START '4';
-INSERT INTO t VALUES (4);
---disable_warnings
-INSERT INTO tm VALUES (4);
---enable_warnings
-INSERT INTO t VALUES (4);
-XA END '4';
-XA PREPARE '4';
-XA ROLLBACK '4';
+--let $command=setup
+--source extra/rpl_tests/rpl_xa_mixed_engines.inc
+
+--echo === COMMIT ===
+--let $command=run
+--let $xa_terminate=XA COMMIT
+--let $xa_prepare_opt=1
+--source extra/rpl_tests/rpl_xa_mixed_engines.inc
+
+--source include/sync_slave_sql_with_master.inc
+--source include/rpl_connection_master.inc
+
+--echo === COMMIT ONE PHASE ===
+
+--let $command=run
+--let $xa_terminate=XA COMMIT
+--let $one_phase=ONE PHASE
+--let $xa_prepare_opt=
+--source extra/rpl_tests/rpl_xa_mixed_engines.inc
+--let $one_phase=
+--source include/sync_slave_sql_with_master.inc
+--source include/rpl_connection_master.inc
+
+--echo === ROLLBACK with PREPARE ===
+
+--let $command=run
+--let $xa_terminate=xa rollback
+--let $xa_prepare_opt=1
+--source extra/rpl_tests/rpl_xa_mixed_engines.inc
+
+--source include/sync_slave_sql_with_master.inc
+--source include/rpl_connection_master.inc
+
+--echo === ROLLBACK with no PREPARE ===
+
+--let $command=run
+--let $xa_terminate=xa rollback
+--let $xa_prepare_opt=
+--source extra/rpl_tests/rpl_xa_mixed_engines.inc
+--let $xa_rollback_only=
 
 --source include/sync_slave_sql_with_master.inc
 
 --let $diff_tables= master:tm, slave:tm
 --source include/diff_tables.inc
 
-# cleanup
-
---connection master
+# Cleanup
 
-DELETE FROM t;
-DROP TABLE t, tm;
+--source include/rpl_connection_master.inc
+--let $command=cleanup
+--source extra/rpl_tests/rpl_xa_mixed_engines.inc
 
 --source include/sync_slave_sql_with_master.inc