MDEV-25553 : Avoid unnecessary rollbacks with SRbb-10.4-MDEV-25553

This patch changes statement rollback for streaming replication.
Previously, a statement rollback was turned into full transaction
rollback in the case where the transaction had already replicated a
fragment. This was introduced in the initial implementation of
streaming replication due to the fact that we do not have a mechanism
to perform a statement rollback on the applying side.
This policy is however overly pessimistic, causing full rollbacks even
in cases where a local statement rollback, would not require a
statement rollback on the applying side. This happens to be case when
the statement itself has not replicated any fragments.
So the patch changes the condition that determines if a statement
rollback should be turned into a full rollback accordingly.

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

12 files changed, 176 insertions, 56 deletions

diff --git a/mysql-test/suite/galera_3nodes_sr/disabled.def b/mysql-test/suite/galera_3nodes_sr/disabled.def
index 0944abd0ad5..900b27860a5 100644
--- a/mysql-test/suite/galera_3nodes_sr/disabled.def
+++ b/mysql-test/suite/galera_3nodes_sr/disabled.def
@@ -1,6 +1,5 @@
 GCF-336 : 
 GCF-582 : 
-GCF-609 : 
 GCF-810A : 
 GCF-810B : 
 GCF-810C : 
diff --git a/mysql-test/suite/galera_3nodes_sr/r/GCF-609.result b/mysql-test/suite/galera_3nodes_sr/r/GCF-609.result
index 8fe13c7e2bf..db7da93c8c0 100644
--- a/mysql-test/suite/galera_3nodes_sr/r/GCF-609.result
+++ b/mysql-test/suite/galera_3nodes_sr/r/GCF-609.result
@@ -1,20 +1,78 @@
+connection node_2;
+connection node_1;
 CREATE TABLE t1 (f1 INTEGER PRIMARY KEY) ENGINE=InnoDB;
+connection node_1;
 SET SESSION wsrep_trx_fragment_size=1;
 SET AUTOCOMMIT=OFF;
 START TRANSACTION;
 INSERT INTO t1 VALUES (1),(2),(3),(4),(5),(6),(7),(8),(9),(10);
+connection node_2;
 SET SESSION wsrep_trx_fragment_size=1;
 SET AUTOCOMMIT=OFF;
 START TRANSACTION;
 INSERT INTO t1 VALUES (11),(12),(13),(14),(15),(16),(17),(18),(19),(20);
 INSERT INTO t1 VALUES (11),(12),(13),(14),(15),(16),(17),(18),(19),(20);
-ERROR 40001: Deadlock found when trying to get lock; try restarting transaction
+ERROR 23000: Duplicate entry '11' for key 'PRIMARY'
 INSERT INTO t1 VALUES (31),(32),(33);
 SELECT COUNT(*) = 0 FROM mysql.wsrep_streaming_log;
 COUNT(*) = 0
 0
+connection node_1;
 SELECT COUNT(*) = 0 FROM mysql.wsrep_streaming_log;
 COUNT(*) = 0
 0
 COMMIT;
+connection node_2;
+COMMIT;
+SELECT * FROM t1;
+f1
+1
+2
+3
+4
+5
+6
+7
+8
+9
+10
+11
+12
+13
+14
+15
+16
+17
+18
+19
+20
+31
+32
+33
+connection node_1;
+SELECT * FROM t1;
+f1
+1
+2
+3
+4
+5
+6
+7
+8
+9
+10
+11
+12
+13
+14
+15
+16
+17
+18
+19
+20
+31
+32
+33
 DROP TABLE t1;
diff --git a/mysql-test/suite/galera_3nodes_sr/t/GCF-609.test b/mysql-test/suite/galera_3nodes_sr/t/GCF-609.test
index fd346cf365b..210b100a9f8 100644
--- a/mysql-test/suite/galera_3nodes_sr/t/GCF-609.test
+++ b/mysql-test/suite/galera_3nodes_sr/t/GCF-609.test
@@ -17,7 +17,7 @@ SET SESSION wsrep_trx_fragment_size=1;
 SET AUTOCOMMIT=OFF;
 START TRANSACTION;
 INSERT INTO t1 VALUES (11),(12),(13),(14),(15),(16),(17),(18),(19),(20);
---error ER_LOCK_DEADLOCK
+--error ER_DUP_ENTRY
 INSERT INTO t1 VALUES (11),(12),(13),(14),(15),(16),(17),(18),(19),(20);
 INSERT INTO t1 VALUES (31),(32),(33);
 
@@ -27,4 +27,11 @@ SELECT COUNT(*) = 0 FROM mysql.wsrep_streaming_log;
 SELECT COUNT(*) = 0 FROM mysql.wsrep_streaming_log;
 COMMIT;
 
+--connection node_2
+COMMIT;
+SELECT * FROM t1;
+
+--connection node_1
+SELECT * FROM t1;
+
 DROP TABLE t1;
diff --git a/mysql-test/suite/galera_sr/r/GCF-572.result b/mysql-test/suite/galera_sr/r/GCF-572.result
index cb4d48b3600..41ae2378a3f 100644
--- a/mysql-test/suite/galera_sr/r/GCF-572.result
+++ b/mysql-test/suite/galera_sr/r/GCF-572.result
@@ -1,18 +1,38 @@
 connection node_2;
 connection node_1;
-CREATE TABLE t1 (f1 INTEGER PRIMARY KEY AUTO_INCREMENT, f2 CHAR(10)) ENGINE=InnoDB;
 connect node_1a, 127.0.0.1, root, , test, $NODE_MYPORT_1;
 connection node_1;
 SET SESSION wsrep_trx_fragment_size = 1;
+CREATE TABLE t1 (f1 INTEGER PRIMARY KEY);
+CREATE TABLE t2 (f1 INTEGER PRIMARY KEY);
+INSERT INTO t2 VALUES (1),(2),(3);
+ALTER TABLE t2 DROP PRIMARY KEY;
+INSERT INTO t2 VALUES (1);
+SET SESSION wsrep_trx_fragment_size = 1;
+START TRANSACTION;
+INSERT INTO t1 SELECT * FROM t2;
+ERROR 40001: Deadlock found when trying to get lock; try restarting transaction
+SELECT * FROM t1;
+f1
+connection node_2;
+SELECT * FROM t1;
+f1
+connection node_1;
+DROP TABLE t1;
+DROP TABLE t2;
+CREATE TABLE t1 (f1 INTEGER PRIMARY KEY AUTO_INCREMENT, f2 CHAR(10)) ENGINE=InnoDB;
+connection node_1;
+SET SESSION wsrep_trx_fragment_size = 1;
 START TRANSACTION;
 INSERT INTO t1 VALUES (1, 'node1');
 connection node_1a;
 INSERT INTO t1 VALUES (5, 'node2');
 connection node_1;
 INSERT INTO t1 VALUES (5, 'node1');
-ERROR 40001: Deadlock found when trying to get lock; try restarting transaction
+ERROR 23000: Duplicate entry '5' for key 'PRIMARY'
 SELECT * FROM t1;
 f1	f2
+1	node1
 5	node2
 SET SESSION wsrep_trx_fragment_size = 10000;
 START TRANSACTION;
@@ -24,6 +44,7 @@ INSERT INTO t1 VALUES(15, 'node2');
 connection node_1;
 SELECT * FROM t1;
 f1	f2
+1	node1
 5	node2
 10	node1
 INSERT INTO t1 VALUES(15, 'node1');
@@ -31,6 +52,7 @@ ERROR 23000: Duplicate entry '15' for key 'PRIMARY'
 COMMIT;
 SELECT * FROM t1;
 f1	f2
+1	node1
 5	node2
 10	node1
 15	node2
diff --git a/mysql-test/suite/galera_sr/r/galera_sr_dupkey_error.result b/mysql-test/suite/galera_sr/r/galera_sr_dupkey_error.result
index b23b934da33..ad843c7c2c6 100644
--- a/mysql-test/suite/galera_sr/r/galera_sr_dupkey_error.result
+++ b/mysql-test/suite/galera_sr/r/galera_sr_dupkey_error.result
@@ -13,7 +13,7 @@ INSERT INTO t1 VALUES (REPEAT('e', 512));
 INSERT INTO t1 VALUES (REPEAT('f', 512));
 connection node_2;
 connection node_1;
-INSERT INTO t1 VALUES (REPEAT('c', 512));
+INSERT INTO t1 VALUES (REPEAT('g', 1024)),(REPEAT('c', 512));
 ERROR 40001: Deadlock found when trying to get lock; try restarting transaction
 connection node_1;
 SELECT COUNT(*) = 0 FROM mysql.wsrep_streaming_log;
diff --git a/mysql-test/suite/galera_sr/r/mysql-wsrep-bugs-900.result b/mysql-test/suite/galera_sr/r/mysql-wsrep-bugs-900.result
new file mode 100644
index 00000000000..a51b5299aa2
--- /dev/null
+++ b/mysql-test/suite/galera_sr/r/mysql-wsrep-bugs-900.result
@@ -0,0 +1,15 @@
+connection node_2;
+connection node_1;
+CREATE TABLE t1 (f1 INTEGER PRIMARY KEY);
+SET SESSION wsrep_trx_fragment_size=1;
+START TRANSACTION;
+INSERT INTO t1 VALUES (1);
+SET SESSION wsrep_cluster_name = ' ';
+ERROR HY000: Variable 'wsrep_cluster_name' is a GLOBAL variable and should be set with SET GLOBAL
+INSERT INTO t1 VALUES (2);
+COMMIT;
+SELECT f1 AS expect_1_and_2 FROM t1;
+expect_1_and_2
+1
+2
+DROP TABLE t1;
diff --git a/mysql-test/suite/galera_sr/t/GCF-572.test b/mysql-test/suite/galera_sr/t/GCF-572.test
index abefb9b08f6..be77451a332 100644
--- a/mysql-test/suite/galera_sr/t/GCF-572.test
+++ b/mysql-test/suite/galera_sr/t/GCF-572.test
@@ -1,15 +1,43 @@
 --source include/galera_cluster.inc
 --source include/have_innodb.inc
 
-CREATE TABLE t1 (f1 INTEGER PRIMARY KEY AUTO_INCREMENT, f2 CHAR(10)) ENGINE=InnoDB;
-
 --connect node_1a, 127.0.0.1, root, , test, $NODE_MYPORT_1
 
 #
-# Test 1: statement rollback is not safe
-# (some fragments were already replicated)
+# Test 1: statement rollback is not safe.
+# Statement has already replicated some fragments
 #
+--connection node_1
+SET SESSION wsrep_trx_fragment_size = 1;
+CREATE TABLE t1 (f1 INTEGER PRIMARY KEY);
+CREATE TABLE t2 (f1 INTEGER PRIMARY KEY);
+INSERT INTO t2 VALUES (1),(2),(3);
+ALTER TABLE t2 DROP PRIMARY KEY;
+INSERT INTO t2 VALUES (1);
 
+SET SESSION wsrep_trx_fragment_size = 1;
+START TRANSACTION;
+
+# The following INSERT .. SELECT inserts a duplicate key,
+# ER_LOCK_DEADLOCK is the only possible outcome at this point.
+# Notice that ER_DUP_ENTRY is NOT an option here because we were
+# forced to rollback the whole transaction (not just the statement)
+--error ER_LOCK_DEADLOCK
+INSERT INTO t1 SELECT * FROM t2;
+SELECT * FROM t1;
+
+--connection node_2
+SELECT * FROM t1;
+
+--connection node_1
+DROP TABLE t1;
+DROP TABLE t2;
+
+#
+# Test 2: statement rollback is safe.
+# Fragments were already replicated, not in the same statement
+#
+CREATE TABLE t1 (f1 INTEGER PRIMARY KEY AUTO_INCREMENT, f2 CHAR(10)) ENGINE=InnoDB;
 --connection node_1
 SET SESSION wsrep_trx_fragment_size = 1;
 START TRANSACTION;
@@ -19,20 +47,16 @@ INSERT INTO t1 VALUES (1, 'node1');
 INSERT INTO t1 VALUES (5, 'node2');
 
 --connection node_1
-# If we try to INSERT a duplicate key, ER_LOCK_DEADLOCK is the only possible
-# outcome at this point. Notice that ER_DUP_ENTRY is NOT an option here
-# because we were forced to rollback the whole transaction (not just the
-# statement)
---error ER_LOCK_DEADLOCK
+# Only the statement is rolled back, expect ER_DUP_ENTRY
+--error ER_DUP_ENTRY
 INSERT INTO t1 VALUES (5, 'node1');
 
 SELECT * FROM t1;
 
 #
-# Test 2: statement rollback is safe
-# (no fragments have been replicated)
+# Test 3: statement rollback is safe
+# No fragments have been replicated
 #
-
 SET SESSION wsrep_trx_fragment_size = 10000;
 
 START TRANSACTION;
diff --git a/mysql-test/suite/galera_sr/t/galera_sr_dupkey_error.test b/mysql-test/suite/galera_sr/t/galera_sr_dupkey_error.test
index a7aca042829..146da425d88 100644
--- a/mysql-test/suite/galera_sr/t/galera_sr_dupkey_error.test
+++ b/mysql-test/suite/galera_sr/t/galera_sr_dupkey_error.test
@@ -22,12 +22,13 @@ INSERT INTO t1 VALUES (REPEAT('f', 512));
 --let $wait_condition = SELECT COUNT(*) > 0 FROM mysql.wsrep_streaming_log;
 
 --connection node_1
-# Deadlock error instead of dupkey since the transaction is SR and
-# statement rollback is not safe.
+# Deadlock error instead of dupkey since the transaction is SR,
+# and the statement has already replicated a fragment (which
+# makes statement rollback unsafe).
 --error ER_LOCK_DEADLOCK
-INSERT INTO t1 VALUES (REPEAT('c', 512));
+INSERT INTO t1 VALUES (REPEAT('g', 1024)),(REPEAT('c', 512));
 
-# Confirm that the wsrep_streaming_log table is now empty, as it was a full transaction rollback
+# Confirm that the wsrep_schema table is now empty, as it was a full transaction rollback
 
 --connection node_1
 SELECT COUNT(*) = 0 FROM mysql.wsrep_streaming_log;
diff --git a/mysql-test/suite/galera_sr/t/mysql-wsrep-bugs-900.test b/mysql-test/suite/galera_sr/t/mysql-wsrep-bugs-900.test
new file mode 100644
index 00000000000..d534fcc5524
--- /dev/null
+++ b/mysql-test/suite/galera_sr/t/mysql-wsrep-bugs-900.test
@@ -0,0 +1,22 @@
+#
+# Statement with no side effects causes unnecessary full rollback
+#
+
+--source include/galera_cluster.inc
+
+CREATE TABLE t1 (f1 INTEGER PRIMARY KEY);
+SET SESSION wsrep_trx_fragment_size=1;
+START TRANSACTION;
+INSERT INTO t1 VALUES (1);
+# Let's cause  some bogus error with a statement that
+# does not cause any replication event.
+# The following used to return error ER_LOCK_DEADLOCK
+# and cause the entire transaction to be rolled back.
+--error ER_GLOBAL_VARIABLE
+SET SESSION wsrep_cluster_name = ' ';
+
+INSERT INTO t1 VALUES (2);
+COMMIT;
+
+SELECT f1 AS expect_1_and_2 FROM t1;
+DROP TABLE t1;
\ No newline at end of file
diff --git a/sql/log.cc b/sql/log.cc
index 49a319eb29d..eb82100d16c 100644
--- a/sql/log.cc
+++ b/sql/log.cc
@@ -10843,35 +10843,6 @@ void wsrep_thd_binlog_stmt_rollback(THD * thd)
   DBUG_VOID_RETURN;
 }
 
-bool wsrep_stmt_rollback_is_safe(THD* thd)
-{
-  bool ret(true);
-
-  DBUG_ENTER("wsrep_binlog_stmt_rollback_is_safe");
-
-  binlog_cache_mngr *cache_mngr= 
-    (binlog_cache_mngr*) thd_get_ha_data(thd, binlog_hton);
-
-
-  if (binlog_hton && cache_mngr)
-  {
-    binlog_cache_data * trx_cache = &cache_mngr->trx_cache;
-    if (thd->wsrep_sr().fragments_certified() > 0 &&
-        (trx_cache->get_prev_position() == MY_OFF_T_UNDEF ||
-         trx_cache->get_prev_position() < thd->wsrep_sr().log_position()))
-    {
-      WSREP_DEBUG("statement rollback is not safe for streaming replication"
-                  " pre-stmt_pos: %llu, frag repl pos: %zu\n"
-                  "Thread: %llu, SQL: %s",
-                  trx_cache->get_prev_position(),
-                  thd->wsrep_sr().log_position(),
-                  thd->thread_id, thd->query());
-       ret = false;
-    }
-  }
-  DBUG_RETURN(ret);
-}
-
 void wsrep_register_binlog_handler(THD *thd, bool trx)
 {
   DBUG_ENTER("register_binlog_handler");
diff --git a/sql/wsrep_mysqld.h b/sql/wsrep_mysqld.h
index db6910030c8..748d93c72aa 100644
--- a/sql/wsrep_mysqld.h
+++ b/sql/wsrep_mysqld.h
@@ -377,8 +377,6 @@ int wsrep_to_buf_helper(
 int wsrep_create_trigger_query(THD *thd, uchar** buf, size_t* buf_len);
 int wsrep_create_event_query(THD *thd, uchar** buf, size_t* buf_len);
 
-bool wsrep_stmt_rollback_is_safe(THD* thd);
-
 void wsrep_init_sidno(const wsrep_uuid_t&);
 bool wsrep_node_is_donor();
 bool wsrep_node_is_synced();
diff --git a/sql/wsrep_trans_observer.h b/sql/wsrep_trans_observer.h
index bb9bd54b02f..8157c14fcdf 100644
--- a/sql/wsrep_trans_observer.h
+++ b/sql/wsrep_trans_observer.h
@@ -345,11 +345,14 @@ static inline int wsrep_before_rollback(THD* thd, bool all)
       }
 
       if (thd->wsrep_trx().is_streaming() &&
-          !wsrep_stmt_rollback_is_safe(thd))
+          (wsrep_fragments_certified_for_stmt(thd) > 0))
       {
         /* Non-safe statement rollback during SR multi statement
-           transasction. Self abort the transaction, the actual rollback
-           and error handling will be done in after statement phase. */
+           transaction. A statement rollback is considered unsafe, if
+           the same statement has already replicated one or more fragments.
+           Self abort the transaction, the actual rollback and error
+           handling will be done in after statement phase. */
+        WSREP_DEBUG("statement rollback is not safe for streaming replication");
         wsrep_thd_self_abort(thd);
         ret= 0;
       }