List:Commits« Previous MessageNext Message »
From:konstantin Date:January 2 2006 4:46pm
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 19:46:37 konstantin@stripped +6 -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 thati t 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.

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

  sql/sql_base.cc
    1.261 06/01/02 19:46:32 konstantin@stripped +15 -7
    - adjust to changed declarations of hash_search, hash_next

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

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

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

  mysys/hash.c
    1.44 06/01/02 19:46:31 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

# 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:	dragonfly.local
# Root:	/opt/local/work/mysql-4.1-7209

--- 1.43/mysys/hash.c	2005-06-01 22:30:56 +04:00
+++ 1.44/mysys/hash.c	2006-01-02 19:46:31 +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 19:46:32 +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 19:46:32 +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 19:46:32 +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 19:46:32 +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 19:46:32 +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#7209konstantin2 Jan