MDEV-29484 Assertion `!trx_free || !trx->locked_connections' failed i…

…n spider_free_trx_conn on LOCK TABLES

In MDEV-28352, we've modified spider_free_trx_conn() so that it frees
a connection only when the connection is locking no remote table.

However, when a user connection to a Spider node is disconnected, the
corresponding connections to remote data nodes from the Spider node
must be freed immediately.

Thus, the modification above leads an assertion error on the debug
build and a hang on the non-debug build. We partly revert MDEV-28352
to fix the problem.

storage/spider/mysql-test/spider/bugfix/r/mdev_29484.result

@@ -0,0 +1,39 @@
+#
+# MDEV-29484 Assertion `!trx_free || !trx->locked_connections' failed in spider_free_trx_conn on LOCK TABLES
+#
+for master_1
+for child2
+child2_1
+child2_2
+child2_3
+for child3
+connection child2_1;
+CREATE DATABASE auto_test_remote;
+USE auto_test_remote;
+CREATE TABLE tbl_a (
+a INT
+) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+connection master_1;
+CREATE DATABASE auto_test_local;
+USE auto_test_local;
+CREATE TABLE tbl_a (
+a INT
+) ENGINE=Spider DEFAULT CHARSET=utf8 COMMENT='table "tbl_a", srv "s_2_1"';
+CREATE TABLE tbl_b (
+a INT
+) ENGINE=Spider DEFAULT CHARSET=utf8;
+LOCK TABLES tbl_a WRITE;
+LOCK TABLES tbl_b READ, tbl_a READ;
+ERROR HY000: Unable to connect to foreign data source: localhost
+disconnect master_1;
+connect  master_1, localhost, root, , , $MASTER_1_MYPORT, $MASTER_1_MYSOCK;
+connection child2_1;
+DROP DATABASE auto_test_remote;
+connection master_1;
+DROP DATABASE auto_test_local;
+for master_1
+for child2
+child2_1
+child2_2
+child2_3
+for child3

storage/spider/mysql-test/spider/bugfix/t/mdev_29484.cnf

@@ -0,0 +1,3 @@
+!include include/default_mysqld.cnf
+!include ../my_1_1.cnf
+!include ../my_2_1.cnf

storage/spider/mysql-test/spider/bugfix/t/mdev_29484.test

@@ -0,0 +1,47 @@
+--echo #
+--echo # MDEV-29484 Assertion `!trx_free || !trx->locked_connections' failed in spider_free_trx_conn on LOCK TABLES
+--echo #
+
+--disable_query_log
+--disable_result_log
+--source ../../t/test_init.inc
+--enable_result_log
+--enable_query_log
+
+--connection child2_1
+CREATE DATABASE auto_test_remote;
+USE auto_test_remote;
+eval CREATE TABLE tbl_a (
+    a INT
+) $CHILD2_1_ENGINE $CHILD2_1_CHARSET;
+
+--connection master_1
+CREATE DATABASE auto_test_local;
+USE auto_test_local;
+
+eval CREATE TABLE tbl_a (
+    a INT
+) $MASTER_1_ENGINE $MASTER_1_CHARSET COMMENT='table "tbl_a", srv "s_2_1"';
+
+eval CREATE TABLE tbl_b (
+    a INT
+) $MASTER_1_ENGINE $MASTER_1_CHARSET;
+
+LOCK TABLES tbl_a WRITE;
+--error 1429
+LOCK TABLES tbl_b READ, tbl_a READ;
+
+--disconnect master_1 # crash
+--source ../../t/connect_master_1.inc
+
+--connection child2_1
+DROP DATABASE auto_test_remote;
+
+--connection master_1
+DROP DATABASE auto_test_local;
+
+--disable_query_log
+--disable_result_log
+--source ../../t/test_deinit.inc
+--enable_result_log
+--enable_query_log

storage/spider/spd_trx.cc

@@ -103,7 +103,6 @@ int spider_free_trx_conn(SPIDER_TRX *trx, bool trx_free)
   HASH *conn_hash= &trx->trx_conn_hash;
 
   DBUG_ENTER("spider_free_trx_conn");
-  DBUG_ASSERT(!trx_free || !trx->locked_connections);
 
   /* Clear the connection queues in any case. */
   while ((conn= (SPIDER_CONN *) my_hash_element(conn_hash, loop_count)))
@@ -114,11 +113,16 @@ int spider_free_trx_conn(SPIDER_TRX *trx, bool trx_free)
 
   if (trx_free || spider_param_conn_recycle_mode(trx->thd) != 2)
   {
-    /* Free connections only when no connection is locked. */
-    if (!trx->locked_connections)
+    loop_count= 0;
+    while ((conn= (SPIDER_CONN *) my_hash_element(&trx->trx_conn_hash,
+                                                  loop_count)))
     {
-      loop_count= 0;
-      while ((conn= (SPIDER_CONN *) my_hash_element(conn_hash, loop_count)))
+      if (conn->table_lock)
+      {
+        DBUG_ASSERT(!trx_free);
+        loop_count++;
+      }
+      else
       {
         spider_free_conn_from_trx(trx, conn, FALSE, trx_free, &loop_count);
       }