| 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: | |
| Category: | MySQL Server | Severity: | S2 (Serious) |
| Version: | 4.1.x | OS: | Any (All) |
| Assigned to: | Jim Winstead | CPU Architecture: | Any |
[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.
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.