MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Dmitry Lenev Date:September 15 2010 2:15pm
Subject:bzr commit into mysql-5.5-runtime branch (dlenev:3137) Bug#56251
View as plain text  
#At file:///home/dlenev/src/bzr/mysql-5.5-rt-56251/ based on revid:jon.hauglid@stripped

 3137 Dmitry Lenev	2010-09-15
      Fix for bug #56251 "Deadlock with INSERT DELAYED and MERGE
      tables".
      
      Attempting to issue an INSERT DELAYED statement for a MERGE
      table might have caused a deadlock if it happened as part of
      a transaction or under LOCK TABLES, and there was a concurrent
      DDL or LOCK TABLES ... WRITE statement which tried to lock one
      of its underlying tables.
      
      The problem occurred when a delayed insert handler thread tried
      to open a MERGE table and discovered that to do this it had also
      to open all underlying tables and hence acquire metadata
      locks on them. Since metadata locks on the underlying tables were
      not pre-acquired by the connection thread executing INSERT DELAYED,
      attempts to do so might lead to waiting. In this case the
      connection thread had to wait for the delayed insert thread.
      If the thread which was preventing the lock on the underlying table
      from being acquired had to wait for the connection thread (due to
      this or other metadata locks), a deadlock occurred. 
      This deadlock was not detected by the MDL deadlock detector since 
      waiting for the handler thread by the connection thread is not
      represented in the wait-for graph.
      
      This patch solves the problem by ensuring that the delayed
      insert handler thread never tries to open underlying tables 
      of a MERGE table. Instead open_tables() is aborted right after
      the parent table is opened and a ER_DELAYED_NOT_SUPPORTED 
      error is emitted (which is passed to the connection thread and
      ultimately to the user).
     @ mysql-test/r/merge.result
        Added test for bug #56251 "Deadlock with INSERT DELAYED and
        MERGE tables".
     @ mysql-test/t/merge.test
        Added test for bug #56251 "Deadlock with INSERT DELAYED and
        MERGE tables".
     @ sql/sql_base.cc
        Changed open_n_lock_single_table() to take prelocking strategy
        as an argument instead of always using DML_prelocking_strategy.
     @ sql/sql_base.h
        Changed open_n_lock_single_table() to take prelocking strategy
        as an argument instead of always using DML_prelocking_strategy.
        Added a version of this function which is compatible with old
        signature.
     @ sql/sql_insert.cc
        When opening MERGE table in delayed insert thread stop and emit
        ER_DELAYED_NOT_SUPPORTED right after opening main table and
        before opening underlying tables. This ensures that we won't
        try to acquire metadata lock on underlying tables which might
        lead to a deadlock.
        This is achieved by using special prelocking strategy which
        abort open_tables() process as soon as we discover that we
        have opened table with engine which doesn't support delayed
        inserts.

    modified:
      mysql-test/r/merge.result
      mysql-test/t/merge.test
      sql/sql_base.cc
      sql/sql_base.h
      sql/sql_insert.cc
=== modified file 'mysql-test/r/merge.result'
--- a/mysql-test/r/merge.result	2010-09-08 08:25:37 +0000
+++ b/mysql-test/r/merge.result	2010-09-15 14:15:31 +0000
@@ -3575,4 +3575,32 @@ ERROR HY000: The definition of table 'v1
 drop view v1;
 drop temporary table tmp;
 drop table t1, t2, t3, m1, m2;
+#
+# Test for bug #56251 "Deadlock with INSERT DELAYED and MERGE tables".
+#
+drop table if exists t1, t2, tm;
+create table t1(a int);
+create table t2(a int);
+create table tm(a int) engine=merge union=(t1, t2);
+begin;
+select * from t1;
+a
+# Connection 'con1'.
+# Sending:
+alter table t1 comment 'test';
+# Connection 'default'.
+# Wait until ALTER TABLE blocks and starts waiting
+# for connection 'default'. It should wait with a
+# pending SNW lock on 't1'.
+# Attempt to perform delayed insert into 'tm' should not lead
+# to a deadlock. Instead error ER_DELAYED_NOT_SUPPORTED should
+# be emitted.
+insert delayed into tm values (1);
+ERROR HY000: DELAYED option not supported for table 'tm'
+# Unblock ALTER TABLE.
+commit;
+# Connection 'con1'.
+# Reaping ALTER TABLE:
+# Connection 'default'.
+drop tables tm, t1, t2;
 End of 6.0 tests

=== modified file 'mysql-test/t/merge.test'
--- a/mysql-test/t/merge.test	2010-09-08 08:25:37 +0000
+++ b/mysql-test/t/merge.test	2010-09-15 14:15:31 +0000
@@ -2,6 +2,9 @@
 # Test of MERGE TABLES
 #
 
+# Save the initial number of concurrent sessions.
+--source include/count_sessions.inc
+
 # MERGE tables require MyISAM tables
 let $default=`select @@global.storage_engine`;
 set global storage_engine=myisam;
@@ -2664,6 +2667,55 @@ drop view v1;
 drop temporary table tmp;
 drop table t1, t2, t3, m1, m2;
 
+
+--echo #
+--echo # Test for bug #56251 "Deadlock with INSERT DELAYED and MERGE tables".
+--echo #
+connect (con1,localhost,root,,);
+connection default;
+--disable_warnings
+drop table if exists t1, t2, tm;
+--enable_warnings
+create table t1(a int);
+create table t2(a int);
+create table tm(a int) engine=merge union=(t1, t2);
+begin;
+select * from t1;
+
+--echo # Connection 'con1'.
+connection con1;
+--echo # Sending:
+--send alter table t1 comment 'test'
+
+--echo # Connection 'default'.
+connection default;
+--echo # Wait until ALTER TABLE blocks and starts waiting
+--echo # for connection 'default'. It should wait with a
+--echo # pending SNW lock on 't1'.
+let $wait_condition=
+  select count(*) = 1 from information_schema.processlist
+  where state = "Waiting for table metadata lock" and
+        info = "alter table t1 comment 'test'";
+--source include/wait_condition.inc
+--echo # Attempt to perform delayed insert into 'tm' should not lead
+--echo # to a deadlock. Instead error ER_DELAYED_NOT_SUPPORTED should
+--echo # be emitted.
+--error ER_DELAYED_NOT_SUPPORTED
+insert delayed into tm values (1);
+--echo # Unblock ALTER TABLE.
+commit;
+
+--echo # Connection 'con1'.
+connection con1;
+--echo # Reaping ALTER TABLE:
+--reap
+
+--echo # Connection 'default'.
+connection default;
+disconnect con1;
+drop tables tm, t1, t2;
+
+
 --echo End of 6.0 tests
 
 --disable_result_log
@@ -2671,3 +2723,7 @@ drop table t1, t2, t3, m1, m2;
 eval set global storage_engine=$default;
 --enable_result_log
 --enable_query_log
+
+# Check that all connections opened by test cases in this file are really
+# gone so execution of other tests won't be affected by their presence.
+--source include/wait_until_count_sessions.inc

=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc	2010-09-09 14:29:14 +0000
+++ b/sql/sql_base.cc	2010-09-15 14:15:31 +0000
@@ -5165,6 +5165,8 @@ static bool check_lock_and_start_stmt(TH
   @param[in]    lock_type       lock to use for table
   @param[in]    flags           options to be used while opening and locking
                                 table (see open_table(), mysql_lock_tables())
+  @param[in]    prelocking_strategy  Strategy which specifies how prelocking
+                                     algorithm should work for this statement.
 
   @return       table
     @retval     != NULL         OK, opened table returned
@@ -5190,7 +5192,8 @@ static bool check_lock_and_start_stmt(TH
 */
 
 TABLE *open_n_lock_single_table(THD *thd, TABLE_LIST *table_l,
-                                thr_lock_type lock_type, uint flags)
+                                thr_lock_type lock_type, uint flags,
+                                Prelocking_strategy *prelocking_strategy)
 {
   TABLE_LIST *save_next_global;
   DBUG_ENTER("open_n_lock_single_table");
@@ -5206,7 +5209,8 @@ TABLE *open_n_lock_single_table(THD *thd
   table_l->required_type= FRMTYPE_TABLE;
 
   /* Open the table. */
-  if (open_and_lock_tables(thd, table_l, FALSE, flags))
+  if (open_and_lock_tables(thd, table_l, FALSE, flags,
+                           prelocking_strategy))
     table_l->table= NULL; /* Just to be sure. */
 
   /* Restore list. */

=== modified file 'sql/sql_base.h'
--- a/sql/sql_base.h	2010-09-09 14:29:14 +0000
+++ b/sql/sql_base.h	2010-09-15 14:15:31 +0000
@@ -246,7 +246,8 @@ bool open_and_lock_tables(THD *thd, TABL
 int open_and_lock_tables_derived(THD *thd, TABLE_LIST *tables, bool derived);
 /* simple open_and_lock_tables without derived handling for single table */
 TABLE *open_n_lock_single_table(THD *thd, TABLE_LIST *table_l,
-                                thr_lock_type lock_type, uint flags);
+                                thr_lock_type lock_type, uint flags,
+                                Prelocking_strategy *prelocking_strategy);
 bool open_normal_and_derived_tables(THD *thd, TABLE_LIST *tables, uint flags);
 bool lock_tables(THD *thd, TABLE_LIST *tables, uint counter, uint flags);
 int decide_logging_format(THD *thd, TABLE_LIST *tables);
@@ -455,6 +456,16 @@ open_tables(THD *thd, TABLE_LIST **table
 }
 
 
+inline TABLE *open_n_lock_single_table(THD *thd, TABLE_LIST *table_l,
+                                       thr_lock_type lock_type, uint flags)
+{
+  DML_prelocking_strategy prelocking_strategy;
+
+  return open_n_lock_single_table(thd, table_l, lock_type, flags,
+                                  &prelocking_strategy);
+}
+
+
 /* open_and_lock_tables with derived handling */
 inline bool open_and_lock_tables(THD *thd, TABLE_LIST *tables,
                                  bool derived, uint flags)

=== modified file 'sql/sql_insert.cc'
--- a/sql/sql_insert.cc	2010-09-01 13:12:42 +0000
+++ b/sql/sql_insert.cc	2010-09-15 14:15:31 +0000
@@ -2496,6 +2496,65 @@ void kill_delayed_threads(void)
 
 
 /**
+  A strategy for the prelocking algorithm which prevents the
+  delayed insert thread from opening tables with engines which
+  do not support delayed inserts.
+
+  Particularly it allows to abort open_tables() as soon as we
+  discover that we have opened a MERGE table, without acquiring
+  metadata locks on underlying tables.
+*/
+
+class Delayed_prelocking_strategy : public Prelocking_strategy
+{
+public:
+  virtual bool handle_routine(THD *thd, Query_tables_list *prelocking_ctx,
+                              Sroutine_hash_entry *rt, sp_head *sp,
+                              bool *need_prelocking);
+  virtual bool handle_table(THD *thd, Query_tables_list *prelocking_ctx,
+                            TABLE_LIST *table_list, bool *need_prelocking);
+  virtual bool handle_view(THD *thd, Query_tables_list *prelocking_ctx,
+                           TABLE_LIST *table_list, bool *need_prelocking);
+};
+
+
+bool Delayed_prelocking_strategy::
+handle_table(THD *thd, Query_tables_list *prelocking_ctx,
+             TABLE_LIST *table_list, bool *need_prelocking)
+{
+  DBUG_ASSERT(table_list->lock_type == TL_WRITE_DELAYED);
+
+  if (!(table_list->table->file->ha_table_flags() & HA_CAN_INSERT_DELAYED))
+  {
+    my_error(ER_DELAYED_NOT_SUPPORTED, MYF(0), table_list->table_name);
+    return TRUE;
+  }
+  return FALSE;
+}
+
+
+bool Delayed_prelocking_strategy::
+handle_routine(THD *thd, Query_tables_list *prelocking_ctx,
+               Sroutine_hash_entry *rt, sp_head *sp,
+               bool *need_prelocking)
+{
+  /* LEX used by the delayed insert thread has no routines. */
+  DBUG_ASSERT(0);
+  return FALSE;
+}
+
+
+bool Delayed_prelocking_strategy::
+handle_view(THD *thd, Query_tables_list *prelocking_ctx,
+            TABLE_LIST *table_list, bool *need_prelocking)
+{
+  /* We don't open views in the delayed insert thread. */
+  DBUG_ASSERT(0);
+  return FALSE;
+}
+
+
+/**
    Open and lock table for use by delayed thread and check that
    this table is suitable for delayed inserts.
 
@@ -2505,21 +2564,21 @@ void kill_delayed_threads(void)
 
 bool Delayed_insert::open_and_lock_table()
 {
+  Delayed_prelocking_strategy prelocking_strategy;
+
+  /*
+    Use special prelocking strategy to get ER_DELAYED_NOT_SUPPORTED
+    error for tables with engines which don't support delayed inserts.
+  */
   if (!(table= open_n_lock_single_table(&thd, &table_list,
                                         TL_WRITE_DELAYED,
-                                        MYSQL_OPEN_IGNORE_GLOBAL_READ_LOCK)))
+                                        MYSQL_OPEN_IGNORE_GLOBAL_READ_LOCK,
+                                        &prelocking_strategy)))
   {
     thd.fatal_error();				// Abort waiting inserts
     return TRUE;
   }
-  if (!(table->file->ha_table_flags() & HA_CAN_INSERT_DELAYED))
-  {
-    /* To rollback InnoDB statement transaction. */
-    trans_rollback_stmt(&thd);
-    my_error(ER_DELAYED_NOT_SUPPORTED, MYF(ME_FATALERROR),
-             table_list.table_name);
-    return TRUE;
-  }
+
   if (table->triggers)
   {
     /*


Attachment: [text/bzr-bundle] bzr/dlenev@mysql.com-20100915141531-seaci6xh1aam8kzc.bundle
Thread
bzr commit into mysql-5.5-runtime branch (dlenev:3137) Bug#56251Dmitry Lenev15 Sep