MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Jon Olav Hauglid Date:December 17 2009 12:43pm
Subject:bzr commit into mysql-5.6-next-mr branch (jon.hauglid:3041) Bug#48724
View as plain text  
#At file:///export/home/z/mysql-next-4284-bug48724/ based on revid:jon.hauglid@stripped

 3041 Jon Olav Hauglid	2009-12-17
      Bug #48724 Deadlock between INSERT DELAYED and FLUSH TABLES
      
      If the handler (or delayed insert) thread failed to lock a table due
      to being killed, the "dead" flag was used to notify the connection thread
      of this failure. However, with the changes introduced by Bug#45949, 
      the handler thread will no longer try to lock the table if it was killed.
      This meant that the "dead" flag would not be set, and the connection
      thread would not notice that the handler thread had failed.
      
      This could happen with concurrent INSERT DELAYED and FLUSH TABLES.
      FLUSH TABLES would kill any active INSERT DELAYED that had opened any
      table(s) to be flushed. This could cause the INSERT DELAYED connection
      thread to be stuck waiting for the handler thread to lock its table,
      while the handler thread would be looping, trying to get the connection
      thread to notice the error.
      
      The root of the problem was that the handler thread had both the "dead"
      flag and "thd->killed" to indicate that it had been killed. Most places
      both were set, but some only set "thd->killed". And 
      Delayed_insert::get_local_table() only checked "dead" while waiting for
      the table to be locked.
      
      This patch removes the "dead" variable and replaces its usage with
      "thd->killed", thereby resolving the issue.

    modified:
      sql/sql_insert.cc
=== modified file 'sql/sql_insert.cc'
--- a/sql/sql_insert.cc	2009-12-16 08:33:54 +0000
+++ b/sql/sql_insert.cc	2009-12-17 12:43:07 +0000
@@ -1761,7 +1761,7 @@ public:
   pthread_mutex_t mutex;
   pthread_cond_t cond,cond_client;
   volatile uint tables_in_use,stacked_inserts;
-  volatile bool status,dead;
+  volatile bool status;
   COPY_INFO info;
   I_List<delayed_row> rows;
   ulong group_count;
@@ -1769,8 +1769,7 @@ public:
 
   Delayed_insert()
     :locks_in_memory(0),
-     table(0),tables_in_use(0),stacked_inserts(0), status(0), dead(0),
-     group_count(0)
+     table(0),tables_in_use(0),stacked_inserts(0), status(0), group_count(0)
   {
     thd.security_ctx->user=thd.security_ctx->priv_user=(char*) delayed_user;
     thd.security_ctx->host=(char*) my_localhost;
@@ -1918,9 +1917,8 @@ Delayed_insert *find_handler(THD *thd, T
   a given consumer (delayed insert thread), only at different
   stages of producer-consumer relationship.
 
-  'dead' and 'status' variables in Delayed_insert are redundant
-  too, since there is already 'di->thd.killed' and
-  di->stacked_inserts.
+  The 'status' variable in Delayed_insert is redundant
+  too, since there is already di->stacked_inserts.
 */
 
 static
@@ -2073,17 +2071,29 @@ TABLE *Delayed_insert::get_local_table(T
   {
     thd_proc_info(client_thd, "waiting for handler lock");
     pthread_cond_signal(&cond);			// Tell handler to lock table
-    while (!dead && !thd.lock && ! client_thd->killed)
+    while (!thd.killed && !thd.lock && ! client_thd->killed)
     {
       pthread_cond_wait(&cond_client,&mutex);
     }
     thd_proc_info(client_thd, "got handler lock");
     if (client_thd->killed)
       goto error;
-    if (dead)
+    if (thd.killed)
     {
-      /* Don't copy over "Server shutdown in progress". */
-      if (thd.stmt_da->sql_errno() == ER_SERVER_SHUTDOWN)
+      /*
+        Copy the error message. Note that we don't treat fatal
+        errors in the delayed thread as fatal errors in the
+        main thread. If delayed thread was killed, we don't
+        want to send "Server shutdown in progress" in the
+        INSERT THREAD.
+
+        The thread could be killed with an error message if
+        di->handle_inserts() or di->open_and_lock_table() fails.
+        The thread could be killed without an error message if
+        killed using mysql_notify_thread_having_shared_lock() or
+        kill_delayed_threads_for_table().
+      */
+      if (!thd.is_error() || thd.stmt_da->sql_errno() == ER_SERVER_SHUTDOWN)
         my_message(ER_QUERY_INTERRUPTED, ER(ER_QUERY_INTERRUPTED), MYF(0));
       else
         my_message(thd.stmt_da->sql_errno(), thd.stmt_da->message(), MYF(0));
@@ -2454,7 +2464,8 @@ pthread_handler_t handle_delayed_insert(
           break;					// Time to die
       }
 
-      if (!di->status && !di->stacked_inserts)
+      /* Shouldn't wait if killed or an insert is waiting. */
+      if (!thd->killed && !di->status && !di->stacked_inserts)
       {
         struct timespec abstime;
         set_timespec(abstime, delayed_insert_timeout);
@@ -2465,7 +2476,7 @@ pthread_handler_t handle_delayed_insert(
         thd_proc_info(&(di->thd), "Waiting for INSERT");
 
         DBUG_PRINT("info",("Waiting for someone to insert rows"));
-        while (!thd->killed)
+        while (!thd->killed && !di->status)
         {
           int error;
 #if defined(HAVE_BROKEN_COND_TIMEDWAIT)
@@ -2481,13 +2492,8 @@ pthread_handler_t handle_delayed_insert(
           }
 #endif
 #endif
-          if (thd->killed || di->status)
-            break;
           if (error == ETIMEDOUT || error == ETIME)
-          {
             thd->killed= THD::KILL_CONNECTION;
-            break;
-          }
         }
         /* We can't lock di->mutex and mysys_var->mutex at the same time */
         pthread_mutex_unlock(&di->mutex);
@@ -2528,14 +2534,12 @@ pthread_handler_t handle_delayed_insert(
             di->table= 0;
             if (di->open_and_lock_table())
             {
-              di->dead= 1;
               thd->killed= THD::KILL_CONNECTION;
             }
           }
           else
           {
             /* Fatal error */
-            di->dead= 1;
             thd->killed= THD::KILL_CONNECTION;
           }
         }
@@ -2546,7 +2550,6 @@ pthread_handler_t handle_delayed_insert(
         if (di->handle_inserts())
         {
           /* Some fatal error */
-          di->dead= 1;
           thd->killed= THD::KILL_CONNECTION;
         }
       }
@@ -2598,7 +2601,6 @@ pthread_handler_t handle_delayed_insert(
 
   close_thread_tables(thd);			// Free the table
   di->table=0;
-  di->dead= 1;                                  // If error
   thd->killed= THD::KILL_CONNECTION;	        // If error
   pthread_cond_broadcast(&di->cond_client);	// Safety
   pthread_mutex_unlock(&di->mutex);


Attachment: [text/bzr-bundle] bzr/jon.hauglid@sun.com-20091217124307-glwitkbztcmh8ajr.bundle
Thread
bzr commit into mysql-5.6-next-mr branch (jon.hauglid:3041) Bug#48724Jon Olav Hauglid17 Dec