List:Commits« Previous MessageNext Message »
From:ingo Date:May 22 2006 7:50pm
Subject:bk commit into 4.0 tree (ingo:1.2180) BUG#17436
View as plain text  
Below is the list of changes that have just been committed into a local
4.0 repository of mydev. When mydev 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.2180 06/05/22 21:50:42 ingo@stripped +11 -0
  Bug#17436 - ALTER TABLE causes pending INSERT DELAYED updates to fail.
  
  The problem was that LOCK TABLE WRITE placed a normal write lock 
  on the table, which could not be changed. Normally ALTER TABLE 
  acquires a special write lock that allows readers. This lock type 
  blocks delayed inserts. But the normal write lock must not block 
  delayed inserts. This is the intended purpose of delayed inserts.
  
  So it was possible that delayed inserts were added to the queue
  before and even while ALTER TABLE was active. But the delayed
  insert thread could not put the records into the table while
  the write lock was in place.
  
  The solution is to trigger the thread at the beginning of ALTER
  TABLE. The thread is granted to bypass the existing write lock.
  And it is marked as blocked for further delayed inserts. The
  ALTER TABLE thread waits until the queued records are flushed.
  After ALTER TABLE the delayed thread is finally terminated.
  After this point in time delayed inserts will create a new
  thread. Everything proceeds as normal.
  
  Delayed inserts coming in during ALTER TABLE will be converted
  to normal inserts and block until the end of ALTER TABLE.

  sql/sql_table.cc
    1.200 06/05/22 21:50:41 ingo@stripped +9 -1
    Bug#17436 - ALTER TABLE causes pending INSERT DELAYED updates to fail.
    Added a call of the new function flush_delayed_inserts()
    to mysql_alter_table() before starting the copy of the data.

  sql/sql_insert.cc
    1.120 06/05/22 21:50:41 ingo@stripped +171 -26
    Bug#17436 - ALTER TABLE causes pending INSERT DELAYED updates to fail.
    Added the elements 'waiting_for_termination' and 'blocked'.
    The former is incremented and decremented by threads that
    want to wait for the delayed threads termination. The latter
    is used block further delayed inserts until the thread is
    terminated.
    Initialized di->thd.di for use as an indicator that the thread
    is not terminated.
    Added the new function kill_delayed_thread() which allows to
    wait for the termination and block further delayed inserts.
    Added a loop to handle_delayed_insert() to handle the wait
    for termination.

  sql/sql_base.cc
    1.195 06/05/22 21:50:41 ingo@stripped +70 -0
    Bug#17436 - ALTER TABLE causes pending INSERT DELAYED updates to fail.
    Added new function flush_delayed_inserts().

  sql/mysql_priv.h
    1.240 06/05/22 21:50:41 ingo@stripped +2 -0
    Bug#17436 - ALTER TABLE causes pending INSERT DELAYED updates to fail.
    Added declarations for the new functions
    kill_delayed_thread() and flush_delayed_inserts().

  mysys/thr_lock.c
    1.40 06/05/22 21:50:41 ingo@stripped +110 -11
    Bug#17436 - ALTER TABLE causes pending INSERT DELAYED updates to fail.
    Added the lock bypass feature: A lock can be granted to bypass
    another lock.
    Changed check_lock() and wait_for_lock().
    Added thr_lock_make_current() and thr_lock_grant_bypass().

  mysql-test/t/delayed.test
    1.7 06/05/22 21:50:41 ingo@stripped +26 -0
    Bug#17436 - ALTER TABLE causes pending INSERT DELAYED updates to fail.
    The test case.

  mysql-test/r/delayed.result
    1.7 06/05/22 21:50:41 ingo@stripped +9 -0
    Bug#17436 - ALTER TABLE causes pending INSERT DELAYED updates to fail.
    The test result.

  myisam/mi_open.c
    1.68 06/05/22 21:50:41 ingo@stripped +7 -4
    Bug#17436 - ALTER TABLE causes pending INSERT DELAYED updates to fail.
    Added DBUG calls.

  myisam/mi_info.c
    1.14 06/05/22 21:50:41 ingo@stripped +9 -0
    Bug#17436 - ALTER TABLE causes pending INSERT DELAYED updates to fail.
    Added a call to mi_get_status() to fetch state info from the
    table share if requested and necessary.

  include/thr_lock.h
    1.15 06/05/22 21:50:41 ingo@stripped +2 -0
    Bug#17436 - ALTER TABLE causes pending INSERT DELAYED updates to fail.
    Added the THR_LOCK_DATA structure element 'bypass'.
    It is designed to receive the address of lock data that may be
    bypassed by this lock.
    Added an access function to set the bypass pointer.

  include/my_base.h
    1.42 06/05/22 21:50:40 ingo@stripped +1 -0
    Bug#17436 - ALTER TABLE causes pending INSERT DELAYED updates to fail.
    Added flag value HA_STATUS_VAR_SHARED for querying state info
    from the table share.

# 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:	ingo
# Host:	chilla.local
# Root:	/home/mydev/mysql-4.0-bug17436

--- 1.41/include/my_base.h	2003-12-12 21:26:56 +01:00
+++ 1.42/include/my_base.h	2006-05-22 21:50:40 +02:00
@@ -227,6 +227,7 @@
 #define HA_STATUS_VARIABLE	16
 #define HA_STATUS_ERRKEY	32
 #define HA_STATUS_AUTO		64
+#define HA_STATUS_VAR_SHARED    128     /* Copy variable state from shared */
 
 	/* Errorcodes given by functions */
 

--- 1.14/include/thr_lock.h	2005-07-26 16:53:32 +02:00
+++ 1.15/include/thr_lock.h	2006-05-22 21:50:41 +02:00
@@ -69,6 +69,7 @@
 typedef struct st_thr_lock_data {
   pthread_t thread;
   struct st_thr_lock_data *next,**prev;
+  struct st_thr_lock_data *bypass;      /* Grant to bypass this lock. */
   struct st_thr_lock *lock;
   pthread_cond_t *cond;
   enum thr_lock_type type;
@@ -109,6 +110,7 @@
 void thr_abort_locks(THR_LOCK *lock);
 my_bool thr_abort_locks_for_thread(THR_LOCK *lock, pthread_t thread);
 void thr_print_locks(void);		/* For debugging */
+void thr_lock_grant_bypass(THR_LOCK_DATA *data, THR_LOCK_DATA *bypass);
 my_bool thr_upgrade_write_delay_lock(THR_LOCK_DATA *data);
 my_bool thr_reschedule_write_lock(THR_LOCK_DATA *data);
 #ifdef	__cplusplus

--- 1.13/myisam/mi_info.c	2003-06-26 06:56:53 +02:00
+++ 1.14/myisam/mi_info.c	2006-05-22 21:50:41 +02:00
@@ -50,6 +50,15 @@
   }
   if (flag & HA_STATUS_VARIABLE)
   {
+    /*
+      In special situations like ALTER TABLE with a LOCK TABLE WRITE and
+      INSERT DELAYED we need to copy the state from the share to local.
+      It can have been modified by another thread and we want to read
+      all rows.
+    */
+    if ((flag & HA_STATUS_VAR_SHARED) && (info->state == &info->save_state))
+      mi_get_status((void*) info);
+
     x->records	 	= info->state->records;
     x->deleted	 	= info->state->del;
     x->delete_length	= info->state->empty;

--- 1.67/myisam/mi_open.c	2004-11-22 19:18:32 +01:00
+++ 1.68/myisam/mi_open.c	2006-05-22 21:50:41 +02:00
@@ -795,6 +795,8 @@
 char *mi_state_info_read(char *ptr, MI_STATE_INFO *state)
 {
   uint i,keys,key_parts,key_blocks;
+  DBUG_ENTER("mi_state_info_read");
+
   memcpy_fixed(&state->header,ptr, sizeof(state->header));
   ptr +=sizeof(state->header);
   keys=(uint) state->header.keys;
@@ -841,26 +843,27 @@
   {
     state->rec_per_key_part[i]= mi_uint4korr(ptr); ptr+=4;
   }
-  return ptr;
+  DBUG_RETURN(ptr);
 }
 
 
 uint mi_state_info_read_dsk(File file, MI_STATE_INFO *state, my_bool pRead)
 {
   char	buff[MI_STATE_INFO_SIZE + MI_STATE_EXTRA_SIZE];
+  DBUG_ENTER("mi_state_info_read_dsk");
 
   if (!myisam_single_user)
   {
     if (pRead)
     {
       if (my_pread(file, buff, state->state_length,0L, MYF(MY_NABP)))
-	return (MY_FILE_ERROR);
+	DBUG_RETURN(MY_FILE_ERROR);
     }
     else if (my_read(file, buff, state->state_length,MYF(MY_NABP)))
-      return (MY_FILE_ERROR);
+      DBUG_RETURN(MY_FILE_ERROR);
     mi_state_info_read(buff, state);
   }
-  return 0;
+  DBUG_RETURN(0);
 }
 
 

--- 1.39/mysys/thr_lock.c	2005-07-26 16:53:32 +02:00
+++ 1.40/mysys/thr_lock.c	2006-05-22 21:50:41 +02:00
@@ -139,8 +139,10 @@
 		count, lock_type, where);
 	return 1;
       }
+      /* Don't complain about allowed or bypass locks. */
       if (same_thread && ! pthread_equal(data->thread,first_thread) &&
-	  last_lock_type != TL_WRITE_ALLOW_WRITE)
+          (last_lock_type != TL_WRITE_ALLOW_WRITE) &&
+          (list->data->bypass != data))
       {
 	fprintf(stderr,
 		"Warning: Found locks from different threads in %s: %s\n",
@@ -178,6 +180,7 @@
 {
   uint old_found_errors=found_errors;
   DBUG_ENTER("check_locks");
+  DBUG_PRINT("info", ("checking locks at: %s", where));
 
   if (found_errors < MAX_FOUND_ERRORS)
   {
@@ -365,12 +368,57 @@
 }
 
 
+/*
+  Make a lock the current lock.
+
+  SYNOPSIS
+    thr_lock_make_current()
+      data                      The lock data for the new current lock.
+      lock                      The lock to change.
+
+  DESCRIPTION
+    Move the lock data to the top of the "running fifo".
+    So it becomes the current lock.
+
+  NOTE
+    Currently this is for write locks only.
+
+  RETURN
+    void
+*/
+
+static void thr_lock_make_current(THR_LOCK_DATA *data, THR_LOCK *lock)
+{
+  /* Remove from running fifo or wait list. */
+  if (((*data->prev)= data->next))
+    data->next->prev= data->prev;
+  else
+  {
+    /* It was last in either the running fifo or the wait list.*/
+    if (lock->write.last == &data->next)
+      lock->write.last= data->prev;
+    else
+      lock->write_wait.last= data->prev;
+  }
+
+  /* Put as first into running fifo. */
+  if ((data->next= lock->write.data))
+    data->next->prev= &data->next;
+  else
+    lock->write.last= &data->next;
+  data->prev= &lock->write.data;
+  lock->write.data= data;
+}
+
+
 static my_bool wait_for_lock(struct st_lock_list *wait, THR_LOCK_DATA *data,
 			     my_bool in_wait_list)
 {
   pthread_cond_t *cond=get_cond();
   struct st_my_thread_var *thread_var=my_thread_var;
   int result;
+  THR_LOCK *lock= data->lock;
+  DBUG_ENTER("wait_for_lock");
 
   if (!in_wait_list)
   {
@@ -380,16 +428,26 @@
   }
 
   /* Set up control struct to allow others to abort locks */
-  thread_var->current_mutex= &data->lock->mutex;
+  thread_var->current_mutex= &lock->mutex;
   thread_var->current_cond=  cond;
-
   data->cond=cond;
-  while (!thread_var->abort || in_wait_list)
+
+  /*
+    If the lock that has to wait owns a grant to bypass the current lock,
+    then we don't need to wait (any longer).
+  */
+  while ((! thread_var->abort || in_wait_list) &&
+         (! data->bypass || (data->bypass != lock->write.data) ||
+          lock->read.data))
   {
-    pthread_cond_wait(cond,&data->lock->mutex);
+    pthread_cond_wait(cond, &lock->mutex);
     if (data->cond != cond)
       break;
   }
+  DBUG_PRINT("info", ("awoke abort: %d  in_wait_list: %d  data: 0x%lx  "
+                      "bypass: 0x%lx  wlock: 0x%lx  rlock: 0x%lx",
+                      thread_var->abort, in_wait_list, data, data->bypass,
+                      lock->write.data, lock->read.data));
 
   if (data->cond || data->type == TL_UNLOCK)
   {
@@ -402,24 +460,31 @@
     }
     data->type=TL_UNLOCK;			/* No lock */
     result=1;					/* Didn't get lock */
-    check_locks(data->lock,"failed wait_for_lock",0);
+    check_locks(lock, "failed wait_for_lock", 0);
   }
   else
   {
+    /*
+      If the lock is allowed to bypass the current lock, then make it
+      the current lock.
+    */
+    if (data->bypass && (data->bypass == lock->write.data))
+      thr_lock_make_current(data, lock);
+
     result=0;
     statistic_increment(locks_waited, &THR_LOCK_lock);
-    if (data->lock->get_status)
-      (*data->lock->get_status)(data->status_param);
-    check_locks(data->lock,"got wait_for_lock",0);
+    if (lock->get_status)
+      (*lock->get_status)(data->status_param);
+    check_locks(lock, "got wait_for_lock", 0);
   }
-  pthread_mutex_unlock(&data->lock->mutex);
+  pthread_mutex_unlock(&lock->mutex);
 
   /* The following must be done after unlock of lock->mutex */
   pthread_mutex_lock(&thread_var->mutex);
   thread_var->current_mutex= 0;
   thread_var->current_cond=  0;
   pthread_mutex_unlock(&thread_var->mutex);
-  return result;
+  DBUG_RETURN(result);
 }
 
 
@@ -1010,6 +1075,32 @@
 }
 
 
+/*
+  Grant a lock to bypass another lock.
+
+  SYNOPSIS
+    thr_lock_grant_bypass()
+      data                      The lock that receives the grant.
+      bypass                    The lock to be bypassed.
+
+  NOTE
+    Currently this works for write locks only.
+
+  RETURN
+    void
+*/
+
+void thr_lock_grant_bypass(THR_LOCK_DATA *data, THR_LOCK_DATA *bypass)
+{
+  DBUG_ENTER("thr_lock_grant_bypass");
+
+  if((data->type >= TL_WRITE_ALLOW_WRITE) &&
+     (bypass->type >= TL_WRITE_ALLOW_WRITE))
+    data->bypass= bypass;
+
+  DBUG_VOID_RETURN;
+}
+
 
 /* Upgrade a WRITE_DELAY lock to a WRITE_LOCK */
 
@@ -1021,21 +1112,27 @@
   pthread_mutex_lock(&lock->mutex);
   if (data->type == TL_UNLOCK || data->type >= TL_WRITE_LOW_PRIORITY)
   {
+    DBUG_PRINT("info", ("lock is already present"));
     pthread_mutex_unlock(&lock->mutex);
     DBUG_RETURN(data->type == TL_UNLOCK);	/* Test if Aborted */
   }
   check_locks(lock,"before upgrading lock",0);
   /* TODO:  Upgrade to TL_WRITE_CONCURRENT_INSERT in some cases */
   data->type=TL_WRITE;				/* Upgrade lock */
+  DBUG_PRINT("info", ("upgraded lock"));
 
   /* Check if someone has given us the lock */
   if (!data->cond)
   {
+    DBUG_PRINT("info", ("received lock from someone"));
     if (!lock->read.data)			/* No read locks */
     {						/* We have the lock */
+      /* Make it the current write lock. */
+      thr_lock_make_current(data, lock);
       if (data->lock->get_status)
 	(*data->lock->get_status)(data->status_param);
       pthread_mutex_unlock(&lock->mutex);
+      DBUG_PRINT("info", ("finished. no read locks"));
       DBUG_RETURN(0);
     }
 
@@ -1051,9 +1148,11 @@
     data->prev= &lock->write_wait.data;
     lock->write_wait.data=data;
     check_locks(lock,"upgrading lock",0);
+    DBUG_PRINT("info", ("swapped lock data"));
   }
   else
   {
+    DBUG_PRINT("info", ("waiting for lock"));
     check_locks(lock,"waiting for lock",0);
   }
   DBUG_RETURN(wait_for_lock(&lock->write_wait,data,1));

--- 1.239/sql/mysql_priv.h	2005-11-29 19:17:37 +01:00
+++ 1.240/sql/mysql_priv.h	2006-05-22 21:50:41 +02:00
@@ -481,6 +481,7 @@
 int mysql_insert(THD *thd,TABLE_LIST *table,List<Item> &fields,
 		 List<List_item> &values, enum_duplicates flag);
 void kill_delayed_threads(void);
+void kill_delayed_thread(THD *thd, THD *target_thd, bool wait_for_termination);
 int mysql_delete(THD *thd, TABLE_LIST *table, COND *conds, ORDER *order,
                  ha_rows rows, ulong options);
 int mysql_truncate(THD *thd, TABLE_LIST *table_list, bool dont_send_ok);
@@ -623,6 +624,7 @@
 int fill_record(List<Item> &fields,List<Item> &values, bool ignore_errors);
 int fill_record(Field **field,List<Item> &values, bool ignore_errors);
 OPEN_TABLE_LIST *list_open_tables(THD *thd, const char *wild);
+void flush_delayed_inserts(THD *thd, const char *db, const char *table_name);
 
 /* sql_calc.cc */
 bool eval_const_cond(COND *cond);

--- 1.194/sql/sql_base.cc	2006-01-23 19:50:25 +01:00
+++ 1.195/sql/sql_base.cc	2006-05-22 21:50:41 +02:00
@@ -2457,6 +2457,7 @@
         if ((in_use->system_thread & SYSTEM_THREAD_DELAYED_INSERT) &&
             ! in_use->killed)
         {
+          DBUG_PRINT("info",("Killing delayed insert thread"));
   	  in_use->killed=1;
 	  pthread_mutex_lock(&in_use->mysys_var->mutex);
 	  if (in_use->mysys_var->current_cond)
@@ -2516,6 +2517,75 @@
   }
   DBUG_RETURN(result);
 }
+
+
+/*
+  Flush delayed inserts from the queue into the table.
+
+  SYNOPSIS
+    flush_delayed_inserts()
+      thd                       The thread handle.
+      db                        The database (schema) name.
+      table_name                The table name.
+
+  DESCRIPTION
+    In cases like ALTER TABLE we need to flush all pending delayed
+    inserts into the table before we start copying it.
+
+  NOTE
+    This will also block further delayed inserts into this table
+    until the delayed insert thread is terminated.
+
+  RETURN
+    void
+*/
+
+void flush_delayed_inserts(THD *thd, const char *db, const char *table_name)
+{
+  char    key[MAX_DBKEY_LENGTH];
+  uint    key_length;
+  bool    found= FALSE;
+  TABLE   *table;
+  TABLE   *thd_table;
+  DBUG_ENTER("flush_delayed_inserts");
+
+  /* Construct the table cache key. */
+  key_length= (uint) (strmov(strmov(key, db) + 1, table_name) - key) + 1;
+
+  /* Find all opens for this table. */
+  for (table= (TABLE*) hash_search(&open_cache, (byte*) key, key_length);
+       table;
+       table= (TABLE*) hash_next(&open_cache, (byte*) key, key_length))
+  {
+    THD *in_use= table->in_use;
+
+    /*
+      Kill any delayed insert thread found for this table
+      and wait for its flush. This blocks the thread until another kill.
+    */
+    if (in_use && ! in_use->killed &&
+        (in_use->system_thread & SYSTEM_THREAD_DELAYED_INSERT))
+    {
+      kill_delayed_thread(thd, in_use, TRUE);
+      found= TRUE;
+    }
+
+  }
+
+  if (found)
+  {
+    /*
+      The delayed thread updated the table statistics in the table share,
+      but this thread may not see it if it has the table open and locked.
+      So copy the state from the share to local.
+    */
+    for (thd_table= thd->open_tables; thd_table; thd_table= thd_table->next)
+      thd_table->file->info(HA_STATUS_VARIABLE | HA_STATUS_VAR_SHARED);
+  }
+
+  DBUG_VOID_RETURN;
+}
+
 
 int setup_ftfuncs(THD *thd)
 {

--- 1.119/sql/sql_insert.cc	2006-05-21 10:45:50 +02:00
+++ 1.120/sql/sql_insert.cc	2006-05-22 21:50:41 +02:00
@@ -19,6 +19,7 @@
 
 #include "mysql_priv.h"
 #include "sql_acl.h"
+#include <assert.h>
 
 static int check_null_fields(THD *thd,TABLE *entry);
 static TABLE *delayed_get_table(THD *thd,TABLE_LIST *table_list);
@@ -543,8 +544,8 @@
   TABLE *table;
   pthread_mutex_t mutex;
   pthread_cond_t cond,cond_client;
-  volatile uint tables_in_use,stacked_inserts;
-  volatile bool status,dead;
+  volatile uint tables_in_use, stacked_inserts, waiting_for_termination;
+  volatile bool status, dead, blocked;
   COPY_INFO info;
   I_List<delayed_row> rows;
   uint group_count;
@@ -552,7 +553,8 @@
 
   delayed_insert()
     :locks_in_memory(0),
-     table(0),tables_in_use(0),stacked_inserts(0), status(0), dead(0),
+     table(0), tables_in_use(0), stacked_inserts(0),
+     waiting_for_termination(0), status(0), dead(0), blocked(0),
      group_count(0)
   {
     thd.user=thd.priv_user=(char*) delayed_user;
@@ -560,6 +562,7 @@
     thd.current_tablenr=0;
     thd.version=refresh_version;
     thd.command=COM_DELAYED_INSERT;
+    thd.di= this;
 
     bzero((char*) &thd.net,sizeof(thd.net));	// Safety
     thd.system_thread= SYSTEM_THREAD_DELAYED_INSERT;
@@ -626,11 +629,15 @@
 delayed_insert *find_handler(THD *thd, TABLE_LIST *table_list)
 {
   thd->proc_info="waiting for delay_list";
+  DBUG_ENTER("find_handler");
+
   pthread_mutex_lock(&LOCK_delayed_insert);	// Protect master list
   I_List_iterator<delayed_insert> it(delayed_threads);
   delayed_insert *di;
   while ((di= it++))
   {
+    DBUG_PRINT("loop", ("checking di '%s.%s'",
+                        di->thd.db, di->table->real_name));
     if (!strcmp(di->thd.db, table_list->db) &&
         !strcmp(table_list->real_name, di->table->real_name))
     {
@@ -639,7 +646,8 @@
     }
   }
   pthread_mutex_unlock(&LOCK_delayed_insert); // For unlink from list
-  return di;
+  DBUG_PRINT("exit", ("returning di 0x%lx", (long) di));
+  DBUG_RETURN(di);
 }
 
 
@@ -750,7 +758,11 @@
   }
 
   pthread_mutex_lock(&di->mutex);
-  table= di->get_local_table(thd);
+  /* If delayed inserts are blocked for this table, switch to normal inserts. */
+  if (di->blocked)
+    table= NULL;
+  else
+    table= di->get_local_table(thd);
   pthread_mutex_unlock(&di->mutex);
   if (table)
     thd->di= di;
@@ -930,6 +942,134 @@
 }
 
 
+/*
+  Kill a delayed insert thread.
+
+  SYNOPSIS
+    kill_delayed_thread()
+      thd                       The thread handle of the running thread.
+      target_thd                The thread handle of the thread to kill.
+      wait_for_terminaion       If to wait until target_thd has flushed
+                                its queued inserts and is ready to die.
+                                This is no final kill. It will block
+                                until another kill.
+
+  NOTE
+    You must _not_ hold LOCK_delayed_insert when you want to wait for
+    termination (i.e. wait_for_termination == TRUE).
+    You _must_ hold LOCK_delayed_insert when you do _not_ want to wait
+    for termination (i.e. wait_for_termination == FALSE).
+    'thd' may be NULL if wait_for_termination is FALSE.
+
+  RETURN
+    void
+*/
+
+void kill_delayed_thread(THD *thd, THD *target_thd, bool wait_for_termination)
+{
+  delayed_insert *di;
+  MYSQL_LOCK     *mylock;
+
+  DBUG_ENTER("kill_delayed_thread");
+
+  /*
+    We must not access di or target_thd, which is a component of di,
+    before we verified that it has not yet been deleted.
+  */
+  if (wait_for_termination)
+  {
+    /* Avoid deletion of di before we could lock it. */
+    VOID(pthread_mutex_lock(&LOCK_delayed_insert));
+
+    /* Check if di is still in the list. */
+    I_List_iterator<delayed_insert> it(delayed_threads);
+    while ((di= it++))
+      if (&di->thd == target_thd)
+        break;
+
+    /* When terminating, the di thread clears thd->di. */
+    if (di)
+      di= target_thd->di; /* Should be either the same or NULL. */
+  }
+  else
+    di= target_thd->di;
+  safe_mutex_assert_owner(&LOCK_delayed_insert);
+
+  /* This is now derived from thd->di, which is NULL on termination. */
+  if (! di)
+  {
+    DBUG_PRINT("info", ("Delayed thread is terminating already."));
+    DBUG_VOID_RETURN;
+  }
+
+  /* Allow the di thread to bypass my write lock (if any). */
+  mylock= thd->lock ? thd->lock : thd->locked_tables;
+  if (mylock)
+  {
+    THR_LOCK_DATA *data= *di->thd.lock->locks;
+    data->cond= NULL; /* Do not abort. */
+    thr_lock_grant_bypass(data, *mylock->locks);
+  }
+
+  /* Ensure that the thread doesn't kill itself while we are looking at it */
+  pthread_mutex_lock(&di->mutex);
+
+  /* Kill the thread. */
+  di->thd.killed= 1;
+
+  /* Wake the thread. */
+  if (di->thd.mysys_var)
+  {
+    pthread_mutex_lock(&di->thd.mysys_var->mutex);
+    if (di->thd.mysys_var->current_cond)
+    {
+      /*
+        We need the following test because the main mutex may be locked
+        in handle_delayed_insert()
+      */
+      if (&di->mutex != di->thd.mysys_var->current_mutex)
+        pthread_mutex_lock(di->thd.mysys_var->current_mutex);
+      pthread_cond_broadcast(di->thd.mysys_var->current_cond);
+      if (&di->mutex != di->thd.mysys_var->current_mutex)
+        pthread_mutex_unlock(di->thd.mysys_var->current_mutex);
+    }
+    pthread_mutex_unlock(&di->thd.mysys_var->mutex);
+  }
+
+  if (wait_for_termination)
+  {
+    VOID(pthread_mutex_unlock(&LOCK_delayed_insert));
+
+    /*
+      Do not allow further delayed inserts. This lets the thread hang around
+      until it is finally terminated.
+    */
+    di->blocked= TRUE;
+
+    while (di->thd.di)
+    {
+      DBUG_PRINT("info", ("Waiting for termination %p", &di->cond_client));
+      di->waiting_for_termination++;
+      pthread_cond_wait(&di->cond_client, &di->mutex);
+      di->waiting_for_termination--;
+      DBUG_PRINT("info", ("Got termination signal %p", &di->cond_client));
+    }
+    /*
+      Do _not_ tell the DI thread that we noticed the termination signal.
+      We want to block it until its final termination.
+      So suppress: pthread_cond_broadcast(&di->cond);
+      Otherwise we would need it as we work with components of di.
+      Hence it must not be deleted before we are done with it.
+      Instead allow for another kill.
+    */
+    di->thd.killed= 0;
+  }
+
+  pthread_mutex_unlock(&di->mutex);
+  DBUG_VOID_RETURN;
+}
+
+
 /* We kill all delayed threads when doing flush-tables */
 
 void kill_delayed_threads(void)
@@ -940,27 +1080,7 @@
   delayed_insert *di;
   while ((di= it++))
   {
-    /* Ensure that the thread doesn't kill itself while we are looking at it */
-    pthread_mutex_lock(&di->mutex);
-    di->thd.killed= 1;
-    if (di->thd.mysys_var)
-    {
-      pthread_mutex_lock(&di->thd.mysys_var->mutex);
-      if (di->thd.mysys_var->current_cond)
-      {
-	/*
-	  We need the following test because the main mutex may be locked
-	  in handle_delayed_insert()
-	*/
-	if (&di->mutex != di->thd.mysys_var->current_mutex)
-	  pthread_mutex_lock(di->thd.mysys_var->current_mutex);
-	pthread_cond_broadcast(di->thd.mysys_var->current_cond);
-	if (&di->mutex != di->thd.mysys_var->current_mutex)
-	  pthread_mutex_unlock(di->thd.mysys_var->current_mutex);
-      }
-      pthread_mutex_unlock(&di->thd.mysys_var->mutex);
-    }
-    pthread_mutex_unlock(&di->mutex);
+    kill_delayed_thread(NULL, &di->thd, FALSE);
   }
   VOID(pthread_mutex_unlock(&LOCK_delayed_insert)); // For unlink from list
 }
@@ -1045,6 +1165,28 @@
     if (thd->killed)
     {
       uint lock_count;
+
+      thd->di= NULL;                                // Flag termination
+      for (;;)
+      {
+        pthread_cond_broadcast(&di->cond_client);   // Signal termination
+        DBUG_PRINT("info", ("Signalled termination"));
+
+        if (! di->waiting_for_termination)
+          break;
+
+        /*
+          We must not destroy the mutex before the client noticed our
+          termination signal. It tries to get the mutex again.
+          We may also hang here if we flushed the queue and must not
+          accept further delayed inserts until finally terminated.
+        */
+        di->thd.mysys_var->current_mutex= &di->mutex;
+        di->thd.mysys_var->current_cond= &di->cond;
+        di->thd.proc_info= "Waiting for termination";
+        pthread_cond_wait(&di->cond, &di->mutex);
+        DBUG_PRINT("info", ("Received final termination signal"));
+      }
       /*
 	Remove this from delay insert list so that no one can request a
 	table from this
@@ -1094,6 +1236,9 @@
 	  break;
 	}
       }
+      DBUG_PRINT("info", ("Awoke  killed: %d  status: %d",
+                          thd->killed, di->status));
+
       /* We can't lock di->mutex and mysys_var->mutex at the same time */
       pthread_mutex_unlock(&di->mutex);
       pthread_mutex_lock(&di->thd.mysys_var->mutex);

--- 1.199/sql/sql_table.cc	2005-11-03 18:24:00 +01:00
+++ 1.200/sql/sql_table.cc	2006-05-22 21:50:41 +02:00
@@ -1628,7 +1628,15 @@
     DBUG_RETURN(error);
   }
 
-  /* Full alter table */
+  /*
+    Full alter table.
+
+    Flush all queued delayed inserts into the table before we start
+    copying. This will also block further delayed inserts into this
+    table until it is finally terminated at the end of this function.
+  */
+  flush_delayed_inserts(thd, db, table_name);
+
   restore_record(table,2);			// Empty record for DEFAULT
   List_iterator<Alter_drop> drop_it(drop_list);
   List_iterator<create_field> def_it(fields);

--- 1.6/mysql-test/r/delayed.result	2002-08-30 20:32:54 +02:00
+++ 1.7/mysql-test/r/delayed.result	2006-05-22 21:50:41 +02:00
@@ -29,3 +29,12 @@
 3	d
 4	e
 drop table t1;
+CREATE TABLE t1 (c1 INT);
+LOCK TABLES t1 WRITE;
+INSERT DELAYED t1 VALUES(123);
+ALTER TABLE t1 MODIFY c1 BIGINT;
+UNLOCK TABLES;
+SELECT * FROM t1;
+c1
+123
+DROP TABLE t1;

--- 1.6/mysql-test/t/delayed.test	2002-08-30 20:32:54 +02:00
+++ 1.7/mysql-test/t/delayed.test	2006-05-22 21:50:41 +02:00
@@ -32,3 +32,29 @@
 --sleep 2
 select * from t1;
 drop table t1;
+
+#
+# Bug#17436 - ALTER TABLE causes pending INSERT DELAYED updates to fail.
+#
+connect (con1,localhost,root,,);
+connect (con2,localhost,root,,);
+#
+connection con1;
+CREATE TABLE t1 (c1 INT);
+LOCK TABLES t1 WRITE;
+#
+connection con2;
+INSERT DELAYED t1 VALUES(123);
+#
+connection con1;
+ALTER TABLE t1 MODIFY c1 BIGINT;
+UNLOCK TABLES;
+#
+connection default;
+# Query returns empty set if bug is present.
+SELECT * FROM t1;
+#
+disconnect con2;
+disconnect con1;
+DROP TABLE t1;
+
Thread
bk commit into 4.0 tree (ingo:1.2180) BUG#17436ingo22 May