WL#1265: Fix proper ALTER/DROP support in the SP cache.

New sp_cache C API. When an SP is dropped, old caches (in other threads)
become invalid and are cleared.
Also, the caches in THD are only created on demand.

Docs/sp-imp-spec.txt

@@ -79,8 +79,7 @@
 
     - Utility functions (sp.{cc,h})
       This contains functions for creating, dropping and finding a stored
-      procedure in the mysql.proc table (or internal cache, when it is
-      implemented).
+      procedure in the mysql.proc table (or the internal cache).
 
 
   - Parsing CREATE PROCEDURE ...
@@ -335,23 +334,12 @@
       Then, before doing anything else in mysql_execute_command(), read all
       functions from the database an keep them in the THD, where the function
       sp_find_function() can find them during the execution.
-      Note: Even when a global in-memory cache is implemented, we must still
-        make sure that the functions are indeed read and cached at this point.
+      Note: Even with an in-memory cache, we must still make sure that the
+            functions are indeed read and cached at this point.
       The code that read and cache functions from the database must also be
       invoked recursively for each read FUNCTION to make sure we have *all* the
       functions we need.
 
-      In the absence of the real in-memory cache for SPs, a temporary solution
-      has been implemented with a per-THD cache for just FUNCTIONs. This is
-      handled by the functions
-
-        void sp_add_fun_to_lex(LEX *lex, LEX_STRING fun);
-        void sp_merge_funs(LEX *dst, LEX *src);
-        int sp_cache_functions(THD *thd, LEX *lex);
-        void sp_clear_function_cache(THD *thd);
-
-      in sp.cc.
-
 
   - Parsing DROP PROCEDURE/FUNCTION
 
@@ -537,7 +525,64 @@
        2    sp_instr_cfetch(0)                  # Contains the variable list
        3    sp_instr_cclose(0)
        4    sp_instr_cpop(1)
-    
+
+
+
+  - The SP cache
+
+    There are two ways to cache SPs:
+
+    1) one global cache, share by all threads/connections,
+    2) one cache per thread.
+
+    There are pros and cons with both methods:
+
+    1) Pros: Save memory, each SP only read from table once,
+       Cons: Needs locking (= serialization at access), requires thread-safe
+             data structures,
+    2) Pros: Fast, no locking required (almost), limited thread-safe
+             requirement,
+       Cons: Uses more memory, each SP read from table once per thread.
+
+    Unfortunately, we cannot use alternative 1 for the time being, as most
+    of the datastructures to be cached (lex and items) are not reentrant
+    and thread-safe. (Things are modifed at execution, we have THD pointers
+    stored everywhere, etc.)
+    This leaves us with alternative 2, one cache per thread; or actually
+    two, since we keep FUNCTIONs and PROCEDUREs in separate caches.
+    This is not that terrible; the only case when it will perform
+    significantly worse than a global cache is when we have an application
+    where new threads are connecting, calling a procedure, and disconnecting,
+    over and over again.
+
+    The cache implementation itself is simple and straightforward, a hashtable
+    wrapped in a class and a C API (see APIs below).
+
+    There is however one issue with multiple caches: dropping and altering
+    procedures. Normally, this should be a very rare event in a running
+    system; it's typically something you do during development and testing,
+    so it's not unthinkable that we would simply ignore the issue and let
+    any threads running with a cached version of an SP keep doing so until
+    its disconnected.
+    But assuming we want to keep the caches consistent with respect to drop
+    and alter, it can be done:
+
+    1) A global counter is needed, initialized to 0 at start.
+    2) At each DROP or ALTER, increase the counter by one.
+    3) Each cache has its own copy of the counter, copied at the last read.
+    4) When looking up a name in the cache, first check if the global counter
+       is larger than the local copy.
+       If so, clear the cache and return "not found", and update the local
+       counter; otherwise, lookup as usual.
+
+    This minimizes the cost to a single brief lock for the access of an
+    integer when operating normally. Only in the event of an actual drop or
+    alter, is the cache cleared. This may seem to be drastic, but since we
+    assume that this is a rare event, it's not a problem.
+    It would of course be possible to have a much more fine-grained solution,
+    keeping track of each SP, but the overhead of doing so is not worth the
+    effort.
+
 
   - Class and function APIs
     This is an outline of the key types. Some types and other details
@@ -991,19 +1036,20 @@
 
     - The cache: sp_cache.h
 
-      class sp_cache
-      {
-        sp_cache();
-
-        void init();
+      /* Initialize the SP caching once at startup */
+      void sp_cache_init();
 
-        void cleanup();
+      /* Clear the cache *cp and set *cp to NULL */
+      void sp_cache_clear(sp_cache **cp);
 
-        void insert(sp_head *sp);
+      /* Insert an SP to cache. If **cp points to NULL, it's set to a
+         new cache */
+      void sp_cache_insert(sp_cache **cp, sp_head *sp);
 
-        sp_head *lookup(char *name, uint namelen);
+      /* Lookup an SP in cache */
+      sp_head *sp_cache_lookup(sp_cache **cp, char *name, uint namelen);
 
-        void remove(sp_head *sp);
-      }
+      /* Remove an SP from cache */
+      void sp_cache_remove(sp_cache **cp, sp_head *sp);
 
 --

sql/mysqld.cc

@@ -38,6 +38,7 @@
 #include <ft_global.h>
 #include <errmsg.h>
 #include "sp_rcontext.h"
+#include "sp_cache.h"
 
 #define mysqld_charset &my_charset_latin1
 
@@ -2140,6 +2141,7 @@ static int init_thread_environment()
   (void) pthread_mutex_init(&LOCK_rpl_status, MY_MUTEX_INIT_FAST);
   (void) pthread_cond_init(&COND_rpl_status, NULL);
 #endif
+  sp_cache_init();
   /* Parameter for threads created for connections */
   (void) pthread_attr_init(&connection_attrib);
   (void) pthread_attr_setdetachstate(&connection_attrib,

sql/sp.cc

@@ -251,13 +251,13 @@ sp_find_procedure(THD *thd, LEX_STRING *name)
 
   DBUG_PRINT("enter", ("name: %*s", name->length, name->str));
 
-  sp= thd->sp_proc_cache->lookup(name->str, name->length);
+  sp= sp_cache_lookup(&thd->sp_proc_cache, name->str, name->length);
   if (! sp)
   {
     if (db_find_routine(thd, TYPE_ENUM_PROCEDURE,
 			name->str, name->length, &sp) == SP_OK)
     {
-      thd->sp_proc_cache->insert(sp);
+      sp_cache_insert(&thd->sp_proc_cache, sp);
     }
   }
 
@@ -286,10 +286,10 @@ sp_drop_procedure(THD *thd, char *name, uint namelen)
   sp_head *sp;
   int ret;
 
-  sp= thd->sp_proc_cache->lookup(name, namelen);
+  sp= sp_cache_lookup(&thd->sp_proc_cache, name, namelen);
   if (sp)
   {
-    thd->sp_proc_cache->remove(sp);
+    sp_cache_remove(&thd->sp_proc_cache, sp);
     delete sp;
   }
   ret= db_drop_routine(thd, TYPE_ENUM_PROCEDURE, name, namelen);
@@ -312,7 +312,7 @@ sp_find_function(THD *thd, LEX_STRING *name)
 
   DBUG_PRINT("enter", ("name: %*s", name->length, name->str));
 
-  sp= thd->sp_func_cache->lookup(name->str, name->length);
+  sp= sp_cache_lookup(&thd->sp_func_cache, name->str, name->length);
   if (! sp)
   {
     if (db_find_routine(thd, TYPE_ENUM_FUNCTION,
@@ -344,10 +344,10 @@ sp_drop_function(THD *thd, char *name, uint namelen)
   sp_head *sp;
   int ret;
 
-  sp= thd->sp_func_cache->lookup(name, namelen);
+  sp= sp_cache_lookup(&thd->sp_func_cache, name, namelen);
   if (sp)
   {
-    thd->sp_func_cache->remove(sp);
+    sp_cache_remove(&thd->sp_func_cache, sp);
     delete sp;
   }
   ret= db_drop_routine(thd, TYPE_ENUM_FUNCTION, name, namelen);
@@ -363,7 +363,7 @@ sp_function_exists(THD *thd, LEX_STRING *name)
   bool ret= FALSE;
   bool opened= FALSE;
 
-  if (thd->sp_func_cache->lookup(name->str, name->length) ||
+  if (sp_cache_lookup(&thd->sp_func_cache, name->str, name->length) ||
       db_find_routine_aux(thd, TYPE_ENUM_FUNCTION,
 			  name->str, name->length, TL_READ,
 			  &table, &opened) == SP_OK)
@@ -419,7 +419,7 @@ sp_cache_functions(THD *thd, LEX *lex)
   {
     LEX_STRING *ls= (LEX_STRING *)hash_element(h, i);
 
-    if (! thd->sp_func_cache->lookup(ls->str, ls->length))
+    if (! sp_cache_lookup(&thd->sp_func_cache, ls->str, ls->length))
     {
       sp_head *sp;
       LEX *oldlex= thd->lex;
@@ -434,7 +434,7 @@ sp_cache_functions(THD *thd, LEX *lex)
 	thd->lex= oldlex;
 	if (ret)
 	  break;
-	thd->sp_func_cache->insert(sp);
+	sp_cache_insert(&thd->sp_func_cache, sp);
       }
       else
       {

sql/sp_cache.cc

@@ -22,6 +22,98 @@
 #include "sp_cache.h"
 #include "sp_head.h"
 
+static pthread_mutex_t Cversion_lock;
+static ulong Cversion = 0;
+
+void
+sp_cache_init()
+{
+  pthread_mutex_init(&Cversion_lock, MY_MUTEX_INIT_FAST);
+}
+
+void
+sp_cache_clear(sp_cache **cp)
+{
+  sp_cache *c= *cp;
+
+  if (c)
+  {
+    delete c;
+    *cp= NULL;
+  }
+}
+
+void
+sp_cache_insert(sp_cache **cp, sp_head *sp)
+{
+  sp_cache *c= *cp;
+
+  if (! c)
+    c= new sp_cache();
+  if (c)
+  {
+    ulong v;
+
+    pthread_mutex_lock(&Cversion_lock); // LOCK
+    v= Cversion;
+    pthread_mutex_unlock(&Cversion_lock); // UNLOCK
+
+    if (c->version < v)
+    {
+      if (*cp)
+	c->remove_all();
+      c->version= v;
+    }
+    c->insert(sp);
+    if (*cp == NULL)
+      *cp= c;
+  }
+}
+
+sp_head *
+sp_cache_lookup(sp_cache **cp, char *name, uint namelen)
+{
+  ulong v;
+  sp_cache *c= *cp;
+
+  if (! c)
+    return NULL;
+
+  pthread_mutex_lock(&Cversion_lock); // LOCK
+  v= Cversion;
+  pthread_mutex_unlock(&Cversion_lock); // UNLOCK
+
+  if (c->version < v)
+  {
+    c->remove_all();
+    c->version= v;
+    return NULL;
+  }
+  return c->lookup(name, namelen);
+}
+
+void
+sp_cache_remove(sp_cache **cp, sp_head *sp)
+{
+  sp_cache *c= *cp;
+
+  if (c)
+  {
+    ulong v;
+
+    pthread_mutex_lock(&Cversion_lock); // LOCK
+    v= Cversion++;
+    pthread_mutex_unlock(&Cversion_lock); // UNLOCK
+
+    if (c->version < v)
+      c->remove_all();
+    else
+      c->remove(sp);
+    c->version= v+1;
+  }
+}
+
+
 static byte *
 hash_get_key_for_sp_head(const byte *ptr, uint *plen,
 			       my_bool first)
@@ -44,6 +136,7 @@ sp_cache::init()
 {
   hash_init(&m_hashtable, system_charset_info, 0, 0, 0,
 	    hash_get_key_for_sp_head, 0, 0);
+  version= 0;
 }
 
 void

sql/sp_cache.h

@@ -23,11 +23,36 @@
 #endif
 
 class sp_head;
+class sp_cache;
+
+/* Initialize the SP caching once at startup */
+void sp_cache_init();
+
+/* Clear the cache *cp and set *cp to NULL */
+void sp_cache_clear(sp_cache **cp);
+
+/* Insert an SP to cache. If 'cp' points to NULL, it's set to a new cache */
+void sp_cache_insert(sp_cache **cp, sp_head *sp);
+
+/* Lookup an SP in cache */
+sp_head *sp_cache_lookup(sp_cache **cp, char *name, uint namelen);
+
+/* Remove an SP from cache */
+void sp_cache_remove(sp_cache **cp, sp_head *sp);
+
+
+/*
+ *
+ * The cache class. Don't use this directly, use the C API above
+ *
+ */
 
 class sp_cache
 {
 public:
 
+  ulong version;
+
   sp_cache();
 
   ~sp_cache();
@@ -56,6 +81,13 @@ public:
     hash_delete(&m_hashtable, (byte *)sp);
   }
 
+  inline void
+  remove_all()
+  {
+    cleanup();
+    init();
+  }
+
 private:
 
   HASH m_hashtable;

sql/sql_class.cc

@@ -156,8 +156,8 @@ THD::THD():user_time(0), is_fatal_error(0),
 	    (hash_get_key) get_var_key,
 	    (hash_free_key) free_user_var, 0);
 
-  sp_proc_cache= new sp_cache();
-  sp_func_cache= new sp_cache();
+  sp_proc_cache= NULL;
+  sp_func_cache= NULL;
 
   /* For user vars replication*/
   if (opt_bin_log)
@@ -260,8 +260,8 @@ void THD::change_user(void)
   hash_init(&user_vars, system_charset_info, USER_VARS_HASH_SIZE, 0, 0,
 	    (hash_get_key) get_var_key,
 	    (hash_free_key) free_user_var, 0);
-  sp_proc_cache->init();
-  sp_func_cache->init();
+  sp_cache_clear(&sp_proc_cache);
+  sp_cache_clear(&sp_func_cache);
 }
 
 
@@ -285,8 +285,8 @@ void THD::cleanup(void)
   close_temporary_tables(this);
   delete_dynamic(&user_var_events);
   hash_free(&user_vars);
-  sp_proc_cache->cleanup();
-  sp_func_cache->cleanup();
+  sp_cache_clear(&sp_proc_cache);
+  sp_cache_clear(&sp_func_cache);
   if (global_read_lock)
     unlock_global_read_lock(this);
   if (ull)
@@ -328,8 +328,8 @@ THD::~THD()
   }
 #endif
 
-  delete sp_proc_cache;
-  delete sp_func_cache;
+  sp_cache_clear(&sp_proc_cache);
+  sp_cache_clear(&sp_func_cache);
 
   DBUG_PRINT("info", ("freeing host"));
   if (host != localhost)			// If not pointer to constant