bzr commit into mysql-5.6-next-mr branch (dlenev:3072) Bug#46272

[Dmitry Lenev]

#At file:///home/dlenev/src/bzr/mysql-next-4284-nl-review/ based on revid:dlenev@stripped

 3072 Dmitry Lenev	2010-01-28
      Tentative patch implementing new type-of-operation-aware metadata locks.
      Fixes bug #46272 "MySQL 5.4.4, new MDL: unnecessary deadlock" and bug
      #37346 "innodb does not detect deadlock between update and alter table".
      
      After-review fixes in progress.

    modified:
      sql/mdl.cc
      sql/mdl.h
      sql/sql_handler.cc
=== modified file 'sql/mdl.cc'
--- a/sql/mdl.cc	2010-01-28 13:19:52 +0000
+++ b/sql/mdl.cc	2010-01-28 15:17:02 +0000
@@ -526,6 +526,7 @@ void MDL_map::remove(MDL_lock *lock)
 MDL_context::MDL_context()
   :m_trans_sentinel(NULL),
   m_thd(NULL),
+  m_needs_thr_lock_abort(FALSE),
   m_waiting_for(NULL),
   m_deadlock_weight(0),
   m_signal(NO_WAKE_UP)
@@ -1496,7 +1497,7 @@ void notify_shared_lock(THD *thd, MDL_ti
       by calling code outside of MDL.
     */
     mysql_notify_thread_having_shared_lock(thd, conflicting_thd,
-                               conflicting_ticket->get_needs_thr_lock_abort());
+                               conflicting_ctx->get_needs_thr_lock_abort());
   }
 }
 

=== modified file 'sql/mdl.h'
--- a/sql/mdl.h	2010-01-28 13:19:52 +0000
+++ b/sql/mdl.h	2010-01-28 15:17:02 +0000
@@ -397,19 +397,6 @@ public:
   }
   enum_mdl_type get_type() const { return m_type; }
   void downgrade_exclusive_lock();
-  void set_needs_thr_lock_abort()
-  {
-    /*
-      QQ: In theory this should be done under protection of MDL_lock::m_mutex.
-          But it is not convenient in practice and probably does not matter.
-          Should we do anything about it?
-    */
-    m_needs_thr_lock_abort= TRUE;
-  }
-  bool get_needs_thr_lock_abort() const
-  {
-    return m_needs_thr_lock_abort;
-  }
 
   bool has_stronger_or_equal_type(enum_mdl_type type) const;
 
@@ -422,8 +409,7 @@ private:
   MDL_ticket(MDL_context *ctx_arg, enum_mdl_type type_arg)
    : m_type(type_arg),
      m_ctx(ctx_arg),
-     m_lock(NULL),
-     m_needs_thr_lock_abort(FALSE)
+     m_lock(NULL)
   {}
 
   static MDL_ticket *create(MDL_context *ctx_arg, enum_mdl_type type_arg);
@@ -438,16 +424,7 @@ private:
 
   /** Pointer to the lock object for this lock ticket. Context private. */
   MDL_lock *m_lock;
-  /**
-    TRUE -  if for this ticket we will break protocol and try to
-            acquire table-level locks while having S lock on some
-            table.
-            To avoid deadlocks which might occur during concurrent
-            upgrade of SNRW lock on such object to X lock we have to
-            abort waits for table-level locks for such connections.
-    FALSE - Otherwise.
-  */
-  bool m_needs_thr_lock_abort;
+
 private:
   MDL_ticket(const MDL_ticket &);               /* not implemented */
   MDL_ticket &operator=(const MDL_ticket &);    /* not implemented */
@@ -550,6 +527,23 @@ public:
 
   void init(THD *thd_arg) { m_thd= thd_arg; }
 
+  void set_needs_thr_lock_abort(bool needs_thr_lock_abort)
+  {
+    /*
+      @note In theory, this member should be modified under protection
+            of some lock since it can be accessed from different threads.
+            In practice, this is not necessary as code which reads this
+            value and so might miss the fact that value was changed will
+            always re-try reading it after small timeout and therefore
+            will see new value eventually.
+    */
+    m_needs_thr_lock_abort= TRUE;
+  }
+  bool get_needs_thr_lock_abort() const
+  {
+    return m_needs_thr_lock_abort;
+  }
+
   bool find_deadlock();
   bool find_deadlock(Deadlock_detection_context *deadlock_ctx);
 
@@ -616,6 +610,16 @@ private:
   */
   MDL_ticket *m_trans_sentinel;
   THD *m_thd;
+  /**
+    TRUE -  if for this context we will break protocol and try to
+            acquire table-level locks while having only S lock on
+            some table.
+            To avoid deadlocks which might occur during concurrent
+            upgrade of SNRW lock on such object to X lock we have to
+            abort waits for table-level locks for such connections.
+    FALSE - Otherwise.
+  */
+  bool m_needs_thr_lock_abort;
 
   /**
     Read-write lock protecting m_waiting_for member.

=== modified file 'sql/sql_handler.cc'
--- a/sql/sql_handler.cc	2010-01-26 18:22:10 +0000
+++ b/sql/sql_handler.cc	2010-01-28 15:17:02 +0000
@@ -293,16 +293,12 @@ bool mysql_ha_open(THD *thd, TABLE_LIST 
     error= TRUE;
   }
   if (!error &&
-      hash_tables->mdl_request.ticket)
+      hash_tables->mdl_request.ticket &&
+      thd->mdl_context.has_lock(mdl_savepoint, hash_tables->mdl_request.ticket))
   {
-    if (thd->mdl_context.has_lock(mdl_savepoint,
-                                  hash_tables->mdl_request.ticket))
-    {
-      /* The ticket returned is within a savepoint. Make a copy.  */
-      error= thd->mdl_context.clone_ticket(&hash_tables->mdl_request);
-      hash_tables->table->mdl_ticket= hash_tables->mdl_request.ticket;
-    }
-    hash_tables->mdl_request.ticket->set_needs_thr_lock_abort();
+    /* The ticket returned is within a savepoint. Make a copy.  */
+    error= thd->mdl_context.clone_ticket(&hash_tables->mdl_request);
+    hash_tables->table->mdl_ticket= hash_tables->mdl_request.ticket;
   }
   if (error)
   {
@@ -318,8 +314,11 @@ bool mysql_ha_open(THD *thd, TABLE_LIST 
   }
   thd->open_tables= backup_open_tables;
   if (hash_tables->mdl_request.ticket)
+  {
     thd->mdl_context.
       move_ticket_after_trans_sentinel(hash_tables->mdl_request.ticket);
+    thd->mdl_context.set_needs_thr_lock_abort(TRUE);
+  }
 
   /* Assert that the above check prevent opening of views and merge tables. */
   DBUG_ASSERT(hash_tables->table->next == NULL);
@@ -379,6 +378,13 @@ bool mysql_ha_close(THD *thd, TABLE_LIST
     DBUG_RETURN(TRUE);
   }
 
+  /*
+    Mark MDL_context as no longer breaking protocol if we have
+    closed last HANDLER.
+  */
+  if (! thd->handler_tables_hash.records)
+    thd->mdl_context.set_needs_thr_lock_abort(FALSE);
+
   my_ok(thd);
   DBUG_PRINT("exit", ("OK"));
   DBUG_RETURN(FALSE);
@@ -742,6 +748,13 @@ void mysql_ha_rm_tables(THD *thd, TABLE_
     hash_tables= next;
   }
 
+  /*
+    Mark MDL_context as no longer breaking protocol if we have
+    closed last HANDLER.
+  */
+  if (! thd->handler_tables_hash.records)
+    thd->mdl_context.set_needs_thr_lock_abort(FALSE);
+
   DBUG_VOID_RETURN;
 }
 


Attachment: [text/bzr-bundle] bzr/[e-mail]