MDEV-22021: Galera database could get inconsistent with rollback to s…

…avepoint When binlog is disabled, WSREP will not behave correctly when SAVEPOINT ROLLBACK is executed and we will not rollback transaction.
MariaDB · Mar 31, 2020 · bdcecfa · bdcecfa
1 parent e2f1f88
commit bdcecfa
Show file tree

Hide file tree

Showing 10 changed files with 139 additions and 25 deletions.
diff --git a/mysql-test/suite/galera/r/MDEV-22021.result b/mysql-test/suite/galera/r/MDEV-22021.result
@@ -0,0 +1,57 @@
+connection node_2;
+connection node_1;
+CREATE TABLE t1 (f1 INTEGER PRIMARY KEY);
+START TRANSACTION;
+INSERT INTO t1 VALUES (1);
+SAVEPOINT sp1;
+INSERT INTO t1 VALUES (2);
+ROLLBACK TO SAVEPOINT sp1;
+COMMIT;
+SELECT COUNT(*) = 1 FROM t1;
+COUNT(*) = 1
+1
+connection node_2;
+SELECT COUNT(*) = 1 FROM t1;
+COUNT(*) = 1
+1
+connection node_1;
+DELETE FROM t1;
+START TRANSACTION;
+SAVEPOINT sp1;
+INSERT INTO t1 VALUES (1);
+SAVEPOINT sp2;
+INSERT INTO t1 VALUES (2);
+ROLLBACK TO SAVEPOINT sp2;
+ROLLBACK TO SAVEPOINT sp1;
+COMMIT;
+SELECT COUNT(*) = 0 FROM t1;
+COUNT(*) = 0
+1
+connection node_2;
+SELECT COUNT(*) = 0 FROM t1;
+COUNT(*) = 0
+1
+connection node_1;
+DELETE FROM t1;
+START TRANSACTION;
+SAVEPOINT sp1;
+INSERT INTO t1 VALUES (1);
+INSERT INTO t1 VALUES (2);
+INSERT INTO t1 VALUES (3);
+INSERT INTO t1 VALUES (4);
+SAVEPOINT sp2;
+INSERT INTO t1 VALUES (5);
+ROLLBACK TO SAVEPOINT sp2;
+INSERT INTO t1 VALUES (6);
+INSERT INTO t1 VALUES (7);
+ROLLBACK TO SAVEPOINT sp1;
+INSERT INTO t1 VALUES (8);
+COMMIT;
+SELECT COUNT(*) = 1 FROM t1;
+COUNT(*) = 1
+1
+connection node_2;
+SELECT COUNT(*) = 1 FROM t1;
+COUNT(*) = 1
+1
+DROP TABLE t1;
diff --git a/mysql-test/suite/galera/t/MDEV-22021.combinations b/mysql-test/suite/galera/t/MDEV-22021.combinations
@@ -0,0 +1,4 @@
+[binlogoff]
+
+[binlogon]
+log-bin
diff --git a/mysql-test/suite/galera/t/MDEV-22021.test b/mysql-test/suite/galera/t/MDEV-22021.test
@@ -0,0 +1,60 @@
+#
+# SAVEPOINT ROLLBACK can introduce inconsistency in cluster.
+#
+
+--source include/galera_cluster.inc
+
+CREATE TABLE t1 (f1 INTEGER PRIMARY KEY);
+
+START TRANSACTION;
+INSERT INTO t1 VALUES (1);
+SAVEPOINT sp1;
+INSERT INTO t1 VALUES (2);
+ROLLBACK TO SAVEPOINT sp1;
+COMMIT;
+
+SELECT COUNT(*) = 1 FROM t1;
+
+--connection node_2
+SELECT COUNT(*) = 1 FROM t1;
+
+--connection node_1
+DELETE FROM t1;
+
+START TRANSACTION;
+SAVEPOINT sp1;
+INSERT INTO t1 VALUES (1);
+SAVEPOINT sp2;
+INSERT INTO t1 VALUES (2);
+ROLLBACK TO SAVEPOINT sp2;
+ROLLBACK TO SAVEPOINT sp1;
+COMMIT;
+
+SELECT COUNT(*) = 0 FROM t1;
+--connection node_2
+SELECT COUNT(*) = 0 FROM t1;
+
+--connection node_1
+DELETE FROM t1;
+
+START TRANSACTION;
+SAVEPOINT sp1;
+INSERT INTO t1 VALUES (1);
+INSERT INTO t1 VALUES (2);
+INSERT INTO t1 VALUES (3);
+INSERT INTO t1 VALUES (4);
+SAVEPOINT sp2;
+INSERT INTO t1 VALUES (5);
+ROLLBACK TO SAVEPOINT sp2;
+INSERT INTO t1 VALUES (6);
+INSERT INTO t1 VALUES (7);
+ROLLBACK TO SAVEPOINT sp1;
+INSERT INTO t1 VALUES (8);
+COMMIT;
+
+SELECT COUNT(*) = 1 FROM t1;
+
+--connection node_2
+SELECT COUNT(*) = 1 FROM t1;
+
+DROP TABLE t1;
diff --git a/mysql-test/suite/galera_sr/r/GCF-571.result b/mysql-test/suite/galera_sr/r/GCF-571.result
@@ -24,7 +24,7 @@ COUNT(*) > 0
 1
 SELECT COUNT(*) = 1 FROM mysql.wsrep_streaming_log WHERE frag LIKE '%SAVEPOINT `A`%';
 COUNT(*) = 1
-0
+1
 connection node_1;
 ROLLBACK TO SAVEPOINT A;
 connection node_1a;
@@ -36,7 +36,7 @@ COUNT(*) > 0
 1
 SELECT COUNT(*) = 1 FROM mysql.wsrep_streaming_log WHERE frag LIKE '%SAVEPOINT `A`%';
 COUNT(*) = 1
-0
+1
 SELECT COUNT(*) = 1 FROM mysql.wsrep_streaming_log WHERE frag LIKE '%ROLLBACK TO `A`%';
 COUNT(*) = 1
 0
@@ -49,7 +49,7 @@ COUNT(*) > 0
 1
 SELECT COUNT(*) = 1 FROM mysql.wsrep_streaming_log WHERE frag LIKE '%SAVEPOINT `A`%';
 COUNT(*) = 1
-0
+1
 SELECT COUNT(*) = 1 FROM mysql.wsrep_streaming_log WHERE frag LIKE '%ROLLBACK TO `A`%';
 COUNT(*) = 1
 0

diff --git a/mysql-test/suite/galera_sr/r/GCF-620.result b/mysql-test/suite/galera_sr/r/GCF-620.result
@@ -14,5 +14,5 @@ COMMIT;
 connection node_2;
 SELECT COUNT(*) = 2 FROM t1;
 COUNT(*) = 2
-0
+1
 DROP TABLE t1;
diff --git a/mysql-test/suite/galera_sr/r/galera_sr_rollback_savepoint.result b/mysql-test/suite/galera_sr/r/galera_sr_rollback_savepoint.result
@@ -33,7 +33,7 @@ COUNT(*) = 5
 1
 SELECT COUNT(*) = 0 FROM t1 WHERE f2 = 'b';
 COUNT(*) = 0
-0
+1
 SELECT COUNT(*) = 5 FROM t1 WHERE f2 = 'c';
 COUNT(*) = 5
 1

diff --git a/mysql-test/suite/galera_sr/r/mysql-wsrep-features#22.result b/mysql-test/suite/galera_sr/r/mysql-wsrep-features#22.result
@@ -31,5 +31,5 @@ COUNT(*) = 6
 connection node_2;
 SELECT COUNT(*) = 6 FROM t1;
 COUNT(*) = 6
-0
+1
 DROP TABLE t1;
diff --git a/sql/log.cc b/sql/log.cc
@@ -2271,9 +2271,6 @@ static int binlog_savepoint_set(handlerton *hton, THD *thd, void *sv)
   int error= 1;
   DBUG_ENTER("binlog_savepoint_set");
 
-  if (wsrep_emulate_bin_log)
-    DBUG_RETURN(0);
-
   char buf[1024];
 
   String log_query(buf, sizeof(buf), &my_charset_bin);
@@ -2306,9 +2303,6 @@ static int binlog_savepoint_rollback(handlerton *hton, THD *thd, void *sv)
 {
   DBUG_ENTER("binlog_savepoint_rollback");
 
-  if (wsrep_emulate_bin_log)
-    DBUG_RETURN(0);
-
   /*
     Write ROLLBACK TO SAVEPOINT to the binlog cache if we have updated some
     non-transactional table. Otherwise, truncate the binlog cache starting
@@ -10798,18 +10792,20 @@ void wsrep_register_binlog_handler(THD *thd, bool trx)
     back a statement or a transaction. However, notifications do not happen
     if the binary log is set as read/write.
   */
-  //binlog_cache_mngr *cache_mngr= thd_get_cache_mngr(thd);
   binlog_cache_mngr *cache_mngr=
     (binlog_cache_mngr*) thd_get_ha_data(thd, binlog_hton);
   /* cache_mngr may be missing e.g. in mtr test ev51914.test */
-  if (cache_mngr && cache_mngr->trx_cache.get_prev_position() == MY_OFF_T_UNDEF)
+  if (cache_mngr)
   {
     /*
       Set an implicit savepoint in order to be able to truncate a trx-cache.
     */
-    my_off_t pos= 0;
-    binlog_trans_log_savepos(thd, &pos);
-    cache_mngr->trx_cache.set_prev_position(pos);
+    if (cache_mngr->trx_cache.get_prev_position() == MY_OFF_T_UNDEF)
+    {
+      my_off_t pos= 0;
+      binlog_trans_log_savepos(thd, &pos);
+      cache_mngr->trx_cache.set_prev_position(pos);
+    }
 
     /*
       Set callbacks in order to be able to call commmit or rollback.

diff --git a/sql/wsrep_high_priority_service.cc b/sql/wsrep_high_priority_service.cc
@@ -453,11 +453,15 @@ Wsrep_applier_service::Wsrep_applier_service(THD* thd)
   thd->wsrep_cs().open(wsrep::client_id(thd->thread_id));
   thd->wsrep_cs().before_command();
   thd->wsrep_cs().debug_log_level(wsrep_debug);
-
+  if (!thd->slave_thread)
+    thd->system_thread_info.rpl_sql_info=
+      new rpl_sql_thread_info(thd->wsrep_rgi->rli->mi->rpl_filter);
 }
 
 Wsrep_applier_service::~Wsrep_applier_service()
 {
+  if (!m_thd->slave_thread)
+    delete m_thd->system_thread_info.rpl_sql_info;
   m_thd->wsrep_cs().after_command_before_result();
   m_thd->wsrep_cs().after_command_after_result();
   m_thd->wsrep_cs().close();

diff --git a/sql/wsrep_thd.cc b/sql/wsrep_thd.cc
@@ -53,11 +53,6 @@ static void wsrep_replication_process(THD *thd,
 
   Wsrep_applier_service applier_service(thd);
 
-  /* thd->system_thread_info.rpl_sql_info isn't initialized. */
-  if (!thd->slave_thread)
-    thd->system_thread_info.rpl_sql_info=
-      new rpl_sql_thread_info(thd->wsrep_rgi->rli->mi->rpl_filter);
-
   WSREP_INFO("Starting applier thread %llu", thd->thread_id);
   enum wsrep::provider::status
     ret= Wsrep_server_state::get_provider().run_applier(&applier_service);
@@ -68,8 +63,6 @@ static void wsrep_replication_process(THD *thd,
   mysql_cond_broadcast(&COND_wsrep_slave_threads);
   mysql_mutex_unlock(&LOCK_wsrep_slave_threads);
 
-  if (!thd->slave_thread)
-    delete thd->system_thread_info.rpl_sql_info;
   delete thd->wsrep_rgi->rli->mi;
   delete thd->wsrep_rgi->rli;