Backport of:

------------------------------------------------------------
revno: 2630.4.18
committer: Dmitry Lenev <dlenev@mysql.com>
branch nick: mysql-6.0-3726-w2
timestamp: Tue 2008-06-03 21:07:58 +0400
message:
  WL#3726 "DDL locking for all metadata objects".

  After review fixes in progress.

  Now during upgrading/downgrading metadata locks we deal with
  individual metadata lock requests rather than with all requests
  for this object in the context. This makes API a bit more clear
  and makes adjust_mdl_locks_upgradability() much nicer.

sql/lock.cc

@@ -976,6 +976,7 @@ bool lock_table_names(THD *thd, TABLE_LIST *table_list)
       goto end;
     mdl_set_lock_type(mdl_lock_data, MDL_EXCLUSIVE);
     mdl_add_lock(&thd->mdl_context, mdl_lock_data);
+    lock_table->mdl_lock_data= mdl_lock_data;
   }
   if (mdl_acquire_exclusive_locks(&thd->mdl_context))
     return 1;

sql/mdl.h

@@ -194,8 +194,8 @@ inline void mdl_set_upgradable(MDL_LOCK_DATA *lock_data)
 
 bool mdl_acquire_shared_lock(MDL_LOCK_DATA *lock_data, bool *retry);
 bool mdl_acquire_exclusive_locks(MDL_CONTEXT *context);
-bool mdl_upgrade_shared_lock_to_exclusive(MDL_CONTEXT *context, int type,
-                                          const char *db, const char *name);
+bool mdl_upgrade_shared_lock_to_exclusive(MDL_CONTEXT *context,
+                                          MDL_LOCK_DATA *lock_data);
 bool mdl_try_acquire_exclusive_lock(MDL_CONTEXT *context,
                                     MDL_LOCK_DATA *lock_data);
 bool mdl_acquire_global_shared_lock(MDL_CONTEXT *context);
@@ -203,9 +203,11 @@ bool mdl_acquire_global_shared_lock(MDL_CONTEXT *context);
 bool mdl_wait_for_locks(MDL_CONTEXT *context);
 
 void mdl_release_locks(MDL_CONTEXT *context);
-void mdl_release_exclusive_locks(MDL_CONTEXT *context);
+void mdl_release_all_locks_for_name(MDL_CONTEXT *context,
+                                    MDL_LOCK_DATA *lock_data);
 void mdl_release_lock(MDL_CONTEXT *context, MDL_LOCK_DATA *lock_data);
-void mdl_downgrade_exclusive_locks(MDL_CONTEXT *context);
+void mdl_downgrade_exclusive_lock(MDL_CONTEXT *context,
+                                  MDL_LOCK_DATA *lock_data);
 void mdl_release_global_shared_lock(MDL_CONTEXT *context);
 
 bool mdl_is_exclusive_lock_owner(MDL_CONTEXT *context, int type, const char *db,

sql/sql_base.cc

@@ -1066,7 +1066,8 @@ bool close_cached_tables(THD *thd, TABLE_LIST *tables, bool have_lock,
     {
       for (TABLE_LIST *table= tables; table; table= table->next_local)
       {
-        TABLE *tab= find_locked_table(thd->open_tables, table->db,
+        /* This should always succeed thanks to check in caller. */
+        TABLE *tab= find_write_locked_table(thd->open_tables, table->db,
                                             table->table_name);
         /*
           Checking TABLE::db_stat is essential in case when we have
@@ -1152,7 +1153,13 @@ bool close_cached_tables(THD *thd, TABLE_LIST *tables, bool have_lock,
     result|= reopen_tables(thd, 1);
     thd->in_lock_tables=0;
     pthread_mutex_unlock(&LOCK_open);
-    mdl_downgrade_exclusive_locks(&thd->mdl_context);
+    /*
+      Since mdl_downgrade_exclusive_lock() won't do anything with shared
+      metadata lock it is much simplier to go through all open tables rather
+      than picking only those tables that were flushed.
+    */
+    for (TABLE *tab= thd->open_tables; tab; tab= tab->next)
+      mdl_downgrade_exclusive_lock(&thd->mdl_context, tab->mdl_lock_data);
   }
   DBUG_RETURN(result);
 }
@@ -2976,7 +2983,7 @@ TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
     lock on this table to shared metadata lock.
   */
   if (table_list->open_type == TABLE_LIST::OPEN_OR_CREATE)
-    mdl_downgrade_exclusive_locks(&thd->mdl_context);
+    mdl_downgrade_exclusive_lock(&thd->mdl_context, table_list->mdl_lock_data);
 
   table->mdl_lock_data= mdl_lock_data;
 
@@ -3992,7 +3999,7 @@ static bool handle_failed_open_table_attempt(THD *thd, TABLE_LIST *table,
 
       thd->warning_info->clear_warning_info(thd->query_id);
       thd->clear_error();                 // Clear error message
-      mdl_release_exclusive_locks(&thd->mdl_context);
+      mdl_release_lock(&thd->mdl_context, table->mdl_lock_data);
       break;
     case OT_REPAIR:
       mdl_set_lock_type(table->mdl_lock_data, MDL_EXCLUSIVE);
@@ -4004,7 +4011,7 @@ static bool handle_failed_open_table_attempt(THD *thd, TABLE_LIST *table,
       pthread_mutex_unlock(&LOCK_open);
 
       result= auto_repair_table(thd, table);
-      mdl_release_exclusive_locks(&thd->mdl_context);
+      mdl_release_lock(&thd->mdl_context, table->mdl_lock_data);
       break;
     default:
       DBUG_ASSERT(0);
@@ -8694,8 +8701,8 @@ int abort_and_upgrade_lock(ALTER_PARTITION_PARAM_TYPE *lpt)
   /* If MERGE child, forward lock handling to parent. */
   mysql_lock_abort(lpt->thd, lpt->table->parent ? lpt->table->parent :
                    lpt->table, TRUE);
-  if (mdl_upgrade_shared_lock_to_exclusive(&lpt->thd->mdl_context, 0,
-                                           lpt->db, lpt->table_name))
+  if (mdl_upgrade_shared_lock_to_exclusive(&lpt->thd->mdl_context,
+                                           lpt->table->mdl_lock_data))
   {
     mysql_lock_downgrade_write(lpt->thd,
                                lpt->table->parent ? lpt->table->parent :

sql/sql_parse.cc

@@ -8114,31 +8114,10 @@ bool parse_sql(THD *thd,
 
 static void adjust_mdl_locks_upgradability(TABLE_LIST *tables)
 {
-  TABLE_LIST *tab, *otab;
-
-  for (tab= tables; tab; tab= tab->next_global)
+  for (TABLE_LIST *tab= tables; tab; tab= tab->next_global)
   {
     if (tab->lock_type >= TL_WRITE_ALLOW_WRITE)
       tab->mdl_upgradable= TRUE;
-    else
-    {
-      /*
-        TODO: To get rid of this loop we need to change our code to do
-              metadata lock upgrade only for those instances of tables
-              which are write locked instead of doing such upgrade for
-              all instances of tables.
-      */
-      for (otab= tables; otab; otab= otab->next_global)
-        if (otab->lock_type >= TL_WRITE_ALLOW_WRITE &&
-            otab->db_length == tab->db_length &&
-            otab->table_name_length == tab->table_name_length &&
-            !strcmp(otab->db, tab->db) &&
-            !strcmp(otab->table_name, tab->table_name))
-        {
-          tab->mdl_upgradable= TRUE;
-          break;
-        }
-    }
   }
 }
 

sql/sql_table.cc

@@ -1904,10 +1904,27 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists,
     else if (thd->locked_tables)
     {
       for (table= tables; table; table= table->next_local)
-        if (!find_temporary_table(thd, table->db, table->table_name) &&
-            !find_write_locked_table(thd->open_tables, table->db,
-                                     table->table_name))
+        if (find_temporary_table(thd, table->db, table->table_name))
+        {
+          /*
+            Since we don't acquire metadata lock if we have found temporary
+            table, we should do something to avoid releasing it at the end.
+          */
+          table->mdl_lock_data= 0;
+        }
+        else
+        {
+          /*
+            Since 'tables' list can't contain duplicates (this is ensured
+            by parser) it is safe to cache pointer to the TABLE instances
+            in its elements.
+          */
+          table->table= find_write_locked_table(thd->open_tables, table->db,
+                                                table->table_name);
+          if (!table->table)
             DBUG_RETURN(1);
+          table->mdl_lock_data= table->table->mdl_lock_data;
+        }
     }
   }
 
@@ -1956,6 +1973,9 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists,
       error= 0;
     }
 
+    /* Probably a non-temporary table. */
+    non_temp_tables_count++;
+
     /*
       If row-based replication is used and the table is not a
       temporary table, we add the table name to the drop statement
@@ -1964,7 +1984,6 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists,
       */
     if (!drop_temporary && thd->current_stmt_binlog_row_based && !dont_log_query)
     {
-      non_temp_tables_count++;
       /*
         Don't write the database name if it is the current one (or if
         thd->db is NULL).
@@ -1985,18 +2004,12 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists,
     {
       if (thd->locked_tables)
       {
-        TABLE *tab= find_locked_table(thd->open_tables, db, table->table_name);
-        if (close_cached_table(thd, tab))
+        if (close_cached_table(thd, table->table))
         {
           error= -1;
           goto err_with_placeholders;
         }
-        /*
-          Leave LOCK TABLES mode if we managed to drop all tables
-          which were locked.
-        */
-        if (thd->locked_tables->table_count == 0)
-          unlock_locked_tables(thd);
+        table->table= 0;
       }
 
       if (thd->killed)
@@ -2175,10 +2188,32 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists,
       doing this. Unfortunately in this case we are likely to get more
       false positives in lock_table_name_if_not_cached() function. So
       it makes sense to remove exclusive meta-data locks in all cases.
+
+      Leave LOCK TABLES mode if we managed to drop all tables which were
+      locked. Additional check for 'non_temp_tables_count' is to avoid
+      leaving LOCK TABLES mode if we have dropped only temporary tables.
     */
-    mdl_release_exclusive_locks(&thd->mdl_context);
+    if (thd->locked_tables && thd->locked_tables->table_count == 0 &&
+        non_temp_tables_count > 0)
+    {
+      unlock_locked_tables(thd);
+      goto end;
+    }
+    for (table= tables; table; table= table->next_local)
+    {
+      if (table->mdl_lock_data)
+      {
+        /*
+          Under LOCK TABLES we may have several instances of table open
+          and locked and therefore have to remove several metadata lock
+          requests associated with them.
+        */
+        mdl_release_all_locks_for_name(&thd->mdl_context, table->mdl_lock_data);
+      }
+    }
   }
 
+end:
   DBUG_RETURN(error);
 }
 
@@ -4122,7 +4157,7 @@ bool mysql_create_table(THD *thd, const char *db, const char *table_name,
 
 unlock:
   if (target_lock_data)
-    mdl_release_exclusive_locks(&thd->mdl_context);
+    mdl_release_lock(&thd->mdl_context, target_lock_data);
   pthread_mutex_lock(&LOCK_lock_db);
   if (!--creating_table && creating_database)
     pthread_cond_signal(&COND_refresh);
@@ -4292,9 +4327,8 @@ bool wait_while_table_is_used(THD *thd, TABLE *table,
   old_lock_type= table->reginfo.lock_type;
   mysql_lock_abort(thd, table, TRUE);	/* end threads waiting on lock */
 
-  if (mdl_upgrade_shared_lock_to_exclusive(&thd->mdl_context, 0,
-                                           table->s->db.str,
-                                           table->s->table_name.str))
+  if (mdl_upgrade_shared_lock_to_exclusive(&thd->mdl_context,
+                                           table->mdl_lock_data))
   {
     mysql_lock_downgrade_write(thd, table, old_lock_type);
     DBUG_RETURN(TRUE);
@@ -4476,6 +4510,10 @@ static int prepare_for_repair(THD *thd, TABLE_LIST *table_list,
     table= &tmp_table;
     pthread_mutex_unlock(&LOCK_open);
   }
+  else
+  {
+    mdl_lock_data= table->mdl_lock_data;
+  }
 
   /* A MERGE table must not come here. */
   DBUG_ASSERT(!table->child_l);
@@ -4574,8 +4612,9 @@ static int prepare_for_repair(THD *thd, TABLE_LIST *table_list,
     closefrm(table, 1);				// Free allocated memory
     pthread_mutex_unlock(&LOCK_open);
   }
-  if (error)
-    mdl_release_exclusive_locks(&thd->mdl_context);
+  /* In case of a temporary table there will be no metadata lock. */
+  if (error && mdl_lock_data)
+    mdl_release_lock(&thd->mdl_context, mdl_lock_data);
   DBUG_RETURN(error);
 }
 
@@ -5539,7 +5578,7 @@ goto binlog;
 
 err:
   if (target_lock_data)
-    mdl_release_exclusive_locks(&thd->mdl_context);
+    mdl_release_lock(&thd->mdl_context, target_lock_data);
   DBUG_RETURN(res);
 }
 
@@ -6477,7 +6516,7 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
                        uint order_num, ORDER *order, bool ignore)
 {
   TABLE *table, *new_table= 0;
-  MDL_LOCK_DATA *target_lock_data= 0;
+  MDL_LOCK_DATA *mdl_lock_data, *target_lock_data= 0;
   int error= 0;
   char tmp_name[80],old_name[32],new_name_buff[FN_REFLEN + 1];
   char new_alias_buff[FN_REFLEN], *table_name, *db, *new_alias, *alias;
@@ -6649,6 +6688,7 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
   if (!(table= open_n_lock_single_table(thd, table_list, TL_WRITE_ALLOW_READ)))
     DBUG_RETURN(TRUE);
   table->use_all_columns();
+  mdl_lock_data= table->mdl_lock_data;
 
   /*
     Prohibit changing of the UNION list of a non-temporary MERGE table
@@ -6894,9 +6934,12 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
               lock here...
       */
       if (new_name != table_name || new_db != db)
-        mdl_release_exclusive_locks(&thd->mdl_context);
+      {
+        mdl_release_lock(&thd->mdl_context, target_lock_data);
+        mdl_release_all_locks_for_name(&thd->mdl_context, mdl_lock_data);
+      }
       else
-        mdl_downgrade_exclusive_locks(&thd->mdl_context);
+        mdl_downgrade_exclusive_lock(&thd->mdl_context, mdl_lock_data);
     }
     DBUG_RETURN(error);
   }
@@ -7575,10 +7618,11 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
       pthread_mutex_lock(&LOCK_open);
       unlink_open_table(thd, table, FALSE);
       pthread_mutex_unlock(&LOCK_open);
-      mdl_release_exclusive_locks(&thd->mdl_context);
+      mdl_release_lock(&thd->mdl_context, target_lock_data);
+      mdl_release_all_locks_for_name(&thd->mdl_context, mdl_lock_data);
     }
     else
-      mdl_downgrade_exclusive_locks(&thd->mdl_context);
+      mdl_downgrade_exclusive_lock(&thd->mdl_context, mdl_lock_data);
   }
 
 end_temporary:
@@ -7634,7 +7678,7 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
     thd->abort_on_warning= save_abort_on_warning;
   }
   if (target_lock_data)
-    mdl_release_exclusive_locks(&thd->mdl_context);
+    mdl_release_lock(&thd->mdl_context, target_lock_data);
   DBUG_RETURN(TRUE);
 
 err_with_placeholders:
@@ -7645,7 +7689,9 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
   */
   unlink_open_table(thd, table, FALSE);
   pthread_mutex_unlock(&LOCK_open);
-  mdl_release_exclusive_locks(&thd->mdl_context);
+  if (target_lock_data)
+    mdl_release_lock(&thd->mdl_context, target_lock_data);
+  mdl_release_all_locks_for_name(&thd->mdl_context, mdl_lock_data);
   DBUG_RETURN(TRUE);
 }
 /* mysql_alter_table */

sql/sql_trigger.cc

@@ -527,8 +527,9 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create)
     locks. Otherwise call to close_thread_tables() will take care about both
     TABLE instance created by reopen_name_locked_table() and meta-data lock.
   */
-  if (thd->locked_tables)
-    mdl_downgrade_exclusive_locks(&thd->mdl_context);
+  if (thd->locked_tables && tables && tables->table)
+    mdl_downgrade_exclusive_lock(&thd->mdl_context,
+                                 tables->table->mdl_lock_data);
 
   if (need_start_waiting)
     start_waiting_global_read_lock(thd);