MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Ingo Struewing Date:February 25 2010 7:52pm
Subject:bzr commit into mysql-backup-backport branch (ingo.struewing:3119)
Bug#50517
View as plain text  
#At file:///home2/mydev/bzrroot/mysql-5.6-backup-backport-mysqldump/ based on revid:charles.bell@stripped

 3119 Ingo Struewing	2010-02-25
      Backport of revid:ingo.struewing@stripped
        Bug#50517 - mysqldump.test sometimes times out
        
        mysqldump timed out sporadically on backup trees.
        mysqldump -x blocked in FLUSH TABLES WITH READ LOCK.
        The global variable protect_against_global_read_lock
        had been set to 1 and no other thread was executing
        a statement.
        
        A successful wait_if_global_read_lock() increments
        protect_against_global_read_lock.
        start_waiting_global_read_lock() decrements it.
        
        There were execution paths where wait_if_global_read_lock()
        could be called and start_waiting_global_read_lock()
        skipped. This patch closes these gaps.
     @ mysql-test/suite/backup/r/backup_bml_transaction.result
        Backport of revid:ingo.struewing@stripped
            Bug#50517 - mysqldump.test sometimes times out
            New test result.
     @ mysql-test/suite/backup/t/backup_bml_transaction.test
        Backport of revid:ingo.struewing@stripped
            Bug#50517 - mysqldump.test sometimes times out
            New test case.
     @ sql/handler.cc
        Backport of revid:ingo.struewing@stripped
            Bug#50517 - mysqldump.test sometimes times out
            Added a call to start_waiting_global_read_lock() if
            wait_if_global_read_lock() succeeds and BCB_instance->get_shared_lock
            fails.
            Combined two branches that depend on 'rw_trans' into one.
            Added sync point 'before_ha_commit_trans_grl'.
     @ sql/sql_rename.cc
        Backport of revid:ingo.struewing@stripped
            Bug#50517 - mysqldump.test sometimes times out
            Fixed error handling to call start_waiting_global_read_lock()
            before return.

    added:
      mysql-test/suite/backup/r/backup_bml_transaction.result
      mysql-test/suite/backup/t/backup_bml_transaction.test
    modified:
      sql/handler.cc
      sql/sql_rename.cc
=== added file 'mysql-test/suite/backup/r/backup_bml_transaction.result'
--- a/mysql-test/suite/backup/r/backup_bml_transaction.result	1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/backup/r/backup_bml_transaction.result	2010-02-25 19:52:51 +0000
@@ -0,0 +1,41 @@
+SET DEBUG_SYNC='RESET';
+USE test;
+DROP TABLE IF EXISTS t1;
+#
+# Bug#50517 - mysqldump.test sometimes times out
+#
+# Create table with a transactional engine.
+CREATE TABLE t1 (c1 INT) ENGINE=InnoDB;
+# Block INSERT near the end of statement execution.
+SET DEBUG_SYNC='before_ha_commit_trans_grl SIGNAL inserted WAIT_FOR finish';
+# INSERT creates a writing transaction, which will be auto
+# committed near end of statement.
+INSERT INTO t1 VALUES (1);
+#
+# connection con1
+# Get id of default connection (no query log).
+# Wait until INSERT reaches sync point.
+SET DEBUG_SYNC='now WAIT_FOR inserted';
+# Kill query of default connection.
+KILL QUERY @id;
+# Kill can be unreliable. Repeat.
+KILL QUERY @id;
+# Let INSERT finish.
+SET DEBUG_SYNC='now SIGNAL finish';
+#
+# connection default
+# Reap commit. Suppress SHOW WARNINGS. Its execution would decrement
+# protect_against_global_read_lock and thus hide the problem.
+# With Bug#50517 in place, we have protect_against_global_read_lock
+# set to 1 and are outside of a statement.
+#
+# connection con1
+# With Bug#50517 in place, the test hangs here: It waits until
+# protect_against_global_read_lock becomes 0...
+FLUSH TABLES WITH READ LOCK;
+# Clean up.
+UNLOCK TABLES;
+#
+# connection default, clean up.
+SET DEBUG_SYNC='RESET';
+DROP TABLE t1;

=== added file 'mysql-test/suite/backup/t/backup_bml_transaction.test'
--- a/mysql-test/suite/backup/t/backup_bml_transaction.test	1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/backup/t/backup_bml_transaction.test	2010-02-25 19:52:51 +0000
@@ -0,0 +1,84 @@
+#
+# Test the effectiveness of the fix for Bug#50517 (mysqldump.test sometimes
+# times out).
+# It needs a modifying transaction, an interrupted (KILL QUERY) commit,
+# and a FLUSH TABLES WITH READ LOCK.
+# Committing a writing transaction activates the buggy code snippet in
+# ha_commit_trans(). wait_if_gloal_read_lock() must be successful, and
+# BCB_instance->get_shared_lock() must fail. The latter can happen if
+# an exclusive lock exists (ongoing backup/restore) and the share lock
+# times out (even with timeout == 0), or if THD::killed is nonzero.
+# I decided to go with KILL QUERY. As writing transaction, I use an INSERT.
+# The commit happens as autocommit. The autocommit is done very late in
+# the statement execution. Right before it, THD::killed is cleared.
+# To be able to run through the code snippet with killed set, I need a
+# new sync point (before_ha_commit_trans_grl) short before it.
+# Synchronizing at this point I can KILL QUERY to set THD:killed.
+#
+
+--source include/have_debug_sync.inc
+--source include/have_innodb.inc
+
+--disable_warnings
+SET DEBUG_SYNC='RESET';
+USE test;
+DROP TABLE IF EXISTS t1;
+--enable_warnings
+
+--echo #
+--echo # Bug#50517 - mysqldump.test sometimes times out
+--echo #
+--let $ID = `SELECT CONNECTION_ID()`
+#
+--echo # Create table with a transactional engine.
+CREATE TABLE t1 (c1 INT) ENGINE=InnoDB;
+#
+--echo # Block INSERT near the end of statement execution.
+SET DEBUG_SYNC='before_ha_commit_trans_grl SIGNAL inserted WAIT_FOR finish';
+#
+--echo # INSERT creates a writing transaction, which will be auto
+--echo # committed near end of statement.
+send INSERT INTO t1 VALUES (1);
+#
+    --echo #
+    --echo # connection con1
+    --connect (con1,localhost,root,,)
+    --echo # Get id of default connection (no query log).
+    --let $not_used = `SELECT @id := $ID`
+    --echo # Wait until INSERT reaches sync point.
+    SET DEBUG_SYNC='now WAIT_FOR inserted';
+    --echo # Kill query of default connection.
+    KILL QUERY @id;
+    --echo # Kill can be unreliable. Repeat.
+    KILL QUERY @id;
+    --echo # Let INSERT finish.
+    SET DEBUG_SYNC='now SIGNAL finish';
+    --disconnect con1
+#
+--echo #
+--echo # connection default
+--connection default
+--echo # Reap commit. Suppress SHOW WARNINGS. Its execution would decrement
+--echo # protect_against_global_read_lock and thus hide the problem.
+--disable_warnings
+reap;
+--enable_warnings
+--echo # With Bug#50517 in place, we have protect_against_global_read_lock
+--echo # set to 1 and are outside of a statement.
+#
+    --echo #
+    --echo # connection con1
+    --connect (con1,localhost,root,,)
+    --echo # With Bug#50517 in place, the test hangs here: It waits until
+    --echo # protect_against_global_read_lock becomes 0...
+    FLUSH TABLES WITH READ LOCK;
+    --echo # Clean up.
+    UNLOCK TABLES;
+    --disconnect con1
+#
+--echo #
+--echo # connection default, clean up.
+--connection default
+SET DEBUG_SYNC='RESET';
+DROP TABLE t1;
+

=== modified file 'sql/handler.cc'
--- a/sql/handler.cc	2010-02-23 19:28:33 +0000
+++ b/sql/handler.cc	2010-02-25 19:52:51 +0000
@@ -1171,28 +1171,34 @@ int ha_commit_trans(THD *thd, bool all)
     /* rw_trans is TRUE when we in a transaction changing data */
     rw_trans= is_real_trans && (rw_ha_count > 0);
 
-    if (rw_trans &&
-        (thd->global_read_lock.wait_if_global_read_lock(thd, FALSE, FALSE) ||
-         !(bcb_registered= BCB_instance->get_shared_lock(thd))))
+    if (rw_trans)
     {
-      ha_rollback_trans(thd, all);
-      DBUG_RETURN(1);
-    }
+      bool ret;
+      DEBUG_SYNC(thd, "before_ha_commit_trans_grl");
 
-    if (rw_trans &&
-        opt_readonly &&
-        !(thd->security_ctx->master_access & SUPER_ACL) &&
-        !thd->slave_thread)
-    {
-      my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--read-only");
-      if (bcb_registered) 
+      if (((ret= thd->global_read_lock.wait_if_global_read_lock(thd, FALSE, FALSE)) ||
+           !(bcb_registered= BCB_instance->get_shared_lock(thd))))
       {
-        BCB_instance->release_shared_lock();
-        bcb_registered=FALSE;
+        if (!ret)
+          thd->global_read_lock.start_waiting_global_read_lock(thd);
+        ha_rollback_trans(thd, all);
+        DBUG_RETURN(1);
+      }
+
+      if (opt_readonly &&
+          !(thd->security_ctx->master_access & SUPER_ACL) &&
+          !thd->slave_thread)
+      {
+        my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--read-only");
+        if (bcb_registered) 
+        {
+          BCB_instance->release_shared_lock();
+          bcb_registered=FALSE;
+        }
+        ha_rollback_trans(thd, all);
+        error= 1;
+        goto end;
       }
-      ha_rollback_trans(thd, all);
-      error= 1;
-      goto end;
     }
 
     DEBUG_SYNC(thd, "within_ha_commit_trans");

=== modified file 'sql/sql_rename.cc'
--- a/sql/sql_rename.cc	2010-02-06 10:28:06 +0000
+++ b/sql/sql_rename.cc	2010-02-25 19:52:51 +0000
@@ -99,7 +99,12 @@ bool mysql_rename_tables(THD *thd, TABLE
             */
             my_error(ER_CANT_RENAME_LOG_TABLE, MYF(0), ren_table->table_name,
                      ren_table->table_name);
-            DBUG_RETURN(1);
+            /*
+              After a successful wait_if_global_read_lock(), we must call
+              start_waiting_global_read_lock() before return.
+            */
+            error= 1;
+            goto err;
           }
         }
         else
@@ -112,7 +117,12 @@ bool mysql_rename_tables(THD *thd, TABLE
             */
             my_error(ER_CANT_RENAME_LOG_TABLE, MYF(0), ren_table->table_name,
                      ren_table->table_name);
-            DBUG_RETURN(1);
+            /*
+              After a successful wait_if_global_read_lock(), we must call
+              start_waiting_global_read_lock() before return.
+            */
+            error= 1;
+            goto err;
           }
           else
           {
@@ -130,7 +140,12 @@ bool mysql_rename_tables(THD *thd, TABLE
       else
         my_error(ER_CANT_RENAME_LOG_TABLE, MYF(0), rename_log_table[1],
                  rename_log_table[1]);
-      DBUG_RETURN(1);
+      /*
+        After a successful wait_if_global_read_lock(), we must call
+        start_waiting_global_read_lock() before return.
+      */
+      error= 1;
+      goto err;
     }
   }
 


Attachment: [text/bzr-bundle] bzr/ingo.struewing@sun.com-20100225195251-wupqpmqcap2ysj2v.bundle
Thread
bzr commit into mysql-backup-backport branch (ingo.struewing:3119)Bug#50517Ingo Struewing25 Feb