List:Commits« Previous MessageNext Message »
From:Konstantin Osipov Date:January 2 2006 7:35pm
Subject:bk commit into 4.1 tree (konstantin:1.2471) BUG#7209
View as plain text  
Below is the list of changes that have just been committed into a local
4.1 repository of kostja. When kostja 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.2471 06/01/02 22:35:17 konstantin@stripped +7 -0
  A fix for Bug#7209 "Client error with "Access Denied" on updates 
  when high concurrency": remove HASH::current_record and make it
  an external search parameter, so that it can not be the cause of a 
  race condition under high concurrent load.
  The bug was in a race condition in table_hash_search,
  when column_priv_hash.current_record was overwritten simultaneously
  by multiple threads, causing the search for a suitable grant record
  to fail.
  No test case as the bug is repeatable only under concurrent load.

  sql/sql_cache.cc
    1.96 06/01/02 22:35:07 konstantin@stripped +6 -4
    - adjust to a changed declaration of hash_replace

  sql/sql_base.cc
    1.261 06/01/02 22:35:07 konstantin@stripped +15 -7
    - adjust to changed declarations of hash_search, hash_nex

  sql/sql_acl.cc
    1.172 06/01/02 22:35:06 konstantin@stripped +5 -4
    - adjust to changed declarations of hash_search, hash_next

  sql/lock.cc
    1.64 06/01/02 22:35:06 konstantin@stripped +3 -2
    - adjust to changed declarations of hash_search, hash_next

  mysys/testhash.c
    1.8 06/01/02 22:35:06 konstantin@stripped +5 -4
    - adjust the test case to changed function declarations

  mysys/hash.c
    1.44 06/01/02 22:35:06 konstantin@stripped +35 -25
    - remove HASH::current_record
    - change declarations of functions that use HASH in read-only mode
      to accept const HASH * instead of HASH *. 
    - implement hash_search; move the old implementation of hash_search
    to hash_first

  include/hash.h
    1.17 06/01/02 22:35:06 konstantin@stripped +10 -4
    - remove current_record from HASH, instead modify hash_first,
    hash_next to accept HASH_SEARCH_STATE as an 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:	konstantin
# Host:	oak.local
# Root:	/home/kostja/mysql/mysql-4.1-7209

--- 1.16/include/hash.h	2005-01-11 13:56:56 +03:00
+++ 1.17/include/hash.h	2006-01-02 22:35:06 +03:00
@@ -33,7 +33,7 @@
 
 typedef struct st_hash {
   uint key_offset,key_length;		/* Length of key if const length */
-  uint records,blength,current_record;
+  uint records,blength;
   uint flags;
   DYNAMIC_ARRAY array;				/* Place for hash_keys */
   hash_get_key get_key;
@@ -41,6 +41,9 @@
   CHARSET_INFO *charset;
 } HASH;
 
+/* A search iterator state */
+typedef uint HASH_SEARCH_STATE;
+
 #define hash_init(A,B,C,D,E,F,G,H) _hash_init(A,B,C,D,E,F,G, H CALLER_INFO)
 my_bool _hash_init(HASH *hash, CHARSET_INFO *charset,
 		   uint default_array_elements, uint key_offset,
@@ -49,12 +52,15 @@
 void hash_free(HASH *tree);
 void my_hash_reset(HASH *hash);
 byte *hash_element(HASH *hash,uint idx);
-gptr hash_search(HASH *info,const byte *key,uint length);
-gptr hash_next(HASH *info,const byte *key,uint length);
+gptr hash_search(const HASH *info, const byte *key, uint length);
+gptr hash_first(const HASH *info, const byte *key, uint length,
+                HASH_SEARCH_STATE *state);
+gptr hash_next(const HASH *info, const byte *key, uint length,
+               HASH_SEARCH_STATE *state);
 my_bool my_hash_insert(HASH *info,const byte *data);
 my_bool hash_delete(HASH *hash,byte *record);
 my_bool hash_update(HASH *hash,byte *record,byte *old_key,uint old_key_length);
-void hash_replace(HASH *hash, uint idx, byte *new_row);
+void hash_replace(HASH *hash, HASH_SEARCH_STATE *state, byte *new_row);
 my_bool hash_check(HASH *hash);			/* Only in debug library */
 
 #define hash_clear(H) bzero((char*) (H),sizeof(*(H)))

--- 1.43/mysys/hash.c	2005-06-01 22:30:56 +04:00
+++ 1.44/mysys/hash.c	2006-01-02 22:35:06 +03:00
@@ -36,9 +36,10 @@
 
 static uint hash_mask(uint hashnr,uint buffmax,uint maxlength);
 static void movelink(HASH_LINK *array,uint pos,uint next_link,uint newlink);
-static int hashcmp(HASH *hash,HASH_LINK *pos,const byte *key,uint length);
+static int hashcmp(const HASH *hash, HASH_LINK *pos, const byte *key,
+                   uint length);
 
-static uint calc_hash(HASH *hash,const byte *key,uint length)
+static uint calc_hash(const HASH *hash, const byte *key, uint length)
 {
   ulong nr1=1, nr2=4;
   hash->charset->coll->hash_sort(hash->charset,(uchar*) key,length,&nr1,&nr2);
@@ -63,7 +64,6 @@
   hash->key_offset=key_offset;
   hash->key_length=key_length;
   hash->blength=1;
-  hash->current_record= NO_RECORD;		/* For the future */
   hash->get_key=get_key;
   hash->free=free_element;
   hash->flags=flags;
@@ -135,7 +135,6 @@
   reset_dynamic(&hash->array);
   /* Set row pointers so that the hash can be reused at once */
   hash->blength= 1;
-  hash->current_record= NO_RECORD;
   DBUG_VOID_RETURN;
 }
 
@@ -147,7 +146,8 @@
 */
 
 static inline char*
-hash_key(HASH *hash,const byte *record,uint *length,my_bool first)
+hash_key(const HASH *hash, const byte *record, uint *length,
+         my_bool first)
 {
   if (hash->get_key)
     return (*hash->get_key)(record,length,first);
@@ -163,8 +163,8 @@
   return (hashnr & ((buffmax >> 1) -1));
 }
 
-static uint hash_rec_mask(HASH *hash,HASH_LINK *pos,uint buffmax,
-			  uint maxlength)
+static uint hash_rec_mask(const HASH *hash, HASH_LINK *pos,
+                          uint buffmax, uint maxlength)
 {
   uint length;
   byte *key= (byte*) hash_key(hash,pos->data,&length,0);
@@ -186,14 +186,25 @@
 }
 
 
-	/* Search after a record based on a key */
-	/* Sets info->current_ptr to found record */
+gptr hash_search(const HASH *hash, const byte *key, uint length)
+{
+  HASH_SEARCH_STATE state;
+  return hash_first(hash, key, length, &state);
+}
+
+/*
+  Search after a record based on a key
+
+  NOTE
+   Assigns the number of the found record to HASH_SEARCH_STATE state
+*/
 
-gptr hash_search(HASH *hash,const byte *key,uint length)
+gptr hash_first(const HASH *hash, const byte *key, uint length,
+                HASH_SEARCH_STATE *current_record)
 {
   HASH_LINK *pos;
   uint flag,idx;
-  DBUG_ENTER("hash_search");
+  DBUG_ENTER("hash_first");
 
   flag=1;
   if (hash->records)
@@ -206,7 +217,7 @@
       if (!hashcmp(hash,pos,key,length))
       {
 	DBUG_PRINT("exit",("found key at %d",idx));
-	hash->current_record= idx;
+	*current_record= idx;
 	DBUG_RETURN (pos->data);
       }
       if (flag)
@@ -218,31 +229,32 @@
     }
     while ((idx=pos->next) != NO_RECORD);
   }
-  hash->current_record= NO_RECORD;
+  *current_record= NO_RECORD;
   DBUG_RETURN(0);
 }
 
 	/* Get next record with identical key */
 	/* Can only be called if previous calls was hash_search */
 
-gptr hash_next(HASH *hash,const byte *key,uint length)
+gptr hash_next(const HASH *hash, const byte *key, uint length,
+               HASH_SEARCH_STATE *current_record)
 {
   HASH_LINK *pos;
   uint idx;
 
-  if (hash->current_record != NO_RECORD)
+  if (*current_record != NO_RECORD)
   {
     HASH_LINK *data=dynamic_element(&hash->array,0,HASH_LINK*);
-    for (idx=data[hash->current_record].next; idx != NO_RECORD ; idx=pos->next)
+    for (idx=data[*current_record].next; idx != NO_RECORD ; idx=pos->next)
     {
       pos=data+idx;
       if (!hashcmp(hash,pos,key,length))
       {
-	hash->current_record= idx;
+	*current_record= idx;
 	return pos->data;
       }
     }
-    hash->current_record=NO_RECORD;
+    *current_record= NO_RECORD;
   }
   return 0;
 }
@@ -282,7 +294,8 @@
     > 0  key of record >  key
  */
 
-static int hashcmp(HASH *hash,HASH_LINK *pos,const byte *key,uint length)
+static int hashcmp(const HASH *hash, HASH_LINK *pos, const byte *key,
+                   uint length)
 {
   uint rec_keylength;
   byte *rec_key= (byte*) hash_key(hash,pos->data,&rec_keylength,1);
@@ -308,7 +321,6 @@
   if (!(empty=(HASH_LINK*) alloc_dynamic(&info->array)))
     return(TRUE);				/* No more memory */
 
-  info->current_record= NO_RECORD;
   data=dynamic_element(&info->array,0,HASH_LINK*);
   halfbuff= info->blength >> 1;
 
@@ -451,7 +463,6 @@
   }
 
   if ( --(hash->records) < hash->blength >> 1) hash->blength>>=1;
-  hash->current_record= NO_RECORD;
   lastpos=data+hash->records;
 
   /* Remove link to record */
@@ -544,7 +555,6 @@
     if ((idx=pos->next) == NO_RECORD)
       DBUG_RETURN(1);			/* Not found in links */
   }
-  hash->current_record= NO_RECORD;
   org_link= *pos;
   empty=idx;
 
@@ -594,10 +604,10 @@
   isn't changed
 */
 
-void hash_replace(HASH *hash, uint idx, byte *new_row)
+void hash_replace(HASH *hash, HASH_SEARCH_STATE *current_record, byte *new_row)
 {
-  if (idx != NO_RECORD)				/* Safety */
-    dynamic_element(&hash->array,idx,HASH_LINK*)->data=new_row;
+  if (*current_record != NO_RECORD)            /* Safety */
+    dynamic_element(&hash->array, *current_record, HASH_LINK*)->data= new_row;
 }
 
 

--- 1.7/mysys/testhash.c	2003-09-19 13:44:28 +04:00
+++ 1.8/mysys/testhash.c	2006-01-02 22:35:06 +03:00
@@ -74,7 +74,7 @@
   bzero((char*) key1,sizeof(key1[0])*1000);
 
   printf("- Creating hash\n");
-  if (hash_init(&hash,recant/2,0,6,0,free_record,0))
+  if (hash_init(&hash, default_charset_info, recant/2, 0, 6, 0, free_record, 0))
     goto err;
   printf("- Writing records:\n");
 
@@ -172,15 +172,16 @@
       break;
   if (key1[j] > 1)
   {
+    HASH_SEARCH_STATE state;
     printf("- Testing identical read\n");
     sprintf(key,"%6d",j);
     pos=1;
-    if (!(recpos=hash_search(&hash,key,0)))
+    if (!(recpos= hash_first(&hash, key, 0, &state)))
     {
       printf("can't find key1: \"%s\"\n",key);
       goto err;
     }
-    while (hash_next(&hash,key,0) && pos < (ulong) (key1[j]+10))
+    while (hash_next(&hash, key, 0, &state) && pos < (ulong) (key1[j]+10))
       pos++;
     if (pos != (ulong) key1[j])
     {
@@ -189,7 +190,7 @@
     }
   }
   printf("- Creating output heap-file 2\n");
-  if (hash_init(&hash2,hash.records,0,0,hash2_key,free_record,0))
+  if (hash_init(&hash2, default_charset_info, hash.records, 0, 0, hash2_key, free_record,0))
     goto err;
 
   printf("- Copying and removing records\n");

--- 1.63/sql/lock.cc	2005-12-07 21:54:06 +03:00
+++ 1.64/sql/lock.cc	2006-01-02 22:35:06 +03:00
@@ -641,6 +641,7 @@
   char  key[MAX_DBKEY_LENGTH];
   char *db= table_list->db;
   uint  key_length;
+  HASH_SEARCH_STATE state;
   DBUG_ENTER("lock_table_name");
   DBUG_PRINT("enter",("db: %s  name: %s", db, table_list->real_name));
 
@@ -651,9 +652,9 @@
 
 
   /* Only insert the table if we haven't insert it already */
-  for (table=(TABLE*) hash_search(&open_cache,(byte*) key,key_length) ;
+  for (table=(TABLE*) hash_first(&open_cache, (byte*)key, key_length, &state);
        table ;
-       table = (TABLE*) hash_next(&open_cache,(byte*) key,key_length))
+       table = (TABLE*) hash_next(&open_cache, (byte*)key, key_length, &state))
     if (table->in_use == thd)
       DBUG_RETURN(0);
 

--- 1.171/sql/sql_acl.cc	2005-12-28 11:23:21 +03:00
+++ 1.172/sql/sql_acl.cc	2006-01-02 22:35:06 +03:00
@@ -1988,14 +1988,15 @@
   char helping [NAME_LEN*2+USERNAME_LENGTH+3];
   uint len;
   GRANT_TABLE *grant_table,*found=0;
+  HASH_SEARCH_STATE state;
 
   len  = (uint) (strmov(strmov(strmov(helping,user)+1,db)+1,tname)-helping)+ 1;
-  for (grant_table=(GRANT_TABLE*) hash_search(&column_priv_hash,
-					      (byte*) helping,
-					      len) ;
+  for (grant_table=(GRANT_TABLE*) hash_first(&column_priv_hash,
+                                             (byte*) helping,
+                                             len, &state) ;
        grant_table ;
        grant_table= (GRANT_TABLE*) hash_next(&column_priv_hash,(byte*) helping,
-					     len))
+                                             len, &state))
   {
     if (exact)
     {

--- 1.260/sql/sql_base.cc	2005-11-15 20:02:31 +03:00
+++ 1.261/sql/sql_base.cc	2006-01-02 22:35:07 +03:00
@@ -799,6 +799,7 @@
   reg1	TABLE *table;
   char	key[MAX_DBKEY_LENGTH];
   uint	key_length;
+  HASH_SEARCH_STATE state;
   DBUG_ENTER("open_table");
 
   /* find a unused table in the open table cache */
@@ -863,9 +864,11 @@
   /* close handler tables which are marked for flush */
   mysql_ha_flush(thd, (TABLE_LIST*) NULL, MYSQL_HA_REOPEN_ON_USAGE, TRUE);
 
-  for (table=(TABLE*) hash_search(&open_cache,(byte*) key,key_length) ;
+  for (table= (TABLE*) hash_first(&open_cache, (byte*) key, key_length,
+                                  &state);
        table && table->in_use ;
-       table = (TABLE*) hash_next(&open_cache,(byte*) key,key_length))
+       table= (TABLE*) hash_next(&open_cache, (byte*) key, key_length,
+                                 &state))
   {
     if (table->version != refresh_version)
     {
@@ -1236,12 +1239,14 @@
 {
   do
   {
+    HASH_SEARCH_STATE state;
     char *key= table->table_cache_key;
     uint key_length=table->key_length;
-    for (TABLE *search=(TABLE*) hash_search(&open_cache,
-					    (byte*) key,key_length) ;
+    for (TABLE *search= (TABLE*) hash_first(&open_cache, (byte*) key,
+                                            key_length, &state);
 	 search ;
-	 search = (TABLE*) hash_next(&open_cache,(byte*) key,key_length))
+         search= (TABLE*) hash_next(&open_cache, (byte*) key,
+                                    key_length, &state))
     {
       if (search->locked_by_flush ||
 	  search->locked_by_name && wait_for_name_lock ||
@@ -2958,11 +2963,14 @@
   key_length=(uint) (strmov(strmov(key,db)+1,table_name)-key)+1;
   for (;;)
   {
+    HASH_SEARCH_STATE state;
     result= signalled= 0;
 
-    for (table=(TABLE*) hash_search(&open_cache,(byte*) key,key_length) ;
+    for (table= (TABLE*) hash_first(&open_cache, (byte*) key, key_length,
+                                    &state);
          table;
-         table = (TABLE*) hash_next(&open_cache,(byte*) key,key_length))
+         table= (TABLE*) hash_next(&open_cache, (byte*) key, key_length,
+                                   &state))
     {
       THD *in_use;
       table->version=0L;		/* Free when thread is ready */

--- 1.95/sql/sql_cache.cc	2005-12-01 15:26:17 +03:00
+++ 1.96/sql/sql_cache.cc	2006-01-02 22:35:07 +03:00
@@ -2873,6 +2873,7 @@
   }
   case Query_cache_block::TABLE:
   {
+    HASH_SEARCH_STATE record_idx;
     DBUG_PRINT("qcache", ("block 0x%lx TABLE", (ulong) block));
     if (*border == 0)
       break;
@@ -2890,7 +2891,7 @@
     byte *key;
     uint key_length;
     key=query_cache_table_get_key((byte*) block, &key_length, 0);
-    hash_search(&tables, (byte*) key, key_length);
+    hash_first(&tables, (byte*) key, key_length, &record_idx);
 
     block->destroy();
     new_block->init(len);
@@ -2924,7 +2925,7 @@
     /* Fix pointer to table name */
     new_block->table()->table(new_block->table()->db() + tablename_offset);
     /* Fix hash to point at moved block */
-    hash_replace(&tables, tables.current_record, (byte*) new_block);
+    hash_replace(&tables, &record_idx, (byte*) new_block);
 
     DBUG_PRINT("qcache", ("moved %lu bytes to 0x%lx, new gap at 0x%lx",
 			len, (ulong) new_block, (ulong) *border));
@@ -2932,6 +2933,7 @@
   }
   case Query_cache_block::QUERY:
   {
+    HASH_SEARCH_STATE record_idx;
     DBUG_PRINT("qcache", ("block 0x%lx QUERY", (ulong) block));
     if (*border == 0)
       break;
@@ -2949,7 +2951,7 @@
     byte *key;
     uint key_length;
     key=query_cache_query_get_key((byte*) block, &key_length, 0);
-    hash_search(&queries, (byte*) key, key_length);
+    hash_first(&queries, (byte*) key, key_length, &record_idx);
     // Move table of used tables 
     memmove((char*) new_block->table(0), (char*) block->table(0),
 	   ALIGN_SIZE(n_tables*sizeof(Query_cache_block_table)));
@@ -3017,7 +3019,7 @@
       net->query_cache_query= (gptr) new_block;
     }
     /* Fix hash to point at moved block */
-    hash_replace(&queries, queries.current_record, (byte*) new_block);
+    hash_replace(&queries, &record_idx, (byte*) new_block);
     DBUG_PRINT("qcache", ("moved %lu bytes to 0x%lx, new gap at 0x%lx",
 			len, (ulong) new_block, (ulong) *border));
     break;
Thread
bk commit into 4.1 tree (konstantin:1.2471) BUG#7209Konstantin Osipov2 Jan