Merge MDEV-8725 into 10.0

Author: knielsen

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

@@ -1689,6 +1689,24 @@ a	b
 include/stop_slave.inc
 SET GLOBAL debug_dbug=@old_dbug;
 include/start_slave.inc
+*** MDEV-8725: Assertion on ROLLBACK statement in the binary log ***
+BEGIN;
+INSERT INTO t2 VALUES (200);
+INSERT INTO t1 VALUES (200);
+INSERT INTO t2 VALUES (201);
+ROLLBACK;
+SELECT * FROM t1 WHERE a>=200 ORDER BY a;
+a
+200
+SELECT * FROM t2 WHERE a>=200 ORDER BY a;
+a
+include/save_master_gtid.inc
+include/sync_with_master_gtid.inc
+SELECT * FROM t1 WHERE a>=200 ORDER BY a;
+a
+200
+SELECT * FROM t2 WHERE a>=200 ORDER BY a;
+a
 include/stop_slave.inc
 SET GLOBAL slave_parallel_threads=@old_parallel_threads;
 include/start_slave.inc

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

@@ -2369,6 +2369,28 @@ SET GLOBAL debug_dbug=@old_dbug;
 
 
 
+--echo *** MDEV-8725: Assertion on ROLLBACK statement in the binary log ***
+--connection server_1
+# Inject an event group terminated by ROLLBACK, by mixing MyISAM and InnoDB
+# in a transaction. The bug was an assertion on the ROLLBACK due to
+# mark_start_commit() being already called.
+--disable_warnings
+BEGIN;
+INSERT INTO t2 VALUES (200);
+INSERT INTO t1 VALUES (200);
+INSERT INTO t2 VALUES (201);
+ROLLBACK;
+--enable_warnings
+SELECT * FROM t1 WHERE a>=200 ORDER BY a;
+SELECT * FROM t2 WHERE a>=200 ORDER BY a;
+--source include/save_master_gtid.inc
+
+--connection server_2
+--source include/sync_with_master_gtid.inc
+SELECT * FROM t1 WHERE a>=200 ORDER BY a;
+SELECT * FROM t2 WHERE a>=200 ORDER BY a;
+
+
 # Clean up.
 --connection server_2
 --source include/stop_slave.inc

sql/log.cc

@@ -7660,14 +7660,13 @@ void
 MYSQL_BIN_LOG::binlog_trigger_immediate_group_commit()
 {
  group_commit_entry *head;
- mysql_mutex_lock(&LOCK_prepare_ordered);
+ mysql_mutex_assert_owner(&LOCK_prepare_ordered);
  head= group_commit_queue;
  if (head)
  {
  head->thd->has_waiter= true;
  mysql_cond_signal(&COND_prepare_ordered);
  }
- mysql_mutex_unlock(&LOCK_prepare_ordered);
 }
 
 
@@ -7686,9 +7685,11 @@ binlog_report_wait_for(THD *thd1, THD *thd2)
 {
  if (opt_binlog_commit_wait_count == 0)
  return;
+ mysql_mutex_lock(&LOCK_prepare_ordered);
  thd2->has_waiter= true;
  if (thd2->waiting_on_group_commit)
  mysql_bin_log.binlog_trigger_immediate_group_commit();
+ mysql_mutex_unlock(&LOCK_prepare_ordered);
 }
 
 

sql/rpl_parallel.cc

@@ -317,13 +317,26 @@ convert_kill_to_deadlock_error(rpl_group_info *rgi)
 }
 
 
-static bool
+/*
+ Check if an event marks the end of an event group. Returns non-zero if so,
+ zero otherwise.
+
+ In addition, returns 1 if the group is committing, 2 if it is rolling back.
+*/
+static int
 is_group_ending(Log_event *ev, Log_event_type event_type)
 {
- return event_type == XID_EVENT ||
- (event_type == QUERY_EVENT &&
- (((Query_log_event *)ev)->is_commit() ||
- ((Query_log_event *)ev)->is_rollback()));
+ if (event_type == XID_EVENT)
+ return 1;
+ if (event_type == QUERY_EVENT)
+ {
+ Query_log_event *qev = (Query_log_event *)ev;
+ if (qev->is_commit())
+ return 1;
+ if (qev->is_rollback())
+ return 2;
+ }
+ return 0;
 }
 
 
@@ -574,7 +587,7 @@ retry_event_group(rpl_group_info *rgi, rpl_parallel_thread *rpt,
  err= 1;
  goto err;
  }
- if (is_group_ending(ev, event_type))
+ if (is_group_ending(ev, event_type) == 1)
  rgi->mark_start_commit();
 
  err= rpt_handle_event(qev, rpt);
@@ -713,7 +726,8 @@ handle_rpl_parallel_thread(void *arg)
  Log_event_type event_type;
  rpl_group_info *rgi= qev->rgi;
  rpl_parallel_entry *entry= rgi->parallel_entry;
- bool end_of_group, group_ending;
+ bool end_of_group;
+ int group_ending;
 
  next_qev= qev->next;
  if (qev->typ == rpl_parallel_thread::queued_event::QUEUED_POS_UPDATE)
@@ -888,7 +902,18 @@ handle_rpl_parallel_thread(void *arg)
 
  group_rgi= rgi;
  group_ending= is_group_ending(qev->ev, event_type);
- if (group_ending && likely(!rgi->worker_error))
+ /*
+ We do not unmark_start_commit() here in case of an explicit ROLLBACK
+ statement. Such events should be very rare, there is no real reason
+ to try to group commit them - on the contrary, it seems best to avoid
+ running them in parallel with following group commits, as with
+ ROLLBACK events we are already deep in dangerous corner cases with
+ mix of transactional and non-transactional tables or the like. And
+ avoiding the mark_start_commit() here allows us to keep an assertion
+ in ha_rollback_trans() that we do not rollback after doing
+ mark_start_commit().
+ */
+ if (group_ending == 1 && likely(!rgi->worker_error))
  {
  /*
  Do an extra check for (deadlock) kill here. This helps prevent a