Merge lp:~akopytov/percona-server/backup_locks_bugs into lp:percona-server/5.6

[Alexey Kopytov]

Bug #1358836: Backup locks do not guarantee consistent SHOW SLAVE STATUS
              with binary log disabled

Bug #1372806: Deadlock with LOCK BINLOG FOR BACKUP + SHOW SLAVE STATUS
              with binary log enabled

The fixes are inter-dependent, hence a single revision for both bugs.

The problem in bug #1358836 was that with the binary log disabled slave
SQL threads did not acquire binlog protection locks when committing in
some cases. The original backup locks implementation was done in
assumption that the only place where master binary logs are updated is
Relay_log_info::inc_event_relay_log_pos(). Which is incorrect and
replication code does not make figuring things like these
easy. Relay_log_info::inc_event_relay_log_pos() is only called for
multi-statement transactions. There are different code paths for
AUTOCOMMIT transactions and other kinds of updates, depending on the
storage engines involved, event types, and whether the slave is
multi-threaded.

Inspecting all possible code paths was difficult, because Relay_log_info
members that have to be protected by an active BINLOG backup lock were
sometimes modified directly, rather than with the ‘setter’ functions,
i.e. set_group_master_log_name() and set_group_master_log_pos().

Fixed by:

- asserting that the current connection has acquired a binlog protection
lock in Relay_log_info::set_group_master_log_name() and
Relay_log_info::set_group_master_log_pos();

- changing all code that accessed those variables directly to use
getter/setter functions instead;

- making sure that whenever a setter function is called (i.e. master
binary log coordinates are updated), binlog protection is acquired;

- renaming the group_master_log_{name,pos} member variables of
Relay_log_info to m_group_master_log_{name,pos} to prevent other code
that modifies those variables directly from creeping in on future
changes or upstream merges;

The problem in bug #1372806 was that with binary log enabled, an SQL
thread would acquire a binlog protection lock after locking the
Relay_log_info::data_lock mutex, which resulted in a deadlock with a
user connection holding a binlog lock while executing SHOW SLAVE STATUS.

Fixed by inspecting the code and making sure that no binlog protection
locks are taken while the Relay_log_info::data_locks mutex is locked by
the same connection, and adding assertions where possible.

[Alexey Kopytov (akopytov)]

http://jenkins.percona.com/view/PS%205.6/job/percona-server-5.6-param/717/

[Laurynas Biveinis (laurynas-biveinis)]

    - Should Relay_log_info::purge_relay_logs set errmsg on failure to
      acquire binlog lock protection?

    - Why does Slave_worker::reset_recovery_info try to acquire the
      binlog protection and proceeds to call the setter without
      considering whether the acquisition succeeded? That will fail
      the DBUG_ASSERT in the setter in the case of failure at
      least. If I understand the context of the code correctly, it
      should be just like the other protection sites instead, with an
      early error return in the case of failure.

    - Have you considered extracting

bool rpl_acquire_backup_binlog_lock_protection(thd, rli)
{
  bool ret= true;
  if (thd && !thd->backup_binlog_lock.is_acquired())
  {
    const ulong timeout= info_thd->variables.lock_wait_timeout;

    DBUG_PRINT("debug", ("Acquiring binlog protection lock"));

    if (rli)
      mysql_mutex_assert_not_owner(&rli->data_lock);

   ret= !info_thd->backup_binlog_lock.acquire_protection(thd,
MDL_EXPLICIT, timeout);
  }
  return ret;
}

and void release(thd, bool protection_acquired)
{
  if (thd && prot_acquired)
  {
    DBUG_PRINT("debug", ("Releasing binlog protection lock"));
    thd->backup_binlog_lock.release_protection(thd);
  }
}

review: Needs Information

[Alexey Kopytov (akopytov)]

>
> - Should Relay_log_info::purge_relay_logs set errmsg on failure to
> acquire binlog lock protection?
>

No, because setting errmsg would cause callers to call
my_error(ER_RELAY_LOG_FAIL, errmsg), which is not what we want, because
that would have been already called by MDL_context::acquire_lock().

> - Why does Slave_worker::reset_recovery_info try to acquire the
> binlog protection and proceeds to call the setter without
> considering whether the acquisition succeeded? That will fail
> the DBUG_ASSERT in the setter in the case of failure at
> least. If I understand the context of the code correctly, it
> should be just like the other protection sites instead, with an
> early error return in the case of failure.
>

For some reasons I wasn’t sure if the callers are able to handle errors
returned by Slave_worker::reset_recovery_info(), but I can’t recall
them. Anyway, this seems to be a rather corner case, so I changed to
return an error instead.

> - Have you considered extracting
>
> bool rpl_acquire_backup_binlog_lock_protection(thd, rli)
> {
> bool ret= true;
> if (thd && !thd->backup_binlog_lock.is_acquired())
> {
> const ulong timeout= info_thd->variables.lock_wait_timeout;
>
> DBUG_PRINT("debug", ("Acquiring binlog protection lock"));
>
> if (rli)
> mysql_mutex_assert_not_owner(&rli->data_lock);
>
> ret= !info_thd->backup_binlog_lock.acquire_protection(thd,
> MDL_EXPLICIT, timeout);
> }
> return ret;
> }
>
> and void release(thd, bool protection_acquired)
> {
> if (thd && prot_acquired)
> {
> DBUG_PRINT("debug", ("Releasing binlog protection lock"));
> thd->backup_binlog_lock.release_protection(thd);
> }
> }

I have, yes. That would prevent some code duplication, but not too much,
as callers would still have to handle errors, the ‘protection_acquired’
flag + some of them also assert that the current connection has not
acquired the binlog lock, while the others do not. Which means the
wrapper code around those acquire/release helper functions would be
about the same number of lines replicating to every site. So I decided
to leave it as is for clarity.

--
Best regards,
Alexey.

[Laurynas Biveinis (laurynas-biveinis)]

OK if Jenkins OK

review: Approve

=== modified file 'mysql-test/suite/rpl/r/rpl_backup_locks.result'
--- mysql-test/suite/rpl/r/rpl_backup_locks.result	2014-02-25 07:30:22 +0000
+++ mysql-test/suite/rpl/r/rpl_backup_locks.result	2014-09-24 16:59:42 +0000
@@ -29,6 +29,13 @@
 UNLOCK TABLES;
 include/check_slave_param.inc [Exec_Master_Log_Pos]
 UNLOCK BINLOG;
+# connection slave
+LOCK BINLOG FOR BACKUP;
+# connection master
+INSERT INTO t_innodb VALUES (1);
+# connection slave
+include/check_slave_param.inc [Exec_Master_Log_Pos]
+UNLOCK BINLOG;
 # connection master
 DROP TABLE t_innodb, t_myisam;
 include/rpl_end.inc

=== added file 'mysql-test/suite/rpl/r/rpl_backup_locks_mts.result'
--- mysql-test/suite/rpl/r/rpl_backup_locks_mts.result	1970-01-01 00:00:00 +0000
+++ mysql-test/suite/rpl/r/rpl_backup_locks_mts.result	2014-09-24 16:59:42 +0000
@@ -0,0 +1,30 @@
+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 master info repository is not secure and is therefore not recommended. Please consider using the USER and PASSWORD connection options for START SLAVE; see the 'START SLAVE Syntax' in the MySQL Manual for more information.
+[connection master]
+CREATE DATABASE db_1;
+CREATE DATABASE db_2;
+CREATE TABLE db_1.test_1 (id INTEGER) engine=Innodb;
+CREATE TABLE db_2.test_1 (id INTEGER) engine=Innodb;
+include/sync_slave_sql_with_master.inc
+include/stop_slave.inc
+SET @save.slave_parallel_workers= @@GLOBAL.slave_parallel_workers;
+SET @@GLOBAL.slave_parallel_workers= 2;
+include/start_slave.inc
+LOCK BINLOG FOR BACKUP;
+INSERT INTO db_1.test_1 VALUES(1);
+INSERT INTO db_2.test_1 VALUES(1);
+include/check_slave_param.inc [Exec_Master_Log_Pos]
+UNLOCK BINLOG;
+include/sync_slave_sql_with_master.inc
+LOCK BINLOG FOR BACKUP;
+INSERT INTO db_1.test_1 VALUES(2);
+INSERT INTO db_2.test_1 VALUES(2);
+include/check_slave_param.inc [Exec_Master_Log_Pos]
+UNLOCK BINLOG;
+DROP DATABASE db_1;
+DROP DATABASE db_2;
+include/sync_slave_sql_with_master.inc
+SET @@GLOBAL.slave_parallel_workers= @save.slave_parallel_workers;
+include/rpl_end.inc

=== added file 'mysql-test/suite/rpl/r/rpl_bug1358836.result'
--- mysql-test/suite/rpl/r/rpl_bug1358836.result	1970-01-01 00:00:00 +0000
+++ mysql-test/suite/rpl/r/rpl_bug1358836.result	2014-09-24 16:59:42 +0000
@@ -0,0 +1,18 @@
+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 master info repository is not secure and is therefore not recommended. Please consider using the USER and PASSWORD connection options for START SLAVE; see the 'START SLAVE Syntax' in the MySQL Manual for more information.
+[connection master]
+CREATE TABLE t1 (
+a int(11) DEFAULT NULL
+) ENGINE=InnoDB DEFAULT CHARSET=latin1;
+# connection slave
+LOCK BINLOG FOR BACKUP;
+# connection master
+INSERT INTO t1 VALUES (1);
+# connection slave
+include/check_slave_param.inc [Exec_Master_Log_Pos]
+UNLOCK BINLOG;
+# connection master
+DROP TABLE t1;
+include/rpl_end.inc

=== modified file 'mysql-test/suite/rpl/t/rpl_backup_locks.test'
--- mysql-test/suite/rpl/t/rpl_backup_locks.test	2014-02-25 07:30:22 +0000
+++ mysql-test/suite/rpl/t/rpl_backup_locks.test	2014-09-24 16:59:42 +0000
@@ -86,6 +86,38 @@
 
 UNLOCK BINLOG;
 
+############################################################################
+# Bug 1372806: Deadlock with LOCK BINLOG FOR BACKUP + SHOW SLAVE STATUS with
+#              binary log enabled
+############################################################################
+
+--connection slave
+--echo # connection slave
+LOCK BINLOG FOR BACKUP;
+
+--let $master_log_pos= query_get_value(SHOW SLAVE STATUS, Exec_Master_Log_Pos, 1)
+
+--connection master
+--echo # connection master
+INSERT INTO t_innodb VALUES (1);
+
+--connection slave
+--echo # connection slave
+let $wait_condition=
+    SELECT COUNT(*) = 1 FROM INFORMATION_SCHEMA.PROCESSLIST
+    WHERE STATE = "Waiting for binlog lock" AND
+    (INFO = "INSERT INTO t_innodb VALUES (1)" OR INFO IS NULL);
+--source include/wait_condition.inc
+
+# The following would deadlock with binlog_format=stmt on SHOW SLAVE STATUS
+# before the fix
+
+--let $slave_param= Exec_Master_Log_Pos
+--let $slave_param_value= $master_log_pos
+--source include/check_slave_param.inc
+
+UNLOCK BINLOG;
+
 --connection master
 --echo # connection master
 

=== added file 'mysql-test/suite/rpl/t/rpl_backup_locks_mts-slave.opt'
--- mysql-test/suite/rpl/t/rpl_backup_locks_mts-slave.opt	1970-01-01 00:00:00 +0000
+++ mysql-test/suite/rpl/t/rpl_backup_locks_mts-slave.opt	2014-09-24 16:59:42 +0000
@@ -0,0 +1,1 @@
+--skip-log-bin --skip-log-slave-updates --slave-transaction-retries=0

=== added file 'mysql-test/suite/rpl/t/rpl_backup_locks_mts.test'
--- mysql-test/suite/rpl/t/rpl_backup_locks_mts.test	1970-01-01 00:00:00 +0000
+++ mysql-test/suite/rpl/t/rpl_backup_locks_mts.test	2014-09-24 16:59:42 +0000
@@ -0,0 +1,84 @@
+###########################################################################
+# MTS tests for binlog backup locks
+###########################################################################
+
+--source include/have_innodb.inc
+--let $rpl_skip_reset_master_and_slave=1
+--source include/master-slave.inc
+
+--connection master
+CREATE DATABASE db_1;
+CREATE DATABASE db_2;
+
+CREATE TABLE db_1.test_1 (id INTEGER) engine=Innodb;
+CREATE TABLE db_2.test_1 (id INTEGER) engine=Innodb;
+
+--connection master
+--source include/sync_slave_sql_with_master.inc
+
+--connection slave
+
+--source include/stop_slave.inc
+
+SET @save.slave_parallel_workers= @@GLOBAL.slave_parallel_workers;
+SET @@GLOBAL.slave_parallel_workers= 2;
+
+--source include/start_slave.inc
+
+LOCK BINLOG FOR BACKUP;
+
+--let $master_log_pos= query_get_value(SHOW SLAVE STATUS, Exec_Master_Log_Pos, 1)
+
+--connection master
+INSERT INTO db_1.test_1 VALUES(1);
+INSERT INTO db_2.test_1 VALUES(1);
+
+--connection slave
+
+let $wait_condition=
+    SELECT COUNT(*) = 2 FROM INFORMATION_SCHEMA.PROCESSLIST
+    WHERE STATE = "Waiting for binlog lock" AND
+    (INFO LIKE "INSERT INTO db\__.test\__ VALUES(1))" OR INFO IS NULL);
+--source include/wait_condition.inc
+
+--let $slave_param= Exec_Master_Log_Pos
+--let $slave_param_value= $master_log_pos
+--source include/check_slave_param.inc
+
+UNLOCK BINLOG;
+
+--connection master
+--source include/sync_slave_sql_with_master.inc
+
+--connection slave
+LOCK BINLOG FOR BACKUP;
+
+--let $master_log_pos= query_get_value(SHOW SLAVE STATUS, Exec_Master_Log_Pos, 1)
+
+--connection master
+INSERT INTO db_1.test_1 VALUES(2);
+INSERT INTO db_2.test_1 VALUES(2);
+
+--connection slave
+
+let $wait_condition=
+    SELECT COUNT(*) = 2 FROM INFORMATION_SCHEMA.PROCESSLIST
+    WHERE STATE = "Waiting for binlog lock" AND
+    (INFO LIKE "INSERT INTO db\__.test\__ VALUES(2)" OR INFO IS NULL);
+--source include/wait_condition.inc
+
+--let $slave_param= Exec_Master_Log_Pos
+--let $slave_param_value= $master_log_pos
+--source include/check_slave_param.inc
+
+UNLOCK BINLOG;
+
+--connection master
+DROP DATABASE db_1;
+DROP DATABASE db_2;
+
+--source include/sync_slave_sql_with_master.inc
+
+SET @@GLOBAL.slave_parallel_workers= @save.slave_parallel_workers;
+
+--source include/rpl_end.inc

=== added file 'mysql-test/suite/rpl/t/rpl_bug1358836-slave.opt'
--- mysql-test/suite/rpl/t/rpl_bug1358836-slave.opt	1970-01-01 00:00:00 +0000
+++ mysql-test/suite/rpl/t/rpl_bug1358836-slave.opt	2014-09-24 16:59:42 +0000
@@ -0,0 +1,1 @@
+--skip-log-bin --skip-log-slave-updates

=== added file 'mysql-test/suite/rpl/t/rpl_bug1358836.test'
--- mysql-test/suite/rpl/t/rpl_bug1358836.test	1970-01-01 00:00:00 +0000
+++ mysql-test/suite/rpl/t/rpl_bug1358836.test	2014-09-24 16:59:42 +0000
@@ -0,0 +1,49 @@
+###############################################################################
+# Bug #1358836: Backup locks do not guarantee consistent SHOW SLAVE STATUS with
+#               binary log disabled
+###############################################################################
+
+--source include/have_innodb.inc
+--let $rpl_skip_reset_master_and_slave=1
+--source include/master-slave.inc
+
+CREATE TABLE t1 (
+  a int(11) DEFAULT NULL
+) ENGINE=InnoDB DEFAULT CHARSET=latin1;
+
+sync_slave_with_master;
+
+--connection slave
+--echo # connection slave
+
+LOCK BINLOG FOR BACKUP;
+
+--let $master_log_pos= query_get_value(SHOW SLAVE STATUS, Exec_Master_Log_Pos, 1)
+
+--connection master
+--echo # connection master
+
+INSERT INTO t1 VALUES (1);
+
+--connection slave
+--echo # connection slave
+
+let $wait_condition=
+    SELECT COUNT(*) = 1 FROM INFORMATION_SCHEMA.PROCESSLIST
+    WHERE STATE = "Waiting for binlog lock" AND
+    (INFO = "INSERT INTO t1 VALUES (1)" OR INFO IS NULL);
+--source include/wait_condition.inc
+
+--let $slave_param= Exec_Master_Log_Pos
+--let $slave_param_value= $master_log_pos
+--source include/check_slave_param.inc
+
+UNLOCK BINLOG;
+
+--connection master
+--echo # connection master
+DROP TABLE t1;
+
+sync_slave_with_master;
+
+--source include/rpl_end.inc

=== modified file 'sql/binlog.cc'
--- sql/binlog.cc	2014-08-22 10:02:24 +0000
+++ sql/binlog.cc	2014-09-24 16:59:42 +0000
@@ -6446,6 +6446,7 @@
   my_xid xid= thd->transaction.xid_state.xid.get_my_xid();
   int error= RESULT_SUCCESS;
   bool stuff_logged= false;
+  bool binlog_prot_acquired= false;
 
   DBUG_PRINT("enter", ("thd: 0x%llx, all: %s, xid: %llu, cache_mngr: 0x%llx",
                        (ulonglong) thd, YESNO(all), (ulonglong) xid,
@@ -6590,8 +6591,15 @@
   {
     int rc;
 
-    /* Block binlog updates if there's an active BINLOG lock. */
-    if (!thd->backup_binlog_lock.is_acquired())
+    /*
+       Block binlog updates if there's an active BINLOG lock.
+
+       We allow binlog lock owner to commit, assuming it knows what it does. We
+       also check if protection has not been acquired earlier, which is possible
+       in slave threads to protect master binlog coordinates.
+    */
+    if (!thd->backup_binlog_lock.is_acquired() &&
+        !thd->backup_binlog_lock.is_protection_acquired())
     {
       const ulong timeout= thd->variables.lock_wait_timeout;
 
@@ -6604,11 +6612,13 @@
 
         DBUG_RETURN(RESULT_ABORTED);
       }
+
+      binlog_prot_acquired= true;
     }
 
     rc= ordered_commit(thd, all);
 
-    if (!thd->backup_binlog_lock.is_acquired())
+    if (binlog_prot_acquired)
     {
       DBUG_PRINT("debug", ("Releasing binlog protection lock"));
       thd->backup_binlog_lock.release_protection(thd);

=== modified file 'sql/log_event.cc'
--- sql/log_event.cc	2014-08-22 10:02:24 +0000
+++ sql/log_event.cc	2014-09-24 16:59:42 +0000
@@ -6910,6 +6910,7 @@
     */
     DBUG_ASSERT(!thd->backup_binlog_lock.is_acquired());
     DBUG_PRINT("debug", ("Acquiring binlog protection lock"));
+    mysql_mutex_assert_not_owner(&rli->data_lock);
     const ulong timeout= thd->variables.lock_wait_timeout;
     if (thd->backup_binlog_lock.acquire_protection(thd, MDL_EXPLICIT, timeout))
     {
@@ -7431,11 +7432,25 @@
   lex_start(thd);
   mysql_reset_thd_for_next_command(thd);
   Relay_log_info *rli_ptr= const_cast<Relay_log_info *>(rli);
+  bool binlog_prot_acquired= false;
 
   /* For a slave Xid_log_event is COMMIT */
   general_log_print(thd, COM_QUERY,
                     "COMMIT /* implicit, from Xid_log_event */");
 
+  if (!thd->backup_binlog_lock.is_acquired())
+  {
+    const ulong timeout= thd->variables.lock_wait_timeout;
+
+    DBUG_PRINT("debug", ("Acquiring binlog protection lock"));
+    mysql_mutex_assert_not_owner(&rli->data_lock);
+    if (thd->backup_binlog_lock.acquire_protection(thd, MDL_EXPLICIT,
+                                                   timeout))
+      return 1;
+
+    binlog_prot_acquired= true;
+  }
+
   mysql_mutex_lock(&rli_ptr->data_lock);
 
   DBUG_PRINT("info", ("do_apply group master %s %llu  group relay %s %llu event %s %llu\n",
@@ -7496,6 +7511,12 @@
   mysql_cond_broadcast(&rli_ptr->data_cond);
   mysql_mutex_unlock(&rli_ptr->data_lock);
 
+  if (binlog_prot_acquired)
+  {
+      DBUG_PRINT("debug", ("Releasing binlog protection lock"));
+      thd->backup_binlog_lock.release_protection(thd);
+  }
+
   return error;
 }
 

=== modified file 'sql/rpl_info_factory.cc'
--- sql/rpl_info_factory.cc	2013-10-15 11:37:57 +0000
+++ sql/rpl_info_factory.cc	2014-09-24 16:59:42 +0000
@@ -529,6 +529,8 @@
   bool error= true;
   enum_return_check return_check_src= ERROR_CHECKING_REPOSITORY;
   enum_return_check return_check_dst= ERROR_CHECKING_REPOSITORY;
+  bool binlog_prot_acquired= false;
+  THD * const thd= current_thd;
   DBUG_ENTER("Rpl_info_factory::decide_repository");
 
   if (option == INFO_REPOSITORY_DUMMY)
@@ -585,6 +587,23 @@
         goto err;
 
       /*
+        Acquire a backup binlog protection lock, because Rpl_info::copy_info()
+        will modify master binary log coordinates.
+      */
+      if (thd && !thd->backup_binlog_lock.is_acquired())
+      {
+        const ulong timeout= thd->variables.lock_wait_timeout;
+
+        DBUG_PRINT("debug", ("Acquiring binlog protection lock"));
+
+        if (thd->backup_binlog_lock.acquire_protection(thd, MDL_EXPLICIT,
+                                                       timeout))
+          goto err;
+
+        binlog_prot_acquired= true;
+      }
+
+      /*
         Transfer information from source to destination and delete the
         source. Note this is not fault-tolerant and a crash before removing
         source may cause the next restart to fail as is_src and is_dest may
@@ -628,6 +647,11 @@
   }
 
 err:
+  if (binlog_prot_acquired)
+  {
+    DBUG_PRINT("debug", ("Releasing binlog protection lock"));
+    thd->backup_binlog_lock.release_protection(thd);
+  }
   DBUG_RETURN(error); 
 }
 

=== modified file 'sql/rpl_rli.cc'
--- sql/rpl_rli.cc	2014-02-27 12:29:47 +0000
+++ sql/rpl_rli.cc	2014-09-24 16:59:42 +0000
@@ -78,7 +78,7 @@
    is_relay_log_recovery(is_slave_recovery),
    save_temporary_tables(0),
    cur_log_old_open_count(0), group_relay_log_pos(0), event_relay_log_pos(0),
-   group_master_log_pos(0),
+   m_group_master_log_pos(0),
    gtid_set(global_sid_map, global_sid_lock),
    log_space_total(0), ignore_log_space_limit(0),
    sql_force_rotate_relay(false),
@@ -120,7 +120,7 @@
 #endif
 
   group_relay_log_name[0]= event_relay_log_name[0]=
-    group_master_log_name[0]= 0;
+    m_group_master_log_name[0]= 0;
   until_log_name[0]= ign_master_log_name_end[0]= 0;
   set_timespec_nsec(last_clock, 0);
   memset(&cache_buf, 0, sizeof(cache_buf));
@@ -688,6 +688,8 @@
   {
     bool pos_reached;
     int cmp_result= 0;
+    const char * const group_master_log_name= get_group_master_log_name();
+    const ulonglong group_master_log_pos= get_group_master_log_pos();
 
     DBUG_PRINT("info",
                ("init_abort_pos_wait: %ld  abort_pos_wait: %ld",
@@ -710,8 +712,8 @@
     */
     if (*group_master_log_name)
     {
-      char *basename= (group_master_log_name +
-                       dirname_length(group_master_log_name));
+      const char * const basename= (group_master_log_name +
+                                    dirname_length(group_master_log_name));
       /*
         First compare the parts before the extension.
         Find the dot in the master's log basename,
@@ -962,6 +964,7 @@
     */
     DBUG_ASSERT(!info_thd->backup_binlog_lock.is_acquired());
     DBUG_PRINT("debug", ("Acquiring binlog protection lock"));
+    mysql_mutex_assert_not_owner(&data_lock);
     if (info_thd->backup_binlog_lock.acquire_protection(info_thd, MDL_EXPLICIT,
                                                         timeout))
       DBUG_RETURN(1);
@@ -971,8 +974,7 @@
   else
   {
     mysql_mutex_assert_owner(&data_lock);
-    DBUG_ASSERT(info_thd->mdl_context.is_lock_owner(MDL_key::BINLOG, "", "",
-                                                    MDL_INTENTION_EXCLUSIVE));
+    DBUG_ASSERT(info_thd->backup_binlog_lock.is_protection_acquired());
   }
 
   inc_event_relay_log_pos();
@@ -1003,10 +1005,10 @@
     With the end_log_pos solution, we avoid computations involving lengthes.
   */
   DBUG_PRINT("info", ("log_pos: %lu  group_master_log_pos: %lu",
-                      (long) log_pos, (long) group_master_log_pos));
+                      (long) log_pos, (long) get_group_master_log_pos()));
 
   if (log_pos > 0)  // 3.23 binlogs don't have log_posx
-    group_master_log_pos= log_pos;
+    set_group_master_log_pos(log_pos);
 
   /*
     In MTS mode FD or Rotate event commit their solitary group to
@@ -1080,6 +1082,7 @@
 {
   int error=0;
   DBUG_ENTER("Relay_log_info::purge_relay_logs");
+  bool binlog_prot_acquired= false;
 
   /*
     Even if inited==0, we still try to empty master_log_* variables. Indeed,
@@ -1097,12 +1100,33 @@
     SLAVE, he will see old, confusing master_log_*. In other words, we reinit
     master_log_* for SHOW SLAVE STATUS to display fine in any case.
   */
-  group_master_log_name[0]= 0;
-  group_master_log_pos= 0;
+
+  if (!thd->backup_binlog_lock.is_acquired())
+  {
+    const ulong timeout= thd->variables.lock_wait_timeout;
+
+    DBUG_PRINT("debug", ("Acquiring binlog protection lock"));
+    mysql_mutex_assert_not_owner(&data_lock);
+    if (thd->backup_binlog_lock.acquire_protection(thd, MDL_EXPLICIT,
+                                                   timeout))
+      return 1;
+
+    binlog_prot_acquired= true;
+  }
+
+  set_group_master_log_name("");
+  set_group_master_log_pos(0);
 
   if (!inited)
   {
     DBUG_PRINT("info", ("inited == 0"));
+
+    if (binlog_prot_acquired)
+    {
+      DBUG_PRINT("debug", ("Releasing binlog protection lock"));
+      thd->backup_binlog_lock.release_protection(thd);
+    }
+
     DBUG_RETURN(0);
   }
 
@@ -1154,6 +1178,13 @@
 #endif
   DBUG_PRINT("info",("log_space_total: %s",llstr(log_space_total,buf)));
   mysql_mutex_unlock(&data_lock);
+
+  if (binlog_prot_acquired)
+  {
+      DBUG_PRINT("debug", ("Releasing binlog protection lock"));
+      thd->backup_binlog_lock.release_protection(thd);
+  }
+
   DBUG_RETURN(error);
 }
 
@@ -1206,10 +1237,10 @@
     {
       if (ev && ev->server_id == (uint32) ::server_id && !replicate_same_server_id)
         DBUG_RETURN(false);
-      log_name= group_master_log_name;
-      log_pos= (!ev)? group_master_log_pos :
+      log_name= get_group_master_log_name();
+      log_pos= (!ev) ? get_group_master_log_pos() :
         ((thd->variables.option_bits & OPTION_BEGIN || !ev->log_pos) ?
-         group_master_log_pos : ev->log_pos - ev->data_written);
+         get_group_master_log_pos() : ev->log_pos - ev->data_written);
     }
     else
     { /* until_condition == UNTIL_RELAY_POS */
@@ -1221,7 +1252,8 @@
     {
       char buf[32];
       DBUG_PRINT("info", ("group_master_log_name='%s', group_master_log_pos=%s",
-                          group_master_log_name, llstr(group_master_log_pos, buf)));
+                          get_group_master_log_name(),
+                          llstr(get_group_master_log_pos(), buf)));
       DBUG_PRINT("info", ("group_relay_log_name='%s', group_relay_log_pos=%s",
                           group_relay_log_name, llstr(group_relay_log_pos, buf)));
       DBUG_PRINT("info", ("(%s) log_name='%s', log_pos=%s",
@@ -1857,8 +1889,8 @@
       error= 1;
       goto err;
     }
-    group_master_log_name[0]= 0;
-    group_master_log_pos= 0;
+    set_group_master_log_name("");
+    set_group_master_log_pos(0);
   }
   else
   {
@@ -2069,6 +2101,7 @@
   int lines= 0;
   char *first_non_digit= NULL;
   ulong temp_group_relay_log_pos= 0;
+  char temp_group_master_log_name[FN_REFLEN];
   ulong temp_group_master_log_pos= 0;
   int temp_sql_delay= 0;
   int temp_internal_id= internal_id;
@@ -2133,8 +2166,8 @@
 
   if (from->get_info((ulong *) &temp_group_relay_log_pos,
                      (ulong) BIN_LOG_HEADER_SIZE) ||
-      from->get_info(group_master_log_name,
-                     (size_t) sizeof(group_relay_log_name),
+      from->get_info(temp_group_master_log_name,
+                     (size_t) sizeof(temp_group_master_log_name),
                      (char *) "") ||
       from->get_info((ulong *) &temp_group_master_log_pos,
                      (ulong) 0))
@@ -2159,7 +2192,8 @@
   }
  
   group_relay_log_pos=  temp_group_relay_log_pos;
-  group_master_log_pos= temp_group_master_log_pos;
+  set_group_master_log_name(temp_group_master_log_name);
+  set_group_master_log_pos(temp_group_master_log_pos);
   sql_delay= (int32) temp_sql_delay;
   internal_id= (uint) temp_internal_id;
 
@@ -2182,8 +2216,8 @@
       to->set_info((int) LINES_IN_RELAY_LOG_INFO_WITH_ID) ||
       to->set_info(group_relay_log_name) ||
       to->set_info((ulong) group_relay_log_pos) ||
-      to->set_info(group_master_log_name) ||
-      to->set_info((ulong) group_master_log_pos) ||
+      to->set_info(get_group_master_log_name()) ||
+      to->set_info((ulong) get_group_master_log_pos()) ||
       to->set_info((int) sql_delay) ||
       to->set_info(recovery_parallel_workers) ||
       to->set_info((int) internal_id))

=== modified file 'sql/rpl_rli.h'
--- sql/rpl_rli.h	2014-06-04 14:48:06 +0000
+++ sql/rpl_rli.h	2014-09-24 16:59:42 +0000
@@ -223,8 +223,8 @@
     !belongs_to_client(); client thread executing BINLOG statement if
     belongs_to_client()).
   */
-  char group_master_log_name[FN_REFLEN];
-  volatile my_off_t group_master_log_pos;
+  char m_group_master_log_name[FN_REFLEN];
+  volatile my_off_t m_group_master_log_pos;
 
   /*
     When it commits, InnoDB internally stores the master log position it has
@@ -417,7 +417,7 @@
   bool is_until_satisfied(THD *thd, Log_event *ev);
   inline ulonglong until_pos()
   {
-    return ((until_condition == UNTIL_MASTER_POS) ? group_master_log_pos :
+    return ((until_condition == UNTIL_MASTER_POS) ? get_group_master_log_pos() :
 	    group_relay_log_pos);
   }
 
@@ -733,15 +733,24 @@
     future_event_relay_log_pos= log_pos;
   }
 
-  inline const char* get_group_master_log_name() { return group_master_log_name; }
-  inline ulonglong get_group_master_log_pos() { return group_master_log_pos; }
+  inline const char* get_group_master_log_name()
+  {
+    return m_group_master_log_name;
+  }
+  inline ulonglong get_group_master_log_pos() { return m_group_master_log_pos; }
   inline void set_group_master_log_name(const char *log_file_name)
   {
-     strmake(group_master_log_name,log_file_name, sizeof(group_master_log_name)-1);
+    DBUG_ASSERT(!info_thd ||
+                info_thd->backup_binlog_lock.is_protection_acquired());
+
+    strmake(m_group_master_log_name, log_file_name,
+            sizeof(m_group_master_log_name) - 1);
   }
   inline void set_group_master_log_pos(ulonglong log_pos)
   {
-    group_master_log_pos= log_pos;
+    DBUG_ASSERT(!info_thd ||
+                info_thd->backup_binlog_lock.is_protection_acquired());
+    m_group_master_log_pos= log_pos;
   }
 
   inline const char* get_group_relay_log_name() { return group_relay_log_name; }
@@ -775,7 +784,7 @@
   }
   inline const char* get_rpl_log_name()
   {
-    return (group_master_log_name[0] ? group_master_log_name : "FIRST");
+    return (m_group_master_log_name[0] ? m_group_master_log_name : "FIRST");
   }
 
 #if MYSQL_VERSION_ID < 40100

=== modified file 'sql/rpl_rli_pdb.cc'
--- sql/rpl_rli_pdb.cc	2014-08-22 10:02:24 +0000
+++ sql/rpl_rli_pdb.cc	2014-09-24 16:59:42 +0000
@@ -300,6 +300,7 @@
   DBUG_ENTER("Slave_worker::read_info");
 
   ulong temp_group_relay_log_pos= 0;
+  char temp_group_master_log_name[FN_REFLEN];
   ulong temp_group_master_log_pos= 0;
   ulong temp_checkpoint_relay_log_pos= 0;
   ulong temp_checkpoint_master_log_pos= 0;
@@ -317,8 +318,8 @@
                      (char *) "") ||
       from->get_info((ulong *) &temp_group_relay_log_pos,
                      (ulong) 0) ||
-      from->get_info(group_master_log_name,
-                     (size_t) sizeof(group_master_log_name),
+      from->get_info(temp_group_master_log_name,
+                     (size_t) sizeof(temp_group_master_log_name),
                      (char *) "") ||
       from->get_info((ulong *) &temp_group_master_log_pos,
                      (ulong) 0) ||
@@ -343,7 +344,8 @@
 
   internal_id=(uint) temp_internal_id;
   group_relay_log_pos=  temp_group_relay_log_pos;
-  group_master_log_pos= temp_group_master_log_pos;
+  set_group_master_log_name(temp_group_master_log_name);
+  set_group_master_log_pos(temp_group_master_log_pos);
   checkpoint_relay_log_pos=  temp_checkpoint_relay_log_pos;
   checkpoint_master_log_pos= temp_checkpoint_master_log_pos;
   checkpoint_seqno= temp_checkpoint_seqno;
@@ -363,8 +365,8 @@
       to->set_info((int) internal_id) ||
       to->set_info(group_relay_log_name) ||
       to->set_info((ulong) group_relay_log_pos) ||
-      to->set_info(group_master_log_name) ||
-      to->set_info((ulong) group_master_log_pos) ||
+      to->set_info(get_group_master_log_name()) ||
+      to->set_info((ulong) get_group_master_log_pos()) ||
       to->set_info(checkpoint_relay_log_name) ||
       to->set_info((ulong) checkpoint_relay_log_pos) ||
       to->set_info(checkpoint_master_log_name) ||
@@ -387,11 +389,33 @@
 */
 bool Slave_worker::reset_recovery_info()
 {
+  bool binlog_prot_acquired= false;
+
   DBUG_ENTER("Slave_worker::reset_recovery_info");
 
+  if (info_thd && !info_thd->backup_binlog_lock.is_acquired())
+  {
+    const ulong timeout= info_thd->variables.lock_wait_timeout;
+
+    DBUG_PRINT("debug", ("Acquiring binlog protection lock"));
+
+    if (info_thd->backup_binlog_lock.acquire_protection(info_thd, MDL_EXPLICIT,
+                                                        timeout))
+      DBUG_RETURN(true);
+
+    binlog_prot_acquired= true;
+  }
+
   set_group_master_log_name("");
   set_group_master_log_pos(0);
 
+  if (binlog_prot_acquired)
+  {
+    DBUG_PRINT("debug", ("Releasing binlog protection lock"));
+    info_thd->backup_binlog_lock.release_protection(info_thd);
+  }
+
+
   DBUG_RETURN(flush_info(true));
 }
 
@@ -410,8 +434,22 @@
 
 bool Slave_worker::commit_positions(Log_event *ev, Slave_job_group* ptr_g, bool force)
 {
+  bool binlog_prot_acquired= false;
   DBUG_ENTER("Slave_worker::checkpoint_positions");
 
+  if (info_thd && !info_thd->backup_binlog_lock.is_acquired())
+  {
+    const ulong timeout= info_thd->variables.lock_wait_timeout;
+
+    DBUG_PRINT("debug", ("Acquiring binlog protection lock"));
+
+    if (info_thd->backup_binlog_lock.acquire_protection(info_thd, MDL_EXPLICIT,
+                                                        timeout))
+      DBUG_RETURN(true);
+
+    binlog_prot_acquired= true;
+  }
+
   /*
     Initial value of checkpoint_master_log_name is learned from
     group_master_log_name. The latter can be passed to Worker
@@ -422,11 +460,10 @@
   */
   if (ptr_g->group_master_log_name != NULL)
   {
-    strmake(group_master_log_name, ptr_g->group_master_log_name,
-            sizeof(group_master_log_name) - 1);
+    set_group_master_log_name(ptr_g->group_master_log_name);
     my_free(ptr_g->group_master_log_name);
     ptr_g->group_master_log_name= NULL;
-    strmake(checkpoint_master_log_name, group_master_log_name,
+    strmake(checkpoint_master_log_name, get_group_master_log_name(),
             sizeof(checkpoint_master_log_name) - 1);
   }
   if (ptr_g->checkpoint_log_name != NULL)
@@ -467,19 +504,19 @@
   bitmap_set_bit(&group_executed, ptr_g->checkpoint_seqno);
   checkpoint_seqno= ptr_g->checkpoint_seqno;
   group_relay_log_pos= ev->future_event_relay_log_pos;
-  group_master_log_pos= ev->log_pos;
+  set_group_master_log_pos(ev->log_pos);
 
   /*
     Directly accessing c_rli->get_group_master_log_name() does not
     represent a concurrency issue because the current code places
     a synchronization point when master rotates.
   */
-  strmake(group_master_log_name, c_rli->get_group_master_log_name(),
-          sizeof(group_master_log_name)-1);
+  set_group_master_log_name(c_rli->get_group_master_log_name());
 
   DBUG_PRINT("mts", ("Committing worker-id %lu group master log pos %llu "
              "group master log name %s checkpoint sequence number %lu.",
-             id, group_master_log_pos, group_master_log_name, checkpoint_seqno));
+                     id, get_group_master_log_pos(),
+                     get_group_master_log_name(), checkpoint_seqno));
 
   DBUG_EXECUTE_IF("mts_debug_concurrent_access",
     {
@@ -487,6 +524,12 @@
     };
   );
 
+  if (binlog_prot_acquired)
+  {
+    DBUG_PRINT("debug", ("Releasing binlog protection lock"));
+    info_thd->backup_binlog_lock.release_protection(info_thd);
+  }
+
   DBUG_RETURN(flush_info(force));
 }
 
@@ -1060,7 +1103,7 @@
       );
     }
 
-    ptr_g->group_master_log_pos= group_master_log_pos;
+    ptr_g->group_master_log_pos= get_group_master_log_pos();
     ptr_g->group_relay_log_pos= group_relay_log_pos;
 
     ptr_g->done= 1;    // GAQ index is available to C now

=== modified file 'sql/rpl_slave.cc'
--- sql/rpl_slave.cc	2014-08-22 10:02:24 +0000
+++ sql/rpl_slave.cc	2014-09-24 16:59:42 +0000
@@ -507,6 +507,21 @@
   int init_error= 0;
   enum_return_check check_return= ERROR_CHECKING_REPOSITORY;
   THD *thd= current_thd;
+  bool binlog_prot_acquired= false;
+
+  if (thd && !thd->backup_binlog_lock.is_acquired())
+  {
+    const ulong timeout= thd->variables.lock_wait_timeout;
+
+    DBUG_PRINT("debug", ("Acquiring binlog protection lock"));
+    mysql_mutex_assert_not_owner(&mi->rli->data_lock);
+
+    if (thd->backup_binlog_lock.acquire_protection(thd, MDL_EXPLICIT,
+                                                   timeout))
+      DBUG_RETURN(1);
+
+    binlog_prot_acquired= true;
+  }
 
   /*
     We need a mutex while we are changing master info parameters to
@@ -570,6 +585,13 @@
 
   mysql_mutex_unlock(&mi->rli->data_lock);
   mysql_mutex_unlock(&mi->data_lock);
+
+  if (binlog_prot_acquired)
+  {
+      DBUG_PRINT("debug", ("Releasing binlog protection lock"));
+      thd->backup_binlog_lock.release_protection(thd);
+  }
+
   DBUG_RETURN(check_return == ERROR_CHECKING_REPOSITORY || init_error);
 }
 
@@ -5066,6 +5088,7 @@
   ulong cnt;
   bool error= FALSE;
   struct timespec curr_clock;
+  bool binlog_prot_acquired= false;
 
   DBUG_ENTER("checkpoint_routine");
 
@@ -5137,9 +5160,32 @@
   sort_dynamic(&rli->least_occupied_workers, (qsort_cmp) ulong_cmp);
 
   if (need_data_lock)
+  {
+    THD * const info_thd= rli->info_thd;
+    const ulong timeout= info_thd->variables.lock_wait_timeout;
+
+    /*
+      Acquire protection against global BINLOG lock before rli->data_lock is
+      locked (otherwise we would also block SHOW SLAVE STATUS).
+    */
+    DBUG_ASSERT(!info_thd->backup_binlog_lock.is_acquired());
+    DBUG_PRINT("debug", ("Acquiring binlog protection lock"));
+    mysql_mutex_assert_not_owner(&rli->data_lock);
+    error= info_thd->backup_binlog_lock.acquire_protection(info_thd,
+                                                           MDL_EXPLICIT,
+                                                           timeout);
+    if (error)
+      goto end;
+
+    binlog_prot_acquired= true;
+
     mysql_mutex_lock(&rli->data_lock);
+  }
   else
+  {
     mysql_mutex_assert_owner(&rli->data_lock);
+    DBUG_ASSERT(rli->info_thd->backup_binlog_lock.is_protection_acquired());
+  }
 
   /*
     "Coordinator::commit_positions" {
@@ -5184,6 +5230,13 @@
   /* end-of "Coordinator::"commit_positions" */
 
 end:
+
+  if (binlog_prot_acquired)
+  {
+    DBUG_PRINT("debug", ("Releasing binlog protection lock"));
+    rli->info_thd->backup_binlog_lock.release_protection(rli->info_thd);
+  }
+
 #ifndef DBUG_OFF
   if (DBUG_EVALUATE_IF("check_slave_debug_group", 1, 0))
     DBUG_SUICIDE();
@@ -8134,6 +8187,7 @@
   my_off_t saved_log_pos= 0;
   my_bool save_relay_log_purge= relay_log_purge;
   bool mts_remove_workers= false;
+  bool binlog_prot_acquired= false;
 
   DBUG_ENTER("change_master");
 
@@ -8481,6 +8535,22 @@
   }
   relay_log_purge= save_relay_log_purge;
 
+  if (!thd->backup_binlog_lock.is_acquired())
+  {
+    const ulong timeout= thd->variables.lock_wait_timeout;
+
+    DBUG_PRINT("debug", ("Acquiring binlog protection lock"));
+    mysql_mutex_assert_not_owner(&mi->rli->data_lock);
+    if (thd->backup_binlog_lock.acquire_protection(thd, MDL_EXPLICIT,
+                                                   timeout))
+    {
+      ret= true;
+      goto err;
+    }
+
+    binlog_prot_acquired= true;
+  }
+
   /*
     Coordinates in rli were spoilt by the 'if (need_relay_log_purge)' block,
     so restore them to good values. If we left them to ''/0, that would work;
@@ -8533,6 +8603,13 @@
   mysql_mutex_unlock(&mi->rli->data_lock);
 
 err:
+
+  if (binlog_prot_acquired)
+  {
+    DBUG_PRINT("debug", ("Releasing binlog protection lock"));
+    thd->backup_binlog_lock.release_protection(thd);
+  }
+
   unlock_slave_threads(mi);
   if (ret == FALSE)
   {

=== modified file 'sql/sql_class.h'
--- sql/sql_class.h	2014-08-22 10:02:24 +0000
+++ sql/sql_class.h	2014-09-24 16:59:42 +0000
@@ -2170,6 +2170,8 @@
 
   bool is_acquired() const { return m_lock != NULL; }
 
+  bool is_protection_acquired() const { return m_prot_lock != NULL; }
+
 private:
   MDL_key::enum_mdl_namespace m_namespace;
   MDL_ticket *m_lock;