Re: bk commit into 5.0 tree (aelkin:1.2543) BUG#28597

[Sergei Golubchik]

Hi!

On Oct 25, Andrei Elkin wrote:
> ChangeSet@stripped, 2007-10-25 12:21:46+03:00, aelkin@stripped +3 -0
>   Bug #28597 Replication doesn't start after upgrading to 5.1.18
>   
>   Since bug@20166, which replaced the binlog file name generating to base on
> pidfile_name instead of 
>   the previous glob_hostname, the binlog file name generating and its storing into
> the binlog index
>   suddenly started to be solely in the absolute path format,
>   including a case when --log-bin option meant a relative path.

make comment lines shorter than 80 characters
   
>   An existed algorithm on master to find a requested binlog file for slave was
> limited to compare only
>   homogenous paths either absolute or relative but never mixed. 
>   In effect of two facts if --log-bin value means a relative path, comparison on the
> buggy 5.1.18 of an
>   allways absolute value (read from the index) with a relave  was doomed.
>   
>   Fixed with leaving `pidfile_name' but stripping off its directory part that
> restores the original logics
>   of storing the names in compatible with --log-bin option format.
>   The comparison algorithm is refined to be able match any format two paths
> converting them to the absolute
>   paths.
>   
>   Two side effects for this fix:
>   
>   correcting bug#27070;
>   ensuring no overrun for buff can happen anymore (Bug#31836 
>   insufficient space reserved for the suffix of relay log file name);
>   bug#31837  --remove_file $MYSQLTEST_VARDIR/tmp/bug14157.sql missed in
> rpl_temporary.test;
> 
>   mysql-test/t/rpl_dual_pos_advance.test@stripped, 2007-10-25 12:21:44+03:00,
> aelkin@stripped +0 -6
>     After correcting the logics of log file name composing workaround for
>     
>       Bug #27070 server logs are created unrequested and in wrong directory
>     
>     is removed.
> 
>   mysql-test/t/rpl_temporary.test@stripped, 2007-10-25 12:21:44+03:00,
> aelkin@stripped +2 -0
>     remaining temp file of the test removed
> 
>   sql/log.cc@stripped, 2007-10-25 12:21:44+03:00, aelkin@stripped +36 -10
>     stripping off the directory part of `pidfile_name' for binlog name generating;
>     a new function static bool equalp_paths() compares two paths;
>     ensuring no overrun for buff can happen anymore (Bug #31836 
>     insufficient space reserved for the suffix of relay log file name)
> 
> diff -Nrup a/sql/log.cc b/sql/log.cc
> --- a/sql/log.cc	2007-07-30 18:27:30 +03:00
> +++ b/sql/log.cc	2007-10-25 12:21:44 +03:00
> @@ -448,13 +448,11 @@ const char *MYSQL_LOG::generate_name(con
>  {
>    if (!log_name || !log_name[0])
>    {
> -    /*
> -      TODO: The following should be using fn_format();  We just need to
> -      first change fn_format() to cut the file name if it's too long.
> -    */
> -    strmake(buff, pidfile_name,FN_REFLEN-5);
> -    strmov(fn_ext(buff),suffix);
> -    return (const char *)buff;
> +    strmake(buff, pidfile_name, FN_REFLEN - strlen(suffix) - 1);
> +    return (const char *)
> +      fn_format(buff, buff, "", suffix,
> +                MYF(MY_UNPACK_FILENAME|MY_REPLACE_EXT|MY_REPLACE_DIR));

Why do you remove directory here ? It's wrong, binlogs should be where
pidfile is (by default), not in the datadir.

>    }
>    // get rid of extension if the log is binary to avoid problems
>    if (strip_ext)
> @@ -796,6 +794,33 @@ err:
>  #endif /* HAVE_REPLICATION */
>  
>  /*
> +  Compare two arbitrary paths represented as null-terminated strings.
> +  If a path arg is relative then `abs_root' absolute path provides the
> +  necessary contribution to build the absolute path out of the path arg.
> +
> +  SYNOPSIS
> +    equalp_paths()
> +    
> +    path_a, path_b  being compared paths
> +    abs_root        an absolute root path
> +
> +  RETURN VALUES
> +  TRUE if the paths are equal, otherwise FALSE
> + */
> +static bool equalp_paths(const char *path_a,  const char *path_b,
> +                         const char *abs_root= mysql_real_data_home)
> +{
> +  size_t len;
> +  uint flag= MYF(MY_RETURN_REAL_PATH|MY_RESOLVE_SYMLINKS);

remove MY_RETURN_REAL_PATH and MY_RESOLVE_SYMLINKS.
We only want to compare two strings, not query filesystem for directory
hierarchy.

> +  char abs_path_a[2*FN_REFLEN];
> +  char abs_path_b[2*FN_REFLEN];
> +  fn_format(abs_path_a, path_a, abs_root, "", flag);
> +  fn_format(abs_path_b, path_b, abs_root, "", flag);
> +  return ((len= strlen(abs_path_a)) == strlen(abs_path_b) && 
> +          memcmp(abs_path_a, abs_path_b, len) == 0);
> +}
> +
> +/*
>    Find the position in the log-index-file for the given log name
>  
>    SYNOPSIS
> @@ -852,11 +877,12 @@ int MYSQL_LOG::find_log_pos(LOG_INFO *li
>  
>      // if the log entry matches, null string matching anything
>      if (!log_name ||
> -	(log_name_len == length-1 && fname[log_name_len] == '\n' &&
> -	 !memcmp(fname, log_name, log_name_len)))
> +        (fname[length - 1] == '\n' &&
> +         !(fname[length - 1]= 0)  &&
> +         equalp_paths(log_name, fname)))

remove equalp_paths function and do fn_format explicitly.
fn_format(fname) - here
fn_format(log_name) - *outside of the loop*

>      {
>        DBUG_PRINT("info",("Found log file entry"));
> -      fname[length-1]=0;			// remove last \n
> +      fname[length - 1]= 0;
>        linfo->index_file_start_offset= offset;
>        linfo->index_file_offset = my_b_tell(&index_file);
>        break;
> 
Regards / Mit vielen Grüssen,
Sergei

-- 
   __  ___     ___ ____  __
  /  |/  /_ __/ __/ __ \/ /   Sergei Golubchik <serg@stripped>
 / /|_/ / // /\ \/ /_/ / /__  Principal Software Developer
/_/  /_/\_, /___/\___\_\___/  MySQL GmbH, Dachauer Str. 37, D-80335 München
       <___/                  Geschäftsführer: Kaj Arnö - HRB
München 162140