List:Commits« Previous MessageNext Message »
From:Luis Soares Date:March 30 2009 1:47pm
Subject:bzr commit into mysql-6.0-rpl branch (luis.soares:2833) Bug#43076
View as plain text  
#At file:///home/lsoares/Workspace/mysql-server/bugfix/43076/6.0-rpl/ based on revid:aelkin@stripped

 2833 Luis Soares	2009-03-30
      BUG#43076: rpl.rpl_idempotency fails sporadically on pushbuild
      
      There were some valgrind warnings in strlen. This happened 
      because SHOW SLAVE STATUS connection thread would race with 
      SQL thread for the error message buffer. Sometimes the connection
      thread was getting a half way completed message (ie, string 
      without NULL terminator, causing valgrind to complain with: 
      "Conditional jump or move depends on uninitialised value(s)".
      
      This patch addresses this issue by synchronizing the error object
      between IO/SQL thread and user thread on SHOW SLAVE STATUS.
     @ sql/rpl_reporting.cc
        Added unconditional synchronization and destructor implementation as requested by reviewer.
     @ sql/rpl_reporting.h
        Added the mutex as discussed within the team. Changes include: mutex 
        variable, mutex initialization.
        
        Declared Slave_reporting_capability copy constructor and copy 
        assignement operator as private and destructor as virtual.
     @ sql/slave.cc
        Added synchronization on SHOW SLAVE STATUS.

    modified:
      sql/rpl_reporting.cc
      sql/rpl_reporting.h
      sql/slave.cc
=== modified file 'sql/rpl_reporting.cc'
--- a/sql/rpl_reporting.cc	2007-06-11 20:15:39 +0000
+++ b/sql/rpl_reporting.cc	2009-03-30 13:47:34 +0000
@@ -13,6 +13,7 @@ Slave_reporting_capability::report(logle
   va_list args;
   va_start(args, msg);
 
+  pthread_mutex_lock(&err_lock);
   switch (level)
   {
   case ERROR_LEVEL:
@@ -38,6 +39,7 @@ Slave_reporting_capability::report(logle
 
   my_vsnprintf(pbuff, pbuffsize, msg, args);
 
+  pthread_mutex_unlock(&err_lock);
   va_end(args);
 
   /* If the msg string ends with '.', do not add a ',' it would be ugly */
@@ -46,3 +48,8 @@ Slave_reporting_capability::report(logle
                   (pbuff[0] && *(strend(pbuff)-1) == '.') ? "" : ",",
                   err_code);
 }
+
+Slave_reporting_capability::~Slave_reporting_capability()
+{
+  pthread_mutex_destroy(&err_lock);
+}

=== modified file 'sql/rpl_reporting.h'
--- a/sql/rpl_reporting.h	2007-06-11 20:15:39 +0000
+++ b/sql/rpl_reporting.h	2009-03-30 13:47:34 +0000
@@ -16,6 +16,8 @@
 class Slave_reporting_capability
 {
 public:
+  /** lock used to synchronize m_last_error on 'SHOW SLAVE STATUS' **/
+  mutable pthread_mutex_t err_lock;
   /**
      Constructor.
 
@@ -24,6 +26,7 @@ public:
   Slave_reporting_capability(char const *thread_name)
     : m_thread_name(thread_name)
   {
+      pthread_mutex_init(&err_lock, MY_MUTEX_INIT_FAST);
   }
 
   /**
@@ -44,7 +47,9 @@ public:
      STATUS</code>.
    */
   void clear_error() {
+    pthread_mutex_lock(&err_lock);
     m_last_error.clear();
+    pthread_mutex_unlock(&err_lock);
   }
 
   /**
@@ -72,6 +77,7 @@ public:
 
   Error const& last_error() const { return m_last_error; }
 
+  virtual ~Slave_reporting_capability()= 0;
 private:
   /**
      Last error produced by the I/O or SQL thread respectively.
@@ -79,6 +85,10 @@ private:
   mutable Error m_last_error;
 
   char const *const m_thread_name;
+
+  // not implemented
+  Slave_reporting_capability(const Slave_reporting_capability& rhs);
+  Slave_reporting_capability& operator=(const Slave_reporting_capability& rhs);
 };
 
 #endif // RPL_REPORTING_H

=== modified file 'sql/slave.cc'
--- a/sql/slave.cc	2009-03-16 11:56:39 +0000
+++ b/sql/slave.cc	2009-03-30 13:47:34 +0000
@@ -1493,6 +1493,8 @@ bool show_master_info(THD* thd, Master_i
 
     pthread_mutex_lock(&mi->data_lock);
     pthread_mutex_lock(&mi->rli.data_lock);
+    pthread_mutex_lock(&mi->err_lock);
+    pthread_mutex_lock(&mi->rli.err_lock);
     protocol->store(mi->host, &my_charset_bin);
     protocol->store(mi->user, &my_charset_bin);
     protocol->store((uint32) mi->port);
@@ -1619,6 +1621,8 @@ bool show_master_info(THD* thd, Master_i
     // Master_Server_id
     protocol->store((uint32) mi->master_id);
 
+    pthread_mutex_unlock(&mi->rli.err_lock);
+    pthread_mutex_unlock(&mi->err_lock);
     pthread_mutex_unlock(&mi->rli.data_lock);
     pthread_mutex_unlock(&mi->data_lock);
 


Attachment: [text/bzr-bundle] bzr/luis.soares@sun.com-20090330134734-o8f47vtwxynzhs9k.bundle
Thread
bzr commit into mysql-6.0-rpl branch (luis.soares:2833) Bug#43076Luis Soares30 Mar