Fix for MDEV-9679 main.delayed fails sporadically

Problem was that notify_shared_lock() didn't abort an insert delayed thread
if it was in thr_upgrade_write_delay_lock().

ALTER TABLE first takes a weak_mdl_lock, then a thr_lock and then tries to upgrade
the mdl_lock.
Delayed insert thread first takes a mdl lock followed by a
thr_upgrade_write_delay_lock()
This caused insert delay to wait for alter table in thr_lock, while
alter table was waiting for the mdl lock by insert delay.

Fixed by telling mdl to run thr_lock_abort() for the insert delay thread table.
We also set thd->mysys_var->abort to 1 for the delay thread when it's killed
by alter table to ensure it doesn't ever get locked in thr_lock.

2 files changed, 64 insertions, 21 deletions

diff --git a/sql/sql_class.cc b/sql/sql_class.cc
index 271c44e1948..9cd4258c0df 100644
--- a/sql/sql_class.cc
+++ b/sql/sql_class.cc
@@ -1878,38 +1878,55 @@ bool THD::notify_shared_lock(MDL_context_owner *ctx_in_use,
 {
   THD *in_use= ctx_in_use->get_thd();
   bool signalled= FALSE;
+  DBUG_ENTER("THD::notify_shared_lock");
+  DBUG_PRINT("enter",("needs_thr_lock_abort: %d", needs_thr_lock_abort));
 
   if ((in_use->system_thread & SYSTEM_THREAD_DELAYED_INSERT) &&
       !in_use->killed)
   {
-    in_use->killed= KILL_CONNECTION;
-    mysql_mutex_lock(&in_use->mysys_var->mutex);
-    if (in_use->mysys_var->current_cond)
-      mysql_cond_broadcast(in_use->mysys_var->current_cond);
-    mysql_mutex_unlock(&in_use->mysys_var->mutex);
+    /* This code is similar to kill_delayed_threads() */
+    DBUG_PRINT("info", ("kill delayed thread"));
+    mysql_mutex_lock(&in_use->LOCK_thd_data);
+    if (in_use->killed < KILL_CONNECTION)
+      in_use->killed= KILL_CONNECTION;
+    if (in_use->mysys_var)
+    {
+      mysql_mutex_lock(&in_use->mysys_var->mutex);
+      if (in_use->mysys_var->current_cond)
+        mysql_cond_broadcast(in_use->mysys_var->current_cond);
+
+      /* Abort if about to wait in thr_upgrade_write_delay_lock */
+      in_use->mysys_var->abort= 1;
+      mysql_mutex_unlock(&in_use->mysys_var->mutex);
+    }
+    mysql_mutex_unlock(&in_use->LOCK_thd_data);
     signalled= TRUE;
   }
 
   if (needs_thr_lock_abort)
   {
     mysql_mutex_lock(&in_use->LOCK_thd_data);
-    for (TABLE *thd_table= in_use->open_tables;
-         thd_table ;
-         thd_table= thd_table->next)
+    /* If not already dying */
+    if (in_use->killed != KILL_CONNECTION_HARD)
     {
-      /*
-        Check for TABLE::needs_reopen() is needed since in some places we call
-        handler::close() for table instance (and set TABLE::db_stat to 0)
-        and do not remove such instances from the THD::open_tables
-        for some time, during which other thread can see those instances
-        (e.g. see partitioning code).
-      */
-      if (!thd_table->needs_reopen())
-        signalled|= mysql_lock_abort_for_thread(this, thd_table);
+      for (TABLE *thd_table= in_use->open_tables;
+           thd_table ;
+           thd_table= thd_table->next)
+      {
+        /*
+          Check for TABLE::needs_reopen() is needed since in some
+          places we call handler::close() for table instance (and set
+          TABLE::db_stat to 0) and do not remove such instances from
+          the THD::open_tables for some time, during which other
+          thread can see those instances (e.g. see partitioning code).
+        */
+        if (!thd_table->needs_reopen())
+          signalled|= mysql_lock_abort_for_thread(this, thd_table);
+      }
+      mysql_mutex_unlock(&in_use->LOCK_thd_data);
     }
-    mysql_mutex_unlock(&in_use->LOCK_thd_data);
   }
-  return signalled;
+  DBUG_RETURN(signalled);
 }
 
 
diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc
index b0151182a88..def5560a011 100644
--- a/sql/sql_insert.cc
+++ b/sql/sql_insert.cc
@@ -2620,14 +2620,16 @@ static void end_delayed_insert(THD *thd)
 
 void kill_delayed_threads(void)
 {
+  DBUG_ENTER("kill_delayed_threads");
   mysql_mutex_lock(&LOCK_delayed_insert); // For unlink from list
 
   I_List_iterator<Delayed_insert> it(delayed_threads);
   Delayed_insert *di;
   while ((di= it++))
   {
-    di->thd.killed= KILL_CONNECTION;
     mysql_mutex_lock(&di->thd.LOCK_thd_data);
+    if (di->thd.killed < KILL_CONNECTION)
+      di->thd.killed= KILL_CONNECTION;
     if (di->thd.mysys_var)
     {
       mysql_mutex_lock(&di->thd.mysys_var->mutex);
@@ -2648,6 +2650,7 @@ void kill_delayed_threads(void)
     mysql_mutex_unlock(&di->thd.LOCK_thd_data);
   }
   mysql_mutex_unlock(&LOCK_delayed_insert); // For unlink from list
+  DBUG_VOID_RETURN;
 }
 
 
@@ -2845,6 +2848,12 @@ pthread_handler_t handle_delayed_insert(void *arg)
     /* Tell client that the thread is initialized */
     mysql_cond_signal(&di->cond_client);
 
+    /*
+      Inform mdl that it needs to call mysql_lock_abort to abort locks
+      for delayed insert.
+    */
+    thd->mdl_context.set_needs_thr_lock_abort(TRUE);
+
     /* Now wait until we get an insert or lock to handle */
     /* We will not abort as long as a client thread uses this thread */
 
@@ -2853,6 +2862,7 @@ pthread_handler_t handle_delayed_insert(void *arg)
       if (thd->killed)
       {
         uint lock_count;
+        DBUG_PRINT("delayed", ("Insert delayed killed"));
         /*
           Remove this from delay insert list so that no one can request a
           table from this
@@ -2869,6 +2879,9 @@ pthread_handler_t handle_delayed_insert(void *arg)
       }
 
       /* Shouldn't wait if killed or an insert is waiting. */
+      DBUG_PRINT("delayed",
+                 ("thd->killed: %d  di->status: %d  di->stacked_inserts: %d",
+                  thd->killed, di->status, di->stacked_inserts));
       if (!thd->killed && !di->status && !di->stacked_inserts)
       {
         struct timespec abstime;
@@ -2908,6 +2921,9 @@ pthread_handler_t handle_delayed_insert(void *arg)
         mysql_mutex_unlock(&di->thd.mysys_var->mutex);
         mysql_mutex_lock(&di->mutex);
       }
+      DBUG_PRINT("delayed",
+                 ("thd->killed: %d  di->tables_in_use: %d  thd->lock: %d",
+                  thd->killed, di->tables_in_use, thd->lock != 0));
 
       if (di->tables_in_use && ! thd->lock && !thd->killed)
       {
@@ -2968,9 +2984,19 @@ pthread_handler_t handle_delayed_insert(void *arg)
   {
     DBUG_ENTER("handle_delayed_insert-cleanup");
     di->table=0;
-    thd->killed= KILL_CONNECTION;	        // If error
     mysql_mutex_unlock(&di->mutex);
 
+    /*
+      Protect against mdl_locks trying to access open tables
+      We use KILL_CONNECTION_HARD here to ensure that
+      THD::notify_shared_lock() dosn't try to access open tables after
+      this.
+    */
+    mysql_mutex_lock(&thd->LOCK_thd_data);
+    thd->killed= KILL_CONNECTION_HARD;	        // If error
+    thd->mdl_context.set_needs_thr_lock_abort(0);
+    mysql_mutex_unlock(&thd->LOCK_thd_data);
+
     close_thread_tables(thd);			// Free the table
     thd->mdl_context.release_transactional_locks();
     mysql_cond_broadcast(&di->cond_client);       // Safety