BUG#17576516 HOST_CACHE_SIZE VALUE IS IGNORED

marcalff · marcalff · commit 3701c5a6ea41 · 2013-10-08T22:48:13.000+02:00
Before this fix, starting the server with
  host_cache_size = N
in the my.cnf file had no effect.
The internal host cache size was always 128.

The root cause is in hostname_cache_init(),
which is using 'HOST_CACHE_SIZE' (a #define to 128)
instead of 'host_cache_size' (the C global variable that corresponds to the
SQL system value named host_cache_size).

Overall, the initialization of the host cache is not satisfactory, and needs
to be cleaned up.

In particular, the fact that there are distinct:
- data path (mysqld.cc setting the host_cache_size global variable, 
  to be used by hostname.cc)
- control path (mysqld.cc calling hostname_cache_init with no parameters)
can only be a source of confusion.

This fix:
- moves the global variable host_cache_size to mysqld, where it belongs,
  because this is a server global variable
- changes the variable type from ulong to uint, given that the size really
  is an unsigned int, per hash_filo::m_size
- changes hostname_cache_init() to take explicitely a size,
  for consistency with hostname_cache_resize(),
  and for a cleaner interface.
diff --git a/sql/hostname.cc b/sql/hostname.cc
@@ -124,7 +124,6 @@ void Host_errors::aggregate(const Host_errors *errors)
 }
 
 static hash_filo *hostname_cache;
-ulong host_cache_size;
 
 void hostname_cache_refresh()
 {
@@ -141,12 +140,12 @@ void hostname_cache_resize(uint size)
   hostname_cache->resize(size);
 }
 
-bool hostname_cache_init()
+bool hostname_cache_init(uint size)
 {
   Host_entry tmp;
   uint key_offset= (uint) ((char*) (&tmp.ip_key) - (char*) &tmp);
 
-  if (!(hostname_cache= new hash_filo(HOST_CACHE_SIZE,
+  if (!(hostname_cache= new hash_filo(size,
                                       key_offset, HOST_ENTRY_KEY_SIZE,
                                       NULL, (my_hash_free_key) free,
                                       &my_charset_bin)))
diff --git a/sql/hostname.h b/sql/hostname.h
@@ -1,4 +1,4 @@
-/* Copyright (c) 2006, 2011, Oracle and/or its affiliates. All rights reserved.
+/* Copyright (c) 2006, 2013, Oracle and/or its affiliates. All rights reserved.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -161,9 +161,6 @@ class Host_entry : public hash_filo_element
   }
 };
 
-/** The size of the host_cache. */
-extern ulong host_cache_size;
-
 #define RC_OK 0
 #define RC_BLOCKED_HOST 1
 int ip_to_hostname(struct sockaddr_storage *ip_storage,
@@ -172,7 +169,7 @@ int ip_to_hostname(struct sockaddr_storage *ip_storage,
 
 void inc_host_errors(const char *ip_string, Host_errors *errors);
 void reset_host_connect_errors(const char *ip_string);
-bool hostname_cache_init();
+bool hostname_cache_init(uint size);
 void hostname_cache_free();
 void hostname_cache_refresh(void);
 uint hostname_cache_size();
diff --git a/sql/mysqld.cc b/sql/mysqld.cc
@@ -399,6 +399,8 @@ bool opt_using_transactions;
 bool volatile abort_loop;
 bool volatile shutdown_in_progress;
 ulong log_warnings;
+uint host_cache_size;
+
 #if defined(_WIN32) && !defined(EMBEDDED_LIBRARY)
 ulong slow_start_timeout;
 #endif
@@ -4590,7 +4592,7 @@ static int init_server_components()
     all things are initialized so that unireg_abort() doesn't fail
   */
   mdl_init();
-  if (table_def_init() | hostname_cache_init())
+  if (table_def_init() | hostname_cache_init(host_cache_size))
     unireg_abort(1);
 
   query_cache_set_min_res_unit(query_cache_min_res_unit);
diff --git a/sql/mysqld.h b/sql/mysqld.h
@@ -284,6 +284,8 @@ extern ulong connection_errors_internal;
 extern ulong connection_errors_max_connection;
 extern ulong connection_errors_peer_addr;
 extern ulong log_warnings;
+/** The size of the host_cache. */
+extern uint host_cache_size;
 void init_sql_statement_names();
 
 /*
diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc
@@ -4175,11 +4175,11 @@ static Sys_var_tz Sys_time_zone(
 
 static bool fix_host_cache_size(sys_var *, THD *, enum_var_type)
 {
-  hostname_cache_resize((uint) host_cache_size);
+  hostname_cache_resize(host_cache_size);
   return false;
 }
 
-static Sys_var_ulong Sys_host_cache_size(
+static Sys_var_uint Sys_host_cache_size(
        "host_cache_size",
        "How many host names should be cached to avoid resolving.",
        GLOBAL_VAR(host_cache_size),

Original file line number	Diff line number	Diff line change
`@@ -124,7 +124,6 @@ void Host_errors::aggregate(const Host_errors *errors)`
`124`	`124`	`}`
`125`	`125`
`126`	`126`	`static hash_filo *hostname_cache;`
`127`		`-ulong host_cache_size;`
`128`	`127`
`129`	`128`	`void hostname_cache_refresh()`
`130`	`129`	`{`
`@@ -141,12 +140,12 @@ void hostname_cache_resize(uint size)`
`141`	`140`	`hostname_cache->resize(size);`
`142`	`141`	`}`
`143`	`142`
`144`		`-bool hostname_cache_init()`
	`143`	`+bool hostname_cache_init(uint size)`
`145`	`144`	`{`
`146`	`145`	`Host_entry tmp;`
`147`	`146`	`uint key_offset= (uint) ((char) (&tmp.ip_key) - (char) &tmp);`
`148`	`147`
`149`		`- if (!(hostname_cache= new hash_filo(HOST_CACHE_SIZE,`
	`148`	`+ if (!(hostname_cache= new hash_filo(size,`
`150`	`149`	`key_offset, HOST_ENTRY_KEY_SIZE,`
`151`	`150`	`NULL, (my_hash_free_key) free,`
`152`	`151`	`&my_charset_bin)))`
Original file line number	Diff line number	Diff line change
`@@ -4175,11 +4175,11 @@ static Sys_var_tz Sys_time_zone(`
`4175`	`4175`
`4176`	`4176`	`static bool fix_host_cache_size(sys_var , THD , enum_var_type)`
`4177`	`4177`	`{`
`4178`		`- hostname_cache_resize((uint) host_cache_size);`
	`4178`	`+ hostname_cache_resize(host_cache_size);`
`4179`	`4179`	`return false;`
`4180`	`4180`	`}`
`4181`	`4181`
`4182`		`-static Sys_var_ulong Sys_host_cache_size(`
	`4182`	`+static Sys_var_uint Sys_host_cache_size(`
`4183`	`4183`	`"host_cache_size",`
`4184`	`4184`	`"How many host names should be cached to avoid resolving.",`
`4185`	`4185`	`GLOBAL_VAR(host_cache_size),`