Bug #10931 Hostname cache completely broken in 4.1

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.

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

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.

Trivial Patch for hashcmp()

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

I've attached the (trivial) patch to fix this bug.

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

Fixed in 4.1.13 and 5.0.8.

Noted in 4.1.13, 5.0.8 changelogs.