Bug #25755137 MASTER IS GENERATING INCOMPLETE TRX ON BINARY LOG (MISSING COMMIT)

The failure is about missed COMMIT Query-log-event which was caused
on the master side by not sufficient cleanup after a failing
atomic DDL. Before sensing the error, the DDL has been binlog-cached.
Yet at both the following statement and session level rollback
  ```binlog_trx_cache_data::flags.with_xid```
left unreset though it should have been.

More to the session level the flag is effectively made
defined on the STATEMENT one by WL9175.
It was supposed that its reset is unavoidable when the SESSION scope
ends with either rollback or commit. That stays for the commit.

For the rollback the binlog caches have a cleanup procedure for
statement rollback (restore_prev_position) and a cleanup procedure for
transaction rollback (reset), where reset() is only called when the cache
is empty. But they cache got emptied
from the only statement by the statement level rollback.
Therefore, reset() is missed in this case so the flag remains up.

Fixed with unflaging ```binlog_trx_cache_data::flags.with_xid``` at
the statement rollback in binlog hton in one step with the cache
rollback for the statement.
If the raised flag survives to the session commit its statement scope
value is "upgraded" to the session level to serve its original
session level semantics.

Author: Andrei Elkin

mysql-test/suite/binlog/r/binlog_atomic_ddl_rollback.result

@@ -0,0 +1,12 @@
+CREATE TABLE t1 (a int);
+CREATE TABLE t2 (a int);
+CREATE VIEW v_12 AS SELECT t1.a AS a, t2.a AS b FROM t1,t2;
+CREATE TEMPORARY TABLE tmp (a INT) ENGINE=innodb;
+ALTER TABLE t2 ROW_FORMAT=COMPRESSED KEY_BLOCK_SIZE=1;
+DROP TABLE t1,t2;
+SET DEBUG_SYNC= 'now SIGNAL continue';
+ERROR 40001: Deadlock found when trying to get lock; try restarting transaction
+include/save_binlog_position.inc
+DELETE FROM tmp;
+include/assert_binlog_events.inc
+DROP VIEW v_12;

mysql-test/suite/binlog/t/binlog_atomic_ddl_rollback.test

@@ -0,0 +1,62 @@
+# ==== Purpose ====
+#
+# Prove a group of events is logged correctly including
+# a terminal Query-log-event(COMMIT), despite a failing atomic DDL
+# in front of the group generating query.
+#
+#
+# ==== References ====
+# BUG25755137 MASTER IS GENERATING INCOMPLETE TRX ON BINARY LOG (MISSING COMMIT)
+#
+--source include/have_debug_sync.inc
+--source include/have_log_bin.inc
+--source include/have_binlog_format_statement.inc
+
+# In order to construct the deadlock error two tables and a view
+# based on them are required.
+CREATE TABLE t1 (a int);
+CREATE TABLE t2 (a int);
+CREATE VIEW v_12 AS SELECT t1.a AS a, t2.a AS b FROM t1,t2;
+
+# The Commit-free group generator.
+CREATE TEMPORARY TABLE tmp (a INT) ENGINE=innodb;
+
+--connection default
+--send ALTER TABLE t2 ROW_FORMAT=COMPRESSED KEY_BLOCK_SIZE=1
+
+# Deadlock's 2nd participant
+--connect (con1, localhost, root,,)
+DROP TABLE t1,t2;
+SET DEBUG_SYNC= 'now SIGNAL continue';
+
+--connection default
+--error ER_LOCK_DEADLOCK
+--reap
+
+#
+# the group generator
+#
+--source include/save_binlog_position.inc
+DELETE FROM tmp;
+
+#
+# Proof:
+# the whole group of BEGIN; DELETE...; COMMIT must be present
+#
+--let $gtid_event= Gtid
+--let $gtid_mode_on= `SELECT @@GLOBAL.GTID_MODE = 'ON'`
+if (!$gtid_mode_on)
+{
+  --let $gtid_event= Anonymous_Gtid
+}
+
+--let $dont_print_pattern= 1
+--let $event_sequence= $gtid_event # !Begin # !Q(DELETE.*) # !Commit
+--source include/assert_binlog_events.inc
+--let $dont_print_pattern= 0
+
+
+# Cleanup:
+
+DROP VIEW v_12;
+

sql/binlog.cc

@@ -633,8 +633,16 @@ class binlog_cache_data
     bool finalized:1;
 
     /*
-      This indicates that the cache contain an XID event.
-     */
+      This indicates that either the cache contain an XID event, or it's
+      an atomic DDL Query-log-event. In the latter case the flag is set up
+      on the statement level, namely when the Query-log-event is cached
+      at time the DDL transaction is not committing.
+      The flag therefore gets reset when the cache is cleaned due to
+      the statement rollback, e.g in case of a DDL post-caching execution
+      error.
+      Any statement scope flag among other things must consider its
+      reset policy when the statement is rolled back.
+    */
     bool with_xid:1;
   } flags;
 
@@ -779,6 +787,11 @@ class binlog_trx_cache_data : public binlog_cache_data
     DBUG_PRINT("enter", ("before_stmt_pos: %llu", (ulonglong) before_stmt_pos));
     binlog_cache_data::truncate(before_stmt_pos);
     before_stmt_pos= MY_OFF_T_UNDEF;
+    /*
+      Binlog statement rollback clears with_xid now as the atomic DDL statement
+      marker which can be set as early as at event creation and caching.
+    */
+    flags.with_xid= false;
     DBUG_PRINT("return", ("before_stmt_pos: %llu", (ulonglong) before_stmt_pos));
     DBUG_VOID_RETURN;
   }
@@ -1707,6 +1720,9 @@ binlog_trx_cache_data::truncate(THD *thd, bool all)
   /*
     If rolling back an entire transaction or a single statement not
     inside a transaction, we reset the transaction cache.
+    Even though formally the atomic DDL statement may not end multi-statement
+    transaction the cache needs full resetting as there must
+    be no other data in it but belonging to the DDL.
   */
   if (ending_trans(thd, all))
   {

sql/sql_table.cc

@@ -10309,6 +10309,8 @@ static bool mysql_inplace_alter_table(THD *thd,
       goto cleanup2;
   }
 
+  DEBUG_SYNC(thd, "action_after_write_bin_log");
+
   if (db_type->flags & HTON_SUPPORTS_ATOMIC_DDL)
   {
     /*