← Back to team overview

maria-developers team mailing list archive

Re: Review of patch for MDEV-4820

 

Kristian Nielsen <knielsen@xxxxxxxxxxxxxxx> writes:

> If you have time to test this then that would be a nice help. I'll see if I
> can come up with a quick patch (ie. later today or tomorrow).

Please try this patch and let me know if you find any issues. I still need to
implement test cases, but it seems to work from quick manual testing.

If you use this to save and restore the internal binlog state across deleting
the binlogs, you should be able to have things work the same as if FLUSH LOGS
+ PURGE BINARY LOGS had been used, and avoid all the small issues that were
caused by the deletion of the binlogs.

I've attached the patch and also appended some text that I indend to add to
the documentation.

 - Kristian.

-----------------------------------------------------------------------
Variable: gtid_binlog_state
Scope: global
Dynamic: Yes
Type: String

The variable gtid_binlog_state holds the internal state of the binlog. The
state consists of the last GTID ever logged to the binary log for every
combination of domain_id and server_id. This information is used by the master
to determine whether a given GTID has been logged to the binlog in the past,
even if it has later been deleted due to binlog purge.

Normally this internal state is not needed by users, as @@gtid_binlog_pos is
more useful in most cases. The main usage of @@gtid_binlog_state is to restore
the state of the binlog after RESET MASTER (or equivalently if the binlog
files are lost). If the value of @@gtid_binlog_state is saved before RESET
MASTER and restored afterwards, the master will retain information about past
history, same as if PURGE BINARY LOGS had been used (of course the actual
events in the binary logs are lost).

Note that to set the value of @@gtid_binlog_state, the binary log must be
empty, that is it must not contain any GTID events and the previous value of
@@gtid_binlog_state must be the empty string. If not, then RESET MASTER must
be used first to erase the binary log first.

For completeness, note that setting @@gtid_binlog_state internally executes a
RESET MASTER. This is normally not noticable as it can only be changed when
the binlog is empty of GTID events. However, if executed eg. immediately after
upgrading to MariaDB 10, it is possible that the binlog is non-empty but
without any GTID events, in which case all such events will be deleted, just
as if RESET MASTER had been run.
-----------------------------------------------------------------------

=== modified file 'mysql-test/suite/sys_vars/r/gtid_slave_pos_basic.result'
--- mysql-test/suite/sys_vars/r/gtid_slave_pos_basic.result	2013-05-22 15:36:48 +0000
+++ mysql-test/suite/sys_vars/r/gtid_slave_pos_basic.result	2013-08-22 12:46:00 +0000
@@ -21,13 +21,13 @@ SELECT @@gtid_slave_pos;
 @@gtid_slave_pos
 1-2-3,2-4-6
 SET GLOBAL gtid_slave_pos= '-1-2-3';
-ERROR HY000: Could not parse GTID list for GTID_POS
+ERROR HY000: Could not parse GTID list
 SET GLOBAL gtid_slave_pos= '1-2 -3';
-ERROR HY000: Could not parse GTID list for GTID_POS
+ERROR HY000: Could not parse GTID list
 SET GLOBAL gtid_slave_pos= '1-2-3 ';
-ERROR HY000: Could not parse GTID list for GTID_POS
+ERROR HY000: Could not parse GTID list
 SET GLOBAL gtid_slave_pos= '1-2-3,2-4';
-ERROR HY000: Could not parse GTID list for GTID_POS
+ERROR HY000: Could not parse GTID list
 SET GLOBAL gtid_slave_pos= '0-1-10,0-2-20';
 ERROR HY000: GTID 0-2-20 and 0-1-10 conflict (duplicate domain id 0)
 SET GLOBAL gtid_slave_pos= '0-1-10,1-2-20,2-3-30,1-20-200,3-4-1';

=== modified file 'sql/log.cc'
--- sql/log.cc	2013-07-17 19:24:29 +0000
+++ sql/log.cc	2013-08-22 13:11:52 +0000
@@ -3703,7 +3703,8 @@ int MYSQL_BIN_LOG::find_next_log(LOG_INF
     1   error
 */
 
-bool MYSQL_BIN_LOG::reset_logs(THD* thd, bool create_new_log)
+bool MYSQL_BIN_LOG::reset_logs(THD* thd, bool create_new_log,
+                               rpl_gtid *init_state, uint32 init_state_len)
 {
   LOG_INFO linfo;
   bool error=0;
@@ -3722,6 +3723,14 @@ bool MYSQL_BIN_LOG::reset_logs(THD* thd,
 
   if (!is_relay_log)
   {
+    if (init_state && !is_empty_state())
+    {
+      my_error(ER_BINLOG_MUST_BE_EMPTY, MYF(0));
+      mysql_mutex_unlock(&LOCK_index);
+      mysql_mutex_unlock(&LOCK_log);
+      DBUG_RETURN(1);
+    }
+
     /*
       Mark that a RESET MASTER is in progress.
       This ensures that a binlog checkpoint will not try to write binlog
@@ -3839,7 +3848,10 @@ bool MYSQL_BIN_LOG::reset_logs(THD* thd,
 
   if (!is_relay_log)
   {
-    rpl_global_gtid_binlog_state.reset();
+    if (init_state)
+      rpl_global_gtid_binlog_state.load(init_state, init_state_len);
+    else
+      rpl_global_gtid_binlog_state.reset();
   }
 
   /* Start logging with a new file */
@@ -5524,6 +5536,30 @@ MYSQL_BIN_LOG::append_state_pos(String *
 }
 
 
+bool
+MYSQL_BIN_LOG::append_state(String *str)
+{
+  bool err;
+
+  mysql_mutex_lock(&rpl_global_gtid_binlog_state.LOCK_binlog_state);
+  err= rpl_global_gtid_binlog_state.append_state(str);
+  mysql_mutex_unlock(&rpl_global_gtid_binlog_state.LOCK_binlog_state);
+  return err;
+}
+
+
+bool
+MYSQL_BIN_LOG::is_empty_state()
+{
+  bool res;
+
+  mysql_mutex_lock(&rpl_global_gtid_binlog_state.LOCK_binlog_state);
+  res= (rpl_global_gtid_binlog_state.count() == 0);
+  mysql_mutex_unlock(&rpl_global_gtid_binlog_state.LOCK_binlog_state);
+  return res;
+}
+
+
 bool
 MYSQL_BIN_LOG::find_in_binlog_state(uint32 domain_id, uint32 server_id,
                                     rpl_gtid *out_gtid)

=== modified file 'sql/log.h'
--- sql/log.h	2013-05-28 11:28:31 +0000
+++ sql/log.h	2013-08-22 13:12:12 +0000
@@ -751,7 +751,8 @@ class MYSQL_BIN_LOG: public TC_LOG, priv
   int register_create_index_entry(const char* entry);
   int purge_index_entry(THD *thd, ulonglong *decrease_log_space,
                         bool need_mutex);
-  bool reset_logs(THD* thd, bool create_new_log);
+  bool reset_logs(THD* thd, bool create_new_log,
+                  rpl_gtid *init_state, uint32 init_state_len);
   void close(uint exiting);
   void clear_inuse_flag_when_closing(File file);
 
@@ -780,6 +781,8 @@ class MYSQL_BIN_LOG: public TC_LOG, priv
   int write_state_to_file();
   int get_most_recent_gtid_list(rpl_gtid **list, uint32 *size);
   bool append_state_pos(String *str);
+  bool append_state(String *str);
+  bool is_empty_state();
   bool find_in_binlog_state(uint32 domain_id, uint32 server_id,
                             rpl_gtid *out_gtid);
   bool lookup_domain_in_binlog_state(uint32 domain_id, rpl_gtid *out_gtid);

=== modified file 'sql/rpl_gtid.cc'
--- sql/rpl_gtid.cc	2013-08-22 10:36:42 +0000
+++ sql/rpl_gtid.cc	2013-08-22 13:22:06 +0000
@@ -719,6 +719,45 @@ gtid_parser_helper(char **ptr, char *end
 }
 
 
+rpl_gtid *
+gtid_parse_string_to_list(const char *str, size_t str_len, uint32 *out_len)
+{
+  char *p= const_cast<char *>(str);
+  char *end= p + str_len;
+  uint32 len= 0, alloc_len= 5;
+  rpl_gtid *list= NULL;
+
+  for (;;)
+  {
+    rpl_gtid gtid;
+
+    if (len >= (((uint32)1 << 28)-1) || gtid_parser_helper(&p, end, &gtid))
+    {
+      my_free(list);
+      return NULL;
+    }
+    if ((!list || len >= alloc_len) &&
+        !(list=
+          (rpl_gtid *)my_realloc(list,
+                                 (alloc_len= alloc_len*2) * sizeof(rpl_gtid),
+                                 MYF(MY_FREE_ON_ERROR|MY_ALLOW_ZERO_PTR))))
+      return NULL;
+    list[len++]= gtid;
+
+    if (p == end)
+      break;
+    if (*p != ',')
+    {
+      my_free(list);
+      return NULL;
+    }
+    ++p;
+  }
+  *out_len= len;
+  return list;
+}
+
+
 /*
   Update the slave replication state with the GTID position obtained from
   master when connecting with old-style (filename,offset) position.
@@ -1231,6 +1270,41 @@ rpl_binlog_state::append_pos(String *str
   }
 
   return false;
+}
+
+
+bool
+rpl_binlog_state::append_state(String *str)
+{
+  uint32 i, j;
+  bool first= true;
+
+  for (i= 0; i < hash.records; ++i)
+  {
+    element *e= (element *)my_hash_element(&hash, i);
+    if (!e->last_gtid)
+    {
+      DBUG_ASSERT(e->hash.records==0);
+      continue;
+    }
+    for (j= 0; j <= e->hash.records; ++j)
+    {
+      const rpl_gtid *gtid;
+      if (j < e->hash.records)
+      {
+        gtid= (rpl_gtid *)my_hash_element(&e->hash, j);
+        if (gtid == e->last_gtid)
+          continue;
+      }
+      else
+        gtid= e->last_gtid;
+
+      if (rpl_slave_state_tostring_helper(str, gtid, &first))
+        return true;
+    }
+  }
+
+  return false;
 }
 
 

=== modified file 'sql/rpl_gtid.h'
--- sql/rpl_gtid.h	2013-08-22 10:36:42 +0000
+++ sql/rpl_gtid.h	2013-08-22 13:12:44 +0000
@@ -163,6 +163,7 @@ struct rpl_binlog_state
   int get_gtid_list(rpl_gtid *gtid_list, uint32 list_size);
   int get_most_recent_gtid_list(rpl_gtid **list, uint32 *size);
   bool append_pos(String *str);
+  bool append_state(String *str);
   rpl_gtid *find(uint32 domain_id, uint32 server_id);
   rpl_gtid *find_most_recent(uint32 domain_id);
 };
@@ -205,5 +206,7 @@ struct slave_connection_state
 extern bool rpl_slave_state_tostring_helper(String *dest, const rpl_gtid *gtid,
                                             bool *first);
 extern int gtid_check_rpl_slave_state_table(TABLE *table);
+extern rpl_gtid *gtid_parse_string_to_list(const char *p, size_t len,
+                                           uint32 *out_len);
 
 #endif  /* RPL_GTID_H */

=== modified file 'sql/rpl_rli.cc'
--- sql/rpl_rli.cc	2013-06-21 19:23:24 +0000
+++ sql/rpl_rli.cc	2013-08-22 13:16:19 +0000
@@ -1011,7 +1011,7 @@ int purge_relay_logs(Relay_log_info* rli
     rli->cur_log_fd= -1;
   }
 
-  if (rli->relay_log.reset_logs(thd, !just_reset))
+  if (rli->relay_log.reset_logs(thd, !just_reset, NULL, 0))
   {
     *errmsg = "Failed during log reset";
     error=1;

=== modified file 'sql/share/errmsg-utf8.txt'
--- sql/share/errmsg-utf8.txt	2013-08-16 13:10:25 +0000
+++ sql/share/errmsg-utf8.txt	2013-08-22 13:02:13 +0000
@@ -6527,7 +6527,7 @@ ER_SQL_DISCOVER_ERROR
 ER_FAILED_GTID_STATE_INIT
         eng "Failed initializing replication GTID state"
 ER_INCORRECT_GTID_STATE
-        eng "Could not parse GTID list for GTID_POS"
+        eng "Could not parse GTID list"
 ER_CANNOT_UPDATE_GTID_STATE
         eng "Could not update replication slave gtid state"
 ER_DUPLICATE_GTID_DOMAIN
@@ -6557,3 +6557,5 @@ ER_STORED_FUNCTION_PREVENTS_SWITCH_GTID_
         eng "Cannot modify @@session.gtid_domain_id or @@session.gtid_seq_no inside a stored function or trigger"
 ER_GTID_POSITION_NOT_FOUND_IN_BINLOG2
 	eng "Connecting slave requested to start from GTID %u-%u-%llu, which is not in the master's binlog. Since the master's binlog contains GTIDs with higher sequence numbers, it probably means that the slave has diverged due to executing extra errorneous transactions"
+ER_BINLOG_MUST_BE_EMPTY
+	eng "This operation is not allowed if any GTID has been logged to the binary log. Run RESET MASTER first to erase the log"

=== modified file 'sql/sql_reload.cc'
--- sql/sql_reload.cc	2013-04-12 13:06:51 +0000
+++ sql/sql_reload.cc	2013-08-22 13:17:52 +0000
@@ -322,7 +322,7 @@ bool reload_acl_and_cache(THD *thd, unsi
   {
     DBUG_ASSERT(thd);
     tmp_write_to_binlog= 0;
-    if (reset_master(thd))
+    if (reset_master(thd, NULL, 0))
     {
       /* NOTE: my_error() has been already called by reset_master(). */
       result= 1;

=== modified file 'sql/sql_repl.cc'
--- sql/sql_repl.cc	2013-08-16 13:10:25 +0000
+++ sql/sql_repl.cc	2013-08-22 12:53:46 +0000
@@ -3474,7 +3474,7 @@ bool change_master(THD* thd, Master_info
   @retval 0 success
   @retval 1 error
 */
-int reset_master(THD* thd)
+int reset_master(THD* thd, rpl_gtid *init_state, uint32 init_state_len)
 {
   if (!mysql_bin_log.is_open())
   {
@@ -3483,7 +3483,7 @@ int reset_master(THD* thd)
     return 1;
   }
 
-  if (mysql_bin_log.reset_logs(thd, 1))
+  if (mysql_bin_log.reset_logs(thd, 1, init_state, init_state_len))
     return 1;
   RUN_HOOK(binlog_transmit, after_reset_master, (thd, 0 /* flags */));
   return 0;

=== modified file 'sql/sql_repl.h'
--- sql/sql_repl.h	2013-07-17 19:24:29 +0000
+++ sql/sql_repl.h	2013-08-22 13:14:48 +0000
@@ -46,7 +46,7 @@ int stop_slave(THD* thd, Master_info* mi
 bool change_master(THD* thd, Master_info* mi, bool *master_info_added);
 bool mysql_show_binlog_events(THD* thd);
 int reset_slave(THD *thd, Master_info* mi);
-int reset_master(THD* thd);
+int reset_master(THD* thd, rpl_gtid *init_state, uint32 init_state_len);
 bool purge_master_logs(THD* thd, const char* to_log);
 bool purge_master_logs_before_date(THD* thd, time_t purge_time);
 bool log_in_use(const char* log_name);

=== modified file 'sql/sys_vars.cc'
--- sql/sys_vars.cc	2013-07-17 19:24:29 +0000
+++ sql/sys_vars.cc	2013-08-22 14:05:11 +0000
@@ -1434,6 +1434,110 @@ static Sys_var_mybool Sys_gtid_strict_mo
        "generate an out-of-order binlog if executed.",
        GLOBAL_VAR(opt_gtid_strict_mode),
        CMD_LINE(OPT_ARG), DEFAULT(FALSE));
+
+
+struct gtid_binlog_state_data { rpl_gtid *list; uint32 list_len; };
+
+bool
+Sys_var_gtid_binlog_state::do_check(THD *thd, set_var *var)
+{
+  String str, *res;
+  struct gtid_binlog_state_data *data;
+  rpl_gtid *list;
+  uint32 list_len;
+
+  DBUG_ASSERT(var->type == OPT_GLOBAL);
+
+  if (!(res= var->value->val_str(&str)))
+    return true;
+  if (thd->in_active_multi_stmt_transaction())
+  {
+    my_error(ER_CANT_DO_THIS_DURING_AN_TRANSACTION, MYF(0));
+    return true;
+  }
+  if (!mysql_bin_log.is_open())
+  {
+    my_error(ER_FLUSH_MASTER_BINLOG_CLOSED, MYF(0));
+    return true;
+  }
+  if (!mysql_bin_log.is_empty_state())
+  {
+    my_error(ER_BINLOG_MUST_BE_EMPTY, MYF(0));
+    return true;
+  }
+  if (res->length() == 0)
+    list= NULL;
+  else if (!(list= gtid_parse_string_to_list(res->ptr(), res->length(),
+                                             &list_len)))
+  {
+    my_error(ER_INCORRECT_GTID_STATE, MYF(0));
+    return true;
+  }
+  if (!(data= (gtid_binlog_state_data *)my_malloc(sizeof(*data), MYF(0))))
+  {
+    my_free(list);
+    my_error(ER_OUT_OF_RESOURCES, MYF(0));
+    return true;
+  }
+  data->list= list;
+  data->list_len= list_len;
+  var->save_result.ptr= data;
+  return false;
+}
+
+
+bool
+Sys_var_gtid_binlog_state::global_update(THD *thd, set_var *var)
+{
+  int res= true;
+  struct gtid_binlog_state_data *data;
+
+  DBUG_ASSERT(var->type == OPT_GLOBAL);
+
+  if (!var->value)
+  {
+    my_error(ER_NO_DEFAULT, MYF(0), var->var->name.str);
+    goto err;
+  }
+  data= (struct gtid_binlog_state_data *)var->save_result.ptr;
+
+  mysql_mutex_unlock(&LOCK_global_system_variables);
+  res= (0 != reset_master(thd, data->list, data->list_len));
+  mysql_mutex_lock(&LOCK_global_system_variables);
+
+err:
+  my_free(data->list);
+  my_free(data);
+
+  return res;
+}
+
+
+uchar *
+Sys_var_gtid_binlog_state::global_value_ptr(THD *thd, LEX_STRING *base)
+{
+  char buf[512];
+  String str(buf, sizeof(buf), system_charset_info);
+  char *p;
+
+  str.length(0);
+  if ((opt_bin_log && mysql_bin_log.append_state(&str)) ||
+      !(p= thd->strmake(str.ptr(), str.length())))
+  {
+    my_error(ER_OUT_OF_RESOURCES, MYF(0));
+    return NULL;
+  }
+
+  return (uchar *)p;
+}
+
+
+static unsigned char opt_gtid_binlog_state_dummy;
+static Sys_var_gtid_binlog_state Sys_gtid_binlog_state(
+       "gtid_binlog_state",
+       "The internal GTID state of the binlog, used to keep track of all "
+       "GTIDs ever logged to the binlog.",
+       GLOBAL_VAR(opt_gtid_binlog_state_dummy), NO_CMD_LINE);
 #endif
 
 

=== modified file 'sql/sys_vars.h'
--- sql/sys_vars.h	2013-08-20 11:48:29 +0000
+++ sql/sys_vars.h	2013-08-22 11:28:38 +0000
@@ -2167,3 +2167,44 @@ class Sys_var_gtid_slave_pos: public sys
   }
   uchar *global_value_ptr(THD *thd, LEX_STRING *base);
 };
+
+
+/**
+  Class for @@global.gtid_binlog_state.
+*/
+class Sys_var_gtid_binlog_state: public sys_var
+{
+public:
+  Sys_var_gtid_binlog_state(const char *name_arg,
+          const char *comment, int flag_args, ptrdiff_t off, size_t size,
+          CMD_LINE getopt)
+    : sys_var(&all_sys_vars, name_arg, comment, flag_args, off, getopt.id,
+              getopt.arg_type, SHOW_CHAR, 0, NULL, VARIABLE_NOT_IN_BINLOG,
+              NULL, NULL, NULL)
+  {
+    option.var_type= GET_STR;
+  }
+  bool do_check(THD *thd, set_var *var);
+  bool session_update(THD *thd, set_var *var)
+  {
+    DBUG_ASSERT(false);
+    return true;
+  }
+  bool global_update(THD *thd, set_var *var);
+  bool check_update_type(Item_result type) { return type != STRING_RESULT; }
+  void session_save_default(THD *thd, set_var *var)
+  {
+    DBUG_ASSERT(false);
+  }
+  void global_save_default(THD *thd, set_var *var)
+  {
+    /* Record the attempt to use default so we can error. */
+    var->value= 0;
+  }
+  uchar *session_value_ptr(THD *thd, LEX_STRING *base)
+  {
+    DBUG_ASSERT(false);
+    return NULL;
+  }
+  uchar *global_value_ptr(THD *thd, LEX_STRING *base);
+};


Follow ups

References