MDEV-13626: Improve innodb.xa_recovery_debug

Improve the test that was imported and adapted for MariaDB in
commit fb217449.

row_undo_step(): Move the DEBUG_SYNC point from trx_rollback_for_mysql().
This DEBUG_SYNC point is executed after rolling back one row.

trx_rollback_for_mysql(): Clarify the comments that describe the scenario,
and remove the DEBUG_SYNC point.

If the statement "if (trx->has_logged_persistent())" and its body are
removed from trx_rollback_for_mysql(), then the test
innodb.xa_recovery_debug will fail because the transaction would still
exist in the XA PREPARE state. If we allow the XA COMMIT statement
to succeed in the test, we would observe an incorrect state of the
XA transaction where the table would contain row (1,NULL). Depending
on whether the XA transaction was committed, the table should either
be empty or contain the record (1,1). The intermediate state of
(1,NULL) should never be observed after completed recovery.

mysql-test/suite/innodb/r/xa_recovery_debug.result

@@ -12,7 +12,7 @@ COUNT(*)
 2
 XA END 'zombie';
 XA PREPARE 'zombie';
-SET DEBUG_SYNC='trx_xa_rollback SIGNAL s1 WAIT_FOR s2';
+SET DEBUG_SYNC='trx_after_rollback_row SIGNAL s1 WAIT_FOR s2';
 XA ROLLBACK 'zombie';
 connection default;
 SET DEBUG_SYNC='now WAIT_FOR s1';
@@ -21,7 +21,6 @@ DELETE FROM t LIMIT 1;
 disconnect con1;
 XA COMMIT 'zombie';
 ERROR XAE04: XAER_NOTA: Unknown XID
-SELECT COUNT(*) FROM t;
-COUNT(*)
-0
+SELECT * FROM t;
+a	b
 DROP TABLE t;

mysql-test/suite/innodb/t/xa_recovery_debug.test

@@ -17,7 +17,7 @@ UPDATE t SET b=1 WHERE a=1;
 SELECT COUNT(*) FROM t;
 XA END 'zombie';
 XA PREPARE 'zombie';
-SET DEBUG_SYNC='trx_xa_rollback SIGNAL s1 WAIT_FOR s2';
+SET DEBUG_SYNC='trx_after_rollback_row SIGNAL s1 WAIT_FOR s2';
 --send XA ROLLBACK 'zombie'
 connection default;
 SET DEBUG_SYNC='now WAIT_FOR s1';
@@ -29,7 +29,11 @@ DELETE FROM t LIMIT 1;
 let $shutdown_timeout=0;
 --source include/restart_mysqld.inc
 disconnect con1;
+# If the trx_undo_set_state_at_prepare() is omitted at the start of
+# XA ROLLBACK, then the XA COMMIT would succeed and the table would
+# incorrectly show the result of the INSERT but not the UPDATE,
+# because we would commit a partially rolled back transaction.
 --error ER_XAER_NOTA
 XA COMMIT 'zombie';
-SELECT COUNT(*) FROM t;
+SELECT * FROM t;
 DROP TABLE t;

storage/innobase/row/row0undo.cc

 /*****************************************************************************
 
 Copyright (c) 1997, 2016, Oracle and/or its affiliates. All Rights Reserved.
-Copyright (c) 2017, 2019, MariaDB Corporation.
+Copyright (c) 2017, 2020, MariaDB Corporation.
 
 This program is free software; you can redistribute it and/or modify it under
 the terms of the GNU General Public License as published by the Free Software
@@ -350,6 +350,12 @@ row_undo_step(
 
 	err = row_undo(node, thr);
 
+#ifdef ENABLED_DEBUG_SYNC
+	if (trx->mysql_thd) {
+		DEBUG_SYNC_C("trx_after_rollback_row");
+	}
+#endif /* ENABLED_DEBUG_SYNC */
+
 	trx->error_state = err;
 
 	if (err != DB_SUCCESS) {

storage/innobase/trx/trx0roll.cc

 /*****************************************************************************
 
 Copyright (c) 1996, 2017, Oracle and/or its affiliates. All Rights Reserved.
-Copyright (c) 2016, 2019, MariaDB Corporation.
+Copyright (c) 2016, 2020, MariaDB Corporation.
 
 This program is free software; you can redistribute it and/or modify it under
 the terms of the GNU General Public License as published by the Free Software
@@ -200,10 +200,21 @@ dberr_t trx_rollback_for_mysql(trx_t* trx)
 	case TRX_STATE_PREPARED_RECOVERED:
 		ut_ad(!trx_is_autocommit_non_locking(trx));
 		if (trx->has_logged_persistent()) {
-			/* Change the undo log state back from
-			TRX_UNDO_PREPARED to TRX_UNDO_ACTIVE
-			so that if the system gets killed,
-			recovery will perform the rollback. */
+			/* The XA ROLLBACK of a XA PREPARE transaction
+			will consist of multiple mini-transactions.
+
+			As the very first step of XA ROLLBACK, we must
+			change the undo log state back from
+			TRX_UNDO_PREPARED to TRX_UNDO_ACTIVE, in order
+			to ensure that recovery will complete the
+			rollback.
+
+			Failure to perform this step could cause a
+			situation where we would roll back part of
+			a XA PREPARE transaction, the server would be
+			killed, and finally, the transaction would be
+			recovered in XA PREPARE state, with some of
+			the actions already having been rolled back. */
 			trx_undo_ptr_t*	undo_ptr = &trx->rsegs.m_redo;
 			mtr_t		mtr;
 			mtr.start();
@@ -219,29 +230,15 @@ dberr_t trx_rollback_for_mysql(trx_t* trx)
 					true, &mtr);
 			}
 			mutex_exit(&trx->rsegs.m_redo.rseg->mutex);
-			/* Persist the XA ROLLBACK, so that crash
-			recovery will replay the rollback in case
-			the redo log gets applied past this point. */
+			/* Write the redo log for the XA ROLLBACK
+			state change to the global buffer. It is
+			not necessary to flush the redo log. If
+			a durable log write of a later mini-transaction
+			takes place for whatever reason, then this state
+			change will be durable as well. */
 			mtr.commit();
 			ut_ad(mtr.commit_lsn() > 0);
 		}
-#ifdef ENABLED_DEBUG_SYNC
-		if (trx->mysql_thd == NULL) {
-			/* We could be executing XA ROLLBACK after
-			XA PREPARE and a server restart. */
-		} else if (!trx->has_logged_persistent()) {
-			/* innobase_close_connection() may roll back a
-			transaction that did not generate any
-			persistent undo log. The DEBUG_SYNC
-			would cause an assertion failure for a
-			disconnected thread.
-
-			NOTE: InnoDB will not know about the XID
-			if no persistent undo log was generated. */
-		} else {
-			DEBUG_SYNC_C("trx_xa_rollback");
-		}
-#endif /* ENABLED_DEBUG_SYNC */
 		return(trx_rollback_for_mysql_low(trx));
 
 	case TRX_STATE_COMMITTED_IN_MEMORY: