MDEV-14693 XA: Assertion `!clust_index->online_log' failed in rollbac…

…k_inplace_alter_table

ha_innobase::commit_inplace_alter_table(): Defer the freeing of ctx->trx
until after the operation has been successfully committed. In this way,
rollback on a partitioned table will be possible.

rollback_inplace_alter_table(): Handle ctx->new_table == NULL when
ctx->trx != NULL.

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

@@ -0,0 +1,15 @@
+#
+# MDEV-14693 XA: Assertion `!clust_index->online_log' failed
+# in rollback_inplace_alter_table
+#
+CREATE TABLE t1 (a INT, b INT) ENGINE=InnoDB PARTITION BY HASH(a) PARTITIONS 2;
+XA START 'xid';
+INSERT INTO t1 VALUES (1,10);
+CREATE DATABASE IF NOT EXISTS db;
+ERROR XAE07: XAER_RMFAIL: The command cannot be executed when global transaction is in the  ACTIVE state
+SET innodb_lock_wait_timeout= 1, lock_wait_timeout= 2;
+ALTER TABLE t1 FORCE;
+ERROR HY000: Lock wait timeout exceeded; try restarting transaction
+XA END 'xid';
+XA ROLLBACK 'xid';
+DROP TABLE t1;

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

@@ -0,0 +1,31 @@
+--source include/have_innodb.inc
+--source include/have_partition.inc
+
+--echo #
+--echo # MDEV-14693 XA: Assertion `!clust_index->online_log' failed
+--echo # in rollback_inplace_alter_table
+--echo #
+
+# A bug in meta-data locking (MDL) for XA transactions causes
+# a bug in InnoDB error handling for ALTER TABLE to be triggered.
+CREATE TABLE t1 (a INT, b INT) ENGINE=InnoDB PARTITION BY HASH(a) PARTITIONS 2;
+XA START 'xid';
+INSERT INTO t1 VALUES (1,10);
+# XA bug: The following releases the MDL on t1!
+--error ER_XAER_RMFAIL
+CREATE DATABASE IF NOT EXISTS db;
+
+--connect (con1,localhost,root,,test)
+SET innodb_lock_wait_timeout= 1, lock_wait_timeout= 2;
+# Here, innodb_lock_wait_timeout would be exceeded, causing the operation
+# to roll back when InnoDB is attempting to commit.
+# (Instead, lock_wait_timeout should be exceeded!)
+--error ER_LOCK_WAIT_TIMEOUT
+ALTER TABLE t1 FORCE;
+
+# Cleanup
+--disconnect con1
+--connection default
+XA END 'xid';
+XA ROLLBACK 'xid';
+DROP TABLE t1;

storage/innobase/handler/handler0alter.cc

@@ -4296,12 +4296,16 @@ rollback_inplace_alter_table(
 	row_mysql_lock_data_dictionary(ctx->trx);
 
 	if (ctx->need_rebuild()) {
-		dberr_t	err;
-		ulint	flags	= ctx->new_table->flags;
-
 		/* DML threads can access ctx->new_table via the
 		online rebuild log. Free it first. */
 		innobase_online_rebuild_log_free(prebuilt->table);
+	}
+
+	if (!ctx->new_table) {
+		ut_ad(ctx->need_rebuild());
+	} else if (ctx->need_rebuild()) {
+		dberr_t	err;
+		ulint	flags	= ctx->new_table->flags;
 
 		/* Since the FTS index specific auxiliary tables has
 		not yet registered with "table->fts" by fts_add_index(),
@@ -5669,21 +5673,6 @@ ha_innobase::commit_inplace_alter_table(
 	ut_ad(prebuilt->table == ctx0->old_table);
 	ha_alter_info->group_commit_ctx = NULL;
 
-	/* Free the ctx->trx of other partitions, if any. We will only
-	use the ctx0->trx here. Others may have been allocated in
-	the prepare stage. */
-
-	for (inplace_alter_handler_ctx** pctx = &ctx_array[1]; *pctx;
-	     pctx++) {
-		ha_innobase_inplace_ctx*	ctx
-			= static_cast<ha_innobase_inplace_ctx*>(*pctx);
-
-		if (ctx->trx) {
-			trx_free_for_mysql(ctx->trx);
-			ctx->trx = NULL;
-		}
-	}
-
 	trx_start_if_not_started_xa(prebuilt->trx);
 
 	for (inplace_alter_handler_ctx** pctx = ctx_array; *pctx; pctx++) {
@@ -6056,10 +6045,6 @@ ha_innobase::commit_inplace_alter_table(
 	covering all partitions. */
 	share->idx_trans_tbl.index_count = 0;
 
-	if (trx == ctx0->trx) {
-		ctx0->trx = NULL;
-	}
-
 	/* Tell the InnoDB server that there might be work for
 	utility threads: */
 
@@ -6082,10 +6067,31 @@ ha_innobase::commit_inplace_alter_table(
 		}
 
 		row_mysql_unlock_data_dictionary(trx);
-		trx_free_for_mysql(trx);
+		if (trx != ctx0->trx) {
+			trx_free_for_mysql(trx);
+		}
 		DBUG_RETURN(true);
 	}
 
+	if (trx == ctx0->trx) {
+		ctx0->trx = NULL;
+	}
+
+	/* Free the ctx->trx of other partitions, if any. We will only
+	use the ctx0->trx here. Others may have been allocated in
+	the prepare stage. */
+
+	for (inplace_alter_handler_ctx** pctx = &ctx_array[1]; *pctx;
+	     pctx++) {
+		ha_innobase_inplace_ctx*	ctx
+			= static_cast<ha_innobase_inplace_ctx*>(*pctx);
+
+		if (ctx->trx) {
+			trx_free_for_mysql(ctx->trx);
+			ctx->trx = NULL;
+		}
+	}
+
 	/* Release the table locks. */
 	trx_commit_for_mysql(prebuilt->trx);
 

storage/xtradb/handler/handler0alter.cc

@@ -4310,12 +4310,16 @@ rollback_inplace_alter_table(
 	row_mysql_lock_data_dictionary(ctx->trx);
 
 	if (ctx->need_rebuild()) {
-		dberr_t	err;
-		ulint	flags	= ctx->new_table->flags;
-
 		/* DML threads can access ctx->new_table via the
 		online rebuild log. Free it first. */
 		innobase_online_rebuild_log_free(prebuilt->table);
+	}
+
+	if (!ctx->new_table) {
+		ut_ad(ctx->need_rebuild());
+	} else if (ctx->need_rebuild()) {
+		dberr_t	err;
+		ulint	flags	= ctx->new_table->flags;
 
 		/* Since the FTS index specific auxiliary tables has
 		not yet registered with "table->fts" by fts_add_index(),
@@ -5677,21 +5681,6 @@ ha_innobase::commit_inplace_alter_table(
 	ut_ad(prebuilt->table == ctx0->old_table);
 	ha_alter_info->group_commit_ctx = NULL;
 
-	/* Free the ctx->trx of other partitions, if any. We will only
-	use the ctx0->trx here. Others may have been allocated in
-	the prepare stage. */
-
-	for (inplace_alter_handler_ctx** pctx = &ctx_array[1]; *pctx;
-	     pctx++) {
-		ha_innobase_inplace_ctx*	ctx
-			= static_cast<ha_innobase_inplace_ctx*>(*pctx);
-
-		if (ctx->trx) {
-			trx_free_for_mysql(ctx->trx);
-			ctx->trx = NULL;
-		}
-	}
-
 	trx_start_if_not_started_xa(prebuilt->trx);
 
 	for (inplace_alter_handler_ctx** pctx = ctx_array; *pctx; pctx++) {
@@ -6057,10 +6046,6 @@ ha_innobase::commit_inplace_alter_table(
 	covering all partitions. */
 	share->idx_trans_tbl.index_count = 0;
 
-	if (trx == ctx0->trx) {
-		ctx0->trx = NULL;
-	}
-
 	/* Tell the InnoDB server that there might be work for
 	utility threads: */
 
@@ -6083,10 +6068,31 @@ ha_innobase::commit_inplace_alter_table(
 		}
 
 		row_mysql_unlock_data_dictionary(trx);
-		trx_free_for_mysql(trx);
+		if (trx != ctx0->trx) {
+			trx_free_for_mysql(trx);
+		}
 		DBUG_RETURN(true);
 	}
 
+	if (trx == ctx0->trx) {
+		ctx0->trx = NULL;
+	}
+
+	/* Free the ctx->trx of other partitions, if any. We will only
+	use the ctx0->trx here. Others may have been allocated in
+	the prepare stage. */
+
+	for (inplace_alter_handler_ctx** pctx = &ctx_array[1]; *pctx;
+	     pctx++) {
+		ha_innobase_inplace_ctx*	ctx
+			= static_cast<ha_innobase_inplace_ctx*>(*pctx);
+
+		if (ctx->trx) {
+			trx_free_for_mysql(ctx->trx);
+			ctx->trx = NULL;
+		}
+	}
+
 	/* Release the table locks. */
 	trx_commit_for_mysql(prebuilt->trx);