MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:dlenev Date:February 16 2006 1:19pm
Subject:bk commit into 5.0 tree (dlenev:1.2023) BUG#16593
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 repository of dlenev. When dlenev 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
  1.2023 06/02/16 16:19:24 dlenev@stripped +4 -0
  Fix for bug #16593 "Deadlock or crash in stress test for case where
  trigger starts trigger".
  
  In short, the deadlock/crash happened when execution of statement, which used
  stored functions or activated triggers, coincided with alteration of the
  tables used by these functions or triggers (in highly concurrent environment).
  
  Bug was caused by the incorrect handling of tables from prelocked set in
  open_tables() functions in situations when refresh happened. This fix replaces
  old smart but not very robust way of handling tables after refresh (which was
  closing only old tables), with new one which simply closes all tables opened so
  far and restarts open_tables().
  Also fixed handling of temporary tables in close_tables_for_reopen().
  
  No test case present since bug manifests itself only in concurrent environment.

  sql/sql_update.cc
    1.184 06/02/16 16:19:19 dlenev@stripped +2 -2
    In order to handle correctly case when table list completely consists from tables
    from prelocked set close_tables_for_reopen() have to accept table list as in/out
    parameter.

  sql/sql_prepare.cc
    1.168 06/02/16 16:19:19 dlenev@stripped +1 -1
    In order to handle correctly case when table list completely consists from tables
    from prelocked set close_tables_for_reopen() have to accept table list as in/out
    parameter.

  sql/sql_base.cc
    1.325 06/02/16 16:19:19 dlenev@stripped +27 -57
    open_tables():
      Removed part of comment  which was out of date.
      Changed handling of case when refresh happens during opening of tables, now
      instead of having code which decides for each table if it should be closed
      we simply close all tables. Old code also incorrectly handled tables from
      prelocked set in this situation which resulted in bug #16593 "Deadlock or
      crash in stress test for case where triggers starting trigger".
    close_tables_for_reopen():
      Now we correctly handle the case when table list completely consists from
      tables from prelocked set. Also now we simply close all tables instead
      leaving temporary tables non-closed (such approach allows easily handle
      correctly tables from prelocked set).

  sql/mysql_priv.h
    1.373 06/02/16 16:19:18 dlenev@stripped +1 -1
    In order to handle correctly case when table list completely consists from tables
    from prelocked set close_tables_for_reopen() have to accept table list as in/out
    parameter.

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	dlenev
# Host:	brandersnatch.site
# Root:	/home/dlenev/src/mysql-5.0-bg16593

--- 1.372/sql/mysql_priv.h	2005-12-27 12:30:49 +03:00
+++ 1.373/sql/mysql_priv.h	2006-02-16 16:19:18 +03:00
@@ -981,7 +981,7 @@
 void intern_close_table(TABLE *entry);
 bool close_thread_table(THD *thd, TABLE **table_ptr);
 void close_temporary_tables(THD *thd);
-void close_tables_for_reopen(THD *thd, TABLE_LIST *tables);
+void close_tables_for_reopen(THD *thd, TABLE_LIST **tables);
 TABLE_LIST *find_table_in_list(TABLE_LIST *table,
                                uint offset_to_list,
                                const char *db_name,

--- 1.324/sql/sql_base.cc	2006-01-12 21:28:18 +03:00
+++ 1.325/sql/sql_base.cc	2006-02-16 16:19:19 +03:00
@@ -1982,22 +1982,11 @@
     statement for which table list for prelocking is already built, let
     us cache routines and try to build such table list.
 
-    NOTE: We can't delay prelocking until we will met some sub-statement
-    which really uses tables, since this will imply that we have to restore
-    its table list to be able execute it in some other context.
-    And current views implementation assumes that view tables are added to
-    global table list only once during PS preparing/first SP execution.
-    Also locking at earlier stage is probably faster altough may decrease
-    concurrency a bit.
-
     NOTE: We will mark statement as requiring prelocking only if we will
     have non empty table list. But this does not guarantee that in prelocked
     mode we will have some locked tables, because queries which use only
     derived/information schema tables and views possible. Thus "counter"
     may be still zero for prelocked statement...
-
-    NOTE: The above notes may be out of date. Please wait for psergey to
-          document new prelocked behavior.
   */
 
   if (!thd->prelocked_mode && !thd->lex->requires_prelocking() &&
@@ -2083,48 +2072,23 @@
 
       if (refresh)				// Refresh in progress
       {
-	/* close all 'old' tables used by this thread */
-	pthread_mutex_lock(&LOCK_open);
-	// if query_id is not reset, we will get an error
-	// re-opening a temp table
-	thd->version=refresh_version;
-	TABLE **prev_table= &thd->open_tables;
-	bool found=0;
-	for (TABLE_LIST *tmp= *start; tmp; tmp= tmp->next_global)
-	{
-	  /* Close normal (not temporary) changed tables */
-	  if (tmp->table && ! tmp->table->s->tmp_table)
-	  {
-	    if (tmp->table->s->version != refresh_version ||
-		! tmp->table->db_stat)
-	    {
-	      VOID(hash_delete(&open_cache,(byte*) tmp->table));
-	      tmp->table=0;
-	      found=1;
-	    }
-	    else
-	    {
-	      *prev_table= tmp->table;		// Relink open list
-	      prev_table= &tmp->table->next;
-	    }
-	  }
-	}
-	*prev_table=0;
-	pthread_mutex_unlock(&LOCK_open);
-	if (found)
-	  VOID(pthread_cond_broadcast(&COND_refresh)); // Signal to refresh
         /*
-          Let us prepare for recalculation of set of prelocked tables.
-          First we pretend that we have finished calculation which we
-          were doing currently. Then we restore list of tables to be
-          opened and set of used routines to the state in which they were
-          before first open_tables() call for this statement (i.e. before
-          we have calculated current set of tables for prelocking).
+          We have met name-locked or old version of table. Now we have
+          to close all tables which are not up to date. We also have to
+          throw away set of prelocked tables (and thus close tables from
+          this set that were open by now) since it possible that one of
+          tables which determined its content was changed.
+
+          Instead of implementing complex/non-robust logic mentioned
+          above we simply close and then reopen all tables.
+
+          In order to prepare for recalculation of set of prelocked tables
+          we pretend that we have finished calculation which we were doing
+          currently.
         */
         if (query_tables_last_own)
           thd->lex->mark_as_requiring_prelocking(query_tables_last_own);
-        thd->lex->chop_off_not_own_tables();
-        sp_remove_not_own_routines(thd->lex);
+        close_tables_for_reopen(thd, start);
 	goto restart;
       }
       result= -1;				// Fatal error
@@ -2335,7 +2299,7 @@
       break;
     if (!need_reopen)
       DBUG_RETURN(-1);
-    close_tables_for_reopen(thd, tables);
+    close_tables_for_reopen(thd, &tables);
   }
   DBUG_RETURN(0);
 }
@@ -2372,7 +2336,7 @@
       break;
     if (!need_reopen)
       DBUG_RETURN(-1);
-    close_tables_for_reopen(thd, tables);
+    close_tables_for_reopen(thd, &tables);
   }
   if (mysql_handle_derived(thd->lex, &mysql_derived_prepare) ||
       (thd->fill_derived_tables() &&
@@ -2600,18 +2564,24 @@
 
   SYNOPSIS
     close_tables_for_reopen()
-      thd     Thread context
-      tables  List of tables which we were trying to open and lock
+      thd    in     Thread context
+      tables in/out List of tables which we were trying to open and lock
 
 */
 
-void close_tables_for_reopen(THD *thd, TABLE_LIST *tables)
+void close_tables_for_reopen(THD *thd, TABLE_LIST **tables)
 {
+  /*
+    If table list consists only from tables from prelocking set, table list
+    for new attempt should be empty, so we have to update list's root pointer.
+  */
+  if (thd->lex->first_not_own_table() == *tables)
+    *tables= 0;
   thd->lex->chop_off_not_own_tables();
   sp_remove_not_own_routines(thd->lex);
-  for (TABLE_LIST *tmp= tables; tmp; tmp= tmp->next_global)
-    if (tmp->table && !tmp->table->s->tmp_table)
-      tmp->table= 0;
+  for (TABLE_LIST *tmp= *tables; tmp; tmp= tmp->next_global)
+    tmp->table= 0;
+  mark_used_tables_as_free_for_reuse(thd, thd->temporary_tables);
   close_thread_tables(thd);
 }
 

--- 1.183/sql/sql_update.cc	2005-12-28 17:20:17 +03:00
+++ 1.184/sql/sql_update.cc	2006-02-16 16:19:19 +03:00
@@ -158,7 +158,7 @@
       break;
     if (!need_reopen)
       DBUG_RETURN(1);
-    close_tables_for_reopen(thd, table_list);
+    close_tables_for_reopen(thd, &table_list);
   }
 
   if (mysql_handle_derived(thd->lex, &mysql_derived_prepare) ||
@@ -813,7 +813,7 @@
     for (TABLE_LIST *tbl= table_list; tbl; tbl= tbl->next_global)
       tbl->cleanup_items();
 
-    close_tables_for_reopen(thd, table_list);
+    close_tables_for_reopen(thd, &table_list);
     goto reopen_tables;
   }
 

--- 1.167/sql/sql_prepare.cc	2006-01-24 15:48:13 +03:00
+++ 1.168/sql/sql_prepare.cc	2006-02-16 16:19:19 +03:00
@@ -1135,7 +1135,7 @@
       break;
     if (!need_reopen)
       goto error;
-    close_tables_for_reopen(thd, table_list);
+    close_tables_for_reopen(thd, &table_list);
   }
 
   /*
Thread
bk commit into 5.0 tree (dlenev:1.2023) BUG#16593dlenev16 Feb