MDEV-24327 wsrep XID checkpointing order with log_slave_updates=OFFbb-10.2-MDEV-24327

If log_slave_updates==OFF, wsrep applier threads used to be configured
with option: thd->variables.option_bits&= ~(OPTION_BIN_LOG);
(i.e. like sql_log_bin=ON). And this was regardless of log-bin configuration.

With this, having configuration of: --log-bin && --log-slave-updates=OFF,
local threads used binlogging, but applier threads did not. And further:
local threads went through binlog group commit, while applier threads did
direct commits. This resulted in situation, where applier threads entered
earlier in wsrep XID checkpointing, and could sync their wsrep XID out of order.
Later local thread commit would see that higher seqno was already checkpointed,
and fire an assert because of this.

As a fix, applier threads are now forced to enable binlogging regardless of
log-slave-updates configuration.

This PR comes with new mtr test: galera.MDEV-24327, which causes a scenario
where applier transaction is applied and committed while earlier local transaction
is parked before commit order monitor enter. A buggy mariadb versoin would fail
for assertion because of wsrep XID checkpoint order violation.

Reviewed-by: Jan Lindström <jan.lindstrom@mariadb.com>

4 files changed, 132 insertions, 5 deletions

diff --git a/mysql-test/suite/galera/r/MDEV-24327.result b/mysql-test/suite/galera/r/MDEV-24327.result
new file mode 100644
index 00000000000..e7cd9d08f4b
--- /dev/null
+++ b/mysql-test/suite/galera/r/MDEV-24327.result
@@ -0,0 +1,33 @@
+CREATE TABLE t1 (f1 INTEGER PRIMARY KEY, f2 CHAR(1));
+INSERT INTO t1 VALUES (1, 'f');
+INSERT INTO t1 VALUES (2, 'g');
+connection node_1;
+SET AUTOCOMMIT=ON;
+START TRANSACTION;
+UPDATE t1 SET f2 = '1' WHERE f1 = 1;
+connect node_1a, 127.0.0.1, root, , test, $NODE_MYPORT_1;
+SET SESSION wsrep_sync_wait=0;
+connection node_1a;
+SET SESSION wsrep_on = 0;
+SET SESSION wsrep_on = 1;
+SET GLOBAL wsrep_provider_options = 'dbug=';
+SET GLOBAL wsrep_provider_options = 'dbug=d,commit_monitor_enter_sync';
+connection node_1;
+COMMIT;
+connection node_1a;
+SET SESSION wsrep_on = 0;
+SET SESSION wsrep_on = 1;
+SET GLOBAL wsrep_provider_options = 'dbug=';
+connection node_2;
+UPDATE t1 SET f2 = '2' WHERE f1 = 2;
+connection node_1a;
+SET GLOBAL wsrep_provider_options = 'signal=commit_monitor_enter_sync';
+SET GLOBAL wsrep_provider_options = 'dbug=';
+connection node_1;
+SELECT * FROM t1;
+f1	f2
+1	1
+2	2
+"node 1 is complete now"
+connection node_2;
+DROP TABLE t1;
diff --git a/mysql-test/suite/galera/t/MDEV-24327.cnf b/mysql-test/suite/galera/t/MDEV-24327.cnf
new file mode 100644
index 00000000000..390a9aab0f4
--- /dev/null
+++ b/mysql-test/suite/galera/t/MDEV-24327.cnf
@@ -0,0 +1,6 @@
+!include ../galera_2nodes.cnf
+
+[mysqld.1]
+log-bin=mariadb-bin
+log-slave-updates=OFF
+
diff --git a/mysql-test/suite/galera/t/MDEV-24327.test b/mysql-test/suite/galera/t/MDEV-24327.test
new file mode 100644
index 00000000000..fe3dbbe2870
--- /dev/null
+++ b/mysql-test/suite/galera/t/MDEV-24327.test
@@ -0,0 +1,87 @@
+#
+# MDEV-24327 wsrep XID checkpointing order violation with log_slave_updates=OFF
+#
+# Here we have configure two node cluster with --log-bin=ON and --log-slave_-updates=OFF
+#
+# a transaction in node executes so far that it has replicated and reached
+# commit phase, We have sync point before entering commit order monitor and
+# the transaction is parked there
+#
+# Then another transaction is executed in node 2, it replicates and commits in node 2
+# and is received and applied in node 1. After applying it will remain waiting for
+# commit order monitor, as it has later seqno than the first transaction in node 1.
+#
+# control connection in node 1 waits to see the 
+# 
+# With the buggy version of MDEV-24327, the applier has however, already synced the
+# wsrep XID checkpoint
+#
+#
+#
+
+--source include/galera_cluster.inc
+--source include/have_innodb.inc
+--source include/have_debug_sync.inc
+--source include/galera_have_debug_sync.inc
+
+CREATE TABLE t1 (f1 INTEGER PRIMARY KEY, f2 CHAR(1));
+INSERT INTO t1 VALUES (1, 'f');
+INSERT INTO t1 VALUES (2, 'g');
+
+--connection node_1
+SET AUTOCOMMIT=ON;
+START TRANSACTION;
+
+UPDATE t1 SET f2 = '1' WHERE f1 = 1;
+
+--connect node_1a, 127.0.0.1, root, , test, $NODE_MYPORT_1
+SET SESSION wsrep_sync_wait=0;
+--connection node_1a
+--let $expected_wsrep_received = `SELECT VARIABLE_VALUE+1 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_received'`
+--source include/galera_wait_sync_point.inc
+--source include/galera_clear_sync_point.inc
+
+# Block the commit, send the COMMIT and wait until it gets blocked
+
+--let $galera_sync_point = commit_monitor_enter_sync
+--source include/galera_set_sync_point.inc
+
+--connection node_1
+--send COMMIT
+
+--connection node_1a
+
+# wait for the commit to block in sync point
+
+--let $galera_sync_point = commit_monitor_enter_sync
+--source include/galera_wait_sync_point.inc
+--source include/galera_clear_sync_point.inc
+
+#
+# replicate non conflicting transaction from node 2
+# it will get later seqno and should sync XID checkpoint after transaction in node 1
+#
+--connection node_2
+UPDATE t1 SET f2 = '2' WHERE f1 = 2;
+
+#
+# wait until update from node 2 has been committed
+# if XID checkpointing order was violated, node 1 would crash for assert
+#
+
+--connection node_1a
+--let $wait_condition = SELECT VARIABLE_VALUE = $expected_wsrep_received FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_received'
+--source include/wait_condition.inc
+
+--let $galera_sync_point = commit_monitor_enter_sync
+--source include/galera_signal_sync_point.inc
+--source include/galera_clear_sync_point.inc
+
+--connection node_1
+--reap
+SELECT * FROM t1;
+--echo "node 1 is complete now"
+
+
+--connection node_2
+DROP TABLE t1;
diff --git a/sql/wsrep_thd.cc b/sql/wsrep_thd.cc
index 2396b8663df..1c43aeaaead 100644
--- a/sql/wsrep_thd.cc
+++ b/sql/wsrep_thd.cc
@@ -146,11 +146,12 @@ static void wsrep_prepare_bf_thd(THD *thd, struct wsrep_thd_shadow* shadow)
 
   // Disable general logging on applier threads
   thd->variables.option_bits |= OPTION_LOG_OFF;
-  // Enable binlogging if opt_log_slave_updates is set
-  if (opt_log_slave_updates)
-    thd->variables.option_bits|= OPTION_BIN_LOG;
-  else
-    thd->variables.option_bits&= ~(OPTION_BIN_LOG);
+
+  /* enable binlogging regardless of log_slave_updates setting
+     this is for ensuring that both local and applier transaction go through
+     same commit ordering algorithm in group commit control
+   */
+  thd->variables.option_bits|= OPTION_BIN_LOG;
 
   if (!thd->wsrep_rgi) thd->wsrep_rgi= wsrep_relay_group_init(thd, "wsrep_relay");