Merge lp:~stewart/drizzle/bug751191 into lp:~drizzle-trunk/drizzle/development

[Stewart Smith]

protect g_refresh_version from races by making it atomic.

http://jenkins.drizzle.org/view/Drizzle-param/job/drizzle-param/841/

[Brian Aker (brianaker)]

We need a test case to show this bug. Refresh works based on a lossy view of the value.

Turning this to atomic will directly effect performance (I know, I once did this patch wondering about refresh).

Really? We need to get rid of refresh entirely.

[Stewart Smith (stewart)]

On Wed, 22 Jun 2011 17:26:37 -0000, Brian Aker <email address hidden> wrote:
> Really? We need to get rid of refresh entirely.

here here.

--
Stewart Smith

Unmerged revisions

2344. By Stewart Smith on 2011-06-21

global g_refresh_version wasn't protected against races. Make it an atomic variable.

=== modified file 'drizzled/main.cc'
--- drizzled/main.cc	2011-06-16 15:27:28 +0000
+++ drizzled/main.cc	2011-06-21 06:21:16 +0000
@@ -71,6 +71,7 @@
 #include <drizzled/sql_base.h>
 #include <drizzled/sql_lex.h>
 #include <drizzled/system_variables.h>
+#include <drizzled/atomics.h>
 
 using namespace drizzled;
 using namespace std;
@@ -80,6 +81,9 @@
 
 extern bool opt_daemon;
 
+namespace drizzled {
+  extern drizzled::atomic<uint64_t> g_refresh_version;
+}
 
 /**
   All global error messages are sent here where the first one is stored
@@ -332,6 +336,8 @@
     server_id= 1;
   }
 
+  drizzled::g_refresh_version.fetch_and_store(1); // needs to be set to 1.
+
   try
   {
     if (init_server_components(modules))

=== modified file 'drizzled/open_tables_state.h'
--- drizzled/open_tables_state.h	2011-04-30 17:00:52 +0000
+++ drizzled/open_tables_state.h	2011-06-21 06:21:16 +0000
@@ -21,10 +21,11 @@
 
 #include <drizzled/common_fwd.h>
 #include <drizzled/lock.h>
+#include <drizzled/atomics.h>
 
 namespace drizzled {
 
-extern uint64_t g_refresh_version;
+extern drizzled::atomic<uint64_t> g_refresh_version;
 
 /**
   Class that holds information about tables which were opened and locked

=== modified file 'drizzled/session.cc'
--- drizzled/session.cc	2011-06-05 23:06:29 +0000
+++ drizzled/session.cc	2011-06-21 06:21:16 +0000
@@ -84,6 +84,7 @@
 #include <drizzled/util/functors.h>
 #include <drizzled/util/storable.h>
 #include <plugin/myisam/myisam.h>
+#include <drizzled/atomics.h>
 
 #include <algorithm>
 #include <climits>
@@ -105,7 +106,7 @@
 
 const char * const Session::DEFAULT_WHERE= "field list";
 
-uint64_t g_refresh_version = 1;
+drizzled::atomic<uint64_t> g_refresh_version;
 
 bool Key_part_spec::operator==(const Key_part_spec& other) const
 {

=== modified file 'drizzled/sql_base.cc'
--- drizzled/sql_base.cc	2011-06-17 13:13:07 +0000
+++ drizzled/sql_base.cc	2011-06-21 06:21:16 +0000
@@ -72,7 +72,7 @@
 
 void table_cache_free()
 {
-  g_refresh_version++;				// Force close of open tables
+  g_refresh_version.increment();	// Force close of open tables
 
   table::getUnused().clear();
   table::getCache().clear();
@@ -169,7 +169,7 @@
     boost::mutex::scoped_lock scopedLock(table::Cache::mutex()); /* Optionally lock for remove tables from open_cahe if not in use */
     if (not tables)
     {
-      g_refresh_version++;				// Force close of open tables
+      g_refresh_version.increment();	// Force close of open tables
       table::getUnused().clear();
     }
     else

=== modified file 'plugin/signal_handler/signal_handler.cc'
--- plugin/signal_handler/signal_handler.cc	2011-06-17 13:00:32 +0000
+++ plugin/signal_handler/signal_handler.cc	2011-06-21 06:21:16 +0000
@@ -205,7 +205,7 @@
     case SIGHUP:
       if (!abort_loop)
       {
-        g_refresh_version++;
+        g_refresh_version.increment();
         drizzled::plugin::StorageEngine::flushLogs(NULL);
       }
       break;