MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Davi Arnaut Date:October 4 2007 8:34pm
Subject:bk commit into 5.0 tree (davi:1.2526) BUG#21587
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 repository of davi. When davi 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-10-04 17:34:41-03:00, davi@stripped +1 -0
  Bug#21587 FLUSH TABLES causes server crash when used with HANDLER statements
  
  This bug is a symptom of the way handler's tables are managed. The
  most different aspect, compared to the conventional behavior, is that
  the handler's tables are long lived, meaning that their lifetimes are
  not bounded by the duration of the command that opened them. For this
  effect the handler code uses its own list (handler_tables instead of
  open_tables) to hold open handler tables so that the tables won't be
  closed at the end of the command/statement. Besides the handler_tables
  list, there is a hash (handler_tables_hash) which is used to associate
  handler aliases to tables and to refresh the tables upon demand (flush
  tables).
  
  The current implementation doesn't work properly with refreshed tables
  -- more precisely when flush commands are issued by other initiators.
  This happens because when a handler open or read statement is being
  processed, the associated table has to be opened or locked and, for this
  matter, the open_tables and handler_tables lists are swapped so that the
  new table being opened is inserted into the handler_tables list. But when
  opening or locking the table, if the refresh version is different from the
  thread refresh version then all used tables in the open_tables list (now
  handler_tables) are refreshed. In the "refreshing" process the handler
  tables are flushed (closed) without being properly unlinked from the
  handler hash.
  
  The current implementation also fails to properly discard handlers of
  dropped tables, but this and other problems are going to be addressed
  in the fixes for bugs 31397 and 31409.
  
  The chosen approach tries to properly save and restore the table state
  so that no table is flushed during the table open and lock operations.
  The logic is almost the same as before with the list swapping, but with
  a working glue code.
  
  The test case for this bug is going to be committed into 5.1 because it
  requires a test feature only avaiable in 5.1 (wait_condition).

  sql/sql_handler.cc@stripped, 2007-10-04 17:34:38-03:00, davi@stripped +41 -12
    Backup and reset the open_tables and handler_table lists when opening
    a new handler table and merge the opened table into the handler_tables
    list afterwards. When locking, do the same but possibly close the table
    if a refresh is pending.

diff -Nrup a/sql/sql_handler.cc b/sql/sql_handler.cc
--- a/sql/sql_handler.cc	2007-08-27 10:37:10 -03:00
+++ b/sql/sql_handler.cc	2007-10-04 17:34:38 -03:00
@@ -65,11 +65,6 @@
 static enum enum_ha_read_modes rkey_to_rnext[]=
 { RNEXT_SAME, RNEXT, RPREV, RNEXT, RPREV, RNEXT, RPREV, RPREV };
 
-#define HANDLER_TABLES_HACK(thd) {      \
-  TABLE *tmp=thd->open_tables;          \
-  thd->open_tables=thd->handler_tables; \
-  thd->handler_tables=tmp; }
-
 static int mysql_ha_flush_table(THD *thd, TABLE **table_ptr, uint mode_flags);
 
 
@@ -187,6 +182,7 @@ bool mysql_ha_open(THD *thd, TABLE_LIST 
   char          *db, *name, *alias;
   uint          dblen, namelen, aliaslen, counter;
   int           error;
+  TABLE         *backup_open_tables, *backup_handler_tables;
   DBUG_ENTER("mysql_ha_open");
   DBUG_PRINT("enter",("'%s'.'%s' as '%s'  reopen: %d",
                       tables->db, tables->table_name, tables->alias,
@@ -215,18 +211,31 @@ bool mysql_ha_open(THD *thd, TABLE_LIST 
     }
   }
 
+  /* save open_ and handler_ tables state */
+  backup_open_tables= thd->open_tables;
+  backup_handler_tables= thd->handler_tables;
+
+  /* no pre-opened tables */
+  thd->open_tables= NULL;
+  /* to avoid flushes */
+  thd->handler_tables= NULL;
+
   /*
     open_tables() will set 'tables->table' if successful.
     It must be NULL for a real open when calling open_tables().
   */
   DBUG_ASSERT(! tables->table);
-  HANDLER_TABLES_HACK(thd);
 
   /* for now HANDLER can be used only for real TABLES */
   tables->required_type= FRMTYPE_TABLE;
   error= open_tables(thd, &tables, &counter, 0);
 
-  HANDLER_TABLES_HACK(thd);
+  /* restore the state and merge the opened table into handler_tables list */
+  thd->handler_tables= thd->open_tables ?
+                       thd->open_tables->next= backup_handler_tables,
+                       thd->open_tables : backup_handler_tables;
+  thd->open_tables= backup_open_tables;
+
   if (error)
     goto err;
 
@@ -351,7 +360,7 @@ bool mysql_ha_read(THD *thd, TABLE_LIST 
                    ha_rows select_limit_cnt, ha_rows offset_limit_cnt)
 {
   TABLE_LIST    *hash_tables;
-  TABLE         *table;
+  TABLE         *table, *backup_open_tables, *backup_handler_tables;
   MYSQL_LOCK    *lock;
   List<Item>	list;
   Protocol	*protocol= thd->protocol;
@@ -361,7 +370,7 @@ bool mysql_ha_read(THD *thd, TABLE_LIST 
   uint          num_rows;
   byte		*key;
   uint		key_len;
-  bool          not_used;
+  bool          need_reopen;
   DBUG_ENTER("mysql_ha_read");
   DBUG_PRINT("enter",("'%s'.'%s' as '%s'",
                       tables->db, tables->table_name, tables->alias));
@@ -375,6 +384,7 @@ bool mysql_ha_read(THD *thd, TABLE_LIST 
   List_iterator<Item> it(list);
   it++;
 
+retry:
   if ((hash_tables= (TABLE_LIST*) hash_search(&thd->handler_tables_hash,
                                               (byte*) tables->alias,
                                               strlen(tables->alias) + 1)))
@@ -427,9 +437,28 @@ bool mysql_ha_read(THD *thd, TABLE_LIST 
   }
   tables->table=table;
 
-  HANDLER_TABLES_HACK(thd);
-  lock= mysql_lock_tables(thd, &tables->table, 1, 0, &not_used);
-  HANDLER_TABLES_HACK(thd);
+  /* save open_ and handler_ tables state */
+  backup_open_tables= thd->open_tables;
+  backup_handler_tables= thd->handler_tables;
+
+  /* no pre-opened tables */
+  thd->open_tables= NULL;
+  /* to avoid flushes */
+  thd->handler_tables= NULL;
+
+  lock= mysql_lock_tables(thd, &tables->table, 1,
+                          MYSQL_LOCK_NOTIFY_IF_NEED_REOPEN, &need_reopen);
+
+  /* restore previous context */
+  thd->handler_tables= backup_handler_tables;
+  thd->open_tables= backup_open_tables;
+
+  if (need_reopen)
+  {
+    mysql_ha_close_table(thd, tables);
+    hash_tables->table= NULL;
+    goto retry;
+  }
 
   if (!lock)
     goto err0; // mysql_lock_tables() printed error message already
Thread
bk commit into 5.0 tree (davi:1.2526) BUG#21587Davi Arnaut4 Oct