Reverted a temporary workaround for bug #56405 "Deadlock

Dmitry Lenev · Dmitry Lenev · commit 0abf8b3f2125 · 2010-09-30T17:29:12.000+04:00
in the MDL deadlock detector".

It is no longer needed as a better fix for this bug has
been pushed.
--BZR--
revision-id: dmitry.lenev@oracle.com-20100930132912-bmit2vzuopz3r2ub
property-branch-nick: mysql-5.5-rt-mrg
testament3-sha1: fdc2eff54b5f4a4388e5a7b1968949d7c5e329ff
diff --git a/mysql-test/suite/perfschema/r/dml_setup_instruments.result b/mysql-test/suite/perfschema/r/dml_setup_instruments.result
@@ -13,7 +13,7 @@ wait/synch/mutex/sql/LOCK_active_mi	YES	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')
diff --git a/sql/mdl.cc b/sql/mdl.cc
@@ -124,6 +124,7 @@ class Deadlock_detection_visitor: public MDL_wait_for_graph_visitor
   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 @@ class Deadlock_detection_visitor: public MDL_wait_for_graph_visitor
   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 @@ class Deadlock_detection_visitor: public MDL_wait_for_graph_visitor
   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 @@ class Deadlock_detection_visitor: public MDL_wait_for_graph_visitor
 
 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_node(MDL_context *node)
 
 void Deadlock_detection_visitor::leave_node(MDL_context *node)
 {
+  --m_current_search_depth;
   if (m_found_deadlock)
     opt_change_victim_to(node);
 }
@@ -244,21 +251,6 @@ Deadlock_detection_visitor::opt_change_victim_to(MDL_context *new_victim)
 }
 
 
-/**
-  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 *waiting_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 *waiting_ticket,
 
 end_leave_node:
   gvisitor->leave_node(src_ctx);
-  --gvisitor->m_current_search_depth;
 
 end:
   mysql_prlock_unlock(&m_rwlock);
diff --git a/sql/mdl.h b/sql/mdl.h
@@ -385,10 +385,7 @@ class MDL_wait_for_graph_visitor
 
   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 @@ class MDL_wait_for_graph_visitor
    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;
 };
 
 /**
diff --git a/sql/sql_base.cc b/sql/sql_base.cc
@@ -100,14 +100,11 @@ bool No_such_table_error_handler::safely_trapped_errors()
   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;
diff --git a/sql/sql_base.h b/sql/sql_base.h
@@ -70,8 +70,6 @@ enum enum_tdc_remove_table_type {TDC_RT_REMOVE_ALL, TDC_RT_REMOVE_NOT_OWN,
 
 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);
diff --git a/sql/table.cc b/sql/table.cc
@@ -3085,30 +3085,7 @@ bool TABLE_SHARE::visit_subgraph(Wait_for_flush *wait_for_flush,
     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_for_flush *wait_for_flush,
     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_for_flush *wait_for_flush,
 
 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;
 }
