List:Commits« Previous MessageNext Message »
From:Nuno Carvalho Date:September 11 2012 3:32pm
Subject:bzr push into mysql-trunk-wl6402 branch (nuno.carvalho:4436 to 4437) WL#6402
View as plain text  
 4437 Nuno Carvalho	2012-09-11
      WL#6402: Non blocking show slave status
      
      Introduced a new mutex, info_thd_lock, to increase the locks granularity
      on slave workflow, this way there is no need to hold run_lock before
      reading info_thd on SHOW SLAVE STATUS.

    modified:
      sql/mysqld.cc
      sql/mysqld.h
      sql/rpl_info.cc
      sql/rpl_info.h
      sql/rpl_info_factory.cc
      sql/rpl_mi.cc
      sql/rpl_mi.h
      sql/rpl_rli.cc
      sql/rpl_rli.h
      sql/rpl_rli_pdb.cc
      sql/rpl_rli_pdb.h
      sql/rpl_slave.cc
      sql/rpl_slave.h
      sql/sql_parse.cc
 4436 Nuno Carvalho	2012-09-06
      WL#6402: Non blocking show slave status
      
      Updated mysqld--help-win test case result file.

    modified:
      mysql-test/r/mysqld--help-win.result
=== modified file 'sql/mysqld.cc'
--- a/sql/mysqld.cc	revid:nuno.carvalho@stripped
+++ b/sql/mysqld.cc	revid:nuno.carvalho@stripped
@@ -9009,9 +9009,9 @@ PSI_mutex_key
   key_LOCK_system_variables_hash, key_LOCK_table_share, key_LOCK_thd_data,
   key_LOCK_user_conn, key_LOCK_uuid_generator, key_LOG_LOCK_log,
   key_master_info_data_lock, key_master_info_run_lock,
-  key_master_info_sleep_lock,
+  key_master_info_sleep_lock, key_master_info_thd_lock,
   key_mutex_slave_reporting_capability_err_lock, key_relay_log_info_data_lock,
-  key_relay_log_info_sleep_lock,
+  key_relay_log_info_sleep_lock, key_relay_log_info_thd_lock,
   key_relay_log_info_log_space_lock, key_relay_log_info_run_lock,
   key_mutex_slave_parallel_pend_jobs, key_mutex_mts_temp_tables_lock,
   key_mutex_slave_parallel_worker,
@@ -9082,9 +9082,11 @@ static PSI_mutex_info all_server_mutexes
   { &key_master_info_data_lock, "Master_info::data_lock", 0},
   { &key_master_info_run_lock, "Master_info::run_lock", 0},
   { &key_master_info_sleep_lock, "Master_info::sleep_lock", 0},
+  { &key_master_info_thd_lock, "Master_info::info_thd_lock", 0},
   { &key_mutex_slave_reporting_capability_err_lock, "Slave_reporting_capability::err_lock", 0},
   { &key_relay_log_info_data_lock, "Relay_log_info::data_lock", 0},
   { &key_relay_log_info_sleep_lock, "Relay_log_info::sleep_lock", 0},
+  { &key_relay_log_info_thd_lock, "Relay_log_info::info_thd_lock", 0},
   { &key_relay_log_info_log_space_lock, "Relay_log_info::log_space_lock", 0},
   { &key_relay_log_info_run_lock, "Relay_log_info::run_lock", 0},
   { &key_mutex_slave_parallel_pend_jobs, "Relay_log_info::pending_jobs_lock", 0},

=== modified file 'sql/mysqld.h'
--- a/sql/mysqld.h	revid:nuno.carvalho@stripped
+++ b/sql/mysqld.h	revid:nuno.carvalho@stripped
@@ -316,9 +316,9 @@ extern PSI_mutex_key
   key_LOCK_table_share, key_LOCK_thd_data,
   key_LOCK_user_conn, key_LOCK_uuid_generator, key_LOG_LOCK_log,
   key_master_info_data_lock, key_master_info_run_lock,
-  key_master_info_sleep_lock,
+  key_master_info_sleep_lock, key_master_info_thd_lock,
   key_mutex_slave_reporting_capability_err_lock, key_relay_log_info_data_lock,
-  key_relay_log_info_sleep_lock,
+  key_relay_log_info_sleep_lock, key_relay_log_info_thd_lock,
   key_relay_log_info_log_space_lock, key_relay_log_info_run_lock,
   key_mutex_slave_parallel_pend_jobs, key_mutex_mts_temp_tables_lock,
   key_mutex_slave_parallel_worker,

=== modified file 'sql/rpl_info.cc'
--- a/sql/rpl_info.cc	revid:nuno.carvalho@stripped
+++ b/sql/rpl_info.cc	revid:nuno.carvalho@stripped
@@ -22,6 +22,7 @@ Rpl_info::Rpl_info(const char* type
                    ,PSI_mutex_key *param_key_info_run_lock,
                    PSI_mutex_key *param_key_info_data_lock,
                    PSI_mutex_key *param_key_info_sleep_lock,
+                   PSI_mutex_key *param_key_info_thd_lock,
                    PSI_mutex_key *param_key_info_data_cond,
                    PSI_mutex_key *param_key_info_start_cond,
                    PSI_mutex_key *param_key_info_stop_cond,
@@ -34,6 +35,7 @@ Rpl_info::Rpl_info(const char* type
   key_info_run_lock(param_key_info_run_lock),
   key_info_data_lock(param_key_info_data_lock),
   key_info_sleep_lock(param_key_info_sleep_lock),
+  key_info_thd_lock(param_key_info_thd_lock),
   key_info_data_cond(param_key_info_data_cond),
   key_info_start_cond(param_key_info_start_cond),
   key_info_stop_cond(param_key_info_stop_cond),
@@ -50,6 +52,8 @@ Rpl_info::Rpl_info(const char* type
                    &data_lock, MY_MUTEX_INIT_FAST);
   mysql_mutex_init(*key_info_sleep_lock,
                     &sleep_lock, MY_MUTEX_INIT_FAST);
+  mysql_mutex_init(*key_info_thd_lock,
+                    &info_thd_lock, MY_MUTEX_INIT_FAST);
   mysql_cond_init(*key_info_data_cond, &data_cond, NULL);
   mysql_cond_init(*key_info_start_cond, &start_cond, NULL);
   mysql_cond_init(*key_info_stop_cond, &stop_cond, NULL);
@@ -58,6 +62,7 @@ Rpl_info::Rpl_info(const char* type
   mysql_mutex_init(NULL, &run_lock, MY_MUTEX_INIT_FAST);
   mysql_mutex_init(NULL, &data_lock, MY_MUTEX_INIT_FAST);
   mysql_mutex_init(NULL, &sleep_lock, MY_MUTEX_INIT_FAST);
+  mysql_mutex_init(NULL, &info_thd_lock, MY_MUTEX_INIT_FAST);
   mysql_cond_init(NULL, &data_cond, NULL);
   mysql_cond_init(NULL, &start_cond, NULL);
   mysql_cond_init(NULL, &stop_cond, NULL);
@@ -72,6 +77,7 @@ Rpl_info::~Rpl_info()
   mysql_mutex_destroy(&run_lock);
   mysql_mutex_destroy(&data_lock);
   mysql_mutex_destroy(&sleep_lock);
+  mysql_mutex_destroy(&info_thd_lock);
   mysql_cond_destroy(&data_cond);
   mysql_cond_destroy(&start_cond);
   mysql_cond_destroy(&stop_cond);

=== modified file 'sql/rpl_info.h'
--- a/sql/rpl_info.h	revid:nuno.carvalho@stripped
+++ b/sql/rpl_info.h	revid:nuno.carvalho@stripped
@@ -30,8 +30,13 @@ public:
     standard lock acquisition order to avoid deadlocks:
     run_lock, data_lock, relay_log.LOCK_log, relay_log.LOCK_index
     run_lock, sleep_lock
+
+    info_thd_lock is to protect operations on info_thd:
+    - before *reading* info_thd we must hold *either* info_thd_lock or
+      run_lock;
+    - before *writing* we must hold *both* info_thd_lock and run_lock.
   */
-  mysql_mutex_t data_lock, run_lock, sleep_lock;
+  mysql_mutex_t data_lock, run_lock, sleep_lock, info_thd_lock;
   /*
     start_cond is broadcast when SQL thread is started
     stop_cond  - when stopped
@@ -41,7 +46,8 @@ public:
   mysql_cond_t data_cond, start_cond, stop_cond, sleep_cond;
 
 #ifdef HAVE_PSI_INTERFACE
-  PSI_mutex_key *key_info_run_lock, *key_info_data_lock, *key_info_sleep_lock;
+  PSI_mutex_key *key_info_run_lock, *key_info_data_lock, *key_info_sleep_lock,
+                *key_info_thd_lock;
 
   PSI_mutex_key *key_info_data_cond, *key_info_start_cond, *key_info_stop_cond,
                 *key_info_sleep_cond;
@@ -140,6 +146,7 @@ protected:
            ,PSI_mutex_key *param_key_info_run_lock,
            PSI_mutex_key *param_key_info_data_lock,
            PSI_mutex_key *param_key_info_sleep_lock,
+           PSI_mutex_key *param_key_info_thd_lock,
            PSI_mutex_key *param_key_info_data_cond,
            PSI_mutex_key *param_key_info_start_cond,
            PSI_mutex_key *param_key_info_stop_cond,

=== modified file 'sql/rpl_info_factory.cc'
--- a/sql/rpl_info_factory.cc	revid:nuno.carvalho@stripped
+++ b/sql/rpl_info_factory.cc	revid:nuno.carvalho@stripped
@@ -99,6 +99,7 @@ Master_info *Rpl_info_factory::create_mi
                             &key_master_info_run_lock,
                             &key_master_info_data_lock,
                             &key_master_info_sleep_lock,
+                            &key_master_info_thd_lock,
                             &key_master_info_data_cond,
                             &key_master_info_start_cond,
                             &key_master_info_stop_cond,
@@ -217,6 +218,7 @@ Relay_log_info *Rpl_info_factory::create
                                 ,&key_relay_log_info_run_lock,
                                 &key_relay_log_info_data_lock,
                                 &key_relay_log_info_sleep_lock,
+                                &key_relay_log_info_thd_lock,
                                 &key_relay_log_info_data_cond,
                                 &key_relay_log_info_start_cond,
                                 &key_relay_log_info_stop_cond,
@@ -382,6 +384,7 @@ Slave_worker *Rpl_info_factory::create_w
                                  ,&key_relay_log_info_run_lock,
                                  &key_relay_log_info_data_lock,
                                  &key_relay_log_info_sleep_lock,
+                                 &key_relay_log_info_thd_lock,
                                  &key_relay_log_info_data_cond,
                                  &key_relay_log_info_start_cond,
                                  &key_relay_log_info_stop_cond,

=== modified file 'sql/rpl_mi.cc'
--- a/sql/rpl_mi.cc	revid:nuno.carvalho@stripped
+++ b/sql/rpl_mi.cc	revid:nuno.carvalho@stripped
@@ -96,6 +96,7 @@ Master_info::Master_info(
                          PSI_mutex_key *param_key_info_run_lock,
                          PSI_mutex_key *param_key_info_data_lock,
                          PSI_mutex_key *param_key_info_sleep_lock,
+                         PSI_mutex_key *param_key_info_thd_lock,
                          PSI_mutex_key *param_key_info_data_cond,
                          PSI_mutex_key *param_key_info_start_cond,
                          PSI_mutex_key *param_key_info_stop_cond,
@@ -106,7 +107,7 @@ Master_info::Master_info(
    :Rpl_info("I/O"
 #ifdef HAVE_PSI_INTERFACE
              ,param_key_info_run_lock, param_key_info_data_lock,
-             param_key_info_sleep_lock,
+             param_key_info_sleep_lock, param_key_info_thd_lock,
              param_key_info_data_cond, param_key_info_start_cond,
              param_key_info_stop_cond, param_key_info_sleep_cond
 #endif

=== modified file 'sql/rpl_mi.h'
--- a/sql/rpl_mi.h	revid:nuno.carvalho@stripped
+++ b/sql/rpl_mi.h	revid:nuno.carvalho@stripped
@@ -370,6 +370,7 @@ private:
               PSI_mutex_key *param_key_info_run_lock,
               PSI_mutex_key *param_key_info_data_lock,
               PSI_mutex_key *param_key_info_sleep_lock,
+              PSI_mutex_key *param_key_info_thd_lock,
               PSI_mutex_key *param_key_info_data_cond,
               PSI_mutex_key *param_key_info_start_cond,
               PSI_mutex_key *param_key_info_stop_cond,

=== modified file 'sql/rpl_rli.cc'
--- a/sql/rpl_rli.cc	revid:nuno.carvalho@stripped
+++ b/sql/rpl_rli.cc	revid:nuno.carvalho@stripped
@@ -56,6 +56,7 @@ Relay_log_info::Relay_log_info(bool is_s
                                ,PSI_mutex_key *param_key_info_run_lock,
                                PSI_mutex_key *param_key_info_data_lock,
                                PSI_mutex_key *param_key_info_sleep_lock,
+                               PSI_mutex_key *param_key_info_thd_lock,
                                PSI_mutex_key *param_key_info_data_cond,
                                PSI_mutex_key *param_key_info_start_cond,
                                PSI_mutex_key *param_key_info_stop_cond,
@@ -66,7 +67,7 @@ Relay_log_info::Relay_log_info(bool is_s
    :Rpl_info("SQL"
 #ifdef HAVE_PSI_INTERFACE
              ,param_key_info_run_lock, param_key_info_data_lock,
-             param_key_info_sleep_lock,
+             param_key_info_sleep_lock, param_key_info_thd_lock,
              param_key_info_data_cond, param_key_info_start_cond,
              param_key_info_stop_cond, param_key_info_sleep_cond
 #endif

=== modified file 'sql/rpl_rli.h'
--- a/sql/rpl_rli.h	revid:nuno.carvalho@stripped
+++ b/sql/rpl_rli.h	revid:nuno.carvalho@stripped
@@ -834,6 +834,7 @@ public:
                  ,PSI_mutex_key *param_key_info_run_lock,
                  PSI_mutex_key *param_key_info_data_lock,
                  PSI_mutex_key *param_key_info_sleep_lock,
+                 PSI_mutex_key *param_key_info_thd_lock,
                  PSI_mutex_key *param_key_info_data_cond,
                  PSI_mutex_key *param_key_info_start_cond,
                  PSI_mutex_key *param_key_info_stop_cond,

=== modified file 'sql/rpl_rli_pdb.cc'
--- a/sql/rpl_rli_pdb.cc	revid:nuno.carvalho@stripped
+++ b/sql/rpl_rli_pdb.cc	revid:nuno.carvalho@stripped
@@ -69,6 +69,7 @@ Slave_worker::Slave_worker(Relay_log_inf
                            ,PSI_mutex_key *param_key_info_run_lock,
                            PSI_mutex_key *param_key_info_data_lock,
                            PSI_mutex_key *param_key_info_sleep_lock,
+                           PSI_mutex_key *param_key_info_thd_lock,
                            PSI_mutex_key *param_key_info_data_cond,
                            PSI_mutex_key *param_key_info_start_cond,
                            PSI_mutex_key *param_key_info_stop_cond,
@@ -79,7 +80,7 @@ Slave_worker::Slave_worker(Relay_log_inf
   : Relay_log_info(FALSE
 #ifdef HAVE_PSI_INTERFACE
                    ,param_key_info_run_lock, param_key_info_data_lock,
-                   param_key_info_sleep_lock,
+                   param_key_info_sleep_lock, param_key_info_thd_lock,
                    param_key_info_data_cond, param_key_info_start_cond,
                    param_key_info_stop_cond, param_key_info_sleep_cond
 #endif
@@ -113,7 +114,9 @@ Slave_worker::~Slave_worker()
   delete_dynamic(&curr_group_exec_parts);
   mysql_mutex_destroy(&jobs_lock);
   mysql_cond_destroy(&jobs_cond);
+  mysql_mutex_lock(&info_thd_lock);
   info_thd= NULL;
+  mysql_mutex_unlock(&info_thd_lock);
   set_rli_description_event(NULL);
 }
 

=== modified file 'sql/rpl_rli_pdb.h'
--- a/sql/rpl_rli_pdb.h	revid:nuno.carvalho@stripped
+++ b/sql/rpl_rli_pdb.h	revid:nuno.carvalho@stripped
@@ -287,6 +287,7 @@ public:
                ,PSI_mutex_key *param_key_info_run_lock,
                PSI_mutex_key *param_key_info_data_lock,
                PSI_mutex_key *param_key_info_sleep_lock,
+               PSI_mutex_key *param_key_info_thd_lock,
                PSI_mutex_key *param_key_info_data_cond,
                PSI_mutex_key *param_key_info_start_cond,
                PSI_mutex_key *param_key_info_stop_cond,

=== modified file 'sql/rpl_slave.cc'
--- a/sql/rpl_slave.cc	revid:nuno.carvalho@stripped
+++ b/sql/rpl_slave.cc	revid:nuno.carvalho@stripped
@@ -2412,12 +2412,10 @@ int register_slave_on_master(MYSQL* mysq
 
   @param mi Pointer to Master_info object for the IO thread.
 
-  @param hold_run_locks Indicates if command must hold run_lock mutexes.
-
   @retval FALSE success
   @retval TRUE failure
 */
-bool show_slave_status(THD* thd, Master_info* mi, bool hold_run_locks)
+bool show_slave_status(THD* thd, Master_info* mi)
 {
   // TODO: fix this for multi-master
   List<Item> field_list;
@@ -2545,27 +2543,16 @@ bool show_slave_status(THD* thd, Master_
 
     /*
       slave_running can be accessed without run_lock but not other
-      non-volotile members like mi->info_thd, which is guarded by the mutex.
-
-      "SHOW SLAVE STATUS NONBLOCKING" do not holds LOCK_active_mi,
-      mi->run_lock and mi->rli->run_lock mutexes, slave status will be
-      returned even when slave is being stopped what may cause
-      inconsistent results.
-      This nonblocking command is intended to be executed by monitoring
-      tools on which is more important to do not block requests then have
-      the last updated status.
+      non-volotile members like mi->info_thd or rli->info_thd, for
+      them either info_thd_lock or run_lock hold is required.
     */
-    if (hold_run_locks)
-      mysql_mutex_lock(&mi->run_lock);
+    mysql_mutex_lock(&mi->info_thd_lock);
     protocol->store(mi->info_thd ? mi->info_thd->get_proc_info() : "", &my_charset_bin);
-    if (hold_run_locks)
-      mysql_mutex_unlock(&mi->run_lock);
+    mysql_mutex_unlock(&mi->info_thd_lock);
 
-    if (hold_run_locks)
-      mysql_mutex_lock(&mi->rli->run_lock);
+    mysql_mutex_lock(&mi->rli->info_thd_lock);
     slave_sql_running_state= const_cast<char *>(mi->rli->info_thd ? mi->rli->info_thd->get_proc_info() : "");
-    if (hold_run_locks)
-      mysql_mutex_unlock(&mi->rli->run_lock);
+    mysql_mutex_unlock(&mi->rli->info_thd_lock);
 
     mysql_mutex_lock(&mi->data_lock);
     mysql_mutex_lock(&mi->rli->data_lock);
@@ -4321,7 +4308,9 @@ err:
 
   mi->abort_slave= 0;
   mi->slave_running= 0;
+  mysql_mutex_lock(&mi->info_thd_lock);
   mi->info_thd= 0;
+  mysql_mutex_unlock(&mi->info_thd_lock);
   /*
     Note: the order of the two following calls (first broadcast, then unlock)
     is important. Otherwise a killer_thread can execute between the calls and
@@ -4407,7 +4396,9 @@ pthread_handler_t handle_slave_worker(vo
     sql_print_error("Failed during slave worker initialization");
     goto err;
   }
+  mysql_mutex_lock(&w->info_thd_lock);
   w->info_thd= thd;
+  mysql_mutex_unlock(&w->info_thd_lock);
   thd->thread_stack = (char*)&thd;
   
   pthread_detach_this_thread();
@@ -5305,7 +5296,9 @@ pthread_handler_t handle_slave_sql(void
 
   thd = new THD; // note that contructor of THD uses DBUG_ !
   thd->thread_stack = (char*)&thd; // remember where our stack is
+  mysql_mutex_lock(&rli->info_thd_lock);
   rli->info_thd= thd;
+  mysql_mutex_unlock(&rli->info_thd_lock);
   
   /* Inform waiting threads that slave has started */
   rli->slave_run_id++;
@@ -5644,7 +5637,9 @@ llstr(rli->get_group_master_log_pos(), l
   net_end(&thd->net); // destructor will not free it, because we are weird
   DBUG_ASSERT(rli->info_thd == thd);
   THD_CHECK_SENTRY(thd);
+  mysql_mutex_lock(&rli->info_thd_lock);
   rli->info_thd= 0;
+  mysql_mutex_unlock(&rli->info_thd_lock);
   set_thd_in_use_temporary_tables(rli);  // (re)set info_thd in use for saved temp tables
 
   thd->release_resources();

=== modified file 'sql/rpl_slave.h'
--- a/sql/rpl_slave.h	revid:nuno.carvalho@stripped
+++ b/sql/rpl_slave.h	revid:nuno.carvalho@stripped
@@ -308,7 +308,7 @@ int start_slave_thread(
 int fetch_master_table(THD* thd, const char* db_name, const char* table_name,
 		       Master_info* mi, MYSQL* mysql, bool overwrite);
 
-bool show_slave_status(THD* thd, Master_info* mi, bool hold_run_locks= true);
+bool show_slave_status(THD* thd, Master_info* mi);
 bool rpl_master_has_bug(const Relay_log_info *rli, uint bug_id, bool report,
                         bool (*pred)(const void *), const void *param);
 bool rpl_master_erroneous_autoinc(THD* thd);

=== modified file 'sql/sql_parse.cc'
--- a/sql/sql_parse.cc	revid:nuno.carvalho@stripped
+++ b/sql/sql_parse.cc	revid:nuno.carvalho@stripped
@@ -2733,7 +2733,7 @@ case SQLCOM_PREPARE:
     /* Accept one of two privileges */
     if (check_global_access(thd, SUPER_ACL | REPL_CLIENT_ACL))
       goto error;
-    res= show_slave_status(thd, active_mi, false);
+    res= show_slave_status(thd, active_mi);
     break;
   }
   case SQLCOM_SHOW_MASTER_STAT:

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-trunk-wl6402 branch (nuno.carvalho:4436 to 4437) WL#6402Nuno Carvalho11 Sep