maria-developers team mailing list archive

[sujatha <sujatha.sivakumar@xxxxxxxxxxx>]

revision-id: 0664e1e766d303187b381ace1c53547d1821e35c (mariadb-10.1.43-252-g0664e1e766d)
parent(s): 85bd5314c56c150d756066806d4a6cb5b682383f
author: Sujatha
committer: Sujatha
timestamp: 2020-08-10 12:29:48 +0530
message:

MDEV-22338: Assertion `!current_stmt_is_commit || !rgi->tables_to_lock' failed in Query_log_event::do_apply_event

Analysis:
=========
With row based replication, master can generate event groups like shown below.

BEGIN
TABLE_MAP
COMMIT

i.e A binlog event group, without any rows event in it. Such groups are
possible when a query operating on transactional table, doesn't modify any
data in the table.  For example a transaction is trying to modify both trans
and non-trans tables and a trigger exists on non-transtable which modifies a
trans table.

CREATE TABLE t1 (id int) engine=innodb;
CREATE TABLE t2 (id int) engine=myisam;
CREATE TRIGGER t2 BEFORE INSERT ON t2 FOR EACH ROW DELETE FROM t1 LIMIT 1;
INSERT INTO t2 VALUES (1);

Above insert will write a record in 't2' table and its corresponding binlog
entry is generated. First table map events are added to appropriate caches.

t2's Table_Map is written into statement_cache.
t1's Table_Map is written into transaction_cache.
'Write_rows' event is added to t2's statement_cache.

Since there are no rows to remove from table 't1' there will not be any
'Delete_rows' event in the transaction cache. There won't be any 'Xid' event
as well. During flush operation 'COMMIT' will be added to the transaction
cache to complete the group.

"t2's statement_cache"  "t1's transaction_cache"

BEGIN                   BEGIN
TABLE_MAP               TABLE_MAP
WRITE_ROWS              COMMIT
COMMIT

On the slave side it has strict rules, that a 'Table_Map' event should be
followed by a rows event. This rule gets violated for 't1' transaction and
results in an assert.

Fix:
====
Replace the assert with if-else block. In a case where TABLE_MAP event is not
followed by rows event invoke 'rows_event_stmt_cleanup' to initiate
appropriate cleanup.

The fix is based on upstream patch:
commit da01f3846b62a2e41111256f9d6f0a2bb2de23c4
Author:	Andrei Elkin <aelkin@xxxxxxxxx>

---
 .../suite/rpl/r/rpl_no_rows_event_assert.result    | 26 ++++++++++++
 .../suite/rpl/t/rpl_no_rows_event_assert.test      | 46 ++++++++++++++++++++++
 sql/log_event.cc                                   | 22 ++++++++++-
 3 files changed, 92 insertions(+), 2 deletions(-)

diff --git a/mysql-test/suite/rpl/r/rpl_no_rows_event_assert.result b/mysql-test/suite/rpl/r/rpl_no_rows_event_assert.result
new file mode 100644
index 00000000000..7661bb62305
--- /dev/null
+++ b/mysql-test/suite/rpl/r/rpl_no_rows_event_assert.result
@@ -0,0 +1,26 @@
+include/master-slave.inc
+[connection master]
+#########################################################################
+#  Test case1: Transaction cache has 'Table_Map' event followed by      #
+#              'COMMIT'.                                                #
+#########################################################################
+CREATE TABLE t1 (id int) ENGINE=INNODB;
+CREATE TABLE t2 (id int) ENGINE=MYISAM;
+CREATE TRIGGER t2 BEFORE INSERT ON t2 FOR EACH ROW DELETE FROM t1 LIMIT 1;
+INSERT INTO t2 VALUES (1);
+DROP TABLE t1,t2;
+#########################################################################
+#  Test case2: Transaction cache has 'Query', 'Table_Map' followed by   #
+#              'COMMIT'.                                                #
+#########################################################################
+CREATE TABLE t1(f INT) ENGINE=INNODB;
+CREATE TABLE t2(f INT) ENGINE=MYISAM;
+CREATE TABLE t3(id INT AUTO_INCREMENT, i INT, PRIMARY KEY (id)) ENGINE=INNODB;
+CREATE TABLE t4(f INT) ENGINE=MYISAM;
+CREATE TRIGGER trig1 BEFORE INSERT ON t2 FOR EACH ROW INSERT INTO t3(i) SELECT * FROM t4 LIMIT 0;
+BEGIN;
+DELETE FROM t1 WHERE f=0;
+INSERT INTO t2 VALUES (2);
+COMMIT;
+DROP TABLE t1,t2,t3,t4;
+include/rpl_end.inc
diff --git a/mysql-test/suite/rpl/t/rpl_no_rows_event_assert.test b/mysql-test/suite/rpl/t/rpl_no_rows_event_assert.test
new file mode 100644
index 00000000000..c5cc7634615
--- /dev/null
+++ b/mysql-test/suite/rpl/t/rpl_no_rows_event_assert.test
@@ -0,0 +1,46 @@
+--source include/have_innodb.inc
+--source include/have_binlog_format_mixed_or_row.inc
+--source include/master-slave.inc
+# ==== References ====
+#
+# MDEV-22338: Assertion `!current_stmt_is_commit || !rgi->tables_to_lock'
+# failed in Query_log_event::do_apply_event
+#
+
+--echo #########################################################################
+--echo #  Test case1: Transaction cache has 'Table_Map' event followed by      #
+--echo #              'COMMIT'.                                                #
+--echo #########################################################################
+--connection master
+CREATE TABLE t1 (id int) ENGINE=INNODB;
+CREATE TABLE t2 (id int) ENGINE=MYISAM;
+CREATE TRIGGER t2 BEFORE INSERT ON t2 FOR EACH ROW DELETE FROM t1 LIMIT 1;
+INSERT INTO t2 VALUES (1);
+--sync_slave_with_master
+
+# Cleanup
+--connection master
+DROP TABLE t1,t2;
+
+--echo #########################################################################
+--echo #  Test case2: Transaction cache has 'Query', 'Table_Map' followed by   #
+--echo #              'COMMIT'.                                                #
+--echo #########################################################################
+
+CREATE TABLE t1(f INT) ENGINE=INNODB;
+CREATE TABLE t2(f INT) ENGINE=MYISAM;
+CREATE TABLE t3(id INT AUTO_INCREMENT, i INT, PRIMARY KEY (id)) ENGINE=INNODB;
+CREATE TABLE t4(f INT) ENGINE=MYISAM;
+CREATE TRIGGER trig1 BEFORE INSERT ON t2 FOR EACH ROW INSERT INTO t3(i) SELECT * FROM t4 LIMIT 0;
+
+BEGIN;
+DELETE FROM t1 WHERE f=0;
+INSERT INTO t2 VALUES (2);
+COMMIT;
+--sync_slave_with_master
+
+# Cleanup
+--connection master
+DROP TABLE t1,t2,t3,t4;
+
+--source include/rpl_end.inc
diff --git a/sql/log_event.cc b/sql/log_event.cc
index a1a442df43f..c37512aa6c1 100644
--- a/sql/log_event.cc
+++ b/sql/log_event.cc
@@ -4329,8 +4329,26 @@ int Query_log_event::do_apply_event(rpl_group_info *rgi,
   thd->clear_error(1);
   current_stmt_is_commit= is_commit();
 
-  DBUG_ASSERT(!current_stmt_is_commit || !rgi->tables_to_lock);
-  rgi->slave_close_thread_tables(thd);
+  if (strcmp("COMMIT", query) == 0 && rgi->tables_to_lock != NULL)
+  {
+    /*
+      Cleaning-up the last statement context:
+      Found a row based event group which didn't modfiy any rows. i.e.
+      BEGIN
+      TABLE_MAP
+      COMMIT
+    */
+    int error;
+    if ((error = rows_event_stmt_cleanup(rgi, thd)))
+    {
+      rli->report(ERROR_LEVEL, error, "Error in cleaning up after an event "
+                  "preceding the commit; the group log file/position: %s %llu",
+                  rli->group_master_log_name,
+                  (ulong) rli->group_master_log_pos);
+    }
+  }
+  else
+    rgi->slave_close_thread_tables(thd);
 
   /*
     Note:   We do not need to execute reset_one_shot_variables() if this