From 89aad233de9371476d9424c7df72d66c37c23e8a Mon Sep 17 00:00:00 2001 From: Kristian Nielsen Date: Sun, 23 Apr 2017 10:49:58 +0200 Subject: [PATCH] MDEV-12179: Per-engine mysql.gtid_slave_pos table Intermediate commit. Move the discovery of mysql.gtid_slave_pos* tables into the SQL thread. This avoids doing things like opening tables and scanning the mysql schema for tables inside of the START SLAVE statement, which might interact badly with existing transaction or table locks. (Even though START SLAVE is documented to implicitly commit any active transactions, this appears not to be the case in current code). Table discovery fits naturally in the SQL thread init code, next to the loading of mysql.gtid_slave_pos state. --- .../suite/rpl/r/rpl_gtid_errorhandling.result | 14 +++++---- .../suite/rpl/t/rpl_gtid_errorhandling.test | 19 ++++++++---- sql/mysqld.cc | 2 +- sql/rpl_mi.cc | 29 +++++++++++-------- sql/rpl_mi.h | 2 +- sql/rpl_parallel.cc | 4 +-- sql/rpl_rli.cc | 14 +++++++-- sql/slave.cc | 8 +++++ sql/sql_repl.cc | 4 --- 9 files changed, 63 insertions(+), 33 deletions(-) diff --git a/mysql-test/suite/rpl/r/rpl_gtid_errorhandling.result b/mysql-test/suite/rpl/r/rpl_gtid_errorhandling.result index 04c3fa7966f..62a5b9c3531 100644 --- a/mysql-test/suite/rpl/r/rpl_gtid_errorhandling.result +++ b/mysql-test/suite/rpl/r/rpl_gtid_errorhandling.result @@ -8,22 +8,26 @@ connection slave; include/stop_slave.inc ALTER TABLE mysql.gtid_slave_pos CHANGE seq_no seq_no VARCHAR(20); START SLAVE; -ERROR HY000: Failed to open mysql.gtid_slave_pos connection master; INSERT INTO t1 VALUES (1); connection slave; -CALL mtr.add_suppression("Incorrect definition of table mysql.gtid_slave_pos"); +CALL mtr.add_suppression("Slave: Failed to open mysql.gtid_slave_pos"); +include/wait_for_slave_sql_error.inc [errno=1944] +include/stop_slave.inc ALTER TABLE mysql.gtid_slave_pos CHANGE seq_no seq_no BIGINT UNSIGNED NOT NULL; ALTER TABLE mysql.gtid_slave_pos DROP PRIMARY KEY; ALTER TABLE mysql.gtid_slave_pos ADD PRIMARY KEY (sub_id, domain_id); START SLAVE; -ERROR HY000: Failed to open mysql.gtid_slave_pos +include/wait_for_slave_sql_error.inc [errno=1944] +include/stop_slave.inc ALTER TABLE mysql.gtid_slave_pos DROP PRIMARY KEY; START SLAVE; -ERROR HY000: Failed to open mysql.gtid_slave_pos +include/wait_for_slave_sql_error.inc [errno=1944] +include/stop_slave.inc ALTER TABLE mysql.gtid_slave_pos ADD PRIMARY KEY (sub_id); START SLAVE; -ERROR HY000: Failed to open mysql.gtid_slave_pos +include/wait_for_slave_sql_error.inc [errno=1944] +include/stop_slave.inc ALTER TABLE mysql.gtid_slave_pos DROP PRIMARY KEY; ALTER TABLE mysql.gtid_slave_pos ADD PRIMARY KEY (domain_id, sub_id); include/start_slave.inc diff --git a/mysql-test/suite/rpl/t/rpl_gtid_errorhandling.test b/mysql-test/suite/rpl/t/rpl_gtid_errorhandling.test index a5946411144..796f6894f19 100644 --- a/mysql-test/suite/rpl/t/rpl_gtid_errorhandling.test +++ b/mysql-test/suite/rpl/t/rpl_gtid_errorhandling.test @@ -10,28 +10,37 @@ CREATE TABLE t1(a INT PRIMARY KEY) ENGINE=InnoDB; --connection slave --source include/stop_slave.inc ALTER TABLE mysql.gtid_slave_pos CHANGE seq_no seq_no VARCHAR(20); ---error ER_GTID_OPEN_TABLE_FAILED START SLAVE; --connection master INSERT INTO t1 VALUES (1); --connection slave -CALL mtr.add_suppression("Incorrect definition of table mysql.gtid_slave_pos"); +CALL mtr.add_suppression("Slave: Failed to open mysql.gtid_slave_pos"); +--let $slave_sql_errno=1944 +--source include/wait_for_slave_sql_error.inc + +--source include/stop_slave.inc ALTER TABLE mysql.gtid_slave_pos CHANGE seq_no seq_no BIGINT UNSIGNED NOT NULL; ALTER TABLE mysql.gtid_slave_pos DROP PRIMARY KEY; ALTER TABLE mysql.gtid_slave_pos ADD PRIMARY KEY (sub_id, domain_id); ---error ER_GTID_OPEN_TABLE_FAILED START SLAVE; +--let $slave_sql_errno=1944 +--source include/wait_for_slave_sql_error.inc +--source include/stop_slave.inc ALTER TABLE mysql.gtid_slave_pos DROP PRIMARY KEY; ---error ER_GTID_OPEN_TABLE_FAILED START SLAVE; +--let $slave_sql_errno=1944 +--source include/wait_for_slave_sql_error.inc +--source include/stop_slave.inc ALTER TABLE mysql.gtid_slave_pos ADD PRIMARY KEY (sub_id); ---error ER_GTID_OPEN_TABLE_FAILED START SLAVE; +--let $slave_sql_errno=1944 +--source include/wait_for_slave_sql_error.inc +--source include/stop_slave.inc ALTER TABLE mysql.gtid_slave_pos DROP PRIMARY KEY; ALTER TABLE mysql.gtid_slave_pos ADD PRIMARY KEY (domain_id, sub_id); --source include/start_slave.inc diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 0b8aaf1de78..a3c8686fee4 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -7790,7 +7790,7 @@ static int show_slaves_running(THD *thd, SHOW_VAR *var, char *buff) var->type= SHOW_LONGLONG; var->value= buff; - *((longlong *)buff)= any_slave_sql_running(); + *((longlong *)buff)= any_slave_sql_running(false); return 0; } diff --git a/sql/rpl_mi.cc b/sql/rpl_mi.cc index f30b7e161f2..4cf09d0bb76 100644 --- a/sql/rpl_mi.cc +++ b/sql/rpl_mi.cc @@ -1558,6 +1558,9 @@ bool give_error_if_slave_running(bool already_locked) /** any_slave_sql_running() + @param + already_locked 0 if we need to lock, 1 if we have LOCK_active_mi_locked + @return 0 No Slave SQL thread is running # Number of slave SQL thread running @@ -1568,26 +1571,28 @@ bool give_error_if_slave_running(bool already_locked) hash entries can't be accessed. */ -uint any_slave_sql_running() +uint any_slave_sql_running(bool already_locked) { uint count= 0; HASH *hash; DBUG_ENTER("any_slave_sql_running"); - mysql_mutex_lock(&LOCK_active_mi); + if (!already_locked) + mysql_mutex_lock(&LOCK_active_mi); if (unlikely(shutdown_in_progress || !master_info_index)) + count= 1; + else { + hash= &master_info_index->master_info_hash; + for (uint i= 0; i< hash->records; ++i) + { + Master_info *mi= (Master_info *)my_hash_element(hash, i); + if (mi->rli.slave_running != MYSQL_SLAVE_NOT_RUN) + count++; + } + } + if (!already_locked) mysql_mutex_unlock(&LOCK_active_mi); - DBUG_RETURN(1); - } - hash= &master_info_index->master_info_hash; - for (uint i= 0; i< hash->records; ++i) - { - Master_info *mi= (Master_info *)my_hash_element(hash, i); - if (mi->rli.slave_running != MYSQL_SLAVE_NOT_RUN) - count++; - } - mysql_mutex_unlock(&LOCK_active_mi); DBUG_RETURN(count); } diff --git a/sql/rpl_mi.h b/sql/rpl_mi.h index 31c0f280ac1..104a7ce3da6 100644 --- a/sql/rpl_mi.h +++ b/sql/rpl_mi.h @@ -377,7 +377,7 @@ void create_logfile_name_with_suffix(char *res_file_name, size_t length, uchar *get_key_master_info(Master_info *mi, size_t *length, my_bool not_used __attribute__((unused))); void free_key_master_info(Master_info *mi); -uint any_slave_sql_running(); +uint any_slave_sql_running(bool already_locked); bool give_error_if_slave_running(bool already_lock); #endif /* HAVE_REPLICATION */ diff --git a/sql/rpl_parallel.cc b/sql/rpl_parallel.cc index 6788f422cbd..5215faaa17b 100644 --- a/sql/rpl_parallel.cc +++ b/sql/rpl_parallel.cc @@ -1465,7 +1465,7 @@ rpl_parallel_change_thread_count(rpl_parallel_thread_pool *pool, */ if (!new_count && !force) { - if (any_slave_sql_running()) + if (any_slave_sql_running(false)) { DBUG_PRINT("warning", ("SQL threads running while trying to reset parallel pool")); @@ -1620,7 +1620,7 @@ err: int rpl_parallel_resize_pool_if_no_slaves(void) { /* master_info_index is set to NULL on shutdown */ - if (opt_slave_parallel_threads > 0 && !any_slave_sql_running()) + if (opt_slave_parallel_threads > 0 && !any_slave_sql_running(false)) return rpl_parallel_inactivate_pool(&global_rpl_thread_pool); return 0; } diff --git a/sql/rpl_rli.cc b/sql/rpl_rli.cc index 2cda87925d1..59138a23daf 100644 --- a/sql/rpl_rli.cc +++ b/sql/rpl_rli.cc @@ -1917,7 +1917,7 @@ find_gtid_slave_pos_tables(THD *thd) { int err= 0; load_gtid_state_cb_data cb_data; - bool any_running; + uint num_running; mysql_mutex_lock(&rpl_global_gtid_slave_state->LOCK_slave_state); bool loaded= rpl_global_gtid_slave_state->loaded; @@ -1949,10 +1949,17 @@ find_gtid_slave_pos_tables(THD *thd) if ((err= gtid_pos_auto_create_tables(&cb_data.table_list))) goto end; - any_running= any_slave_sql_running(); + mysql_mutex_lock(&LOCK_active_mi); + num_running= any_slave_sql_running(true); mysql_mutex_lock(&rpl_global_gtid_slave_state->LOCK_slave_state); - if (!any_running) + if (num_running <= 1) { + /* + If no slave is running now, the count will be 1, since this SQL thread + which is starting is included in the count. In this case, we can safely + replace the list, no-one can be trying to read it without lock. + */ + DBUG_ASSERT(num_running == 1); rpl_global_gtid_slave_state->set_gtid_pos_tables_list(cb_data.table_list, cb_data.default_entry); cb_data.table_list= NULL; @@ -2012,6 +2019,7 @@ find_gtid_slave_pos_tables(THD *thd) } } mysql_mutex_unlock(&rpl_global_gtid_slave_state->LOCK_slave_state); + mysql_mutex_unlock(&LOCK_active_mi); end: if (cb_data.table_list) diff --git a/sql/slave.cc b/sql/slave.cc index 443e5b7a5f1..a892bba82c9 100644 --- a/sql/slave.cc +++ b/sql/slave.cc @@ -5279,6 +5279,14 @@ pthread_handler_t handle_slave_sql(void *arg) if (mi->using_gtid != Master_info::USE_GTID_NO || opt_gtid_strict_mode) goto err; } + /* Re-load the set of mysql.gtid_slave_posXXX tables available. */ + if (find_gtid_slave_pos_tables(thd)) + { + rli->report(ERROR_LEVEL, thd->get_stmt_da()->sql_errno(), NULL, + "Error processing replication GTID position tables: %s", + thd->get_stmt_da()->message()); + goto err; + } /* execute init_slave variable */ if (opt_init_slave.length) diff --git a/sql/sql_repl.cc b/sql/sql_repl.cc index 087fc293359..7c30aa38f1a 100644 --- a/sql/sql_repl.cc +++ b/sql/sql_repl.cc @@ -3031,10 +3031,6 @@ int start_slave(THD* thd , Master_info* mi, bool net_report) if (check_access(thd, SUPER_ACL, any_db, NULL, NULL, 0, 0)) DBUG_RETURN(-1); - /* Re-load the set of mysql.gtid_slave_posXXX tables available. */ - if (find_gtid_slave_pos_tables(thd)) - DBUG_RETURN(-1); - create_logfile_name_with_suffix(master_info_file_tmp, sizeof(master_info_file_tmp), master_info_file, 0,