Description:
Summary:
We should change SET GTID_PURGED so that it does not add any GTIDs to Previous_gtids_log_event and does not rotate the binary log. Instead it should add GTIDs to the table mysql.gtid_executed.
Background:
In 5.6, there is no table to store GTIDs, and GTIDs are not stored in the binary log at all when GTID_MODE=OFF. In particular, GTID_PURGED is normally equal to the Previous_gtids_log_event of the oldest binary log. In two circumstances, GTID_PURGED may instead be equal to the Previous_gtids_log_event of another binary log:
1. A SET GTID_PURGED statement causes the binary log to rotate, so that the new binary log contains the correct Previous_gtids_log_event. Thus, there may be a number of binary logs having Previous_gtids_log_event containing an empty set, preceding the binary log containing the 'real' Previous_gtids_log_event.
2. If GTID_MODE = OFF, no Previous_gtids_log_event is generated. Thus, if server switched from GTID_MODE=OFF to GTID_MODE=ON, there may be any number of binary logs having no Previous_gtids_log_event at all, preceding the binary log containing the 'real' Previous_gtids_log_event.
So a precise recovery procedure that computes GTID_PURGED will read it from the first binary log that has one of the following:
- Both a Previous_gtids_log_event and a Gtid_log_event.
- A Previous_gtids_log_event that is not empty.
When any of these criteria is met, it is sure that the user has not executed SET GTID_PURGED after this binary log, since SET GTID_PURGED is only allowed when GTID_EXECUTED is empty.
This recovery procedure can be very slow: when gtid_mode=off it has to scan the headers of all binary logs. Therefore we introduced the binlog_gtid_simple_recovery option, which assumes that GTID_PURGED is empty if the first binray log does not contain a Previous_gtids_log_event. The routine will thus set the wrong value for GTID_PURGED in two cases:
C1. the GTID_MODE changed from OFF to ON and not all old binary logs were purged;
C2. the user executed SET GTID_PURGED and not all old binary logs were purged.
In 5.7, the situation is different, because (1) we store GTIDs in the system table mysql.gtid_executed; (2) we generate Previous_gtids_log_event regardless of GTID_MODE. The recovery routine that initializes GTID_PURGED has also changed in 5.7: it adds those GTIDs which exist in the table but not in the binary log.
Thus, in 5.7 we normally never have to skip over binary logs that do not have a Previous_gtids_log_event, because such binary logs normally do not exist. (The only case when such binary logs exist is if the server was just upgrade from 5.6 to 5.7 and still has some old binary logs.) However, we still skip over binary logs that contain a Previous_gtids_log_event that is empty and does not contain any Gtid_log_event, since it is possible that user issued SET GTID_PURGED in a later binary log.
Therefore, in 5.7 binlog_simple_gtid_recovery is less unsafe, because in case C1 it computes the wrong value for GTID_PURGED only if some binary logs were generated by a 5.6 server, i.e., for a short while after an upgrade.
Problem:
Still, in 5.7 binlog_simple_gtid_recovery will compute wrong value for GTID_PURGED just after a SET GTID_PURGED statement. Since SET GTID_PURGED normally is done while restoring a backup, it is probably common to have server restart short after restoring the backup (e.g. to tweak some parameter before putting it into production).
Solution:
It would be much better if SET GTID_PURGED only added GTIDs to the table mysql.gtid_executed, and did not add anything to Previous_gtids_log_event. Then, C2 is only a problem in case SET GTID_PURGED was executed in 5.6.
In other words, binlog_gtid_simple_recovery will compute wrong value for GTID_PURGED only just after an upgrade from 5.6.
How to repeat:
--source include/have_gtid.inc
--let $rpl_topology= none
--source include/rpl_init.inc
SET GLOBAL GTID_PURGED = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa:1';
SELECT @@GLOBAL.GTID_PURGED;
--let $rpl_server_parameters= --binlog-gtid-simple-recovery=1
--let $rpl_server_number= 1
--source include/rpl_restart_server.inc
SELECT @@GLOBAL.GTID_PURGED;
--let $rpl_server_parameters=
--let $rpl_server_number= 1
--source include/rpl_restart_server.inc
SELECT @@GLOBAL.GTID_PURGED;
Suggested fix:
diff --git a/sql/rpl_gtid.h b/sql/rpl_gtid.h
index 91211ae..b0db48d 100644
--- a/sql/rpl_gtid.h
+++ b/sql/rpl_gtid.h
@@ -2735,7 +2735,7 @@ public:
char *, bool) for format details.
@return RETURN_STATUS_OK or RETURN_STATUS_REPORTED_ERROR.
*/
- enum_return_status add_lost_gtids(const char *text);
+ enum_return_status add_lost_gtids(const Gtid_set *gtid_set);
/// Return a pointer to the Gtid_set that contains the lost groups.
const Gtid_set *get_lost_gtids() const { return &lost_gtids; }
/*
@@ -2848,7 +2848,7 @@ public:
@retval
-1 Error
*/
- int save(Gtid_set *gtid_set);
+ int save(const Gtid_set *gtid_set);
/**
Save the set of gtids logged in the last binlog into gtid_executed table.
diff --git a/sql/rpl_gtid_persist.cc b/sql/rpl_gtid_persist.cc
index b75f71a..47c863d 100644
--- a/sql/rpl_gtid_persist.cc
+++ b/sql/rpl_gtid_persist.cc
@@ -288,7 +288,7 @@ end:
}
-int Gtid_table_persistor::save(THD *thd, Gtid *gtid)
+int Gtid_table_persistor::save(THD *thd, const Gtid *gtid)
{
DBUG_ENTER("Gtid_table_persistor::save(THD *thd, Gtid *gtid)");
int error= 0;
@@ -332,7 +332,7 @@ end:
}
-int Gtid_table_persistor::save(Gtid_set *gtid_set)
+int Gtid_table_persistor::save(const Gtid_set *gtid_set)
{
DBUG_ENTER("Gtid_table_persistor::save(Gtid_set *gtid_set)");
int ret= 0;
@@ -371,7 +371,7 @@ end:
}
-int Gtid_table_persistor::save(TABLE *table, Gtid_set *gtid_set)
+int Gtid_table_persistor::save(TABLE *table, const Gtid_set *gtid_set)
{
DBUG_ENTER("Gtid_table_persistor::save(TABLE* table, "
"Gtid_set *gtid_set)");
diff --git a/sql/rpl_gtid_persist.h b/sql/rpl_gtid_persist.h
index 7d89b69..9b1891b 100644
--- a/sql/rpl_gtid_persist.h
+++ b/sql/rpl_gtid_persist.h
@@ -117,7 +117,7 @@ public:
@retval
-1 Error
*/
- int save(THD *thd, Gtid *gtid);
+ int save(THD *thd, const Gtid *gtid);
/**
Insert the gtid set into table.
@@ -129,7 +129,7 @@ public:
@retval
-1 Error
*/
- int save(Gtid_set *gtid_set);
+ int save(const Gtid_set *gtid_set);
/**
Delete all rows from the table.
@@ -301,7 +301,7 @@ private:
@retval
-1 Error
*/
- int save(TABLE *table, Gtid_set *gtid_set);
+ int save(TABLE *table, const Gtid_set *gtid_set);
/* Prevent user from invoking default assignment function. */
Gtid_table_persistor &operator=(const Gtid_table_persistor &info);
/* Prevent user from invoking default constructor function. */
diff --git a/sql/rpl_gtid_state.cc b/sql/rpl_gtid_state.cc
index 237404c..ab2aecb 100644
--- a/sql/rpl_gtid_state.cc
+++ b/sql/rpl_gtid_state.cc
@@ -677,12 +677,12 @@ enum_return_status Gtid_state::ensure_sidno()
}
-enum_return_status Gtid_state::add_lost_gtids(const char *text)
+enum_return_status Gtid_state::add_lost_gtids(const Gtid_set *gtid_set)
{
DBUG_ENTER("Gtid_state::add_lost_gtids()");
sid_lock->assert_some_wrlock();
- DBUG_PRINT("info", ("add_lost_gtids '%s'", text));
+ gtid_set->dbug_print("add_lost_gtids");
if (!executed_gtids.is_empty())
{
@@ -700,8 +700,11 @@ enum_return_status Gtid_state::add_lost_gtids(const char *text)
}
DBUG_ASSERT(lost_gtids.is_empty());
- PROPAGATE_REPORTED_ERROR(lost_gtids.add_gtid_text(text));
- PROPAGATE_REPORTED_ERROR(executed_gtids.add_gtid_text(text));
+ if (save(gtid_set))
+ RETURN_REPORTED_ERROR;
+ PROPAGATE_REPORTED_ERROR(gtids_only_in_table.add_gtid_set(gtid_set));
+ PROPAGATE_REPORTED_ERROR(lost_gtids.add_gtid_set(gtid_set));
+ PROPAGATE_REPORTED_ERROR(executed_gtids.add_gtid_set(gtid_set));
DBUG_RETURN(RETURN_STATUS_OK);
}
@@ -750,7 +753,7 @@ int Gtid_state::save(THD *thd)
}
-int Gtid_state::save(Gtid_set *gtid_set)
+int Gtid_state::save(const Gtid_set *gtid_set)
{
DBUG_ENTER("Gtid_state::save(Gtid_set *gtid_set)");
int ret= gtid_table_persistor->save(gtid_set);
diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc
index 894c686..b663087 100644
--- a/sql/sys_vars.cc
+++ b/sql/sys_vars.cc
@@ -4942,14 +4942,23 @@ bool Sys_var_gtid_purged::global_update(THD *thd, set_var *var)
DBUG_ENTER("Sys_var_gtid_purged::global_update");
#ifdef HAVE_REPLICATION
bool error= false;
- int rotate_res= 0;
global_sid_lock->wrlock();
char *previous_gtid_executed= gtid_state->get_executed_gtids()->to_string();
char *previous_gtid_lost= gtid_state->get_lost_gtids()->to_string();
- enum_return_status ret= gtid_state->add_lost_gtids(var->save_result.string_value.str);
- char *current_gtid_executed= gtid_state->get_executed_gtids()->to_string();
- char *current_gtid_lost= gtid_state->get_lost_gtids()->to_string();
+ char *current_gtid_executed;
+ char *current_gtid_lost;
+ enum_return_status ret;
+ Gtid_set gtid_set(global_sid_map, var->save_result.string_value.str,
+ &ret, global_sid_lock);
+ if (ret != RETURN_STATUS_OK)
+ {
+ error= true;
+ goto end;
+ }
+ ret= gtid_state->add_lost_gtids(>id_set);
+ current_gtid_executed= gtid_state->get_executed_gtids()->to_string();
+ current_gtid_lost= gtid_state->get_lost_gtids()->to_string();
global_sid_lock->unlock();
if (RETURN_STATUS_OK != ret)
{
@@ -4963,14 +4972,6 @@ bool Sys_var_gtid_purged::global_update(THD *thd, set_var *var)
sql_print_information(ER(ER_GTID_EXECUTED_WAS_CHANGED),
previous_gtid_executed, current_gtid_executed);
- // Rotate logs to have Previous_gtid_event on last binlog.
- rotate_res= mysql_bin_log.rotate_and_purge(thd, true);
- if (rotate_res)
- {
- error= true;
- goto end;
- }
-
end:
my_free(previous_gtid_executed);
my_free(previous_gtid_lost);
Description: Summary: We should change SET GTID_PURGED so that it does not add any GTIDs to Previous_gtids_log_event and does not rotate the binary log. Instead it should add GTIDs to the table mysql.gtid_executed. Background: In 5.6, there is no table to store GTIDs, and GTIDs are not stored in the binary log at all when GTID_MODE=OFF. In particular, GTID_PURGED is normally equal to the Previous_gtids_log_event of the oldest binary log. In two circumstances, GTID_PURGED may instead be equal to the Previous_gtids_log_event of another binary log: 1. A SET GTID_PURGED statement causes the binary log to rotate, so that the new binary log contains the correct Previous_gtids_log_event. Thus, there may be a number of binary logs having Previous_gtids_log_event containing an empty set, preceding the binary log containing the 'real' Previous_gtids_log_event. 2. If GTID_MODE = OFF, no Previous_gtids_log_event is generated. Thus, if server switched from GTID_MODE=OFF to GTID_MODE=ON, there may be any number of binary logs having no Previous_gtids_log_event at all, preceding the binary log containing the 'real' Previous_gtids_log_event. So a precise recovery procedure that computes GTID_PURGED will read it from the first binary log that has one of the following: - Both a Previous_gtids_log_event and a Gtid_log_event. - A Previous_gtids_log_event that is not empty. When any of these criteria is met, it is sure that the user has not executed SET GTID_PURGED after this binary log, since SET GTID_PURGED is only allowed when GTID_EXECUTED is empty. This recovery procedure can be very slow: when gtid_mode=off it has to scan the headers of all binary logs. Therefore we introduced the binlog_gtid_simple_recovery option, which assumes that GTID_PURGED is empty if the first binray log does not contain a Previous_gtids_log_event. The routine will thus set the wrong value for GTID_PURGED in two cases: C1. the GTID_MODE changed from OFF to ON and not all old binary logs were purged; C2. the user executed SET GTID_PURGED and not all old binary logs were purged. In 5.7, the situation is different, because (1) we store GTIDs in the system table mysql.gtid_executed; (2) we generate Previous_gtids_log_event regardless of GTID_MODE. The recovery routine that initializes GTID_PURGED has also changed in 5.7: it adds those GTIDs which exist in the table but not in the binary log. Thus, in 5.7 we normally never have to skip over binary logs that do not have a Previous_gtids_log_event, because such binary logs normally do not exist. (The only case when such binary logs exist is if the server was just upgrade from 5.6 to 5.7 and still has some old binary logs.) However, we still skip over binary logs that contain a Previous_gtids_log_event that is empty and does not contain any Gtid_log_event, since it is possible that user issued SET GTID_PURGED in a later binary log. Therefore, in 5.7 binlog_simple_gtid_recovery is less unsafe, because in case C1 it computes the wrong value for GTID_PURGED only if some binary logs were generated by a 5.6 server, i.e., for a short while after an upgrade. Problem: Still, in 5.7 binlog_simple_gtid_recovery will compute wrong value for GTID_PURGED just after a SET GTID_PURGED statement. Since SET GTID_PURGED normally is done while restoring a backup, it is probably common to have server restart short after restoring the backup (e.g. to tweak some parameter before putting it into production). Solution: It would be much better if SET GTID_PURGED only added GTIDs to the table mysql.gtid_executed, and did not add anything to Previous_gtids_log_event. Then, C2 is only a problem in case SET GTID_PURGED was executed in 5.6. In other words, binlog_gtid_simple_recovery will compute wrong value for GTID_PURGED only just after an upgrade from 5.6. How to repeat: --source include/have_gtid.inc --let $rpl_topology= none --source include/rpl_init.inc SET GLOBAL GTID_PURGED = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa:1'; SELECT @@GLOBAL.GTID_PURGED; --let $rpl_server_parameters= --binlog-gtid-simple-recovery=1 --let $rpl_server_number= 1 --source include/rpl_restart_server.inc SELECT @@GLOBAL.GTID_PURGED; --let $rpl_server_parameters= --let $rpl_server_number= 1 --source include/rpl_restart_server.inc SELECT @@GLOBAL.GTID_PURGED; Suggested fix: diff --git a/sql/rpl_gtid.h b/sql/rpl_gtid.h index 91211ae..b0db48d 100644 --- a/sql/rpl_gtid.h +++ b/sql/rpl_gtid.h @@ -2735,7 +2735,7 @@ public: char *, bool) for format details. @return RETURN_STATUS_OK or RETURN_STATUS_REPORTED_ERROR. */ - enum_return_status add_lost_gtids(const char *text); + enum_return_status add_lost_gtids(const Gtid_set *gtid_set); /// Return a pointer to the Gtid_set that contains the lost groups. const Gtid_set *get_lost_gtids() const { return &lost_gtids; } /* @@ -2848,7 +2848,7 @@ public: @retval -1 Error */ - int save(Gtid_set *gtid_set); + int save(const Gtid_set *gtid_set); /** Save the set of gtids logged in the last binlog into gtid_executed table. diff --git a/sql/rpl_gtid_persist.cc b/sql/rpl_gtid_persist.cc index b75f71a..47c863d 100644 --- a/sql/rpl_gtid_persist.cc +++ b/sql/rpl_gtid_persist.cc @@ -288,7 +288,7 @@ end: } -int Gtid_table_persistor::save(THD *thd, Gtid *gtid) +int Gtid_table_persistor::save(THD *thd, const Gtid *gtid) { DBUG_ENTER("Gtid_table_persistor::save(THD *thd, Gtid *gtid)"); int error= 0; @@ -332,7 +332,7 @@ end: } -int Gtid_table_persistor::save(Gtid_set *gtid_set) +int Gtid_table_persistor::save(const Gtid_set *gtid_set) { DBUG_ENTER("Gtid_table_persistor::save(Gtid_set *gtid_set)"); int ret= 0; @@ -371,7 +371,7 @@ end: } -int Gtid_table_persistor::save(TABLE *table, Gtid_set *gtid_set) +int Gtid_table_persistor::save(TABLE *table, const Gtid_set *gtid_set) { DBUG_ENTER("Gtid_table_persistor::save(TABLE* table, " "Gtid_set *gtid_set)"); diff --git a/sql/rpl_gtid_persist.h b/sql/rpl_gtid_persist.h index 7d89b69..9b1891b 100644 --- a/sql/rpl_gtid_persist.h +++ b/sql/rpl_gtid_persist.h @@ -117,7 +117,7 @@ public: @retval -1 Error */ - int save(THD *thd, Gtid *gtid); + int save(THD *thd, const Gtid *gtid); /** Insert the gtid set into table. @@ -129,7 +129,7 @@ public: @retval -1 Error */ - int save(Gtid_set *gtid_set); + int save(const Gtid_set *gtid_set); /** Delete all rows from the table. @@ -301,7 +301,7 @@ private: @retval -1 Error */ - int save(TABLE *table, Gtid_set *gtid_set); + int save(TABLE *table, const Gtid_set *gtid_set); /* Prevent user from invoking default assignment function. */ Gtid_table_persistor &operator=(const Gtid_table_persistor &info); /* Prevent user from invoking default constructor function. */ diff --git a/sql/rpl_gtid_state.cc b/sql/rpl_gtid_state.cc index 237404c..ab2aecb 100644 --- a/sql/rpl_gtid_state.cc +++ b/sql/rpl_gtid_state.cc @@ -677,12 +677,12 @@ enum_return_status Gtid_state::ensure_sidno() } -enum_return_status Gtid_state::add_lost_gtids(const char *text) +enum_return_status Gtid_state::add_lost_gtids(const Gtid_set *gtid_set) { DBUG_ENTER("Gtid_state::add_lost_gtids()"); sid_lock->assert_some_wrlock(); - DBUG_PRINT("info", ("add_lost_gtids '%s'", text)); + gtid_set->dbug_print("add_lost_gtids"); if (!executed_gtids.is_empty()) { @@ -700,8 +700,11 @@ enum_return_status Gtid_state::add_lost_gtids(const char *text) } DBUG_ASSERT(lost_gtids.is_empty()); - PROPAGATE_REPORTED_ERROR(lost_gtids.add_gtid_text(text)); - PROPAGATE_REPORTED_ERROR(executed_gtids.add_gtid_text(text)); + if (save(gtid_set)) + RETURN_REPORTED_ERROR; + PROPAGATE_REPORTED_ERROR(gtids_only_in_table.add_gtid_set(gtid_set)); + PROPAGATE_REPORTED_ERROR(lost_gtids.add_gtid_set(gtid_set)); + PROPAGATE_REPORTED_ERROR(executed_gtids.add_gtid_set(gtid_set)); DBUG_RETURN(RETURN_STATUS_OK); } @@ -750,7 +753,7 @@ int Gtid_state::save(THD *thd) } -int Gtid_state::save(Gtid_set *gtid_set) +int Gtid_state::save(const Gtid_set *gtid_set) { DBUG_ENTER("Gtid_state::save(Gtid_set *gtid_set)"); int ret= gtid_table_persistor->save(gtid_set); diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc index 894c686..b663087 100644 --- a/sql/sys_vars.cc +++ b/sql/sys_vars.cc @@ -4942,14 +4942,23 @@ bool Sys_var_gtid_purged::global_update(THD *thd, set_var *var) DBUG_ENTER("Sys_var_gtid_purged::global_update"); #ifdef HAVE_REPLICATION bool error= false; - int rotate_res= 0; global_sid_lock->wrlock(); char *previous_gtid_executed= gtid_state->get_executed_gtids()->to_string(); char *previous_gtid_lost= gtid_state->get_lost_gtids()->to_string(); - enum_return_status ret= gtid_state->add_lost_gtids(var->save_result.string_value.str); - char *current_gtid_executed= gtid_state->get_executed_gtids()->to_string(); - char *current_gtid_lost= gtid_state->get_lost_gtids()->to_string(); + char *current_gtid_executed; + char *current_gtid_lost; + enum_return_status ret; + Gtid_set gtid_set(global_sid_map, var->save_result.string_value.str, + &ret, global_sid_lock); + if (ret != RETURN_STATUS_OK) + { + error= true; + goto end; + } + ret= gtid_state->add_lost_gtids(>id_set); + current_gtid_executed= gtid_state->get_executed_gtids()->to_string(); + current_gtid_lost= gtid_state->get_lost_gtids()->to_string(); global_sid_lock->unlock(); if (RETURN_STATUS_OK != ret) { @@ -4963,14 +4972,6 @@ bool Sys_var_gtid_purged::global_update(THD *thd, set_var *var) sql_print_information(ER(ER_GTID_EXECUTED_WAS_CHANGED), previous_gtid_executed, current_gtid_executed); - // Rotate logs to have Previous_gtid_event on last binlog. - rotate_res= mysql_bin_log.rotate_and_purge(thd, true); - if (rotate_res) - { - error= true; - goto end; - } - end: my_free(previous_gtid_executed); my_free(previous_gtid_lost);