Bug#17332 - changing key_buffer_size on a running server

            can crash under load

Resizing a key cache while it was in heavy use could crash the
server. There were several race conditions.

I reworked some of the algorithms to fix the race conditions.

No test case. Repeating the crashes requires heavy concurrent
load on the key cache. A test script is attached to the bug report.

More explanations to the changes are contained in a text file
attached to the bug report.


include/keycache.h:
  Bug#17332 - changing key_buffer_size on a running server
              can crash under load
  Added KEY_CACHE components in_resize and waiting_for_resize_cnt.
mysys/mf_keycache.c:
  Bug#17332 - changing key_buffer_size on a running server
              can crash under load
  
  Changed resize_key_cache() to not disable the key cache
  after the flush phase. Changed queue handling to use
  standard functions. Wake all threads waiting on resize_queue.
  We can now have read/write threads waiting there (see below).
  
  Combined add_to_queue() and the wait loops that were always
  following it to the new function wait_on_queue().
  Combined release_queue() and the condition that was always
  preceding it to the new function release_whole_queue().
  
  Added code to flag and respect the exceptional situation
  BLOCK_IN_EVICTION.
  
  Rewrote the resize branch of find_key_block().
  
  Added code to the eviction handling in find_key_block()
  to catch more exceptional cases.
  
  Changed key_cache_read(), key_cache_insert() and key_cache_write()
  so that they lock keycache->cache_lock whenever the key cache is
  initialized. Checking for a disabled cache and incrementing and
  decrementing the "resize counter" is always done within the lock.
  Locking and unlocking as well as counting the "resize counter" is
  now done once outside the loop. All three functions can now handle
  a NULL return from find_key_block. This happens in the flush phase
  of a resize and demands direct file I/O. Care is taken for
  secondary requests (PAGE_WAIT_TO_BE_READ) to wait in any case.
  Moved block status changes behind the copying of buffer data.
  key_cache_insert() does now read the block if the caller did
  supply less data than a full cache block.
  key_cache_write() does now take care of parallel running flushes
  (BLOCK_FOR_UPDATE, BLOCK_IN_FLUSHWRITE).
  
  Changed free_block() to un-initialize block variables in the
  correct order and respect an exceptional BLOCK_IN_EVICTION state.
  
  Changed flushing to take care for parallel running writes.
  Changed flushing to avoid freeing blocks in eviction.
  Changed flushing to consider that parallel writes can move blocks
  from the file_blocks hash to the changed_blocks hash.
  Changed flushing to take care for other parallel flushes.
  Changed flushing to assure that it ends with everything flushed.
  
  Added some comments and debugging statements. ;-)
mysys/my_static.c:
  Bug#17332 - changing key_buffer_size on a running server
              can crash under load
  Removed an unused global variable.
sql/handler.cc:
  Bug#17332 - changing key_buffer_size on a running server
              can crash under load
  Changed types of local variables to match their use
  for init_key_cache().
sql/sql_table.cc:
  Bug#17332 - changing key_buffer_size on a running server
              can crash under load
  Changed TL_READ to TL_READ_NO_INSERT in mysql_preload_keys.
storage/myisam/ha_myisam.cc:
  Bug#17332 - changing key_buffer_size on a running server
              can crash under load
  Moved an automatic (stack) variable to the scope where it is used.
storage/myisam/mi_preload.c:
  Bug#17332 - changing key_buffer_size on a running server
              can crash under load
  Added some preliminary code to
  - allow LOAD INDEX to load indexes of different block size,
  - align load chunks to key cache blocks.

include/keycache.h

@@ -44,6 +44,7 @@ typedef struct st_keycache_wqueue
 typedef struct st_key_cache
 {
   my_bool key_cache_inited;
+  my_bool in_resize;             /* true during resize operation             */
   my_bool resize_in_flush;       /* true during flush of resize operation    */
   my_bool can_be_used;           /* usage of cache for read/write is allowed */
   uint key_cache_shift;
@@ -72,6 +73,11 @@ typedef struct st_key_cache
   BLOCK_LINK *used_ins;          /* ptr to the insertion block in LRU chain  */
   pthread_mutex_t cache_lock;    /* to lock access to the cache structure    */
   KEYCACHE_WQUEUE resize_queue;  /* threads waiting during resize operation  */
+  /*
+    Waiting for a zero resize count. Using a queue for symmetry though
+    only one thread can wait here.
+  */
+  KEYCACHE_WQUEUE waiting_for_resize_cnt;
   KEYCACHE_WQUEUE waiting_for_hash_link; /* waiting for a free hash link     */
   KEYCACHE_WQUEUE waiting_for_block;    /* requests waiting for a free block */
   BLOCK_LINK *changed_blocks[CHANGED_BLOCKS_HASH]; /* hash for dirty file bl.*/

mysys/my_static.c

@@ -48,9 +48,6 @@ struct st_remember _my_sig_remember[MAX_SIGNALS]={{0,0}};
 sigset_t my_signals;			/* signals blocked by mf_brkhant */
 #endif
 
-	/* from mf_keycache.c */
-my_bool key_cache_inited=0;
-
 	/* from mf_reccache.c */
 ulong my_default_record_cache_size=RECORD_CACHE_SIZE;
 

sql/handler.cc

@@ -2716,8 +2716,8 @@ int ha_init_key_cache(const char *name, KEY_CACHE *key_cache)
   if (!key_cache->key_cache_inited)
   {
     pthread_mutex_lock(&LOCK_global_system_variables);
-    long tmp_buff_size= (long) key_cache->param_buff_size;
-    long tmp_block_size= (long) key_cache->param_block_size;
+    ulong tmp_buff_size= (ulong) key_cache->param_buff_size;
+    uint tmp_block_size= (uint) key_cache->param_block_size;
     uint division_limit= key_cache->param_division_limit;
     uint age_threshold=  key_cache->param_age_threshold;
     pthread_mutex_unlock(&LOCK_global_system_variables);

sql/sql_table.cc

@@ -4575,8 +4575,13 @@ int reassign_keycache_tables(THD *thd, KEY_CACHE *src_cache,
 bool mysql_preload_keys(THD* thd, TABLE_LIST* tables)
 {
   DBUG_ENTER("mysql_preload_keys");
+  /*
+    We cannot allow concurrent inserts. The storage engine reads
+    directly from the index file, bypassing the cache. It could read
+    outdated information if parallel inserts into cache blocks happen.
+  */
   DBUG_RETURN(mysql_admin_table(thd, tables, 0,
-				"preload_keys", TL_READ, 0, 0, 0, 0,
+				"preload_keys", TL_READ_NO_INSERT, 0, 0, 0, 0,
 				&handler::preload_keys, 0));
 }
 

storage/myisam/ha_myisam.cc

@@ -858,6 +858,7 @@ int ha_myisam::preload_keys(THD* thd, HA_CHECK_OPT *check_opt)
   ulonglong map= ~(ulonglong) 0;
   TABLE_LIST *table_list= table->pos_in_table_list;
   my_bool ignore_leaves= table_list->ignore_leaves;
+  char buf[ERRMSGSIZE+20];
 
   DBUG_ENTER("ha_myisam::preload_keys");
 
@@ -889,7 +890,6 @@ int ha_myisam::preload_keys(THD* thd, HA_CHECK_OPT *check_opt)
       errmsg= "Failed to allocate buffer";
       break;
     default:
-      char buf[ERRMSGSIZE+20];
       my_snprintf(buf, ERRMSGSIZE,
                   "Failed to read from index file (errno: %d)", my_errno);
       errmsg= buf;

storage/myisam/mi_preload.c

@@ -58,12 +58,39 @@ int mi_preload(MI_INFO *info, ulonglong key_map, my_bool ignore_leaves)
   /* Check whether all indexes use the same block size */
   for (i= 1 ; i < keys ; i++)
   {
+#if !defined(INGO_TEST_LOADIDX_OFF)
+    /* Allow non-IGNORE-LEAVES index loading even with different block sizes. */
+    if (ignore_leaves && (keyinfo[i].block_length != block_length))
+      DBUG_RETURN(my_errno= HA_ERR_NON_UNIQUE_BLOCK_SIZE);
+    set_if_bigger(block_length, keyinfo[i].block_length);
+#else
     if (keyinfo[i].block_length != block_length)
       DBUG_RETURN(my_errno= HA_ERR_NON_UNIQUE_BLOCK_SIZE);
+#endif
   }
 
+#if !defined(INGO_TEST_LOADIDX_OFF)
+  /* Align non-IGNORE-LEAVES index loads. */
+  if (!ignore_leaves)
+  {
+    /* Round up to the next multiple of key_cache_block_size. */
+    length= ((info->preload_buff_size +
+              share->key_cache->key_cache_block_size - 1) /
+             share->key_cache->key_cache_block_size *
+             share->key_cache->key_cache_block_size);
+    /* Round down to the next multiple of key_cache_block_size. */
+    pos= (share->base.keystart / share->key_cache->key_cache_block_size *
+          share->key_cache->key_cache_block_size);
+  }
+  else
+  {
+    length= info->preload_buff_size/block_length * block_length;
+    set_if_bigger(length, block_length);
+  }
+#else
   length= info->preload_buff_size/block_length * block_length;
   set_if_bigger(length, block_length);
+#endif
 
   if (!(buff= (uchar *) my_malloc(length, MYF(MY_WME))))
     DBUG_RETURN(my_errno= HA_ERR_OUT_OF_MEM);