List:Commits« Previous MessageNext Message »
From:konstantin Date:July 13 2007 8:18pm
Subject:bk commit into 5.1 tree (kostja:1.2528) BUG#21074
View as plain text  
Below is the list of changes that have just been committed into a local
5.1 repository of kostja. When kostja does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2007-07-14 00:18:12+04:00, kostja@bodhi.(none) +5 -0
  A follow up after the patch for Bug#21074 - even though
  we now have exclusive name lock on the table name in mysql_rm_table_part2,
  we still should keep LOCK_open - some storage engines are not
  ready for locking scope change and assume that LOCK_open is kept.
  Still, the binary logging and query cache invalidation calls
  moved out of LOCK_open scope.
  Fixes some of the broken 5.1-runtime tests (tests break on asserts).

  sql/ha_ndbcluster.cc@stripped, 2007-07-14 00:18:08+04:00, kostja@bodhi.(none) +2 -4
    Do not lock LOCK_open for mysql_rm_table_part2 - it does that
    for us now.

  sql/mysql_priv.h@stripped, 2007-07-14 00:18:08+04:00, kostja@bodhi.(none) +1 -2
    Remove an unused flag.

  sql/sql_class.h@stripped, 2007-07-14 00:18:08+04:00, kostja@bodhi.(none) +1 -1
    Fix an unrelated compiler warning.

  sql/sql_db.cc@stripped, 2007-07-14 00:18:08+04:00, kostja@bodhi.(none) +1 -1
    Adjust to the changed signature.

  sql/sql_table.cc@stripped, 2007-07-14 00:18:08+04:00, kostja@bodhi.(none) +12 -23
    mysql_rm_table_part2: we need to keep LOCK_open while calling
    storage engine functions, even though now 
    we have an exclusive lock on the table name. Some of them assume that it's 
    kept and attempt to unlock it.
     

diff -Nrup a/sql/ha_ndbcluster.cc b/sql/ha_ndbcluster.cc
--- a/sql/ha_ndbcluster.cc	2007-07-02 23:00:12 +04:00
+++ b/sql/ha_ndbcluster.cc	2007-07-14 00:18:08 +04:00
@@ -7019,8 +7019,6 @@ int ndbcluster_find_files(handlerton *ht
     }
   }
 
-  // Lock mutex before deleting and creating frm files
-  pthread_mutex_lock(&LOCK_open);
   if (!global_read_lock)
   {
     // Delete old files
@@ -7037,14 +7035,14 @@ int ndbcluster_find_files(handlerton *ht
                                  FALSE,   /* if_exists */
                                  FALSE,   /* drop_temporary */ 
                                  FALSE,   /* drop_view */
-                                 TRUE,    /* dont_log_query*/ 
-                                 FALSE);  /* need lock open */
+                                 TRUE     /* dont_log_query*/);
 
       /* Clear error message that is returned when table is deleted */
       thd->clear_error();
     }
   }
 
+  pthread_mutex_lock(&LOCK_open);
   // Create new files
   List_iterator_fast<char> it2(create_list);
   while ((file_name=it2++))
diff -Nrup a/sql/mysql_priv.h b/sql/mysql_priv.h
--- a/sql/mysql_priv.h	2007-07-02 23:00:12 +04:00
+++ b/sql/mysql_priv.h	2007-07-14 00:18:08 +04:00
@@ -901,8 +901,7 @@ void mysql_client_binlog_statement(THD *
 bool mysql_rm_table(THD *thd,TABLE_LIST *tables, my_bool if_exists,
                     my_bool drop_temporary);
 int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists,
-                         bool drop_temporary, bool drop_view, bool log_query,
-                         bool need_lock_open);
+                         bool drop_temporary, bool drop_view, bool log_query);
 bool quick_rm_table(handlerton *base,const char *db,
                     const char *table_name, uint flags);
 void close_cached_table(THD *thd, TABLE *table);
diff -Nrup a/sql/sql_class.h b/sql/sql_class.h
--- a/sql/sql_class.h	2007-07-02 02:03:22 +04:00
+++ b/sql/sql_class.h	2007-07-14 00:18:08 +04:00
@@ -1999,8 +1999,8 @@ class select_insert :public select_resul
 class select_create: public select_insert {
   ORDER *group;
   TABLE_LIST *create_table;
-  TABLE_LIST *select_tables;
   HA_CREATE_INFO *create_info;
+  TABLE_LIST *select_tables;
   Alter_info *alter_info;
   Field **field;
 public:
diff -Nrup a/sql/sql_db.cc b/sql/sql_db.cc
--- a/sql/sql_db.cc	2007-07-02 23:00:12 +04:00
+++ b/sql/sql_db.cc	2007-07-14 00:18:08 +04:00
@@ -1113,7 +1113,7 @@ static long mysql_rm_known_files(THD *th
     }
   }
   if (thd->killed ||
-      (tot_list && mysql_rm_table_part2(thd, tot_list, 1, 0, 1, 1, 1)))
+      (tot_list && mysql_rm_table_part2(thd, tot_list, 1, 0, 1, 1)))
     goto err;
 
   /* Remove RAID directories */
diff -Nrup a/sql/sql_table.cc b/sql/sql_table.cc
--- a/sql/sql_table.cc	2007-07-02 23:00:12 +04:00
+++ b/sql/sql_table.cc	2007-07-14 00:18:08 +04:00
@@ -1430,11 +1430,7 @@ bool mysql_rm_table(THD *thd,TABLE_LIST 
     LOCK_open during wait_if_global_read_lock(), other threads could not
     close their tables. This would make a pretty deadlock.
   */
-  error= mysql_rm_table_part2(thd, tables, if_exists, drop_temporary, 0, 0, 1);
-  pthread_mutex_lock(&thd->mysys_var->mutex);
-  thd->mysys_var->current_mutex= 0;
-  thd->mysys_var->current_cond= 0;
-  pthread_mutex_unlock(&thd->mysys_var->mutex);
+  error= mysql_rm_table_part2(thd, tables, if_exists, drop_temporary, 0, 0);
 
   if (need_start_waiters)
     start_waiting_global_read_lock(thd);
@@ -1477,7 +1473,7 @@ bool mysql_rm_table(THD *thd,TABLE_LIST 
 
 int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists,
 			 bool drop_temporary, bool drop_view,
-			 bool dont_log_query, bool need_lock_open)
+			 bool dont_log_query)
 {
   TABLE_LIST *table;
   char path[FN_REFLEN], *alias;
@@ -1489,8 +1485,7 @@ int mysql_rm_table_part2(THD *thd, TABLE
   String built_query;
   DBUG_ENTER("mysql_rm_table_part2");
 
-  if (need_lock_open)
-    pthread_mutex_lock(&LOCK_open);
+  pthread_mutex_lock(&LOCK_open);
 
   LINT_INIT(alias);
   LINT_INIT(path_length);
@@ -1528,14 +1523,10 @@ int mysql_rm_table_part2(THD *thd, TABLE
 
   if (!drop_temporary && lock_table_names_exclusively(thd, tables))
   {
-    if (need_lock_open)
-      pthread_mutex_unlock(&LOCK_open);
+    pthread_mutex_unlock(&LOCK_open);
     DBUG_RETURN(1);
   }
 
-  if (need_lock_open)
-    pthread_mutex_unlock(&LOCK_open);
-
   /* Don't give warnings for not found errors, as we already generate notes */
   thd->no_warnings_for_error= 1;
 
@@ -1545,7 +1536,7 @@ int mysql_rm_table_part2(THD *thd, TABLE
     handlerton *table_type;
     enum legacy_db_type frm_db_type;
 
-    mysql_ha_flush(thd, table, MYSQL_HA_CLOSE_FINAL, !need_lock_open);
+    mysql_ha_flush(thd, table, MYSQL_HA_CLOSE_FINAL, 1);
     if (!close_temporary_table(thd, table))
     {
       tmp_table_deleted=1;
@@ -1582,8 +1573,6 @@ int mysql_rm_table_part2(THD *thd, TABLE
     {
       TABLE *locked_table;
       abort_locked_tables(thd, db, table->table_name);
-      if (need_lock_open)
-        pthread_mutex_lock(&LOCK_open);
       remove_table_from_cache(thd, db, table->table_name,
 	                      RTFC_WAIT_OTHER_THREAD_FLAG |
 			      RTFC_CHECK_KILLED_FLAG);
@@ -1594,9 +1583,6 @@ int mysql_rm_table_part2(THD *thd, TABLE
       if ((locked_table= drop_locked_tables(thd, db, table->table_name)))
         table->table= locked_table;
 
-      if (need_lock_open)
-        pthread_mutex_unlock(&LOCK_open);
-
       if (thd->killed)
       {
         thd->no_warnings_for_error= 0;
@@ -1663,6 +1649,11 @@ int mysql_rm_table_part2(THD *thd, TABLE
       wrong_tables.append(String(table->table_name,system_charset_info));
     }
   }
+  /*
+    It's safe to unlock LOCK_open: we have an exclusive lock 
+    on the table name.
+  */
+  pthread_mutex_unlock(&LOCK_open);
   thd->tmp_table_used= tmp_table_deleted;
   error= 0;
   if (wrong_tables.length())
@@ -1722,11 +1713,9 @@ int mysql_rm_table_part2(THD *thd, TABLE
       */
     }
   }
-  if (need_lock_open)
-    pthread_mutex_lock(&LOCK_open);
+  pthread_mutex_lock(&LOCK_open);
   unlock_table_names(thd, tables, (TABLE_LIST*) 0);
-  if (need_lock_open)
-    pthread_mutex_unlock(&LOCK_open);
+  pthread_mutex_unlock(&LOCK_open);
   thd->no_warnings_for_error= 0;
   DBUG_RETURN(error);
 }
Thread
bk commit into 5.1 tree (kostja:1.2528) BUG#21074konstantin13 Jul