bzr commit into mysql-trunk-runtime branch (kostja:3038)

[Konstantin Osipov]

#At file:///opt/local/work/trunk-runtime-stage/ based on revid:kostja@stripped

 3038 Konstantin Osipov	2010-06-04
      Add class MDL_wait.

    modified:
      sql/mdl.cc
      sql/mdl.h
=== modified file 'sql/mdl.cc'
--- a/sql/mdl.cc	2010-06-04 09:01:31 +0000
+++ b/sql/mdl.cc	2010-06-04 14:29:40 +0000
@@ -715,13 +715,9 @@ 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)
+  m_waiting_for(NULL)
 {
   mysql_prlock_init(key_MDL_context_waiting_for_rwlock, &m_waiting_for_lock);
-  mysql_mutex_init(key_MDL_context_signal_mutex, &m_signal_lock, NULL);
-  mysql_cond_init(key_MDL_context_signal_mutex, &m_signal_cond, NULL);
 }
 
 
@@ -742,8 +738,6 @@ void MDL_context::destroy()
   DBUG_ASSERT(m_tickets.is_empty());
 
   mysql_prlock_destroy(&m_waiting_for_lock);
-  mysql_mutex_destroy(&m_signal_lock);
-  mysql_cond_destroy(&m_signal_cond);
 }
 
 
@@ -953,6 +947,21 @@ static inline void mdl_exit_cond(THD *th
 }
 
 
+MDL_wait::MDL_wait()
+  :m_signal(NO_WAKE_UP)
+{
+  mysql_mutex_init(key_MDL_context_signal_mutex, &m_signal_lock, NULL);
+  mysql_cond_init(key_MDL_context_signal_mutex, &m_signal_cond, NULL);
+}
+
+
+MDL_wait::~MDL_wait()
+{
+  mysql_mutex_destroy(&m_signal_lock);
+  mysql_cond_destroy(&m_signal_cond);
+}
+
+
 /**
   Wait for the signal to be posted for this context.
 
@@ -965,8 +974,9 @@ static inline void mdl_exit_cond(THD *th
   @returns Signal posted.
 */
 
-MDL_context::mdl_signal_type
-MDL_context::timed_wait(struct timespec *abs_timeout, bool signal_timeout)
+MDL_wait::mdl_signal_type
+MDL_wait::timed_wait(THD *thd, struct timespec *abs_timeout,
+                     bool signal_timeout)
 {
   const char *old_msg;
   mdl_signal_type result;
@@ -975,7 +985,7 @@ MDL_context::timed_wait(struct timespec 
 
   mysql_mutex_lock(&m_signal_lock);
 
-  old_msg= MDL_ENTER_COND(m_thd, mysys_var, &m_signal_cond, &m_signal_lock);
+  old_msg= MDL_ENTER_COND(thd, mysys_var, &m_signal_cond, &m_signal_lock);
 
   while (!m_signal && !mysys_var->abort &&
          wait_result != ETIMEDOUT && wait_result != ETIME)
@@ -1005,7 +1015,7 @@ MDL_context::timed_wait(struct timespec 
   else
     result= m_signal;
 
-  MDL_EXIT_COND(m_thd, mysys_var, &m_signal_lock, old_msg);
+  MDL_EXIT_COND(thd, mysys_var, &m_signal_lock, old_msg);
 
   return result;
 }
@@ -1114,7 +1124,7 @@ void MDL_lock::reschedule_waiters()
   {
     if (can_grant_lock(ticket->get_type(), ticket->get_ctx()))
     {
-      if (ticket->get_ctx()->post_signal(MDL_context::GRANTED_WAKE_UP))
+      if (ticket->get_ctx()->m_wait.post_signal(MDL_wait::GRANTED_WAKE_UP))
       {
         /*
           We have failed to post signal to waiter. This can be due to fact
@@ -1702,7 +1712,7 @@ MDL_context::acquire_lock(MDL_request *m
   MDL_lock *lock;
   MDL_ticket *ticket;
   struct timespec abs_timeout;
-  mdl_signal_type wait_result;
+  MDL_wait::mdl_signal_type wait_result;
   set_timespec(abs_timeout, lock_wait_timeout);
 
   if (try_acquire_lock_impl(mdl_request, &ticket))
@@ -1728,7 +1738,7 @@ MDL_context::acquire_lock(MDL_request *m
 
   lock->m_waiting.add_ticket(ticket);
 
-  wait_reset();
+  m_wait.wait_reset();
 
   if (ticket->is_upgradable_or_exclusive())
     lock->notify_shared_locks(this);
@@ -1753,10 +1763,11 @@ MDL_context::acquire_lock(MDL_request *m
       timeout_is_near= cmp_timespec(abs_shortwait, abs_timeout) > 0;
 
       wait_result=
-        timed_wait((timeout_is_near ? &abs_timeout : &abs_shortwait),
-                   timeout_is_near);
+        m_wait.timed_wait(m_thd,
+                          (timeout_is_near ? &abs_timeout : &abs_shortwait),
+                          timeout_is_near);
 
-      if (wait_result == TIMEOUT_WAKE_UP && !timeout_is_near)
+      if (wait_result == MDL_wait::TIMEOUT_WAKE_UP && !timeout_is_near)
       {
         mysql_prlock_wrlock(&lock->m_rwlock);
         lock->notify_shared_locks(this);
@@ -1767,23 +1778,23 @@ MDL_context::acquire_lock(MDL_request *m
     }
   }
   else
-    wait_result= timed_wait(&abs_timeout, TRUE);
+    wait_result= m_wait.timed_wait(m_thd, &abs_timeout, TRUE);
 
-  stop_waiting();
+  done_waiting_for();
 
-  if (wait_result != GRANTED_WAKE_UP)
+  if (wait_result != MDL_wait::GRANTED_WAKE_UP)
   {
     lock->remove_ticket(&MDL_lock::m_waiting, ticket);
     MDL_ticket::destroy(ticket);
     switch (wait_result)
     {
-    case VICTIM_WAKE_UP:
+    case MDL_wait::VICTIM_WAKE_UP:
       my_error(ER_LOCK_DEADLOCK, MYF(0));
       break;
-    case TIMEOUT_WAKE_UP:
+    case MDL_wait::TIMEOUT_WAKE_UP:
       my_error(ER_LOCK_WAIT_TIMEOUT, MYF(0));
       break;
-    case KILLED_WAKE_UP:
+    case MDL_wait::KILLED_WAKE_UP:
       break;
     default:
       DBUG_ASSERT(0);
@@ -1798,7 +1809,7 @@ MDL_context::acquire_lock(MDL_request *m
     concurrent thread (@sa MDL_lock:reschedule_waiters()).
     So all we need to do is to update MDL_context and MDL_request objects.
   */
-  DBUG_ASSERT(wait_result == GRANTED_WAKE_UP);
+  DBUG_ASSERT(wait_result == MDL_wait::GRANTED_WAKE_UP);
 
   m_tickets.push_front(ticket);
 
@@ -1979,7 +1990,7 @@ bool MDL_lock::find_deadlock(MDL_ticket 
   Ticket_iterator granted_it(m_granted);
   Ticket_iterator waiting_it(m_waiting);
 
-  if (src_ctx->peek_signal() != MDL_context::NO_WAKE_UP)
+  if (src_ctx->m_wait.peek_signal() != MDL_wait::NO_WAKE_UP)
   {
     /*
       State of MDL_lock object and MDL_context::m_waiting_for member are
@@ -2173,7 +2184,7 @@ void MDL_context::find_deadlock()
         that the victim has received some other signal and is
         about to stop its waiting/to break deadlock loop.
       */
-      (void) victim->post_signal(VICTIM_WAKE_UP);
+      (void) victim->m_wait.post_signal(MDL_wait::VICTIM_WAKE_UP);
       mysql_prlock_unlock(&victim->m_waiting_for_lock);
       /*
         After adding new arc to waiting graph we found that it participates
@@ -2195,7 +2206,7 @@ void MDL_context::find_deadlock()
         Failure to post signal to ourselves is OK as this means that our
         lock request was satisfied and deadlock went away.
       */
-      (void) victim->post_signal(VICTIM_WAKE_UP);
+      (void) victim->m_wait.post_signal(MDL_wait::VICTIM_WAKE_UP);
       DBUG_ASSERT(&victim->m_waiting_for_lock == &m_waiting_for_lock);
       mysql_prlock_unlock(&victim->m_waiting_for_lock);
       return;

=== modified file 'sql/mdl.h'
--- a/sql/mdl.h	2010-06-04 09:01:31 +0000
+++ b/sql/mdl.h	2010-06-04 14:29:40 +0000
@@ -447,6 +447,68 @@ private:
 };
 
 
+/**
+  A reliable way to wait on an MDL lock.
+*/
+
+class MDL_wait
+{
+public:
+  MDL_wait();
+  ~MDL_wait();
+
+  enum mdl_signal_type { NO_WAKE_UP = 0,
+                         GRANTED_WAKE_UP,
+                         VICTIM_WAKE_UP,
+                         TIMEOUT_WAKE_UP,
+                         KILLED_WAKE_UP };
+
+  bool post_signal(mdl_signal_type signal)
+  {
+    bool result= FALSE;
+    mysql_mutex_lock(&m_signal_lock);
+    if (m_signal == NO_WAKE_UP)
+    {
+      m_signal= signal;
+      mysql_cond_signal(&m_signal_cond);
+    }
+    else
+      result= TRUE;
+    mysql_mutex_unlock(&m_signal_lock);
+    return result;
+  }
+
+  mdl_signal_type peek_signal()
+  {
+    mdl_signal_type result;
+    mysql_mutex_lock(&m_signal_lock);
+    result= m_signal;
+    mysql_mutex_unlock(&m_signal_lock);
+    return result;
+  }
+  void wait_reset()
+  {
+    mysql_mutex_lock(&m_signal_lock);
+    m_signal= NO_WAKE_UP;
+    mysql_mutex_unlock(&m_signal_lock);
+  }
+
+  mdl_signal_type timed_wait(THD *thd, struct timespec *abs_timeout,
+                             bool signal_timeout);
+private:
+  /**
+    Condvar which is used for waiting until this context's pending
+    request can be satisfied or this thread has to perform actions
+    to resolve a potential deadlock (we subscribe to such
+    notification by adding a ticket corresponding to the request
+    to an appropriate queue of waiters).
+  */
+  mysql_mutex_t m_signal_lock;
+  mysql_cond_t m_signal_cond;
+  mdl_signal_type m_signal;
+};
+
+
 typedef I_P_List<MDL_request, I_P_List_adapter<MDL_request,
                  &MDL_request::next_in_list,
                  &MDL_request::prev_in_list>,
@@ -471,12 +533,6 @@ public:
 
   typedef Ticket_list::Iterator Ticket_iterator;
 
-  enum mdl_signal_type { NO_WAKE_UP = 0,
-                         GRANTED_WAKE_UP,
-                         VICTIM_WAKE_UP,
-                         TIMEOUT_WAKE_UP,
-                         KILLED_WAKE_UP };
-
   MDL_context();
   void destroy();
 
@@ -540,30 +596,6 @@ public:
                     already has received some signal or closed
                     signal slot.
   */
-  bool post_signal(mdl_signal_type signal)
-  {
-    bool result= FALSE;
-    mysql_mutex_lock(&m_signal_lock);
-    if (m_signal == NO_WAKE_UP)
-    {
-      m_signal= signal;
-      mysql_cond_signal(&m_signal_cond);
-    }
-    else
-      result= TRUE;
-    mysql_mutex_unlock(&m_signal_lock);
-    return result;
-  }
-
-  mdl_signal_type peek_signal()
-  {
-    mdl_signal_type result;
-    mysql_mutex_lock(&m_signal_lock);
-    result= m_signal;
-    mysql_mutex_unlock(&m_signal_lock);
-    return result;
-  }
-
   void init(THD *thd_arg) { m_thd= thd_arg; }
 
   void set_needs_thr_lock_abort(bool needs_thr_lock_abort)
@@ -584,6 +616,12 @@ public:
   }
 
   bool find_deadlock(Deadlock_detection_visitor *dvisitor);
+public:
+  /**
+    If our request for a lock is scheduled, or aborted by the deadlock
+    detector, the result is recorded in this class.
+  */
+  MDL_wait m_wait;
 private:
   /**
     All MDL tickets acquired by this connection.
@@ -666,19 +704,13 @@ private:
           otherwise. @sa Comment for MDL_lock::m_rwlock.
   */
   mysql_prlock_t m_waiting_for_lock;
-  MDL_ticket *m_waiting_for;
-  uint m_deadlock_weight;
   /**
-    Condvar which is used for waiting until this context's pending
-    request can be satisfied or this thread has to perform actions
-    to resolve a potential deadlock (we subscribe to such
-    notification by adding a ticket corresponding to the request
-    to an appropriate queue of waiters).
-  */
-  mysql_mutex_t m_signal_lock;
-  mysql_cond_t m_signal_cond;
-  mdl_signal_type m_signal;
-
+    Tell the deadlock detector what lock this session is waiting for.
+    In principle, this is redundant, as information can be found
+    by inspecting waiting queues, but we'd very much like it to be
+    readily available to the wait-for graph iterator.
+   */
+  MDL_ticket *m_waiting_for;
 private:
   MDL_ticket *find_ticket(MDL_request *mdl_req,
                           bool *is_transactional);
@@ -688,6 +720,7 @@ private:
 
   void find_deadlock();
 
+  /** Inform the deadlock detector there is an edge in the wait-for graph. */
   void will_wait_for(MDL_ticket *pending_ticket)
   {
     mysql_prlock_wrlock(&m_waiting_for_lock);
@@ -695,22 +728,13 @@ private:
     mysql_prlock_unlock(&m_waiting_for_lock);
   }
 
-  void stop_waiting()
+  /** Remove the wait-for edge from the graph after we're done waiting. */
+  void done_waiting_for()
   {
     mysql_prlock_wrlock(&m_waiting_for_lock);
     m_waiting_for= NULL;
     mysql_prlock_unlock(&m_waiting_for_lock);
   }
-
-  void wait_reset()
-  {
-    mysql_mutex_lock(&m_signal_lock);
-    m_signal= NO_WAKE_UP;
-    mysql_mutex_unlock(&m_signal_lock);
-  }
-
-  mdl_signal_type timed_wait(struct timespec *abs_timeout, bool signal_timeout);
-
 private:
   MDL_context(const MDL_context &rhs);          /* not implemented */
   MDL_context &operator=(MDL_context &rhs);     /* not implemented */


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