BUG#34398622 gr_suspect_member_resumes_after_crash_mysql bug failed

tiagoportelajorge · tiagoportelajorge · commit aaab65d41259 · 2023-06-02T16:11:41.000+02:00
Problem
========================
gr_suspect_member_resumes_after_crash_mysql is failing sporadically
on our continuous integration platform on SLL runs

Analysis
========================
MySQL connections used in XCom are not fully controlled by whom who
creates them but by:
- The Network Connection Manager that lies in XCom;
- The server when it goes into a stop.

In this case, it means that the GR plugin might be stopping, but
the server is still accepting connections that are to be delegated
to GCS.

In a group with 2 nodes (A,B), a Node B XCom client that creates a
connection to join a group is not aware that the new connection is not
making any progress on Node A server-side, thus it blocks on a con_read
that will never return with success.

This lack of return derived from the fact that:
- Plugin was stopping and it would never return anything on that
  connection;
- Server side plugin would get stuck trying to acquire a read lock
  on a resource that is not available when the server is either stopping
  or starting.

Adding to that, connections that are received when the plugin is
stopping might end up in a limbo, because they were accepted but never
registered on the Network Connection Manager from XCom.

Fix
========================

The fix has several fronts, all of them on the server side:
- We changed the sql_parse.cc code to return client errors during the
  connection delegation itself. This allows for a client to detect
  any of the errors described above, fail-fast and retry the connection.

- Change the lock acquisition to a "try lock", returning an error if
  we can't obtain a lock to delegate the connection, when the plugin
  is executing an exclusive operation on gcs_operations.cc.

- Changed the wait loop for a delegated server connection to check if
  the plugin is still enabled. If the plugin is tear down, those
  connections are terminated.

Change-Id: I0528e1df9adcd985181a233e226b6352af4eae7c
diff --git a/mysql-test/suite/group_replication/include/gr_suspect_member_resumes_after_crash.inc b/mysql-test/suite/group_replication/include/gr_suspect_member_resumes_after_crash.inc
@@ -63,7 +63,7 @@ call mtr.add_suppression("Error connecting to all peers. Member join failed. Loc
 call mtr.add_suppression("Unable to start MySQL Network Provider*.*");
 call mtr.add_suppression("Timeout while waiting for the group communication engine to be ready*.*");
 call mtr.add_suppression("The group communication engine is not ready for the member to join*.*");
-
+call mtr.add_suppression(".*Failed to accept a MySQL connection for Group Replication. Group Replication plugin has an ongoing exclusive operation, like START, STOP or FORCE MEMBERS.*");
 set session sql_log_bin=1;
 
 
diff --git a/mysql-test/suite/group_replication/r/gr_failed_server_mysql_connection.result b/mysql-test/suite/group_replication/r/gr_failed_server_mysql_connection.result
@@ -0,0 +1,65 @@
+include/group_replication.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 server1]
+#
+# 1. Bootstrap a group with M1.
+#
+#######
+[connection server1]
+include/start_and_bootstrap_group_replication.inc
+include/disable_binlog.inc
+call mtr.add_suppression(".*Failed to accept a MySQL connection for Group Replication. Group Replication plugin has an ongoing exclusive operation, like START, STOP or FORCE MEMBERS.*");
+include/restore_binlog.inc
+[connection server2]
+include/disable_binlog.inc
+call mtr.add_suppression("Timeout on wait for view after joining group");
+call mtr.add_suppression("Timeout while waiting for the group communication engine to be ready!");
+call mtr.add_suppression("The group communication engine is not ready for the member to join. Local port: *.*");
+call mtr.add_suppression("read failed");
+call mtr.add_suppression("The member was unable to join the group. Local port: *.*");
+call mtr.add_suppression("Error connecting to all peers. Member join failed. Local port: *.*");
+include/restore_binlog.inc
+#
+# 2. Enable fail_incoming_connection_ongoing_operation and try to
+#    join M2.
+#
+#######
+[connection server1]
+# Adding debug point 'fail_incoming_connection_ongoing_operation' to @@GLOBAL.debug
+[connection server2]
+#
+# 3. Join M2 will fail. Assert that the error message exists
+#    in the log of M1.
+#
+#######
+SET GLOBAL group_replication_group_name= "GROUP_REPLICATION_GROUP_NAME";
+START GROUP_REPLICATION;
+ERROR HY000: The server is not configured properly to be an active member of the group. Please see more details on error log.
+[connection server1]
+include/assert.inc ['Failed to accept a MySQL connection for Group Replication. Group Replication plugin has an ongoing exclusive operation, like START, STOP or FORCE MEMBERS.']
+#
+# 4. Start M2 with the send command
+#
+#######
+[connection server2]
+START GROUP_REPLICATION;;
+[connection server_1_1]
+#
+# 5. Sleep for 10 seconds and then clear
+#    fail_incoming_connection_ongoing_operation
+#
+#######
+# Removing debug point 'fail_incoming_connection_ongoing_operation' from @@GLOBAL.debug
+#
+# 6. reap the start command and M2 must be able to join the group.
+#
+#######
+[connection server2]
+#
+# 7. Clean-up
+#
+######
+[connection server1]
+include/group_replication_end.inc
diff --git a/mysql-test/suite/group_replication/r/gr_suspect_member_resumes_after_crash.result b/mysql-test/suite/group_replication/r/gr_suspect_member_resumes_after_crash.result
@@ -24,6 +24,7 @@ call mtr.add_suppression("Error connecting to all peers. Member join failed. Loc
 call mtr.add_suppression("Unable to start MySQL Network Provider*.*");
 call mtr.add_suppression("Timeout while waiting for the group communication engine to be ready*.*");
 call mtr.add_suppression("The group communication engine is not ready for the member to join*.*");
+call mtr.add_suppression(".*Failed to accept a MySQL connection for Group Replication. Group Replication plugin has an ongoing exclusive operation, like START, STOP or FORCE MEMBERS.*");
 set session sql_log_bin=1;
 
 ############################################################
diff --git a/mysql-test/suite/group_replication/r/gr_suspect_member_resumes_after_crash_mysql.result b/mysql-test/suite/group_replication/r/gr_suspect_member_resumes_after_crash_mysql.result
@@ -24,6 +24,7 @@ call mtr.add_suppression("Error connecting to all peers. Member join failed. Loc
 call mtr.add_suppression("Unable to start MySQL Network Provider*.*");
 call mtr.add_suppression("Timeout while waiting for the group communication engine to be ready*.*");
 call mtr.add_suppression("The group communication engine is not ready for the member to join*.*");
+call mtr.add_suppression(".*Failed to accept a MySQL connection for Group Replication. Group Replication plugin has an ongoing exclusive operation, like START, STOP or FORCE MEMBERS.*");
 set session sql_log_bin=1;
 
 ############################################################
diff --git a/mysql-test/suite/group_replication/t/gr_failed_server_mysql_connection-master.opt b/mysql-test/suite/group_replication/t/gr_failed_server_mysql_connection-master.opt
@@ -0,0 +1 @@
+--loose-group_replication_communication_stack=MySQL
diff --git a/mysql-test/suite/group_replication/t/gr_failed_server_mysql_connection-slave.opt b/mysql-test/suite/group_replication/t/gr_failed_server_mysql_connection-slave.opt
@@ -0,0 +1 @@
+--loose-group_replication_communication_stack=MySQL
diff --git a/mysql-test/suite/group_replication/t/gr_failed_server_mysql_connection.test b/mysql-test/suite/group_replication/t/gr_failed_server_mysql_connection.test
@@ -0,0 +1,121 @@
+################################################################################
+# This test will emulate server-side errors on MySQL connections, such as
+# the plugin has an ongoing operation that does not allow connections to
+# be accepted.
+#
+# Test:
+# 0. The test requires two servers: M1 and M2.
+# 1. Bootstrap a group with M1.
+# 2. Enable fail_incoming_connection_ongoing_operation and try to join M2.
+# 3. Join M2 will fail. Assert that the error message exists in the log of M1.
+# 4. Start M2 with the send command
+# 5. Sleep for 10 seconds and then clear
+#    fail_incoming_connection_ongoing_operation
+# 6. reap the start command and M2 must be able to join the group.
+# 7. Clean-up
+################################################################################
+--source include/big_test.inc
+--source include/have_group_replication_mysql_communication_stack.inc
+--source include/have_group_replication_plugin.inc
+--let $rpl_skip_group_replication_start= 1
+--source include/group_replication.inc
+
+--echo #
+--echo # 1. Bootstrap a group with M1.
+--echo #
+--echo #######
+
+--let $rpl_connection_name= server1
+--source include/rpl_connection.inc
+--source include/start_and_bootstrap_group_replication.inc
+--source include/disable_binlog.inc
+call mtr.add_suppression(".*Failed to accept a MySQL connection for Group Replication. Group Replication plugin has an ongoing exclusive operation, like START, STOP or FORCE MEMBERS.*");
+--source include/restore_binlog.inc
+
+--let $rpl_connection_name= server2
+--source include/rpl_connection.inc
+--source include/disable_binlog.inc
+call mtr.add_suppression("Timeout on wait for view after joining group");
+call mtr.add_suppression("Timeout while waiting for the group communication engine to be ready!");
+call mtr.add_suppression("The group communication engine is not ready for the member to join. Local port: *.*");
+call mtr.add_suppression("read failed");
+call mtr.add_suppression("The member was unable to join the group. Local port: *.*");
+call mtr.add_suppression("Error connecting to all peers. Member join failed. Local port: *.*");
+--source include/restore_binlog.inc
+
+--echo #
+--echo # 2. Enable fail_incoming_connection_ongoing_operation and try to
+--echo #    join M2.
+--echo #
+--echo #######
+
+--let $rpl_connection_name= server1
+--source include/rpl_connection.inc
+
+--let $debug_point = fail_incoming_connection_ongoing_operation
+--source include/add_debug_point.inc
+
+--let $rpl_connection_name= server2
+--source include/rpl_connection.inc
+
+--echo #
+--echo # 3. Join M2 will fail. Assert that the error message exists
+--echo #    in the log of M1.
+--echo #
+--echo #######
+
+--replace_result $group_replication_group_name GROUP_REPLICATION_GROUP_NAME
+--eval SET GLOBAL group_replication_group_name= "$group_replication_group_name"
+
+--error ER_GROUP_REPLICATION_CONFIGURATION
+START GROUP_REPLICATION;
+
+--let $rpl_connection_name= server1
+--source include/rpl_connection.inc
+
+--let $assert_text= 'Failed to accept a MySQL connection for Group Replication. Group Replication plugin has an ongoing exclusive operation, like START, STOP or FORCE MEMBERS.'
+--let $assert_cond= "[SELECT COUNT(*) as count FROM performance_schema.error_log WHERE error_code=\'MY-014081\' AND data LIKE \"%Failed to accept a MySQL connection for Group Replication%\", count, 1]" >= "1"
+--source include/assert.inc
+
+--echo #
+--echo # 4. Start M2 with the send command
+--echo #
+--echo #######
+
+--let $rpl_connection_name= server2
+--source include/rpl_connection.inc
+
+--send START GROUP_REPLICATION;
+
+--let $rpl_connection_name= server_1_1
+--source include/rpl_connection.inc
+
+--echo #
+--echo # 5. Sleep for 10 seconds and then clear
+--echo #    fail_incoming_connection_ongoing_operation
+--echo #
+--echo #######
+--sleep 10
+
+--let $debug_point = fail_incoming_connection_ongoing_operation
+--source include/remove_debug_point.inc
+
+--echo #
+--echo # 6. reap the start command and M2 must be able to join the group.
+--echo #
+--echo #######
+--let $rpl_connection_name= server2
+--source include/rpl_connection.inc
+
+--reap
+
+--echo #
+--echo # 7. Clean-up
+--echo #
+--echo ######
+
+--let $rpl_connection_name= server1
+--source include/rpl_connection.inc
+
+--source include/group_replication_end.inc
+
diff --git a/plugin/group_replication/src/gcs_mysql_network_provider.cc b/plugin/group_replication/src/gcs_mysql_network_provider.cc
@@ -34,8 +34,8 @@
 #include "sql/rpl_group_replication.h"
 
 // Forward declaration of Group Replication callback...
-void handle_group_replication_incoming_connection(THD *thd, int fd,
-                                                  SSL *ssl_ctx);
+int handle_group_replication_incoming_connection(THD *thd, int fd,
+                                                 SSL *ssl_ctx);
 
 bool Gcs_mysql_network_provider_auth_interface_impl::get_credentials(
     std::string &username, std::string &password) {
diff --git a/plugin/group_replication/src/gcs_operations.cc b/plugin/group_replication/src/gcs_operations.cc
@@ -814,12 +814,28 @@ Gcs_mysql_network_provider *Gcs_operations::get_mysql_network_provider() {
   DBUG_TRACE;
   Gcs_mysql_network_provider *result = nullptr;
 
-  gcs_operations_lock->rdlock();
-  if (gcs_interface != nullptr && gcs_mysql_net_provider != nullptr &&
-      gcs_interface->is_initialized()) {
-    result = gcs_mysql_net_provider.get();
+  auto fail_incoming_connection_ongoing_operation_log = []() {
+    LogPluginErr(ERROR_LEVEL,
+                 ER_GRP_RPL_MYSQL_NETWORK_PROVIDER_SERVER_ERROR_COMMAND_ERR,
+                 "Group Replication plugin has an ongoing exclusive operation, "
+                 "like START, STOP or FORCE MEMBERS");
+  };
+
+  DBUG_EXECUTE_IF("fail_incoming_connection_ongoing_operation", {
+    fail_incoming_connection_ongoing_operation_log();
+    return result;
+  });
+
+  Checkable_rwlock::Guard g(*gcs_operations_lock,
+                            Checkable_rwlock::TRY_READ_LOCK);
+  if (g.is_rdlocked()) {
+    if (gcs_interface != nullptr && gcs_mysql_net_provider != nullptr &&
+        gcs_interface->is_initialized()) {
+      result = gcs_mysql_net_provider.get();
+    }
+  } else {
+    fail_incoming_connection_ongoing_operation_log();
   }
-  gcs_operations_lock->unlock();
 
   return result;
 }
diff --git a/plugin/group_replication/src/plugin.cc b/plugin/group_replication/src/plugin.cc
@@ -366,18 +366,23 @@ bool get_allow_single_leader() {
  * @param thd     THD object of the connection
  * @param fd      File descriptor of the connections
  * @param ssl_ctx SSL data of the connection
+ *
+ * @return int Returns 1 in case of any error. 0 otherwise.
  */
-void handle_group_replication_incoming_connection(THD *thd, int fd,
-                                                  SSL *ssl_ctx) {
+
+int handle_group_replication_incoming_connection(THD *thd, int fd,
+                                                 SSL *ssl_ctx) {
   auto *new_connection = new Network_connection(fd, ssl_ctx);
   new_connection->has_error = false;
+  int error_return = 1;
 
-  Gcs_mysql_network_provider *mysql_provider =
-      gcs_module->get_mysql_network_provider();
-
-  if (mysql_provider) {
+  if (auto mysql_provider = gcs_module->get_mysql_network_provider();
+      mysql_provider) {
     mysql_provider->set_new_connection(thd, new_connection);
+    error_return = 0;
   }
+
+  return error_return;
 }
 
 /**
@@ -1789,6 +1794,7 @@ bool attempt_rejoin() {
   */
   DBUG_EXECUTE_IF("group_replication_fail_rejoin", goto end;);
   view_change_notifier->start_view_modification();
+
   join_state =
       gcs_module->join(*events_handler, *events_handler, view_change_notifier);
   if (join_state == GCS_OK) {
diff --git a/share/messages_to_error_log.txt b/share/messages_to_error_log.txt
@@ -12334,6 +12334,9 @@ ER_THREAD_STILL_ALIVE
 ER_NUM_THREADS_STILL_ALIVE
   eng "Waiting for forceful disconnection of %ld thread(s) to end."
 
+ER_GRP_RPL_MYSQL_NETWORK_PROVIDER_SERVER_ERROR_COMMAND_ERR
+   eng "Failed to accept a MySQL connection for Group Replication. %s. Please retry."
+
 # DO NOT add server-to-client messages here;
 # they go in messages_to_clients.txt
 # in the same directory as this file.
diff --git a/sql/rpl_group_replication.h b/sql/rpl_group_replication.h
@@ -87,7 +87,7 @@ bool get_group_replication_view_change_uuid(std::string &uuid);
 bool is_group_replication_member_secondary();
 
 // Callback definition for socket donation
-typedef void (*gr_incoming_connection_cb)(THD *thd, int fd, SSL *ssl_ctx);
+typedef int (*gr_incoming_connection_cb)(THD *thd, int fd, SSL *ssl_ctx);
 void set_gr_incoming_connection(gr_incoming_connection_cb x);
 
 #endif /* RPL_GROUP_REPLICATION_INCLUDED */
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
diff --git a/unittest/gunit/group_replication/group_replication_gcs_mysql_network_provider-t.cc b/unittest/gunit/group_replication/group_replication_gcs_mysql_network_provider-t.cc

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+--loose-group_replication_communication_stack=MySQL`