From: Nuno Carvalho Date: September 11 2012 3:32pm Subject: bzr push into mysql-trunk-wl6402 branch (nuno.carvalho:4436 to 4437) WL#6402 List-Archive: http://lists.mysql.com/commits/144723 Message-Id: <20120911153239.23459.42250.4437@localhost> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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 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(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).