Hi Alfranio,
I have some comments below.
Just my few cents,
Mats Kindahl
Alfranio Correia wrote:
> #At
> file:///home/acorreia/workspace.sun/repository.mysql/bzrwork/bug-40337/mysql-6.0-rpl/
> based on revid:luis.soares@stripped
>
> 2810 Alfranio Correia 2009-02-04
> BUG#40337 Fsyncing master and relay log to disk after every event is too slow
[snip]
> === modified file 'sql/rpl_rli.h'
> --- a/sql/rpl_rli.h 2008-08-18 05:43:50 +0000
> +++ b/sql/rpl_rli.h 2009-02-04 16:11:20 +0000
[snip]
> + /*
> + * Identifies when the recovery process is going on.
> + * See sql/slave.cc:init_recovery for further details.
> + */
> + bool is_relay_log_recovery;
I don't see an initializer for this field in the constructor above. (See below.)
[snip]
> === modified file 'sql/slave.cc'
> --- a/sql/slave.cc 2009-01-26 16:03:39 +0000
> +++ b/sql/slave.cc 2009-02-04 16:11:20 +0000
[snip]
> @@ -254,6 +255,10 @@ int init_slave()
> goto err;
> }
>
> + active_mi->rli.is_relay_log_recovery= relay_log_recovery;
> + if (active_mi->rli.is_relay_log_recovery && init_recovery(active_mi))
> + goto err;
> +
Don't you need to move the first line to just after you have created the
Master_info, and then add a guard inside init_master_info so that
init_relay_log_info() does not try to open (at least) the relay log file since
that is corrupt? Also, isn't it the case that the contents read from
relay-log.info will be thrown away if you first run init_relay_log_info() and
then init_recovery()? In that case, it seems redundant to execute
init_relay_log_info() at all.
Actually, I would prefer if you could initialize the is_relay_log_recovery field
in the constructor to avoid it having a random value at the wrong time, so
something like:
active_mi = new Master_info(relay_log_recovery);
... and then change the constructor for Master_info and Relay_log_info accordingly.
> @@ -269,15 +274,86 @@ int init_slave()
> goto err;
> }
> }
> + active_mi->rli.is_relay_log_recovery= 0;
> pthread_mutex_unlock(&LOCK_active_mi);
> DBUG_RETURN(0);
>
> err:
> + active_mi->rli.is_relay_log_recovery= 0;
> pthread_mutex_unlock(&LOCK_active_mi);
> DBUG_RETURN(1);
This is two pieces of code that is almost identical. I think it could be a good
idea to try to merge these cases into one piece of code.
--
Mats Kindahl
Lead Software Developer
Replication Team
MySQL AB, www.mysql.com
