Followup to Bug#45225 Locking: hang if drop table with no timeout

This patch prevents system threads and system table accesses from
using user-specified values for "lock_wait_timeout". Instead all
such accesses are done using the default value (1 year).

This prevents background tasks (such as replication, events, 
accessing stored function definitions, logging, reading time-zone
information, etc.) from failing in cases where the global value
of "lock_wait_timeout" is set very low.

The patch also simplifies the open tables API. Rather than adding
another convenience function for opening and locking system tables,
this patch removes most of the existing convenience functions for
open_and_lock_tables_derived(). Before, open_and_lock_tables() was
a convenience function that enforced derived tables handling, while
open_and_lock_tables_derived() was the main function where derived
tables handling was optional. Now, this convencience function is
gone and the main function is renamed to open_and_lock_tables(). 

No test case added as it would have required the use of --sleep to
check that system threads and system tables have a different timeout
value from the user-specified "lock_wait_timeout" system variable.

1 files changed, 31 insertions, 34 deletions

diff --git a/sql/sql_base.cc b/sql/sql_base.cc
index d5a664df0d0..0f572350647 100644
--- a/sql/sql_base.cc
+++ b/sql/sql_base.cc
@@ -122,7 +122,8 @@ static bool open_table_entry_fini(THD *thd, TABLE_SHARE *share, TABLE *entry);
 static bool auto_repair_table(THD *thd, TABLE_LIST *table_list);
 static void free_cache_entry(TABLE *entry);
 static bool tdc_wait_for_old_versions(THD *thd,
-                                      MDL_request_list *mdl_requests);
+                                      MDL_request_list *mdl_requests,
+                                      ulong timeout);
 static bool
 has_write_table_with_auto_increment(TABLE_LIST *tables);
 
@@ -2363,8 +2364,7 @@ open_table_get_mdl_lock(THD *thd, TABLE_LIST *table_list,
     mdl_requests.push_front(mdl_request);
     mdl_requests.push_front(global_request);
 
-    if (thd->mdl_context.acquire_locks(&mdl_requests,
-                                       thd->variables.lock_wait_timeout))
+    if (thd->mdl_context.acquire_locks(&mdl_requests, ot_ctx->get_timeout()))
       return 1;
   }
   else
@@ -3308,7 +3308,7 @@ unlink_all_closed_tables(THD *thd, MYSQL_LOCK *lock, size_t reopen_count)
 bool
 Locked_tables_list::reopen_tables(THD *thd)
 {
-  Open_table_context ot_ctx_unused(thd);
+  Open_table_context ot_ctx_unused(thd, LONG_TIMEOUT);
   bool lt_refresh_unused;
   size_t reopen_count= 0;
   MYSQL_LOCK *lock;
@@ -3744,13 +3744,14 @@ end_with_lock_open:
 
 /** Open_table_context */
 
-Open_table_context::Open_table_context(THD *thd)
+Open_table_context::Open_table_context(THD *thd, ulong timeout)
   :m_action(OT_NO_ACTION),
    m_start_of_statement_svp(thd->mdl_context.mdl_savepoint()),
    m_has_locks((thd->in_multi_stmt_transaction() &&
                 thd->mdl_context.has_locks()) ||
                 thd->mdl_context.trans_sentinel()),
-   m_global_mdl_request(NULL)
+   m_global_mdl_request(NULL),
+   m_timeout(timeout)
 {}
 
 
@@ -3845,11 +3846,10 @@ recover_from_failed_open(THD *thd, MDL_request *mdl_request,
   switch (m_action)
   {
     case OT_WAIT_MDL_LOCK:
-      result= thd->mdl_context.wait_for_lock(mdl_request,
-                                             thd->variables.lock_wait_timeout);
+      result= thd->mdl_context.wait_for_lock(mdl_request, get_timeout());
       break;
     case OT_WAIT_TDC:
-      result= tdc_wait_for_old_versions(thd, &m_mdl_requests);
+      result= tdc_wait_for_old_versions(thd, &m_mdl_requests, get_timeout());
       DBUG_ASSERT(thd->mysys_var->current_mutex == NULL);
       break;
     case OT_DISCOVER:
@@ -3866,8 +3866,7 @@ recover_from_failed_open(THD *thd, MDL_request *mdl_request,
         mdl_requests.push_front(&mdl_global_request);
 
         if ((result=
-             thd->mdl_context.acquire_locks(&mdl_requests,
-                                            thd->variables.lock_wait_timeout)))
+             thd->mdl_context.acquire_locks(&mdl_requests, get_timeout())))
           break;
 
         DBUG_ASSERT(mdl_request->key.mdl_namespace() == MDL_key::TABLE);
@@ -3899,8 +3898,7 @@ recover_from_failed_open(THD *thd, MDL_request *mdl_request,
         mdl_requests.push_front(&mdl_global_request);
 
         if ((result=
-             thd->mdl_context.acquire_locks(&mdl_requests,
-                                            thd->variables.lock_wait_timeout)))
+             thd->mdl_context.acquire_locks(&mdl_requests, get_timeout())))
           break;
 
         DBUG_ASSERT(mdl_request->key.mdl_namespace() == MDL_key::TABLE);
@@ -4397,8 +4395,7 @@ open_tables_acquire_upgradable_mdl(THD *thd, TABLE_LIST *tables_start,
     mdl_requests.push_front(global_request);
   }
 
-  if (thd->mdl_context.acquire_locks(&mdl_requests,
-                                     thd->variables.lock_wait_timeout))
+  if (thd->mdl_context.acquire_locks(&mdl_requests, ot_ctx->get_timeout()))
     return TRUE;
 
   for (table= tables_start; table && table != tables_end;
@@ -4457,7 +4454,8 @@ bool open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags,
   TABLE_LIST **table_to_open;
   Sroutine_hash_entry **sroutine_to_open;
   TABLE_LIST *tables;
-  Open_table_context ot_ctx(thd);
+  Open_table_context ot_ctx(thd, (flags & MYSQL_LOCK_IGNORE_TIMEOUT) ?
+                            LONG_TIMEOUT : thd->variables.lock_wait_timeout);
   bool error= FALSE;
   MEM_ROOT new_frm_mem;
   bool has_prelocking_list;
@@ -4961,7 +4959,7 @@ TABLE *open_n_lock_single_table(THD *thd, TABLE_LIST *table_l,
   table_l->required_type= FRMTYPE_TABLE;
 
   /* Open the table. */
-  if (open_and_lock_tables_derived(thd, table_l, FALSE, flags))
+  if (open_and_lock_tables(thd, table_l, FALSE, flags))
     table_l->table= NULL; /* Just to be sure. */
 
   /* Restore list. */
@@ -4999,7 +4997,8 @@ TABLE *open_ltable(THD *thd, TABLE_LIST *table_list, thr_lock_type lock_type,
                    uint lock_flags)
 {
   TABLE *table;
-  Open_table_context ot_ctx(thd);
+  Open_table_context ot_ctx(thd, (lock_flags & MYSQL_LOCK_IGNORE_TIMEOUT) ?
+                            LONG_TIMEOUT : thd->variables.lock_wait_timeout);
   bool refresh;
   bool error;
   DBUG_ENTER("open_ltable");
@@ -5098,20 +5097,13 @@ end:
   @note
     The lock will automaticaly be freed by close_thread_tables()
 
-  @note
-    There are several convenience functions, e.g. :
-    - simple_open_n_lock_tables(thd, tables)  without derived handling
-    - open_and_lock_tables(thd, tables)       with derived handling
-    Both inline functions call open_and_lock_tables_derived() with
-    the third argument set appropriately.
-
   @retval FALSE  OK.
   @retval TRUE   Error
 */
 
-bool open_and_lock_tables_derived(THD *thd, TABLE_LIST *tables,
-                                  bool derived, uint flags,
-                                  Prelocking_strategy *prelocking_strategy)
+bool open_and_lock_tables(THD *thd, TABLE_LIST *tables,
+                          bool derived, uint flags,
+                          Prelocking_strategy *prelocking_strategy)
 {
   uint counter;
   bool need_reopen;
@@ -5125,7 +5117,7 @@ bool open_and_lock_tables_derived(THD *thd, TABLE_LIST *tables,
     statement.
   */
   MDL_ticket *start_of_statement_svp= thd->mdl_context.mdl_savepoint();
-  DBUG_ENTER("open_and_lock_tables_derived");
+  DBUG_ENTER("open_and_lock_tables");
   DBUG_PRINT("enter", ("derived handling: %d", derived));
 
   for ( ; ; ) 
@@ -8536,16 +8528,18 @@ void tdc_remove_table(THD *thd, enum_tdc_remove_table_type remove_type,
 
    @param thd      Thread context
    @param context  Metadata locking context with locks.
+   @param timeout  Seconds to wait before reporting ER_LOCK_WAIT_TIMEOUT.
 */
 
 static bool
-tdc_wait_for_old_versions(THD *thd, MDL_request_list *mdl_requests)
+tdc_wait_for_old_versions(THD *thd, MDL_request_list *mdl_requests,
+                          ulong timeout)
 {
   TABLE_SHARE *share;
   const char *old_msg;
   MDL_request *mdl_request;
   struct timespec abstime;
-  set_timespec(abstime, thd->variables.lock_wait_timeout);
+  set_timespec(abstime, timeout);
   int wait_result= 0;
 
   while (!thd->killed)
@@ -8812,8 +8806,9 @@ open_system_tables_for_read(THD *thd, TABLE_LIST *table_list,
   lex->reset_n_backup_query_tables_list(&query_tables_list_backup);
   thd->reset_n_backup_open_tables_state(backup);
 
-  if (open_and_lock_tables_derived(thd, table_list, FALSE,
-                                   MYSQL_LOCK_IGNORE_FLUSH))
+  if (open_and_lock_tables(thd, table_list, FALSE,
+                           MYSQL_LOCK_IGNORE_FLUSH |
+                           MYSQL_LOCK_IGNORE_TIMEOUT))
   {
     lex->restore_backup_query_tables_list(&query_tables_list_backup);
     goto error;
@@ -8875,7 +8870,8 @@ open_system_table_for_update(THD *thd, TABLE_LIST *one_table)
 {
   DBUG_ENTER("open_system_table_for_update");
 
-  TABLE *table= open_ltable(thd, one_table, one_table->lock_type, 0);
+  TABLE *table= open_ltable(thd, one_table, one_table->lock_type,
+                            MYSQL_LOCK_IGNORE_TIMEOUT);
   if (table)
   {
     DBUG_ASSERT(table->s->table_category == TABLE_CATEGORY_SYSTEM);
@@ -8902,6 +8898,7 @@ open_log_table(THD *thd, TABLE_LIST *one_table, Open_tables_backup *backup)
   uint flags= ( MYSQL_LOCK_IGNORE_GLOBAL_READ_LOCK |
                 MYSQL_LOCK_IGNORE_GLOBAL_READ_ONLY |
                 MYSQL_LOCK_IGNORE_FLUSH |
+                MYSQL_LOCK_IGNORE_TIMEOUT |
                 MYSQL_LOCK_PERF_SCHEMA);
   TABLE *table;
   /* Save value that is changed in mysql_lock_tables() */