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

@@ -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);