Below is the list of changes that have just been committed into a local
5.0 repository of psergey. When psergey does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
ChangeSet
1.1951 05/08/06 03:08:19 sergefp@stripped +4 -0
Fix for BUG#12228: SP cache code:
* sp_cache_insert, sp_cache_remove, sp_cache_invalidate must not delete sp_head* objects
as they may be called when this sp_head is being executed.
* Added sp_cache_flush_obsolete() function that may delete sp_head objects.
* Removed sp_cache_remove as there is no need for it now - changing one SP requires all
invalidates all others because of sp_lex_keeper::query_own_last.
There is no test case because it doesn't seem to be possible to cause thread races to end
the same way they end in heavy-load test. This patch removes the crash in heavy test.
sql/sql_parse.cc
1.458 05/08/06 03:08:15 sergefp@stripped +2 -0
Added sp_cache_flush_obsolete() call at query end, where we don't hold any sp_head* pointers.
sql/sp_cache.h
1.12 05/08/06 03:08:15 sergefp@stripped +23 -78
Fix for BUG#12228:
* Move class sp_cache to sp_cache.cc it is not needed in .h file
* Added comments
sql/sp_cache.cc
1.11 05/08/06 03:08:15 sergefp@stripped +105 -49
Fix for BUG#12228:
* Move class sp_cache to here from sp_cache.h
* Document the functions.
* sp_cache_insert, sp_cache_remove, sp_cache_invalidate must not delete sp_head* objects
as they may be called when this sp_head is being executed.
* Added sp_cache_flush_obsolete() function that may delete sp_head objects.
* Removed sp_cache_remove as there is no need for it now - changing one SP requires all
invalidates all others because of sp_lex_keeper::query_own_last.
sql/sp.cc
1.87 05/08/06 03:08:14 sergefp@stripped +4 -12
Fix for BUG#12228: When a routine is removed from the cache, invalidate all caches in all
threads. We need to do so because of sp_lex_keeper::prelocking_tables.
# This is a BitKeeper patch. What follows are the unified diffs for the
# set of deltas contained in the patch. The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User: sergefp
# Host: newbox.mylan
# Root: /home/psergey/mysql-5.0-bug12228-r4
--- 1.457/sql/sql_parse.cc 2005-08-03 10:13:59 +00:00
+++ 1.458/sql/sql_parse.cc 2005-08-06 03:08:15 +00:00
@@ -5411,6 +5411,8 @@
thd->cleanup_after_query();
DBUG_ASSERT(thd->change_list.is_empty());
}
+ sp_cache_flush_obsolete(&thd->sp_proc_cache);
+ sp_cache_flush_obsolete(&thd->sp_func_cache);
DBUG_VOID_RETURN;
}
--- 1.86/sql/sp.cc 2005-08-03 10:13:59 +00:00
+++ 1.87/sql/sp.cc 2005-08-06 03:08:14 +00:00
@@ -986,13 +986,11 @@
sp_drop_procedure(THD *thd, sp_name *name)
{
int ret;
- bool found;
DBUG_ENTER("sp_drop_procedure");
DBUG_PRINT("enter", ("name: %*s", name->m_name.length, name->m_name.str));
- found= sp_cache_remove(&thd->sp_proc_cache, name);
ret= db_drop_routine(thd, TYPE_ENUM_PROCEDURE, name);
- if (!found && !ret)
+ if (!ret)
sp_cache_invalidate();
DBUG_RETURN(ret);
}
@@ -1002,13 +1000,11 @@
sp_update_procedure(THD *thd, sp_name *name, st_sp_chistics *chistics)
{
int ret;
- bool found;
DBUG_ENTER("sp_update_procedure");
DBUG_PRINT("enter", ("name: %*s", name->m_name.length, name->m_name.str));
- found= sp_cache_remove(&thd->sp_proc_cache, name);
ret= db_update_routine(thd, TYPE_ENUM_PROCEDURE, name, chistics);
- if (!found && !ret)
+ if (!ret)
sp_cache_invalidate();
DBUG_RETURN(ret);
}
@@ -1099,13 +1095,11 @@
sp_drop_function(THD *thd, sp_name *name)
{
int ret;
- bool found;
DBUG_ENTER("sp_drop_function");
DBUG_PRINT("enter", ("name: %*s", name->m_name.length, name->m_name.str));
- found= sp_cache_remove(&thd->sp_func_cache, name);
ret= db_drop_routine(thd, TYPE_ENUM_FUNCTION, name);
- if (!found && !ret)
+ if (!ret)
sp_cache_invalidate();
DBUG_RETURN(ret);
}
@@ -1115,13 +1109,11 @@
sp_update_function(THD *thd, sp_name *name, st_sp_chistics *chistics)
{
int ret;
- bool found;
DBUG_ENTER("sp_update_procedure");
DBUG_PRINT("enter", ("name: %*s", name->m_name.length, name->m_name.str));
- found= sp_cache_remove(&thd->sp_func_cache, name);
ret= db_update_routine(thd, TYPE_ENUM_FUNCTION, name, chistics);
- if (!found && !ret)
+ if (!ret)
sp_cache_invalidate();
DBUG_RETURN(ret);
}
--- 1.10/sql/sp_cache.cc 2005-06-05 14:19:12 +00:00
+++ 1.11/sql/sp_cache.cc 2005-08-06 03:08:15 +00:00
@@ -24,17 +24,74 @@
static pthread_mutex_t Cversion_lock;
static ulong Cversion = 0;
-void
-sp_cache_init()
+
+/*
+ Cache of stored routines.
+*/
+
+class sp_cache
+{
+public:
+ ulong version;
+
+ sp_cache();
+ ~sp_cache();
+
+ inline void insert(sp_head *sp)
+ {
+ /* TODO: why don't we check return value? */
+ my_hash_insert(&m_hashtable, (const byte *)sp);
+ }
+
+ inline sp_head *lookup(char *name, uint namelen)
+ {
+ return (sp_head *)hash_search(&m_hashtable, (const byte *)name, namelen);
+ }
+
+ inline bool remove(char *name, uint namelen)
+ {
+ sp_head *sp= lookup(name, namelen);
+ if (sp)
+ {
+ hash_delete(&m_hashtable, (byte *)sp);
+ return TRUE;
+ }
+ return FALSE;
+ }
+
+ inline void remove_all()
+ {
+ cleanup();
+ init();
+ }
+
+private:
+ void init();
+ void cleanup();
+
+ /*
+ unique_hashtable<SP-qname, owner-ptr<sp_head*> >
+ */
+ HASH m_hashtable;
+}; // class sp_cache
+
+/* Initialize the SP caching once at startup */
+
+void sp_cache_init()
{
pthread_mutex_init(&Cversion_lock, MY_MUTEX_INIT_FAST);
}
-void
-sp_cache_clear(sp_cache **cp)
+
+/*
+ Clear the cache *cp and set *cp to NULL.
+ NOTE
+ This function doesn't invalidate other caches.
+*/
+
+void sp_cache_clear(sp_cache **cp)
{
sp_cache *c= *cp;
-
if (c)
{
delete c;
@@ -42,8 +99,10 @@
}
}
-void
-sp_cache_insert(sp_cache **cp, sp_head *sp)
+
+/* Insert a routine into the cache. */
+
+void sp_cache_insert(sp_cache **cp, sp_head *sp)
{
sp_cache *c= *cp;
@@ -51,76 +110,74 @@
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, sp_name *name)
+
+/*
+ Look up a routine in the cache.
+ NOTE
+ An obsolete (but not more obsolete then since last
+ sp_cache_flush_obsolete call) routine may be returned.
+*/
+
+sp_head *sp_cache_lookup(sp_cache **cp, sp_name *name)
{
ulong v;
sp_cache *c= *cp;
-
if (! c)
return NULL;
+ return c->lookup(name->m_qname.str, name->m_qname.length);
+}
+
+/*
+ Invalidate all routines in all caches.
+ NOTE
+ This is called when a VIEW definition is modifed. We can't destroy sp_head
+ objects here as one may modify VIEW definitions from prelocking-free SPs.
+*/
+void sp_cache_invalidate()
+{
pthread_mutex_lock(&Cversion_lock); // LOCK
- v= Cversion;
+ Cversion++;
pthread_mutex_unlock(&Cversion_lock); // UNLOCK
-
- if (c->version < v)
- {
- c->remove_all();
- c->version= v;
- return NULL;
- }
- return c->lookup(name->m_qname.str, name->m_qname.length);
}
-bool
-sp_cache_remove(sp_cache **cp, sp_name *name)
+
+/*
+ Remove out-of-date SPs from the cache.
+ NOTE
+ This invalidates pointers to sp_head objects this thread uses.
+ In practice that means 'dont call this function when inside SP'.
+*/
+
+void sp_cache_flush_obsolete(sp_cache **cp)
{
sp_cache *c= *cp;
- bool found= FALSE;
+ sp_head *sp;
if (c)
{
ulong v;
-
pthread_mutex_lock(&Cversion_lock); // LOCK
- v= Cversion++;
+ v= Cversion;
pthread_mutex_unlock(&Cversion_lock); // UNLOCK
-
if (c->version < v)
+ {
+ /* We need to delete all elements. */
c->remove_all();
- else
- found= c->remove(name->m_qname.str, name->m_qname.length);
- c->version= v+1;
+ c->version= v;
+ }
}
- return found;
}
-void
-sp_cache_invalidate()
-{
- pthread_mutex_lock(&Cversion_lock); // LOCK
- Cversion++;
- pthread_mutex_unlock(&Cversion_lock); // UNLOCK
-}
+/*************************************************************************
+ Internal functions
+ *************************************************************************/
static byte *
hash_get_key_for_sp_head(const byte *ptr, uint *plen,
@@ -136,7 +193,6 @@
hash_free_sp_head(void *p)
{
sp_head *sp= (sp_head *)p;
-
delete sp;
}
--- 1.11/sql/sp_cache.h 2005-08-03 10:12:26 +00:00
+++ 1.12/sql/sp_cache.h 2005-08-06 03:08:15 +00:00
@@ -25,94 +25,39 @@
/*
Stored procedures/functions cache. This is used as follows:
* Each thread has its own cache.
- * Each sp_head object is put into its thread cache before it is used, and
+ * Each sp_head object is put into its thread cache before it is used, and
then remains in the cache until deleted.
*/
class sp_head;
class sp_cache;
-/* Initialize the SP caching once at startup */
-void sp_cache_init();
+/*
+ Cache usage scenarios:
+ 1. Application-wide init:
+ sp_cache_init();
+
+ 2. SP execution in thread:
+ 2.1 While holding sp_head* pointers:
+
+ // look up a routine in the cache (no checks if it is date or not)
+ sp_cache_lookup();
+
+ sp_cache_insert();
+ sp_cache_invalidate();
+
+ 2.2 When not holding any sp_head* pointers (at query end):
+ sp_cache_flush_obsolete();
+
+ 3. Before thread exit:
+ sp_cache_clear();
+*/
-/* Clear the cache *cp and set *cp to NULL */
+void sp_cache_init();
void sp_cache_clear(sp_cache **cp);
-
-/* Insert an SP into 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, sp_name *name);
-
-/*
- Remove an SP from cache, and also bump the Cversion number so all other
- caches are invalidated.
- Returns true if something was removed.
-*/
-bool sp_cache_remove(sp_cache **cp, sp_name *name);
-
-/* Invalidate all existing SP caches by bumping Cversion number. */
void sp_cache_invalidate();
-
-
-/*
- *
- * The cache class. Don't use this directly, use the C API above
- *
- */
-
-class sp_cache
-{
-public:
-
- ulong version;
-
- sp_cache();
-
- ~sp_cache();
-
- void
- init();
-
- void
- cleanup();
-
- inline void
- insert(sp_head *sp)
- {
- my_hash_insert(&m_hashtable, (const byte *)sp);
- }
-
- inline sp_head *
- lookup(char *name, uint namelen)
- {
- return (sp_head *)hash_search(&m_hashtable, (const byte *)name, namelen);
- }
-
- inline bool
- remove(char *name, uint namelen)
- {
- sp_head *sp= lookup(name, namelen);
-
- if (sp)
- {
- hash_delete(&m_hashtable, (byte *)sp);
- return TRUE;
- }
- return FALSE;
- }
-
- inline void
- remove_all()
- {
- cleanup();
- init();
- }
-
-private:
-
- HASH m_hashtable;
-
-}; // class sp_cache
+void sp_cache_flush_obsolete(sp_cache **cp);
#endif /* _SP_CACHE_H_ */
| Thread |
|---|
| • bk commit into 5.0 tree (sergefp:1.1951) BUG#12228 | Sergey Petrunia | 6 Aug |