List:Commits« Previous MessageNext Message »
From:Dmitry Lenev Date:September 30 2010 2:07pm
Subject:bzr push into mysql-5.5-runtime branch (Dmitry.Lenev:3149 to 3150) Bug#56405
View as plain text  
 3150 Dmitry Lenev	2010-09-30
      Reverted a temporary workaround for bug #56405 "Deadlock
      in the MDL deadlock detector".
      
      It is no longer needed as a better fix for this bug has
      been pushed.

    modified:
      mysql-test/suite/perfschema/r/dml_setup_instruments.result
      sql/mdl.cc
      sql/mdl.h
      sql/sql_base.cc
      sql/sql_base.h
      sql/table.cc
 3149 Jon Olav Hauglid	2010-09-30 [merge]
      Merge from mysql-5.5-bugteam to mysql-5.5-runtime

    modified:
      BUILD/build_mccge.sh
      BUILD/check-cpu
      CMakeLists.txt
      client/CMakeLists.txt
      client/mysqltest.cc
      client/sql_string.cc
      cmake/cmake_parse_arguments.cmake
      cmake/install_macros.cmake
      cmake/libutils.cmake
      cmake/mysql_add_executable.cmake
      cmake/plugin.cmake
      config/ac-macros/maintainer.m4
      configure.in
      include/CMakeLists.txt
      include/Makefile.am
      include/atomic/x86-gcc.h
      include/mysql.h
      include/mysql.h.pp
      libmysql/libmysql.c
      mysql-test/collections/default.experimental
      mysql-test/include/index_merge_ror_cpk.inc
      mysql-test/lib/mtr_cases.pm
      mysql-test/mysql-test-run.pl
      mysql-test/r/func_if.result
      mysql-test/r/func_misc.result
      mysql-test/r/gis.result
      mysql-test/r/index_merge_innodb.result
      mysql-test/r/index_merge_myisam.result
      mysql-test/r/join.result
      mysql-test/r/multi_update.result
      mysql-test/r/mysqltest.result
      mysql-test/r/order_by.result
      mysql-test/r/partition.result
      mysql-test/r/partition_column.result
      mysql-test/r/partition_column_prune.result
      mysql-test/r/partition_hash.result
      mysql-test/r/partition_innodb.result
      mysql-test/r/partition_pruning.result
      mysql-test/r/partition_range.result
      mysql-test/r/row.result
      mysql-test/r/select.result
      mysql-test/r/subselect.result
      mysql-test/r/subselect4.result
      mysql-test/r/timezone2.result
      mysql-test/r/trigger_notembedded.result
      mysql-test/r/type_timestamp.result
      mysql-test/r/user_var.result
      mysql-test/suite/binlog/r/binlog_row_mix_innodb_myisam.result
      mysql-test/suite/binlog/r/binlog_stm_mix_innodb_myisam.result
      mysql-test/suite/innodb/r/innodb-autoinc.result
      mysql-test/suite/innodb/r/innodb_mysql.result
      mysql-test/suite/innodb/t/innodb-autoinc.test
      mysql-test/suite/innodb/t/innodb_mysql.test
      mysql-test/suite/parts/r/partition_alter3_innodb.result
      mysql-test/suite/parts/r/partition_alter3_myisam.result
      mysql-test/suite/perfschema/r/dml_setup_instruments.result
      mysql-test/suite/rpl/r/rpl_begin_commit_rollback.result
      mysql-test/suite/rpl/r/rpl_innodb_mixed_dml.result
      mysql-test/suite/rpl/r/rpl_mixed_mixing_engines.result
      mysql-test/suite/rpl/r/rpl_non_direct_mixed_mixing_engines.result
      mysql-test/suite/rpl/r/rpl_non_direct_row_mixing_engines.result
      mysql-test/suite/rpl/r/rpl_non_direct_stm_mixing_engines.result
      mysql-test/suite/rpl/r/rpl_row_mixing_engines.result
      mysql-test/suite/rpl/r/rpl_stm_mixing_engines.result
      mysql-test/suite/rpl/t/disabled.def
      mysql-test/suite/rpl/t/rpl_drop.test
      mysql-test/t/func_gconcat.test
      mysql-test/t/func_if.test
      mysql-test/t/func_misc.test
      mysql-test/t/gis.test
      mysql-test/t/join.test
      mysql-test/t/log_tables_debug.test
      mysql-test/t/multi_update.test
      mysql-test/t/mysqltest.test
      mysql-test/t/order_by.test
      mysql-test/t/partition.test
      mysql-test/t/partition_pruning.test
      mysql-test/t/row.test
      mysql-test/t/select.test
      mysql-test/t/subselect4.test
      mysql-test/t/timezone2.test
      mysql-test/t/trigger_notembedded.test
      mysql-test/t/type_timestamp.test
      mysql-test/t/user_var.test
      mysql-test/valgrind.supp
      mysys/my_getopt.c
      scripts/CMakeLists.txt
      scripts/make_win_bin_dist
      sql/field.cc
      sql/ha_partition.cc
      sql/ha_partition.h
      sql/item.cc
      sql/item_cmpfunc.cc
      sql/item_func.cc
      sql/item_func.h
      sql/item_geofunc.h
      sql/item_subselect.cc
      sql/log.cc
      sql/log.h
      sql/mdl.cc
      sql/mdl.h
      sql/net_serv.cc
      sql/opt_range.cc
      sql/spatial.cc
      sql/sql_base.cc
      sql/sql_base.h
      sql/sql_cache.cc
      sql/sql_select.cc
      sql/sql_select.h
      sql/sql_string.cc
      sql/sql_string.h
      sql/sql_table.cc
      sql/sql_trigger.cc
      sql/table.cc
      sql/tztime.cc
      storage/innobase/btr/btr0cur.c
      storage/innobase/handler/ha_innodb.cc
      storage/innobase/include/btr0cur.h
      storage/innobase/include/dict0dict.h
      storage/innobase/include/dict0dict.ic
      storage/innobase/row/row0mysql.c
      storage/innobase/row/row0upd.c
      storage/myisam/mi_range.c
      storage/myisam/mi_search.c
      storage/myisam/rt_index.c
      storage/myisam/rt_split.c
      tests/mysql_client_test.c
=== modified file 'mysql-test/suite/perfschema/r/dml_setup_instruments.result'
--- a/mysql-test/suite/perfschema/r/dml_setup_instruments.result	2010-09-24 16:26:24 +0000
+++ b/mysql-test/suite/perfschema/r/dml_setup_instruments.result	2010-09-30 13:29:12 +0000
@@ -13,7 +13,7 @@ wait/synch/mutex/sql/LOCK_active_mi	YES	
 wait/synch/mutex/sql/LOCK_audit_mask	YES	YES
 wait/synch/mutex/sql/LOCK_connection_count	YES	YES
 wait/synch/mutex/sql/LOCK_crypt	YES	YES
-wait/synch/mutex/sql/LOCK_dd_owns_lock_open	YES	YES
+wait/synch/mutex/sql/LOCK_delayed_create	YES	YES
 select * from performance_schema.SETUP_INSTRUMENTS
 where name like 'Wait/Synch/Rwlock/sql/%'
   and name not in ('wait/synch/rwlock/sql/CRYPTO_dynlock_value::lock')

=== modified file 'sql/mdl.cc'
--- a/sql/mdl.cc	2010-09-06 17:29:02 +0000
+++ b/sql/mdl.cc	2010-09-30 13:29:12 +0000
@@ -124,6 +124,7 @@ public:
   Deadlock_detection_visitor(MDL_context *start_node_arg)
     : m_start_node(start_node_arg),
       m_victim(NULL),
+      m_current_search_depth(0),
       m_found_deadlock(FALSE)
   {}
   virtual bool enter_node(MDL_context *node);
@@ -132,8 +133,6 @@ public:
   virtual bool inspect_edge(MDL_context *dest);
 
   MDL_context *get_victim() const { return m_victim; }
-
-  void abort_traversal(MDL_context *node);
 private:
   /**
     Change the deadlock victim to a new one if it has lower deadlock
@@ -148,6 +147,13 @@ private:
   MDL_context *m_start_node;
   /** If a deadlock is found, the context that identifies the victim. */
   MDL_context *m_victim;
+  /** Set to the 0 at start. Increased whenever
+    we descend into another MDL context (aka traverse to the next
+    wait-for graph node). When MAX_SEARCH_DEPTH is reached, we
+    assume that a deadlock is found, even if we have not found a
+    loop.
+  */
+  uint m_current_search_depth;
   /** TRUE if we found a deadlock. */
   bool m_found_deadlock;
   /**
@@ -181,7 +187,7 @@ private:
 
 bool Deadlock_detection_visitor::enter_node(MDL_context *node)
 {
-  m_found_deadlock= m_current_search_depth >= MAX_SEARCH_DEPTH;
+  m_found_deadlock= ++m_current_search_depth >= MAX_SEARCH_DEPTH;
   if (m_found_deadlock)
   {
     DBUG_ASSERT(! m_victim);
@@ -201,6 +207,7 @@ bool Deadlock_detection_visitor::enter_n
 
 void Deadlock_detection_visitor::leave_node(MDL_context *node)
 {
+  --m_current_search_depth;
   if (m_found_deadlock)
     opt_change_victim_to(node);
 }
@@ -245,21 +252,6 @@ Deadlock_detection_visitor::opt_change_v
 
 
 /**
-  Abort traversal of a wait-for graph and report a deadlock.
-
-  @param node Node which we were about to visit when abort
-              was initiated.
-*/
-
-void Deadlock_detection_visitor::abort_traversal(MDL_context *node)
-{
-  DBUG_ASSERT(! m_victim);
-  m_found_deadlock= TRUE;
-  opt_change_victim_to(node);
-}
-
-
-/**
   Get a bit corresponding to enum_mdl_type value in a granted/waiting bitmaps
   and compatibility matrices.
 */
@@ -2064,13 +2056,8 @@ bool MDL_lock::visit_subgraph(MDL_ticket
     are visiting it but this is OK: in the worst case we might do some
     extra work and one more context might be chosen as a victim.
   */
-  ++gvisitor->m_current_search_depth;
-
   if (gvisitor->enter_node(src_ctx))
-  {
-    --gvisitor->m_current_search_depth;
     goto end;
-  }
 
   /*
     We do a breadth-first search first -- that is, inspect all
@@ -2127,7 +2114,6 @@ bool MDL_lock::visit_subgraph(MDL_ticket
 
 end_leave_node:
   gvisitor->leave_node(src_ctx);
-  --gvisitor->m_current_search_depth;
 
 end:
   mysql_prlock_unlock(&m_rwlock);

=== modified file 'sql/mdl.h'
--- a/sql/mdl.h	2010-09-06 17:29:02 +0000
+++ b/sql/mdl.h	2010-09-30 13:29:12 +0000
@@ -385,10 +385,7 @@ public:
 
   virtual bool inspect_edge(MDL_context *dest) = 0;
   virtual ~MDL_wait_for_graph_visitor();
-  MDL_wait_for_graph_visitor() :m_lock_open_count(0),
-                                m_current_search_depth(0)
-  { }
-  virtual void abort_traversal(MDL_context *node) = 0;
+  MDL_wait_for_graph_visitor() :m_lock_open_count(0) {}
 public:
   /**
    XXX, hack: During deadlock search, we may need to
@@ -399,17 +396,6 @@ public:
    LOCK_open since it has significant performance impacts.
   */
   uint m_lock_open_count;
-  /**
-    Set to the 0 at start. Increased whenever
-    we descend into another MDL context (aka traverse to the next
-    wait-for graph node). When MAX_SEARCH_DEPTH is reached, we
-    assume that a deadlock is found, even if we have not found a
-    loop.
-
-    XXX: This member belongs to this class only temporarily until
-         bug #56405 is fixed.
-  */
-  uint m_current_search_depth;
 };
 
 /**

=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc	2010-09-30 10:43:43 +0000
+++ b/sql/sql_base.cc	2010-09-30 13:29:12 +0000
@@ -100,14 +100,11 @@ bool No_such_table_error_handler::safely
   TABLE_SHAREs, refresh_version and the table id counter.
 */
 mysql_mutex_t LOCK_open;
-mysql_mutex_t LOCK_dd_owns_lock_open;
-uint dd_owns_lock_open= 0;
 
 #ifdef HAVE_PSI_INTERFACE
-static PSI_mutex_key key_LOCK_open, key_LOCK_dd_owns_lock_open;
+static PSI_mutex_key key_LOCK_open;
 static PSI_mutex_info all_tdc_mutexes[]= {
-  { &key_LOCK_open, "LOCK_open", PSI_FLAG_GLOBAL },
-  { &key_LOCK_dd_owns_lock_open, "LOCK_dd_owns_lock_open", PSI_FLAG_GLOBAL }
+  { &key_LOCK_open, "LOCK_open", PSI_FLAG_GLOBAL }
 };
 
 /**
@@ -302,8 +299,6 @@ bool table_def_init(void)
   init_tdc_psi_keys();
 #endif
   mysql_mutex_init(key_LOCK_open, &LOCK_open, MY_MUTEX_INIT_FAST);
-  mysql_mutex_init(key_LOCK_dd_owns_lock_open, &LOCK_dd_owns_lock_open,
-                   MY_MUTEX_INIT_FAST);
   oldest_unused_share= &end_of_unused_share;
   end_of_unused_share.prev= &oldest_unused_share;
 
@@ -347,7 +342,6 @@ void table_def_free(void)
     table_def_inited= 0;
     /* Free table definitions. */
     my_hash_free(&table_def_cache);
-    mysql_mutex_destroy(&LOCK_dd_owns_lock_open);
     mysql_mutex_destroy(&LOCK_open);
   }
   DBUG_VOID_RETURN;

=== modified file 'sql/sql_base.h'
--- a/sql/sql_base.h	2010-09-30 10:43:43 +0000
+++ b/sql/sql_base.h	2010-09-30 13:29:12 +0000
@@ -70,8 +70,6 @@ enum enum_tdc_remove_table_type {TDC_RT_
 
 bool check_dup(const char *db, const char *name, TABLE_LIST *tables);
 extern mysql_mutex_t LOCK_open;
-extern mysql_mutex_t LOCK_dd_owns_lock_open;
-extern uint dd_owns_lock_open;
 bool table_cache_init(void);
 void table_cache_free(void);
 bool table_def_init(void);

=== modified file 'sql/table.cc'
--- a/sql/table.cc	2010-09-30 10:43:43 +0000
+++ b/sql/table.cc	2010-09-30 13:29:12 +0000
@@ -3085,30 +3085,7 @@ bool TABLE_SHARE::visit_subgraph(Wait_fo
     holding a write-lock on MDL_lock::m_rwlock.
   */
   if (gvisitor->m_lock_open_count++ == 0)
-  {
-    /*
-      To circumvent bug #56405 "Deadlock in the MDL deadlock detector"
-      we don't try to lock LOCK_open mutex if some thread doing
-      deadlock detection already owns it and current search depth is
-      greater than 0. Instead we report a deadlock.
-
-      TODO/FIXME: The proper fix for this bug is to use rwlocks for
-                  protection of table shares/instead of LOCK_open.
-                  Unfortunately it requires more effort/has significant
-                  performance effect.
-    */
-    mysql_mutex_lock(&LOCK_dd_owns_lock_open);
-    if (gvisitor->m_current_search_depth > 0 && dd_owns_lock_open > 0)
-    {
-      mysql_mutex_unlock(&LOCK_dd_owns_lock_open);
-      --gvisitor->m_lock_open_count;
-      gvisitor->abort_traversal(src_ctx);
-      return TRUE;
-    }
-    ++dd_owns_lock_open;
-    mysql_mutex_unlock(&LOCK_dd_owns_lock_open);
     mysql_mutex_lock(&LOCK_open);
-  }
 
   I_P_List_iterator <TABLE, TABLE_share> tables_it(used_tables);
 
@@ -3123,12 +3100,8 @@ bool TABLE_SHARE::visit_subgraph(Wait_fo
     goto end;
   }
 
-  ++gvisitor->m_current_search_depth;
   if (gvisitor->enter_node(src_ctx))
-  {
-    --gvisitor->m_current_search_depth;
     goto end;
-  }
 
   while ((table= tables_it++))
   {
@@ -3151,16 +3124,10 @@ bool TABLE_SHARE::visit_subgraph(Wait_fo
 
 end_leave_node:
   gvisitor->leave_node(src_ctx);
-  --gvisitor->m_current_search_depth;
 
 end:
   if (gvisitor->m_lock_open_count-- == 1)
-  {
     mysql_mutex_unlock(&LOCK_open);
-    mysql_mutex_lock(&LOCK_dd_owns_lock_open);
-    --dd_owns_lock_open;
-    mysql_mutex_unlock(&LOCK_dd_owns_lock_open);
-  }
 
   return result;
 }


Attachment: [text/bzr-bundle] bzr/dmitry.lenev@oracle.com-20100930132912-bmit2vzuopz3r2ub.bundle
Thread
bzr push into mysql-5.5-runtime branch (Dmitry.Lenev:3149 to 3150) Bug#56405Dmitry Lenev30 Sep