From: Brandon Nesterenko
This patch adds an MTR test to show that threads awaiting semi-sync
ACKs can be awoken before their ACK is received from the slave, due
to the ack receiver thread broadcasting on a single condition
variable for all threads. The test causes the new assertion to fail.
---
.../rpl/t/rpl_semi_sync_cond_var_per_thd.cnf | 10 +++
.../rpl/t/rpl_semi_sync_cond_var_per_thd.test | 64 +++++++++++++++++++
sql/semisync_master.cc | 8 +++
3 files changed, 82 insertions(+)
create mode 100644 mysql-test/suite/rpl/t/rpl_semi_sync_cond_var_per_thd.cnf
create mode 100644 mysql-test/suite/rpl/t/rpl_semi_sync_cond_var_per_thd.test
diff --git a/mysql-test/suite/rpl/t/rpl_semi_sync_cond_var_per_thd.cnf b/mysql-test/suite/rpl/t/rpl_semi_sync_cond_var_per_thd.cnf
new file mode 100644
index 00000000000..e8e03e71ec8
--- /dev/null
+++ b/mysql-test/suite/rpl/t/rpl_semi_sync_cond_var_per_thd.cnf
@@ -0,0 +1,10 @@
+!include ../my.cnf
+
+[mysqld.1]
+log-warnings=9
+rpl_semi_sync_master_enabled=1
+rpl_semi_sync_master_wait_point=AFTER_COMMIT
+
+[mysqld.2]
+log-warnings=9
+rpl_semi_sync_slave_enabled=1
diff --git a/mysql-test/suite/rpl/t/rpl_semi_sync_cond_var_per_thd.test b/mysql-test/suite/rpl/t/rpl_semi_sync_cond_var_per_thd.test
new file mode 100644
index 00000000000..f8fa0a99d9c
--- /dev/null
+++ b/mysql-test/suite/rpl/t/rpl_semi_sync_cond_var_per_thd.test
@@ -0,0 +1,64 @@
+#
+# This test ensures that, when using semi-sync with the wait_point
+# AFTER_COMMIT, each thread awaiting an ACK is only woken up when its ACK (or
+# an ACK for a later commit in binlog) has been received from the slave.
+#
+# Prior to MDEV-33551, all threads would be woken up for each ACK received,
+# leading to large slowdowns, as each thread would check if the ACK was for it
+# in mutual exclusion from the others.
+#
+# To ensure this, a DBUG_ASSERT is added into
+# Repl_semi_sync_master::commit_trx() to ensure that the ACK binlog coordinate
+# is at or after the coordinate we are waiting on. Then, we use binlog group
+# commit to commit a series of transactions, such that each will await an ACK
+# concurrently.
+#
+# References:
+# MDEV-33551: Semi-sync Wait Point AFTER_COMMIT Slow on Workloads with Heavy
+# Concurrency
+#
+--source include/have_binlog_format_row.inc
+--source include/have_debug.inc
+--source include/master-slave.inc
+
+--connection master
+set @save_bgc_count= @@global.binlog_commit_wait_count;
+set @save_bgc_usec= @@global.binlog_commit_wait_usec;
+set @@global.binlog_commit_wait_count=3;
+set @@global.binlog_commit_wait_usec=10000000;
+
+--echo # Ensure semi-sync is on
+--connection slave
+let $status_var= rpl_semi_sync_slave_status;
+let $status_var_value= ON;
+source include/wait_for_status_var.inc;
+
+--connection master
+let $status_var= rpl_semi_sync_master_status;
+let $status_var_value= ON;
+source include/wait_for_status_var.inc;
+
+--echo # Create three transactions to binlog group commit together
+--connection master
+--send create table t1 (a int)
+--connection server_1
+--send create table t2 (a int)
+--connection default
+--send create table t3 (a int)
+
+--connection master
+--reap
+--connection server_1
+--reap
+--connection default
+--reap
+
+
+--echo #
+--echo # Cleanup
+--connection master
+set @@global.binlog_commit_wait_count=@save_bgc_count;
+set @@global.binlog_commit_wait_usec=@save_bgc_usec;
+drop table t1, t2, t3;
+
+--source include/rpl_end.inc
diff --git a/sql/semisync_master.cc b/sql/semisync_master.cc
index 8cc721e5737..0eaf0f0e0e2 100644
--- a/sql/semisync_master.cc
+++ b/sql/semisync_master.cc
@@ -979,6 +979,14 @@ int Repl_semi_sync_master::commit_trx(const char* trx_wait_binlog_name,
{
rpl_semi_sync_master_trx_wait_num++;
rpl_semi_sync_master_trx_wait_time += wait_time;
+ /*
+ Assert we have either recieved our ACK; or have timed out and are
+ awoken in an off state.
+ */
+ DBUG_ASSERT(!get_master_enabled() || !is_on() || thd->is_killed() ||
+ 0 <= Active_tranx::compare(
+ m_reply_file_name, m_reply_file_pos,
+ trx_wait_binlog_name, trx_wait_binlog_pos));
}
}
}
--
2.30.2
