List:Commits« Previous MessageNext Message »
From:Andrei Elkin Date:October 30 2008 8:48pm
Subject:Re: bzr commit into mysql-5.0-bugteam branch (alfranio.correia:2705) Bug#40337
View as plain text  
Alfranio, hello.

> #At file:///home/acorreia/workspace.sun/playground.mysql/mysql-5.0.68-custom_build/
>
>  2705 Alfranio Correia	2008-10-29
>       Introduced a recovery mechanism that throws away relay-log.bin* retrieved
>       from a master in case of a crash and updates the master.info based on
>       the relay.info. This is done because such files may be corrupted after
>       a crash. We are assuming that relay.info is fsynced per transaction.
>       See BUG#40337.

As I confessed the suggest fix of 1-3 items is principally okay with
me except some details.
And here they are.

This part of bug#40337 looks as the most difficult.

I have not found in your patch that there is a detection of a crash.
I am in strong opinion that we really need one prior to call your new
function.
Otherwise, as we talked, there is a chance to loose of lot of data the
IO thread received but the SQL thread has not processed.
Notice, that the SQL slave could be simply offline.
As I see you are trying to force the user to provide crash evidence
through the new option relay_log_recovery. Would not it be much
cheaper to write a stand-alone script to fix the coordinates in the
master.info?

An approach that I can see we need to follow is 
1. to find out evidence of a crash,
2. next to find out the last event (if any) executed from that relay log, 
3. and finally to set up the binlog retrieval coordinates for the master to be
   the next event after it.

If there is no evidence of a crash, no need to mess around with
master, relay-log.info.
If no event was executed from the last relay log then the binlog
retrieval coordinates can be set to the next event after the last executed
in the case the last executed is in the proceeding relay
log. Otherwise, if there are some more unprocessed relay log we will
have to find out the last event in the last properly closed relay log.


Evidence of crash can be found out through a special flag bit 
(check out LOG_EVENT_BINLOG_IN_USE_F) that is stored at the log closing time.
Alas, it works only for the binlog but not for the relay log.
Still, this technique can be rather easily adopted to the relay log.
and that's what I see we really need to implement.

So I am offering now to implement pp. 1-3 where p.1 implies extending
the relay log open/close operations with the binlog-like flagging.

regards,

Andrei

> modified:
>   mysql-test/r/rpl_flush_log_loop.result
>   sql/mysql_priv.h
>   sql/mysqld.cc
>   sql/set_var.cc
>   sql/slave.cc
>
> per-file messages:
>   mysql-test/r/rpl_flush_log_loop.result
>     Fixed a test case broken after creating the patch.
>   sql/mysql_priv.h
>     Created parameter --relay-log-recovery, default = 0 thus keeping the original
> behavior.
>   sql/mysqld.cc
>     Created parameter --relay-log-recovery, default = 0 thus keeping the original
> behavior.
>   sql/set_var.cc
>     Created parameter --relay-log-recovery, default = 0 thus keeping the original
> behavior.
>   sql/slave.cc
>     Provided a recovery mechanism for the master.info and relay-log.bin* based on the
> relay.info.
> === modified file 'mysql-test/r/rpl_flush_log_loop.result'
> --- a/mysql-test/r/rpl_flush_log_loop.result	2007-10-10 07:21:11 +0000
> +++ b/mysql-test/r/rpl_flush_log_loop.result	2008-10-29 16:37:16 +0000
> @@ -10,6 +10,7 @@ relay_log	MYSQLTEST_VARDIR/master-data/r
>  relay_log_index	
>  relay_log_info_file	relay-log.info
>  relay_log_purge	ON
> +relay_log_recovery	OFF
>  relay_log_space_limit	0
>  stop slave;
>  change master to master_host='127.0.0.1',master_user='root',
>
> === modified file 'sql/mysql_priv.h'
> --- a/sql/mysql_priv.h	2008-10-29 12:27:59 +0000
> +++ b/sql/mysql_priv.h	2008-10-29 16:37:16 +0000
> @@ -1314,7 +1314,7 @@ extern ulong expire_logs_days;
>  extern uint sync_binlog_period, sync_relaylog_period;
>  extern ulong opt_tc_log_size, tc_log_max_pages_used, tc_log_page_size;
>  extern ulong tc_log_page_waits;
> -extern my_bool relay_log_purge, opt_innodb_safe_binlog, opt_innodb;
> +extern my_bool relay_log_purge, relay_log_recovery, opt_innodb_safe_binlog,
> opt_innodb;
>  extern uint test_flags,select_errors,ha_open_options;
>  extern uint protocol_version, mysqld_port, dropping_tables;
>  extern uint delay_key_write_options, lower_case_table_names;
>
> === modified file 'sql/mysqld.cc'
> --- a/sql/mysqld.cc	2008-10-29 12:27:59 +0000
> +++ b/sql/mysqld.cc	2008-10-29 16:37:16 +0000
> @@ -404,7 +404,7 @@ ulong opt_ndb_cache_check_time;
>  const char *opt_ndb_mgmd;
>  ulong opt_ndb_nodeid;
>  #endif
> -my_bool opt_readonly, use_temp_pool, relay_log_purge;
> +my_bool opt_readonly, use_temp_pool, relay_log_purge, relay_log_recovery;
>  my_bool opt_sync_frm, opt_allow_suspicious_udfs;
>  my_bool opt_secure_auth= 0;
>  char* opt_secure_file_priv= 0;
> @@ -4919,6 +4919,7 @@ enum options_mysqld
>    OPT_QUERY_CACHE_TYPE, OPT_QUERY_CACHE_WLOCK_INVALIDATE, OPT_RECORD_BUFFER,
>    OPT_RECORD_RND_BUFFER, OPT_DIV_PRECINCREMENT, OPT_RELAY_LOG_SPACE_LIMIT,
>    OPT_RELAY_LOG_PURGE,
> +  OPT_RELAY_LOG_RECOVERY,
>    OPT_SLAVE_NET_TIMEOUT, OPT_SLAVE_COMPRESSED_PROTOCOL, OPT_SLOW_LAUNCH_TIME,
>    OPT_SLAVE_TRANS_RETRIES, OPT_READONLY, OPT_DEBUGGING,
>    OPT_SORT_BUFFER, OPT_TABLE_CACHE,
> @@ -6304,6 +6305,11 @@ The minimum value for this variable is 4
>     (gptr*) &relay_log_purge,
>     (gptr*) &relay_log_purge, 0, GET_BOOL, NO_ARG,
>     1, 0, 1, 0, 1, 0},
> +  {"relay_log_recovery", OPT_RELAY_LOG_RECOVERY,
> +   "Enables automatic relay log recovery right after the database startup, which
> means that the IO Thread starts re-fetching from the master right after the last
> transaction processed. Use this parameter together with the sync relay log.",
> +   (gptr*) &relay_log_recovery,
> +   (gptr*) &relay_log_recovery, 0, GET_BOOL, NO_ARG,
> +   0, 0, 1, 0, 1, 0},
>    {"relay_log_space_limit", OPT_RELAY_LOG_SPACE_LIMIT,
>     "Maximum space to use for all relay logs.",
>     (gptr*) &relay_log_space_limit,
>
> === modified file 'sql/set_var.cc'
> --- a/sql/set_var.cc	2008-10-29 12:27:59 +0000
> +++ b/sql/set_var.cc	2008-10-29 16:37:16 +0000
> @@ -323,6 +323,8 @@ sys_var_thd_ulong	sys_div_precincrement(
>  #ifdef HAVE_REPLICATION
>  sys_var_bool_ptr	sys_relay_log_purge("relay_log_purge",
>                                              &relay_log_purge);
> +sys_var_bool_ptr	sys_relay_log_recovery("relay_log_recovery",
> +                                            &relay_log_recovery);
>  #endif
>  sys_var_long_ptr	sys_rpl_recovery_rank("rpl_recovery_rank",
>  					      &rpl_recovery_rank);
> @@ -734,6 +736,7 @@ sys_var *sys_variables[]=
>    &sys_read_rnd_buff_size,
>  #ifdef HAVE_REPLICATION
>    &sys_relay_log_purge,
> +  &sys_relay_log_recovery,
>  #endif
>    &sys_rpl_recovery_rank,
>    &sys_safe_updates,
> @@ -1054,6 +1057,7 @@ struct show_var_st init_vars[]= {
>    {"relay_log_index", (char*) &opt_relaylog_index_name, SHOW_CHAR_PTR},
>    {"relay_log_info_file", (char*) &relay_log_info_file, SHOW_CHAR_PTR},
>    {sys_relay_log_purge.name,  (char*) &sys_relay_log_purge,         SHOW_SYS},
> +  {sys_relay_log_recovery.name,  (char*) &sys_relay_log_recovery,   SHOW_SYS},
>    {"relay_log_space_limit",  (char*) &relay_log_space_limit,       
> SHOW_LONGLONG},
>  #endif
>    {sys_rpl_recovery_rank.name,(char*) &sys_rpl_recovery_rank,       SHOW_SYS},
>
> === modified file 'sql/slave.cc'
> --- a/sql/slave.cc	2008-10-29 12:27:59 +0000
> +++ b/sql/slave.cc	2008-10-29 16:37:16 +0000
> @@ -67,6 +67,7 @@ static inline bool io_slave_killed(THD* 
>  static inline bool sql_slave_killed(THD* thd,RELAY_LOG_INFO* rli);
>  static int count_relay_log_space(RELAY_LOG_INFO* rli);
>  static int init_slave_thread(THD* thd, SLAVE_THD_TYPE thd_type);
> +static int init_recovery(MASTER_INFO* mi);
>  static int safe_connect(THD* thd, MYSQL* mysql, MASTER_INFO* mi);
>  static int safe_reconnect(THD* thd, MYSQL* mysql, MASTER_INFO* mi,
>  			  bool suppress_warnings);
> @@ -174,6 +175,9 @@ int init_slave()
>    if (server_id && !master_host && active_mi->host[0])
>      master_host= active_mi->host;
>  
> +  if (init_recovery(active_mi))
> +    goto err;
> +
>    /* If server id is not set, start_slave_thread() will say it */
>  
>    if (master_host && !opt_skip_slave_start)
> @@ -197,6 +201,73 @@ err:
>    DBUG_RETURN(1);
>  }
>  
> +/*
> +* Updates the master info based on the information stored in the relay info and
> +* ignores relay logs previously retrieved by the IO thread starting fetching again
> +* based on to the group_master_log_pos and group_master_log_name.
> +*
> +*  There is no need for a mutex 
> +*  as the caller (i.e. init_slave) already has one acquired.
> +*
> +*  The recovery is done just after the database startup which means that
> +*  start slave and stop slave commands do not throw away any relay log 
> +*  retrieved in the meantime.
> +*
> +*  In the feature, we can improve this routine in order to avoid throwing
> +*  away logs that are safely stored in the disk. Eventually, the old
> +*  relay logs will be purged by the normal purge mechanism.
> +*
> +* Specifically,
> +*
> +*  1 - mi->master_log_pos  <-- rli->group_master_log_pos
> +*  2 - mi->master_log_name <-- rli->group_master_log_name
> +*  3 - It moves the relay info to the new relay log file, by
> +*   rli->group_relay_log_pos <-- rli->group_relay_log_pos <--
> BIN_LOG_HEADER_SIZE;
> +*   rli->group_relay_log_name, rli->group_relay_log_pos
> +*
> +* If there is an error, it returns (1), otherwise returns (0). 
> +*/
> +static int init_recovery(MASTER_INFO* mi)
> +{
> +  const char* errmsg=0;
> +  DBUG_ENTER("init_recovery");
> +
> +  RELAY_LOG_INFO* rli = &mi->rli;
> +  if (relay_log_recovery && rli->group_master_log_name[0])
> +  {
> +    mi->master_log_pos = max(BIN_LOG_HEADER_SIZE,
> +                              rli->group_master_log_pos);
> +    strmake(mi->master_log_name, rli->group_master_log_name,
> +      sizeof(mi->master_log_name)-1);
> +
> +    strmake(rli->group_relay_log_name, rli->relay_log.get_log_fname(),
> +            sizeof(rli->group_relay_log_name)-1);
> +    strmake(rli->event_relay_log_name, rli->relay_log.get_log_fname(),
> +            sizeof(mi->rli.event_relay_log_name)-1);
> +
> +    rli->group_relay_log_pos = rli->group_relay_log_pos =
> BIN_LOG_HEADER_SIZE;
> +
> +    if (init_relay_log_pos(rli,
> +                           rli->group_relay_log_name,
> +                           rli->group_relay_log_pos,
> +                           0 /*no data lock*/,
> +                           &errmsg, 0))
> +      DBUG_RETURN(1);
> +
> +    if (flush_master_info(mi, 0))
> +    {
> +      sql_print_error("Failed to flush master info file");
> +      DBUG_RETURN(1);
> +    }
> +    if (flush_relay_log_info(rli))
> +    {
> +       sql_print_error("Failed to flush master info file");
> +       DBUG_RETURN(1);
> +    }
> +  }
> +
> +  DBUG_RETURN(0);
> +}
>  
>  static void free_table_ent(TABLE_RULE_ENT* e)
>  {
>
>
> -- 
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe:    http://lists.mysql.com/commits?unsub=1
Thread
bzr commit into mysql-5.0-bugteam branch (alfranio.correia:2705) Bug#40337Alfranio Correia29 Oct
  • Re: bzr commit into mysql-5.0-bugteam branch (alfranio.correia:2705) Bug#40337Andrei Elkin30 Oct
    • Re: bzr commit into mysql-5.0-bugteam branch (alfranio.correia:2705)Bug#40337alfranio correia31 Oct
      • Re: bzr commit into mysql-5.0-bugteam branch (alfranio.correia:2705)Bug#40337alfranio correia31 Oct
      • Re: bzr commit into mysql-5.0-bugteam branch (alfranio.correia:2705) Bug#40337Andrei Elkin31 Oct
  • Re: bzr commit into mysql-5.0-bugteam branch (alfranio.correia:2705)Bug#40337He Zhenxing3 Nov
    • Re: bzr commit into mysql-5.0-bugteam branch (alfranio.correia:2705)Bug#40337alfranio correia12 Nov
Re: bzr commit into mysql-5.0-bugteam branch (alfranio.correia:2705)Bug#40337He Zhenxing15 Nov
  • Re: bzr commit into mysql-5.0-bugteam branch (alfranio.correia:2705)Bug#40337alfranio correia15 Nov
    • Re: bzr commit into mysql-5.0-bugteam branch (alfranio.correia:2705)Bug#40337He Zhenxing15 Nov