diff options
-rw-r--r-- | mysql-test/suite/rpl/r/rpl_gtid_crash.result | 7 | ||||
-rw-r--r-- | mysql-test/suite/rpl/r/rpl_gtid_crash_myisam.result | 24 | ||||
-rw-r--r-- | mysql-test/suite/rpl/t/rpl_gtid_crash.test | 37 | ||||
-rw-r--r-- | mysql-test/suite/rpl/t/rpl_gtid_crash_myisam-master.opt | 1 | ||||
-rw-r--r-- | mysql-test/suite/rpl/t/rpl_gtid_crash_myisam.test | 64 | ||||
-rw-r--r-- | sql/log.cc | 255 | ||||
-rw-r--r-- | sql/log.h | 7 | ||||
-rw-r--r-- | sql/slave.cc | 3 |
8 files changed, 296 insertions, 102 deletions
diff --git a/mysql-test/suite/rpl/r/rpl_gtid_crash.result b/mysql-test/suite/rpl/r/rpl_gtid_crash.result index ef98c66bee0..9c3b9a95323 100644 --- a/mysql-test/suite/rpl/r/rpl_gtid_crash.result +++ b/mysql-test/suite/rpl/r/rpl_gtid_crash.result @@ -90,5 +90,12 @@ a 6 7 8 +*** MDEV-4725: Incorrect recovery when crash in the middle of writing an event group *** +SET GLOBAL debug_dbug="+d,crash_before_writing_xid"; +INSERT INTO t1 VALUES (9), (10); +SHOW VARIABLES like 'gtid_strict_mode'; +Variable_name Value +gtid_strict_mode ON +include/start_slave.inc DROP TABLE t1; include/rpl_end.inc diff --git a/mysql-test/suite/rpl/r/rpl_gtid_crash_myisam.result b/mysql-test/suite/rpl/r/rpl_gtid_crash_myisam.result new file mode 100644 index 00000000000..55465d7e81b --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_gtid_crash_myisam.result @@ -0,0 +1,24 @@ +include/rpl_init.inc [topology=1->2] +*** Test crashing master with InnoDB disabled, the binlog gtid state should still be correctly recovered. *** +CREATE TABLE t1 (a INT PRIMARY KEY) ENGINE=MyISAM; +include/stop_slave.inc +CHANGE MASTER TO master_use_gtid=slave_pos; +include/start_slave.inc +INSERT INTO t1 VALUES (1); +INSERT INTO t1 VALUES (2); +SELECT * FROM t1 ORDER BY a; +a +1 +2 +FLUSH TABLES; +SET SESSION debug_dbug="+d,crash_dispatch_command_before"; +SELECT 1; +Got one of the listed errors +INSERT INTO t1 VALUES (3); +SELECT * FROM t1 ORDER BY a; +a +1 +2 +3 +DROP TABLE t1; +include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_gtid_crash.test b/mysql-test/suite/rpl/t/rpl_gtid_crash.test index 804aae1114d..18a391aa100 100644 --- a/mysql-test/suite/rpl/t/rpl_gtid_crash.test +++ b/mysql-test/suite/rpl/t/rpl_gtid_crash.test @@ -256,6 +256,43 @@ EOF SELECT * FROM t1 ORDER BY a; +--echo *** MDEV-4725: Incorrect recovery when crash in the middle of writing an event group *** + +--connection server_2 +--write_file $MYSQLTEST_VARDIR/tmp/mysqld.2.expect +wait +EOF + +--let $old_gtid_strict= `SELECT @@gtid_strict_mode` +SET GLOBAL debug_dbug="+d,crash_before_writing_xid"; + +--connection server_1 +INSERT INTO t1 VALUES (9), (10); +--save_master_pos + +--connection server_2 +--source include/wait_until_disconnected.inc + +# The bug was that during crash recovery we would update the binlog state +# with the GTID of the half-written event group at the end of the slaves +# binlog, even though this event group was not committed. +# We restart the server with gtid_strict_mode; this way, we get an error +# about duplicate gtid when the slave re-executes the event group, if the +# binlog crash recovery is incorrect. +--append_file $MYSQLTEST_VARDIR/tmp/mysqld.2.expect +restart: --gtid_strict_mode=1 +EOF + +--enable_reconnect +--source include/wait_until_connected_again.inc + +SHOW VARIABLES like 'gtid_strict_mode'; +--source include/start_slave.inc +--sync_with_master +--disable_query_log +eval SET GLOBAL gtid_strict_mode= $old_gtid_strict; +--enable_query_log + --connection server_1 DROP TABLE t1; diff --git a/mysql-test/suite/rpl/t/rpl_gtid_crash_myisam-master.opt b/mysql-test/suite/rpl/t/rpl_gtid_crash_myisam-master.opt new file mode 100644 index 00000000000..32711eb9726 --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_gtid_crash_myisam-master.opt @@ -0,0 +1 @@ +--skip-stack-trace --skip-core-file --skip-innodb diff --git a/mysql-test/suite/rpl/t/rpl_gtid_crash_myisam.test b/mysql-test/suite/rpl/t/rpl_gtid_crash_myisam.test new file mode 100644 index 00000000000..faf388f5bed --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_gtid_crash_myisam.test @@ -0,0 +1,64 @@ +--source include/have_debug.inc +# Valgrind does not work well with test that crashes the server +--source include/not_valgrind.inc + +--let $rpl_topology=1->2 +--source include/rpl_init.inc + +--echo *** Test crashing master with InnoDB disabled, the binlog gtid state should still be correctly recovered. *** + +--connection server_1 +CREATE TABLE t1 (a INT PRIMARY KEY) ENGINE=MyISAM; +--save_master_pos + +--connection server_2 +--sync_with_master +--source include/stop_slave.inc +CHANGE MASTER TO master_use_gtid=slave_pos; +--source include/start_slave.inc + +--connection server_1 +INSERT INTO t1 VALUES (1); +INSERT INTO t1 VALUES (2); +--save_master_pos + +--connection server_2 +--sync_with_master +SELECT * FROM t1 ORDER BY a; + +--connection server_1 + +--write_file $MYSQLTEST_VARDIR/tmp/mysqld.1.expect +wait +EOF + +FLUSH TABLES; +SET SESSION debug_dbug="+d,crash_dispatch_command_before"; +--error 2006,2013 +SELECT 1; + +--source include/wait_until_disconnected.inc + +--append_file $MYSQLTEST_VARDIR/tmp/mysqld.1.expect +restart-rpl_gtid_crash.test +EOF + +--connection server_1 +--enable_reconnect +--source include/wait_until_connected_again.inc + +INSERT INTO t1 VALUES (3); +--save_master_pos + +--connection server_2 +--sync_with_master +SELECT * FROM t1 ORDER BY a; + +--connection server_1 +DROP TABLE t1; + +--connection default +--enable_reconnect +--source include/wait_until_connected_again.inc + +--source include/rpl_end.inc diff --git a/sql/log.cc b/sql/log.cc index c1fd8aacb9a..a0845106fad 100644 --- a/sql/log.cc +++ b/sql/log.cc @@ -2937,7 +2937,8 @@ MYSQL_BIN_LOG::MYSQL_BIN_LOG(uint *sync_period) bytes_written(0), file_id(1), open_count(1), group_commit_queue(0), group_commit_queue_busy(FALSE), num_commits(0), num_group_commits(0), - sync_period_ptr(sync_period), sync_counter(0), state_read(false), + sync_period_ptr(sync_period), sync_counter(0), + state_file_deleted(false), binlog_state_recover_done(false), is_relay_log(0), signal_cnt(0), checksum_alg_reset(BINLOG_CHECKSUM_ALG_UNDEF), relay_log_checksum_alg(BINLOG_CHECKSUM_ALG_UNDEF), @@ -3139,12 +3140,19 @@ bool MYSQL_BIN_LOG::open(const char *log_name, DBUG_ENTER("MYSQL_BIN_LOG::open"); DBUG_PRINT("enter",("log_type: %d",(int) log_type_arg)); - if (!is_relay_log && read_state_from_file()) - DBUG_RETURN(1); + if (!is_relay_log) + { + if (!binlog_state_recover_done) + { + binlog_state_recover_done= true; + if (do_binlog_recovery(opt_bin_logname, false)) + DBUG_RETURN(1); + } - if (!is_relay_log && !binlog_background_thread_started && - start_binlog_background_thread()) - DBUG_RETURN(1); + if (!binlog_background_thread_started && + start_binlog_background_thread()) + DBUG_RETURN(1); + } if (init_and_set_log_file_name(log_name, new_name, log_type_arg, io_cache_type_arg)) @@ -3431,6 +3439,25 @@ bool MYSQL_BIN_LOG::open(const char *log_name, my_free(binlog_xid_count_list.get()); binlog_xid_count_list.push_back(new_xid_list_entry); mysql_mutex_unlock(&LOCK_xid_list); + + /* + Now that we have synced a new binlog file with an initial Gtid_list + event, it is safe to delete the binlog state file. We will write out + a new, updated file at shutdown, and if we crash before we can recover + the state from the newly written binlog file. + + Since the state file will contain out-of-date data as soon as the first + new GTID is binlogged, it is better to remove it, to avoid any risk of + accidentally reading incorrect data later. + */ + if (!state_file_deleted) + { + char buf[FN_REFLEN]; + fn_format(buf, opt_bin_logname, mysql_data_home, ".state", + MY_UNPACK_FILENAME); + my_delete(buf, MY_SYNC_DIR); + state_file_deleted= true; + } } log_state= LOG_OPENED; @@ -5488,10 +5515,6 @@ MYSQL_BIN_LOG::read_state_from_file() bool opened= false; bool inited= false; - if (state_read) - return 0; - state_read= true; - fn_format(buf, opt_bin_logname, mysql_data_home, ".state", MY_UNPACK_FILENAME); if ((file_no= mysql_file_open(key_file_binlog_state, buf, @@ -8559,7 +8582,6 @@ int TC_LOG::using_heuristic_recover() int TC_LOG_BINLOG::open(const char *opt_name) { - LOG_INFO log_info; int error= 1; DBUG_ASSERT(total_ha_2pc > 1); @@ -8580,65 +8602,8 @@ int TC_LOG_BINLOG::open(const char *opt_name) return 1; } - if ((error= find_log_pos(&log_info, NullS, 1))) - { - if (error != LOG_INFO_EOF) - sql_print_error("find_log_pos() failed (error: %d)", error); - else - error= 0; - goto err; - } - - { - const char *errmsg; - IO_CACHE log; - File file; - Log_event *ev=0; - Format_description_log_event fdle(BINLOG_VERSION); - char log_name[FN_REFLEN]; - - if (! fdle.is_valid()) - goto err; - - do - { - strmake_buf(log_name, log_info.log_file_name); - } while (!(error= find_next_log(&log_info, 1))); - - if (error != LOG_INFO_EOF) - { - sql_print_error("find_log_pos() failed (error: %d)", error); - goto err; - } - - if ((file= open_binlog(&log, log_name, &errmsg)) < 0) - { - sql_print_error("%s", errmsg); - goto err; - } - - if ((ev= Log_event::read_log_event(&log, 0, &fdle, - opt_master_verify_checksum)) && - ev->get_type_code() == FORMAT_DESCRIPTION_EVENT && - ev->flags & LOG_EVENT_BINLOG_IN_USE_F) - { - sql_print_information("Recovering after a crash using %s", opt_name); - error= recover(&log_info, log_name, &log, - (Format_description_log_event *)ev); - state_read= true; - } - else - error= read_state_from_file(); - - delete ev; - end_io_cache(&log); - mysql_file_close(file, MYF(MY_WME)); - - if (error) - goto err; - } - -err: + error= do_binlog_recovery(opt_name, true); + binlog_state_recover_done= true; return error; } @@ -9014,9 +8979,9 @@ start_binlog_background_thread() int TC_LOG_BINLOG::recover(LOG_INFO *linfo, const char *last_log_name, IO_CACHE *first_log, - Format_description_log_event *fdle) + Format_description_log_event *fdle, bool do_xa) { - Log_event *ev; + Log_event *ev= NULL; HASH xids; MEM_ROOT mem_root; char binlog_checkpoint_name[FN_REFLEN]; @@ -9025,13 +8990,19 @@ int TC_LOG_BINLOG::recover(LOG_INFO *linfo, const char *last_log_name, IO_CACHE log; File file= -1; const char *errmsg; +#ifdef HAVE_REPLICATION + rpl_gtid last_gtid; + bool last_gtid_standalone= false; + bool last_gtid_valid= false; +#endif if (! fdle->is_valid() || - my_hash_init(&xids, &my_charset_bin, TC_LOG_PAGE_SIZE/3, 0, - sizeof(my_xid), 0, 0, MYF(0))) + (do_xa && my_hash_init(&xids, &my_charset_bin, TC_LOG_PAGE_SIZE/3, 0, + sizeof(my_xid), 0, 0, MYF(0)))) goto err1; - init_alloc_root(&mem_root, TC_LOG_PAGE_SIZE, TC_LOG_PAGE_SIZE, MYF(0)); + if (do_xa) + init_alloc_root(&mem_root, TC_LOG_PAGE_SIZE, TC_LOG_PAGE_SIZE, MYF(0)); fdle->flags&= ~LOG_EVENT_BINLOG_IN_USE_F; // abort on the first error @@ -9051,22 +9022,23 @@ int TC_LOG_BINLOG::recover(LOG_INFO *linfo, const char *last_log_name, 0, fdle, opt_master_verify_checksum)) && ev->is_valid()) { - switch (ev->get_type_code()) + enum Log_event_type typ= ev->get_type_code(); + switch (typ) { case XID_EVENT: { - Xid_log_event *xev=(Xid_log_event *)ev; - uchar *x= (uchar *) memdup_root(&mem_root, (uchar*) &xev->xid, - sizeof(xev->xid)); - if (!x || my_hash_insert(&xids, x)) + if (do_xa) { - delete ev; - goto err2; + Xid_log_event *xev=(Xid_log_event *)ev; + uchar *x= (uchar *) memdup_root(&mem_root, (uchar*) &xev->xid, + sizeof(xev->xid)); + if (!x || my_hash_insert(&xids, x)) + goto err2; + break; } - break; } case BINLOG_CHECKPOINT_EVENT: - if (first_round) + if (first_round && do_xa) { uint dir_len; Binlog_checkpoint_log_event *cev= (Binlog_checkpoint_log_event *)ev; @@ -9084,8 +9056,8 @@ int TC_LOG_BINLOG::recover(LOG_INFO *linfo, const char *last_log_name, cev->binlog_file_name, FN_REFLEN - 1 - dir_len); binlog_checkpoint_found= true; } - break; } + break; case GTID_LIST_EVENT: if (first_round) { @@ -9097,28 +9069,49 @@ int TC_LOG_BINLOG::recover(LOG_INFO *linfo, const char *last_log_name, } break; +#ifdef HAVE_REPLICATION case GTID_EVENT: if (first_round) { Gtid_log_event *gev= (Gtid_log_event *)ev; - rpl_gtid gtid; /* Update the binlog state with any GTID logged after Gtid_list. */ - gtid.domain_id= gev->domain_id; - gtid.server_id= gev->server_id; - gtid.seq_no= gev->seq_no; - if (rpl_global_gtid_binlog_state.update_nolock(>id, false)) - goto err2; + last_gtid.domain_id= gev->domain_id; + last_gtid.server_id= gev->server_id; + last_gtid.seq_no= gev->seq_no; + last_gtid_standalone= + ((gev->flags2 & Gtid_log_event::FL_STANDALONE) ? true : false); + last_gtid_valid= true; } break; +#endif default: /* Nothing. */ break; } + +#ifdef HAVE_REPLICATION + if (last_gtid_valid && + ((last_gtid_standalone && !ev->is_part_of_group(typ)) || + (!last_gtid_standalone && + (typ == XID_EVENT || + (typ == QUERY_EVENT && + (((Query_log_event *)ev)->is_commit() || + ((Query_log_event *)ev)->is_rollback())))))) + { + if (rpl_global_gtid_binlog_state.update_nolock(&last_gtid, false)) + goto err2; + last_gtid_valid= false; + } +#endif + delete ev; + ev= NULL; } + if (!do_xa) + break; /* If the last binlog checkpoint event points to an older log, we have to scan all logs from there also, to get all possible XIDs to recover. @@ -9171,21 +9164,28 @@ int TC_LOG_BINLOG::recover(LOG_INFO *linfo, const char *last_log_name, } } - if (ha_recover(&xids)) - goto err2; + if (do_xa) + { + if (ha_recover(&xids)) + goto err2; - free_root(&mem_root, MYF(0)); - my_hash_free(&xids); + free_root(&mem_root, MYF(0)); + my_hash_free(&xids); + } return 0; err2: + delete ev; if (file >= 0) { end_io_cache(&log); mysql_file_close(file, MYF(MY_WME)); } - free_root(&mem_root, MYF(0)); - my_hash_free(&xids); + if (do_xa) + { + free_root(&mem_root, MYF(0)); + my_hash_free(&xids); + } err1: sql_print_error("Crash recovery failed. Either correct the problem " "(if it's, for example, out of memory error) and restart, " @@ -9195,6 +9195,67 @@ err1: } +int +MYSQL_BIN_LOG::do_binlog_recovery(const char *opt_name, bool do_xa_recovery) +{ + LOG_INFO log_info; + const char *errmsg; + IO_CACHE log; + File file; + Log_event *ev= 0; + Format_description_log_event fdle(BINLOG_VERSION); + char log_name[FN_REFLEN]; + int error; + + if ((error= find_log_pos(&log_info, NullS, 1))) + { + if (error != LOG_INFO_EOF) + sql_print_error("find_log_pos() failed (error: %d)", error); + else + error= 0; + return error; + } + + if (! fdle.is_valid()) + return 1; + + do + { + strmake_buf(log_name, log_info.log_file_name); + } while (!(error= find_next_log(&log_info, 1))); + + if (error != LOG_INFO_EOF) + { + sql_print_error("find_log_pos() failed (error: %d)", error); + return error; + } + + if ((file= open_binlog(&log, log_name, &errmsg)) < 0) + { + sql_print_error("%s", errmsg); + return 1; + } + + if ((ev= Log_event::read_log_event(&log, 0, &fdle, + opt_master_verify_checksum)) && + ev->get_type_code() == FORMAT_DESCRIPTION_EVENT && + ev->flags & LOG_EVENT_BINLOG_IN_USE_F) + { + sql_print_information("Recovering after a crash using %s", opt_name); + error= recover(&log_info, log_name, &log, + (Format_description_log_event *)ev, do_xa_recovery); + } + else + error= read_state_from_file(); + + delete ev; + end_io_cache(&log); + mysql_file_close(file, MYF(MY_WME)); + + return error; +} + + #ifdef INNODB_COMPATIBILITY_HOOKS /** Get the file name of the MySQL binlog. diff --git a/sql/log.h b/sql/log.h index ed3daa56444..73518d2594f 100644 --- a/sql/log.h +++ b/sql/log.h @@ -521,8 +521,8 @@ class MYSQL_BIN_LOG: public TC_LOG, private MYSQL_LOG */ uint *sync_period_ptr; uint sync_counter; - /* Protect against reading the binlog state file twice. */ - bool state_read; + bool state_file_deleted; + bool binlog_state_recover_done; inline uint get_sync_period() { @@ -661,7 +661,8 @@ public: int unlog(ulong cookie, my_xid xid); void commit_checkpoint_notify(void *cookie); int recover(LOG_INFO *linfo, const char *last_log_name, IO_CACHE *first_log, - Format_description_log_event *fdle); + Format_description_log_event *fdle, bool do_xa); + int do_binlog_recovery(const char *opt_name, bool do_xa_recovery); #if !defined(MYSQL_CLIENT) int flush_and_set_pending_rows_event(THD *thd, Rows_log_event* event, diff --git a/sql/slave.cc b/sql/slave.cc index 76c9a100104..76fbc10db5b 100644 --- a/sql/slave.cc +++ b/sql/slave.cc @@ -5988,10 +5988,9 @@ static Log_event* next_event(rpl_group_info *rgi, ulonglong *event_size) IO_CACHE* cur_log = rli->cur_log; mysql_mutex_t *log_lock = rli->relay_log.get_log_lock(); const char* errmsg=0; - THD *thd = rgi->thd; DBUG_ENTER("next_event"); - DBUG_ASSERT(thd != 0 && thd == rli->sql_driver_thd); + DBUG_ASSERT(rgi->thd != 0 && rgi->thd == rli->sql_driver_thd); *event_size= 0; #ifndef DBUG_OFF |