MDEV-5492 - Reduce usage of LOCK_open: TABLE::in_usebb-10.2-mdev5492

- moved TABLE::in_use updates out of TDC_element::LOCK_table_share
- use atomic load/store so that foreign threads accessing TABLE::in_use get
  consistent value
- relaxed assertions so that TABLE::in_use == 0 is now acceptable and is skipped
  by kill_delayed_threads_for_table() and TABLE_SHARE::visit_subgraph()

In all cases it is guaranteed that THD object stays valid until
TDC_element::LOCK_table_share is locked or TDC_element::all_tables_refs is
not 0.

It is safe to skip TABLE::in_use == 0 in kill_delayed_threads_for_table() and
TABLE_SHARE::visit_subgraph() because TABLE is either being released or it is
being acquired and will be released immediately.

5 files changed, 15 insertions, 19 deletions

diff --git a/sql/sql_base.cc b/sql/sql_base.cc
index 08a9647cf56..a9409c901bf 100644
--- a/sql/sql_base.cc
+++ b/sql/sql_base.cc
@@ -307,7 +307,7 @@ static my_bool list_open_tables_callback(TDC_element *element,
   TDC_element::All_share_tables_list::Iterator it(element->all_tables);
   TABLE *table;
   while ((table= it++))
-    if (table->in_use)
+    if (my_atomic_loadptr_explicit(&table->in_use, MY_MEMORY_ORDER_RELAXED))
       ++(*arg->start_list)->in_use;
   mysql_mutex_unlock(&element->LOCK_table_share);
   (*arg->start_list)->locked= 0;                   /* Obsolete. */
@@ -373,16 +373,16 @@ void kill_delayed_threads_for_table(TDC_element *element)
   TABLE *tab;
 
   mysql_mutex_assert_owner(&element->LOCK_table_share);
+  DBUG_ASSERT(element->flushed);
 
   if (!delayed_insert_threads)
     return;
 
   while ((tab= it++))
   {
-    THD *in_use= tab->in_use;
+    THD *in_use= my_atomic_loadptr_explicit(&tab->in_use, MY_MEMORY_ORDER_RELAXED);
 
-    DBUG_ASSERT(in_use && tab->s->tdc->flushed);
-    if ((in_use->system_thread & SYSTEM_THREAD_DELAYED_INSERT) &&
+    if (in_use && (in_use->system_thread & SYSTEM_THREAD_DELAYED_INSERT) &&
         ! in_use->killed)
     {
       in_use->killed= KILL_SYSTEM_THREAD;
diff --git a/sql/sql_test.cc b/sql/sql_test.cc
index 50d51dcc8cc..4cbc2f3708d 100644
--- a/sql/sql_test.cc
+++ b/sql/sql_test.cc
@@ -85,7 +85,7 @@ static my_bool print_cached_tables_callback(TDC_element *element,
   TDC_element::All_share_tables_list::Iterator it(element->all_tables);
   while ((entry= it++))
   {
-    THD *in_use= entry->in_use;
+    THD *in_use= my_atomic_loadptr_explicit(&entry->in_use, MY_MEMORY_ORDER_RELAXED);
     printf("%-14.14s %-32s%6ld%8ld%6d  %s\n",
            entry->s->db.str, entry->s->table_name.str, element->version,
            in_use ? (long) in_use->thread_id : (long) 0,
diff --git a/sql/table.cc b/sql/table.cc
index dc1730b5b6f..c153ca3ef43 100644
--- a/sql/table.cc
+++ b/sql/table.cc
@@ -3931,6 +3931,7 @@ bool TABLE_SHARE::visit_subgraph(Wait_for_flush *wait_for_flush,
   */
   mysql_mutex_lock(&tdc->LOCK_table_share);
   tdc->all_tables_refs++;
+  DBUG_ASSERT(tdc->flushed);
   mysql_mutex_unlock(&tdc->LOCK_table_share);
 
   TDC_element::All_share_tables_list::Iterator tables_it(tdc->all_tables);
@@ -3951,8 +3952,8 @@ bool TABLE_SHARE::visit_subgraph(Wait_for_flush *wait_for_flush,
 
   while ((table= tables_it++))
   {
-    DBUG_ASSERT(table->in_use && tdc->flushed);
-    if (gvisitor->inspect_edge(&table->in_use->mdl_context))
+    THD *thd= my_atomic_loadptr_explicit(&table->in_use, MY_MEMORY_ORDER_RELAXED);
+    if (thd && gvisitor->inspect_edge(&thd->mdl_context))
     {
       goto end_leave_node;
     }
@@ -3961,8 +3962,8 @@ bool TABLE_SHARE::visit_subgraph(Wait_for_flush *wait_for_flush,
   tables_it.rewind();
   while ((table= tables_it++))
   {
-    DBUG_ASSERT(table->in_use && tdc->flushed);
-    if (table->in_use->mdl_context.visit_subgraph(gvisitor))
+    THD *thd= my_atomic_loadptr_explicit(&table->in_use, MY_MEMORY_ORDER_RELAXED);
+    if (thd && thd->mdl_context.visit_subgraph(gvisitor))
     {
       goto end_leave_node;
     }
diff --git a/sql/table_cache.cc b/sql/table_cache.cc
index 9176810d1b2..eb0a3295a9e 100644
--- a/sql/table_cache.cc
+++ b/sql/table_cache.cc
@@ -343,6 +343,8 @@ bool tc_release_table(TABLE *table)
   DBUG_ASSERT(table->in_use);
   DBUG_ASSERT(table->file);
 
+  my_atomic_storeptr_explicit(&table->in_use, 0, MY_MEMORY_ORDER_RELAXED);
+
   if (table->needs_reopen() || tc_records() > tc_size)
   {
     mysql_mutex_lock(&table->s->tdc->LOCK_table_share);
@@ -354,13 +356,6 @@ bool tc_release_table(TABLE *table)
   mysql_mutex_lock(&table->s->tdc->LOCK_table_share);
   if (table->s->tdc->flushed)
     goto purge;
-  /*
-    in_use doesn't really need mutex protection, but must be reset after
-    checking tdc.flushed and before this table appears in free_tables.
-    Resetting in_use is needed only for print_cached_tables() and
-    list_open_tables().
-  */
-  table->in_use= 0;
   /* Add table to the list of unused TABLE objects for this share. */
   table->s->tdc->free_tables.push_front(table);
   mysql_mutex_unlock(&table->s->tdc->LOCK_table_share);
@@ -369,7 +364,6 @@ bool tc_release_table(TABLE *table)
 purge:
   tc_remove_table(table);
   mysql_mutex_unlock(&table->s->tdc->LOCK_table_share);
-  table->in_use= 0;
   intern_close_table(table);
   return true;
 }
diff --git a/sql/table_cache.h b/sql/table_cache.h
index af1386cb9cd..d68f18e9132 100644
--- a/sql/table_cache.h
+++ b/sql/table_cache.h
@@ -91,16 +91,17 @@ public:
 
     mysql_mutex_lock(&LOCK_table_share);
     table= free_tables.pop_front();
+    mysql_mutex_unlock(&LOCK_table_share);
+
     if (table)
     {
       DBUG_ASSERT(!table->in_use);
-      table->in_use= thd;
+      my_atomic_storeptr_explicit(&table->in_use, thd, MY_MEMORY_ORDER_RELAXED);
       /* The ex-unused table must be fully functional. */
       DBUG_ASSERT(table->db_stat && table->file);
       /* The children must be detached from the table. */
       DBUG_ASSERT(!table->file->extra(HA_EXTRA_IS_ATTACHED_CHILDREN));
     }
-    mysql_mutex_unlock(&LOCK_table_share);
     return table;
   }