Bug #10931 Hostname cache completely broken in 4.1
Submitted: 27 May 2005 20:54 Modified: 21 Jun 2005 11:00
Reporter: Jeremy Cole (Basic Quality Contributor) (OCA) Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server Severity:S2 (Serious)
Version:4.1.x OS:Any (All)
Assigned to: Jim Winstead CPU Architecture:Any

[27 May 2005 20:54] Jeremy Cole
Description:
Currently the hostname cache in 4.1 is completely broken.  It does not serve any useful purpose in caching, as far as I can tell, and may actually slow things down significantly.  The difference from 4.0 comes down to the following lines from mysys/hash.c in hashcmp(), which changed between 4.0 and 4.1:

From 4.0:

   276  static int hashcmp(HASH *hash,HASH_LINK *pos,const byte *key,uint length)
   277  {
   278    uint rec_keylength;
   279    byte *rec_key= (byte*) hash_key(hash,pos->data,&rec_keylength,1);
   280    return (length && length != rec_keylength) ||
   281      (hash->flags & HASH_CASE_INSENSITIVE ?
   282       my_casecmp(rec_key,key,rec_keylength) :
   283       memcmp(rec_key,key,rec_keylength));
   284  }

From 4.1:

   267  static int hashcmp(HASH *hash,HASH_LINK *pos,const byte *key,uint length
)
   268  {
   269    uint rec_keylength;
   270    byte *rec_key= (byte*) hash_key(hash,pos->data,&rec_keylength,1);
   271    return ((length && length != rec_keylength) ||
   272            my_strnncoll(hash->charset, (uchar*) rec_key, rec_keylength,
   273                         (uchar*) key, length));
   274  }

The special case of if(length==0) {compare using the default length (rec_keylength)} is eliminated in 4.1, by passing length instead of rec_keylength as the 5th argument to my_strnncoll().

This special case is actually used in sql/hostname.cc in several functions and places, as follows:

    88      if (!(entry=(host_entry*) hostname_cache->search((gptr) &in->s_addr,0)))

   119    if ((entry=(host_entry*) hostname_cache->search((gptr) &in->s_addr,0)))

   128    if ((entry=(host_entry*) hostname_cache->search((gptr) &in->s_addr,0)))

   145      if ((entry=(host_entry*) hostname_cache->search((gptr) &in->s_addr,0)))

Those calls to hostname_cache->search() are hash_filo::search(gptr key, uint length) in sql/hash_filo.h where length is passed on to hash_search() untouched:

    90      hash_filo_element *entry=(hash_filo_element*)
    91        hash_search(&cache,(byte*) key,length);

In mysys/hash.c, hash_search(HASH *hash, const byte *key, uint length) then passes the length parameter on to hashcmp() as follows:

   206        if (!hashcmp(hash,pos,key,length))

Finally, hashcmp(HASH *hash, HASH_LINK *pos, const byte *key, uint length) again passes length untouched on to my_strnncoll() as follows:

   272            my_strnncoll(hash->charset, (uchar*) rec_key, rec_keylength,
   273                         (uchar*) key, length));

This means that any calls to hostname_cache->search(x, 0) will always return NO_RECORD.  This will cause hostnames to be repeatedly added to the host cache instead of their entry being found.  In addition, the cache cannot be used as a cache, since the records cannot be found.

How to repeat:
N/A.

Suggested fix:
If the special case for length==0 was intentionally removed, calls to hostname_cache->search(x, 0) should be changed to hostname_cache->search(x, 4) instead.

If the special case was not intentionally removed, the length parameter to my_strnncoll should be changed to rec_keylength instead.
[27 May 2005 22:16] Timothy Smith
Jeremy,

It looks like the code change intended to simply switch from using my_casecmp/memcmp to my_strnncoll.  That is, the use of length instead of rec_keylength in 4.1 appears to be a simple typo.  I will let another developer more intimate with the code make that decision, though.

Thanks for the bug report.

Timothy
[27 May 2005 22:21] Timothy Smith
By the way, this change happened on April 1st, 2003, in this change set:

ChangeSet 1.1504.1.7 2003/04/01 14:17:28 bar@bar.mysql.r18.ru
  my_strncasecmp() is not used anymore. Use my_strncoll() instead.
mysys/hash.c 1.26 2003/04/01 14:17:25 bar@bar.mysql.r18.ru
  my_strncasecmp() is not used anymore. Use my_strncoll() instead.
[31 May 2005 19:45] Jeremy Cole
Trivial Patch for hashcmp()

Attachment: broken_hashcmp.patch (application/octet-stream, text), 426 bytes.

[31 May 2005 19:45] Jeremy Cole
I've attached the (trivial) patch to fix this bug.
[1 Jun 2005 18:30] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/internals/25474
[10 Jun 2005 4:56] Jim Winstead
Fixed in 4.1.13 and 5.0.8.
[10 Jun 2005 19:32] Paul DuBois
Noted in 4.1.13, 5.0.8 changelogs.