Bug#16387720: BLOCKED WITH FLUSH TABLES WITH READ LOCK +

Sujatha Sivakumar · Sujatha Sivakumar · commit c529846d00f3 · 2013-05-24T06:23:14.000+05:30
SHOW SLAVE STATUS

Problem:
=======
If a client thread on a slave does FLUSH TABLES WITH READ
LOCK; then master does some updates, SHOW SLAVE STATUS in
the same client will be blocked. If the blocked slave server
is killed and restarted when GTID's are enabled, one GTID
gets missed leaving the slave in an out of sync state.

Using 'relay-log-recovery=1' should do a crash safe slave
recovery but it is not happening.

Analysis:
========

RPL info tables "slave_master_info" and "slave_relay_log_info"
are used to store slave's thread's positions.  When a FTWRL
is issued on slave this command blocks all operations on
tables. Hence when some update operations are done on master
the slave thread will be able to open the tables but while 
closing, an internal commit operation will be blocked. These
blocked info tables make the "SHOW SLAVE STATUS" to hang.

When "relay-log-recovery=1" during crash recovery all the
partial written events will be discarded the master info
table is initialised with the information read from relay
log info table. The "Retrieved GTID" set should be cleared
so that partial read events are discarded and they are
fetched once again. Since this is not happening
"The Retried GTID" is considered to be executed and the 
actual transaction is skipped.

Fix:
===

Info tables should be made independent of global read lock.
At the time of opening the RPL info tables the following
"MYSQL_OPEN_IGNORE_GLOBAL_READ_LOCK" flag is set to make
the info tables not to block when the FTWRL is in progress.
Hence a similar flag is introduced in "ha_commit_trans"
code which will allow commit to complete even if a global
read lock is active. This flag can be used to allow changes
to internal tables (e.g. slave status tables).

To fix the missing GTID issue using "relay-log-recovery"
option, during recovery process retrieved GTID set is cleared.
diff --git a/mysql-test/suite/rpl/t/rpl_check_gtid-slave.opt b/mysql-test/suite/rpl/t/rpl_check_gtid-slave.opt
diff --git a/mysql-test/suite/rpl/t/rpl_check_gtid.test b/mysql-test/suite/rpl/t/rpl_check_gtid.test
@@ -788,10 +788,12 @@ while ($i != 5)
     --let $gtid_ini= 1
     --let $gtid_end= 1
   }
+  # Post relay log recovery the previous GTIDs set should
+  # be empty.
   if ($i != 1)
   {
-    --let $gtid_set_ini= 1
-    --let $gtid_set_end= `SELECT $i - 1`
+    --let $gtid_set_ini= 0
+    --let $gtid_set_end= 0
     --let $gtid_ini= `SELECT $i`
     --let $gtid_end= `SELECT $i`
   }
@@ -884,10 +886,12 @@ while ($i != 5)
     --let $gtid_ini= 1
     --let $gtid_end= 1
   }
+  # Post relay log recovery the previous GTIDs set should
+  # be empty.
   if ($i != 1)
   {
-    --let $gtid_set_ini= 1
-    --let $gtid_set_end= `SELECT $i - 1`
+    --let $gtid_set_ini= 0
+    --let $gtid_set_end= 0
     --let $gtid_ini= `SELECT $i`
     --let $gtid_end= `SELECT $i`
   }
diff --git a/sql/handler.cc b/sql/handler.cc
@@ -1296,6 +1296,11 @@ ha_check_and_coalesce_trx_read_only(THD *thd, Ha_trx_info *ha_list,
 
 
 /**
+  @param[in] ignore_global_read_lock   Allow commit to complete even if a
+                                       global read lock is active. This can be
+                                       used to allow changes to internal tables
+                                       (e.g. slave status tables).
+
   @retval
     0   ok
   @retval
@@ -1309,7 +1314,8 @@ ha_check_and_coalesce_trx_read_only(THD *thd, Ha_trx_info *ha_list,
     stored functions or triggers. So we simply do nothing now.
     TODO: This should be fixed in later ( >= 5.1) releases.
 */
-int ha_commit_trans(THD *thd, bool all)
+
+int ha_commit_trans(THD *thd, bool all, bool ignore_global_read_lock)
 {
   int error= 0;
   /*
@@ -1379,7 +1385,7 @@ 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)
+    if (rw_trans && !ignore_global_read_lock)
     {
       /*
         Acquire a metadata lock which will ensure that COMMIT is blocked
diff --git a/sql/handler.h b/sql/handler.h
@@ -3404,7 +3404,7 @@ int ha_release_temporary_latches(THD *thd);
 /* transactions: interface to handlerton functions */
 int ha_start_consistent_snapshot(THD *thd);
 int ha_commit_or_rollback_by_xid(THD *thd, XID *xid, bool commit);
-int ha_commit_trans(THD *thd, bool all);
+int ha_commit_trans(THD *thd, bool all, bool ignore_global_read_lock= false);
 int ha_rollback_trans(THD *thd, bool all);
 int ha_prepare(THD *thd);
 int ha_recover(HASH *commit_list);
diff --git a/sql/rpl_info_table_access.cc b/sql/rpl_info_table_access.cc
@@ -1,4 +1,4 @@
-/* Copyright (c) 2010, 2012, Oracle and/or its affiliates. All rights reserved.
+/* Copyright (c) 2010, 2013, Oracle and/or its affiliates. All rights reserved.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -155,14 +155,25 @@ bool Rpl_info_table_access::close_table(THD *thd, TABLE* table,
     if (error)
       ha_rollback_trans(thd, FALSE);
     else
-      ha_commit_trans(thd, FALSE);
-
+    {
+      /*
+        To make the commit not to block with global read lock set
+        "ignore_global_read_lock" flag to true.
+       */
+      ha_commit_trans(thd, FALSE, TRUE);
+    }
     if (saved_current_thd != current_thd)
     {
       if (error)
         ha_rollback_trans(thd, TRUE);
       else
-        ha_commit_trans(thd, TRUE);
+      {
+        /*
+          To make the commit not to block with global read lock set
+          "ignore_global_read_lock" flag to true.
+         */
+        ha_commit_trans(thd, TRUE, TRUE);
+      }
     }
     /*
       In order not to break execution of current statement we have to
diff --git a/sql/rpl_slave.cc b/sql/rpl_slave.cc
@@ -488,6 +488,13 @@ int init_recovery(Master_info* mi, const char** errmsg)
     rli->set_event_relay_log_pos(BIN_LOG_HEADER_SIZE);
   }
 
+  /*
+    Clear the retrieved GTID set so that events that are written partially
+    will be fetched again.
+    */
+  global_sid_lock->wrlock();
+  (const_cast<Gtid_set *>(rli->get_gtid_set()))->clear();
+  global_sid_lock->unlock();
   DBUG_RETURN(error);
 }
 

Original file line number	Diff line number	Diff line change
`@@ -788,10 +788,12 @@ while ($i != 5)`
`788`	`788`	`--let $gtid_ini= 1`
`789`	`789`	`--let $gtid_end= 1`
`790`	`790`	`}`
	`791`	`+ # Post relay log recovery the previous GTIDs set should`
	`792`	`+ # be empty.`
`791`	`793`	`if ($i != 1)`
`792`	`794`	`{`
`793`		`- --let $gtid_set_ini= 1`
`794`		- --let $gtid_set_end= `SELECT $i - 1`
	`795`	`+ --let $gtid_set_ini= 0`
	`796`	`+ --let $gtid_set_end= 0`
`795`	`797`	--let $gtid_ini= `SELECT $i`
`796`	`798`	--let $gtid_end= `SELECT $i`
`797`	`799`	`}`
`@@ -884,10 +886,12 @@ while ($i != 5)`
`884`	`886`	`--let $gtid_ini= 1`
`885`	`887`	`--let $gtid_end= 1`
`886`	`888`	`}`
	`889`	`+ # Post relay log recovery the previous GTIDs set should`
	`890`	`+ # be empty.`
`887`	`891`	`if ($i != 1)`
`888`	`892`	`{`
`889`		`- --let $gtid_set_ini= 1`
`890`		- --let $gtid_set_end= `SELECT $i - 1`
	`893`	`+ --let $gtid_set_ini= 0`
	`894`	`+ --let $gtid_set_end= 0`
`891`	`895`	--let $gtid_ini= `SELECT $i`
`892`	`896`	--let $gtid_end= `SELECT $i`
`893`	`897`	`}`
Original file line number	Diff line number	Diff line change
`@@ -488,6 +488,13 @@ int init_recovery(Master_info* mi, const char** errmsg)`
`488`	`488`	`rli->set_event_relay_log_pos(BIN_LOG_HEADER_SIZE);`
`489`	`489`	`}`
`490`	`490`
	`491`	`+ /*`
	`492`	`+ Clear the retrieved GTID set so that events that are written partially`
	`493`	`+ will be fetched again.`
	`494`	`+ */`
	`495`	`+ global_sid_lock->wrlock();`
	`496`	`+ (const_cast<Gtid_set *>(rli->get_gtid_set()))->clear();`
	`497`	`+ global_sid_lock->unlock();`
`491`	`498`	`DBUG_RETURN(error);`
`492`	`499`	`}`
`493`	`500`