BUG#35164090 NDB MTA fails to retry temporary error

blaudden · bjornmu · commit b5cc47e8cc72 · 2023-03-16T18:07:26.000+01:00
Temporary error that occurs while applying a transaction using MTA(multi
threaded applier) for NDB is not properly retried. The problem is only
visible when using more than one worker.

The problem causes replication applier to stop with error similar to
below, the "Got error -1" is significant:
 "Worker 1 failed executing transaction 'ANONYMOUS' at source
  log master-bin.000001, end_log_pos 1341; Could not execute Write_rows
  event on table test.t1; Got error -1 - 'Unknown error -1'
  during COMMIT, Error_code: 1180; the event's source log
  master-bin.000001, end_log_pos 1341"

Fix by:
- returning a MySQL mapped error code, thus avoiding that -1 is used as
error code.
- push warning(s) indicating temporary error to the warning stack, this
is important in order for the replication applier to classify the error
as temporary.

Add tests showing how a provoked deadlock (which is considered temporary
and should be retried) stops the applier when no transaction retries are
allowed and sucessfully retries when allowed.

Change-Id: I0ffc17beda65637d45fdba8c3d7dc4fe328f4527
diff --git a/mysql-test/suite/ndb_rpl/r/ndb_rpl_temp_error_retry.result b/mysql-test/suite/ndb_rpl/r/ndb_rpl_temp_error_retry.result
@@ -0,0 +1,62 @@
+include/master-slave.inc
+Warnings:
+Note	####	Sending passwords in plain text without SSL/TLS is extremely insecure.
+Note	####	Storing MySQL user name or password information in the connection metadata repository is not secure and is therefore not recommended. Please consider using the USER and PASSWORD connection options for START REPLICA; see the 'START REPLICA Syntax' in the MySQL Manual for more information.
+[connection master]
+##################################################################
+#
+# Check that applier retries temporary errors
+# - first show that applier stops when no retries are allowed.
+# - then reconfigure to allow transaction retries and show how
+#   applier is retrying until the provoked error is cleared
+#
+[connection slave]
+call mtr.add_suppression(".*Lock timed out; Retry transaction.*");
+call mtr.add_suppression(".*worker thread retried transaction 1 time.*");
+call mtr.add_suppression(".*coordinator and worker threads are stopped.*");
+[connection master]
+CREATE TABLE t1 (
+nid int NOT NULL,
+nom char(4) default NULL,
+prenom char(4) default NULL,
+PRIMARY KEY USING HASH (nid)
+) ENGINE=ndbcluster DEFAULT CHARSET=latin1;
+INSERT INTO t1 VALUES (1,"XYZ1","ABC1");
+include/sync_slave_sql_with_master.inc
+[connection slave]
+# Lock the row on replica
+BEGIN;
+UPDATE t1 SET nom="LOCK" WHERE nid=1;
+# Set number of retries low so retries are not allowed
+SET GLOBAL replica_transaction_retries=1;
+[connection master]
+# Change the row on source, this will cause lock wait timeout when
+# applying on replica
+UPDATE t1 SET nom="DEAD" WHERE nid=1;
+[connection slave1]
+# Wait for deadlock to be detected and applier to stop since
+# retries where not allowed. Use different connection.
+include/wait_for_slave_sql_error.inc [errno=1205]
+# Reconfigure retries high enough to allow retries, start replication
+SET GLOBAL replica_transaction_retries=10;
+include/start_slave.inc
+# Wait for deadlock to be detected and retried at least two times.
+[connection slave]
+# Switch back to the connection holding lock on replica, commit the
+# transaction to release lock and thus let the currently retrying
+# applier succeed
+select * from t1 order by nid;
+nid	nom	prenom
+1	LOCK	ABC1
+COMMIT;
+[connection master]
+include/sync_slave_sql_with_master.inc
+[connection slave]
+# Verify that the row was applied sucessfully on replica
+SELECT * FROM t1;
+nid	nom	prenom
+1	DEAD	ABC1
+[connection master]
+# Cleanup
+DROP TABLE t1;
+include/rpl_end.inc
diff --git a/mysql-test/suite/ndb_rpl/t/ndb_rpl_temp_error_retry.test b/mysql-test/suite/ndb_rpl/t/ndb_rpl_temp_error_retry.test
@@ -0,0 +1,78 @@
+--source include/have_ndb.inc
+--source suite/ndb_rpl/ndb_master-slave.inc
+
+--echo ##################################################################
+--echo #
+--echo # Check that applier retries temporary errors
+--echo # - first show that applier stops when no retries are allowed.
+--echo # - then reconfigure to allow transaction retries and show how
+--echo #   applier is retrying until the provoked error is cleared
+--echo #
+
+--source include/rpl_connection_slave.inc
+# Supress warnings caused by provoked temporary error retries
+call mtr.add_suppression(".*Lock timed out; Retry transaction.*");
+call mtr.add_suppression(".*worker thread retried transaction 1 time.*");
+call mtr.add_suppression(".*coordinator and worker threads are stopped.*");
+
+--source include/rpl_connection_master.inc
+CREATE TABLE t1 (
+  nid int NOT NULL,
+  nom char(4) default NULL,
+  prenom char(4) default NULL,
+  PRIMARY KEY USING HASH (nid)
+) ENGINE=ndbcluster DEFAULT CHARSET=latin1;
+INSERT INTO t1 VALUES (1,"XYZ1","ABC1");
+
+--source include/sync_slave_sql_with_master.inc
+
+--source include/rpl_connection_slave.inc
+--echo # Lock the row on replica
+BEGIN;
+UPDATE t1 SET nom="LOCK" WHERE nid=1;
+
+--echo # Set number of retries low so retries are not allowed
+SET GLOBAL replica_transaction_retries=1;
+
+--source include/rpl_connection_master.inc
+--echo # Change the row on source, this will cause lock wait timeout when
+--echo # applying on replica
+UPDATE t1 SET nom="DEAD" WHERE nid=1;
+
+--source include/rpl_connection_slave1.inc
+--echo # Wait for deadlock to be detected and applier to stop since
+--echo # retries where not allowed. Use different connection.
+--let $slave_sql_errno= convert_error(ER_LOCK_WAIT_TIMEOUT)
+--let $show_slave_sql_error= 0
+--source include/wait_for_slave_sql_error.inc
+
+--echo # Reconfigure retries high enough to allow retries, start replication
+SET GLOBAL replica_transaction_retries=10;
+--source include/start_slave.inc
+
+--echo # Wait for deadlock to be detected and retried at least two times.
+let $wait_condition=
+  SELECT APPLYING_TRANSACTION_RETRIES_COUNT >= 2 FROM
+    performance_schema.replication_applier_status_by_worker
+      WHERE CHANNEL_NAME = "";
+--source include/wait_condition.inc
+
+--source include/rpl_connection_slave.inc
+--echo # Switch back to the connection holding lock on replica, commit the
+--echo # transaction to release lock and thus let the currently retrying
+--echo # applier succeed
+select * from t1 order by nid;
+COMMIT;
+
+--source include/rpl_connection_master.inc
+--source include/sync_slave_sql_with_master.inc
+
+--source include/rpl_connection_slave.inc
+--echo # Verify that the row was applied sucessfully on replica
+SELECT * FROM t1;
+
+--source include/rpl_connection_master.inc
+--echo # Cleanup
+DROP TABLE t1;
+
+--source include/rpl_end.inc
diff --git a/storage/ndb/plugin/ha_ndbcluster.cc b/storage/ndb/plugin/ha_ndbcluster.cc
@@ -7784,11 +7784,21 @@ int ndbcluster_commit(handlerton *, THD *thd, bool all) {
     if (applier && applier->get_num_workers() > 1) {
       if (thd_ndb->m_unsent_bytes) {
         DBUG_PRINT("info", ("Applier preparing defined operations"));
-        return execute_no_commit(thd_ndb, trans, true);
+        res = execute_no_commit(thd_ndb, trans, true);
+        if (res != 0) {
+          // Fatal transaction error occured
+          const NdbError &trans_error = trans->getNdbError();
+          if (trans_error.code == 4350) {  // Transaction already aborted
+            thd_ndb->push_ndb_error_warning(trans_error);
+            res = HA_ERR_ROLLED_BACK;
+          } else {
+            res = ndbcluster_print_error(trans, thd_ndb->m_handler);
+          }
+        }
       }
     }
 
-    return 0;
+    return res;
   }
   thd_ndb->save_point_count = 0;
 
