List:Commits« Previous MessageNext Message »
From:Alfranio Correia Date:October 5 2010 11:44am
Subject:Re: bzr commit into mysql-5.5 branch (mats.kindahl:3087) WL#5465
View as plain text  
Hi Mats,

Please, update the following result files before pushing the patch:

. sys_vars.all_vars
. rpl.rpl_flushlog_loop
. main.mysqld--help-win
. main.mysqld--help-notwin
. rpl.rpl_variables

Please, update the following test cases and result files before pushing the patch:

. main.variables-notembedded
. main.flush2
. sys_vars.relay_log_index_basic


The following tests are sporadically failing. Although, I think the failures are not
related to your patch, please, check if they are happening in PB and there are
appropriated
bug reports.


. main.single_delete_update
. rpl.rpl_*_mixing_engines
. rpl.rpl_non_direct_*_mixing_engines

Cheers.


On 10/05/2010 09:52 AM, Alfranio Correia wrote:
> Hi Mats,
>
> Patch approved.
> See minor comments in-line.
>
> Cheers.
>
> On 09/27/2010 06:30 PM, Mats Kindahl wrote:
>> #At file:///home/bzr/mkindahl/w5465-5.5/ based on
>> revid:joerg@stripped
>>
>> 3087 Mats Kindahl 2010-09-27
>> WL#5465: System variables: paths to relay log and binary log files
>>
>> Adding new variables with paths to where the binary log files and
>> the relay log files are located. Three new global read-only variables
>> are added and one has changed contents.
>>
>> log_bin_basename
>> This variable hold the full path and name used for the binary logs:
>> not including the extension of the files.
>>
>> log_bin_index
>> This variable contain the full path to the binary log index file.
>>
>> relay_log_basename
>> This variable hold the full path and name used for the relay logs:
>> not including the extension of the files.
>>
>> relay_log_index
>> This variable now contain the full path to the relay log index file.
>> Previously, it just contained the value supplied to mysqld option
>> relay-log-index, which was then used to compute the path to the
>> index file.
>> @ libmysqld/CMakeLists.txt
>> Adding file rpl_init.cc.
>> @ mysql-test/suite/rpl/t/rpl_variables.test
>> Extending test to print value of replication file variables.
>> @ sql/CMakeLists.txt
>> Adding new file rpl_init.cc.
>> @ sql/mysqld.cc
>> Exporting opt_binlog_index_name.
>> @ sql/rpl_init.cc
>> Initial revision.
>> @ sql/rpl_init.h
>> Initial revision.
>> @ sql/slave.h
>> Exporting variable storing the value of log-bin-index.
>> @ sql/sys_vars.cc
>> Adding variables relay_log_basename, relay_log_index, log_bin_basename,
>> and log_bin_index as non-option variables.
>>
>> added:
>> sql/rpl_init.cc
>> sql/rpl_init.h
>> modified:
>> libmysqld/CMakeLists.txt
>> mysql-test/suite/rpl/r/rpl_variables.result
>> mysql-test/suite/rpl/t/rpl_variables.test
>> sql/CMakeLists.txt
>> sql/mysqld.cc
>> sql/slave.h
>> sql/sys_vars.cc
>> === modified file 'libmysqld/CMakeLists.txt'
>> --- a/libmysqld/CMakeLists.txt 2010-08-18 11:29:04 +0000
>> +++ b/libmysqld/CMakeLists.txt 2010-09-27 17:30:05 +0000
>> @@ -83,7 +83,7 @@ SET(SQL_EMBEDDED_SOURCES emb_qcache.cc l
>> ../sql/sql_alter.cc ../sql/sql_partition_admin.cc
>> ../sql/event_parse_data.cc
>> ../sql/sql_signal.cc ../sql/rpl_handler.cc
>> - ../sql/rpl_utility.cc
>> + ../sql/rpl_utility.cc ../sql/rpl_init.cc
>> ../sql/sys_vars.cc
>> ${CMAKE_BINARY_DIR}/sql/sql_builtin.cc
>> ../sql/mdl.cc ../sql/transaction.cc
>>
>> === modified file 'mysql-test/suite/rpl/r/rpl_variables.result'
>> --- a/mysql-test/suite/rpl/r/rpl_variables.result 2008-08-14 09:38:22
>> +0000
>> +++ b/mysql-test/suite/rpl/r/rpl_variables.result 2010-09-27 17:30:05
>> +0000
>> @@ -47,6 +47,19 @@ include/start_slave.inc
>> [on slave]
>> SET @@global.init_slave = 'SELECT 1';
>> [on master]
>> +SELECT @@pid_file, @@datadir;
>> +@@pid_file MYSQL_TEST_DIR/var/run/mysqld.1.pid
>> +@@datadir MYSQL_TEST_DIR/var/mysqld.1/data/
>> +**** Relay log variables
>> +SELECT @@relay_log, @@relay_log_index, @@relay_log_basename;
>> +@@relay_log NULL
>> +@@relay_log_index
>> MYSQL_TEST_DIR/var/mysqld.1/data/mysqld-relay-bin.index
>> +@@relay_log_basename MYSQL_TEST_DIR/var/mysqld.1/data/mysqld-relay-bin
>> +**** Binary log variables
>> +SELECT @@log_bin, @@log_bin_index, @@log_bin_basename;
>> +@@log_bin 1
>> +@@log_bin_index MYSQL_TEST_DIR/var/mysqld.1/data/master-bin-bin.index
>> +@@log_bin_basename MYSQL_TEST_DIR/var/mysqld.1/data/master-bin-bin
>> CREATE TABLE tstmt (id INT AUTO_INCREMENT PRIMARY KEY,
>> truth BOOLEAN,
>> num INT,
>>
>> === modified file 'mysql-test/suite/rpl/t/rpl_variables.test'
>> --- a/mysql-test/suite/rpl/t/rpl_variables.test 2008-07-17 16:26:59 +0000
>> +++ b/mysql-test/suite/rpl/t/rpl_variables.test 2010-09-27 17:30:05 +0000
>> @@ -122,6 +122,15 @@ SET @@global.init_slave = 'SELECT 1';
>>
>> --echo [on master]
>> connection master;
>> +# checking values of read-only variables
>> +--replace_result $MYSQL_TEST_DIR MYSQL_TEST_DIR
>> +query_vertical SELECT @@pid_file, @@datadir;
>> +--echo **** Relay log variables
>> +--replace_result $MYSQL_TEST_DIR MYSQL_TEST_DIR
>> +query_vertical SELECT @@relay_log, @@relay_log_index,
>> @@relay_log_basename;
>> +--echo **** Binary log variables
>> +--replace_result $MYSQL_TEST_DIR MYSQL_TEST_DIR
>> +query_vertical SELECT @@log_bin, @@log_bin_index, @@log_bin_basename;
>>
>> # Tables where everything happens.
>> CREATE TABLE tstmt (id INT AUTO_INCREMENT PRIMARY KEY,
>>
>> === modified file 'sql/CMakeLists.txt'
>> --- a/sql/CMakeLists.txt 2010-08-31 11:06:56 +0000
>> +++ b/sql/CMakeLists.txt 2010-09-27 17:30:05 +0000
>> @@ -69,7 +69,8 @@ SET (SQL_SOURCE
>> event_queue.cc event_db_repository.cc
>> sql_tablespace.cc events.cc ../sql-common/my_user.c
>> partition_info.cc rpl_utility.cc rpl_injector.cc sql_locale.cc
>> - rpl_rli.cc rpl_mi.cc sql_servers.cc sql_audit.cc
>> + rpl_init.cc rpl_rli.cc rpl_mi.cc
>> + sql_servers.cc sql_audit.cc
>> sql_connect.cc scheduler.cc sql_partition_admin.cc
>> sql_profile.cc event_parse_data.cc sql_alter.cc
>> sql_signal.cc rpl_handler.cc mdl.cc sql_admin.cc
>>
>> === modified file 'sql/mysqld.cc'
>> --- a/sql/mysqld.cc 2010-08-30 14:07:40 +0000
>> +++ b/sql/mysqld.cc 2010-09-27 17:30:05 +0000
>> @@ -67,6 +67,7 @@
>> #include "scheduler.h"
>> #include "debug_sync.h"
>> #include "sql_callback.h"
>> +#include "rpl_init.h"
>>
>> #ifdef WITH_PERFSCHEMA_STORAGE_ENGINE
>> #include "../storage/perfschema/pfs_server.h"
>> @@ -656,7 +657,8 @@ static my_bool opt_do_pstack;
>> static my_bool opt_bootstrap, opt_myisam_log;
>> static int cleanup_done;
>> static ulong opt_specialflag;
>> -static char *opt_update_logname, *opt_binlog_index_name;
>> +static char *opt_update_logname;
>> +char *opt_binlog_index_name;
>> char *mysql_home_ptr, *pidfile_name_ptr;
>> /** Initial command line arguments (count), after load_defaults().*/
>> static int defaults_argc;
>> @@ -3080,6 +3082,31 @@ static inline char *make_default_log_nam
>> return make_log_name(buff, default_logfile_name, log_ext);
>> }
>>
>> +/**
>> + Create a replication file name or base for file names.
>> +
>> + Only the basename of this string will be used.
>> +
>> + @param[in] opt Value of option, or NULL
>> + @param[in] def Default value if option value is not set.
>> + @param[in] ext Extension to use for the path
>> + */
>> +static inline const char *
>> +make_replication_log_name(const char *opt,
>> + const char *def,
>> + const char *ext)
>> +{
>> + DBUG_ENTER("make_replication_log_name");
>> + DBUG_PRINT("enter", ("opt: %s, def: %s, ext: %s", opt, def, ext));
>> + char buff[FN_REFLEN];
>> + const char *base= opt ? opt : def;
>> + DBUG_PRINT("debug", ("mysql_real_data_home_ptr: %s",
>> mysql_real_data_home_ptr));
>> + fn_format(buff, base, mysql_real_data_home_ptr, ext,
>> + MY_REPLACE_DIR | MY_REPLACE_EXT | MY_UNPACK_FILENAME |
>> MY_RETURN_REAL_PATH);
>
> I think you should check if the function fails and also use MY_SAFE_PATH.
> See mysys/mf_format.cc:
>
> /* To long path, return original or NULL */
> size_t tmp_length;
> if (flag & MY_SAFE_PATH)
> DBUG_RETURN(NullS);
>
>> + DBUG_RETURN(strdup(buff));
>
> It is unlikely but strdup may also fail.
>
>> +}
>> +
>> +
>> static int init_common_variables()
>> {
>> char buff[FN_REFLEN];
>> @@ -3148,6 +3175,11 @@ static int init_common_variables()
>> strmov(fn_ext(pidfile_name),".pid"); // Add proper extension
>
>>
>> /*
>> + Create the default names for binary and relay log files.
>> + */
>
> I Think this comment is in the wrong place.
>
>> +
>> +
>> + /*
>> The default-storage-engine entry in my_long_options should have a
>> non-null default value. It was earlier intialized as
>> (longlong)"MyISAM" in my_long_options but this triggered a
>> @@ -3433,7 +3465,7 @@ static int init_common_variables()
>> make_default_log_name(buff, ".log"));
>> FIX_LOG_VAR(opt_slow_logname,
>> make_default_log_name(buff, "-slow.log"));
>> -
>> +
>> #if defined(ENABLED_DEBUG_SYNC)
>> /* Initialize the debug sync facility. See debug_sync.cc. */
>> if (debug_sync_init())
>> @@ -3782,6 +3814,12 @@ static int init_server_components()
>> unireg_abort(1);
>> }
>>
>> + /*
>> + Replication Setup.
>> +
>> + TODO: Most of this code could probably be moved to rpl_init.cc.
>> + */
>> +
>> /* need to configure logging before initializing storage engines */
>> if (opt_log_slave_updates&& !opt_bin_log)
>> {
>> @@ -3854,6 +3892,7 @@ a file name for --log-bin-index option",
>> "master and has his hostname changed!! Please "
>> "use '--log-bin=%s' to avoid this problem.", ln);
>> }
>> + DBUG_PRINT("debug", ("mysql_bin_log.generate_name(): %s", ln));
>> if (ln == buf)
>> {
>> my_free(opt_bin_logname);
>> @@ -3865,6 +3904,11 @@ a file name for --log-bin-index option",
>> }
>> }
>>
>> + g_log_bin_basename= make_replication_log_name(opt_bin_logname,
>> pidfile_name, "-bin");
>> + g_relay_log_basename= make_replication_log_name(opt_relay_logname,
>> pidfile_name, "-relay-bin");
>> + g_log_bin_index= make_replication_log_name(opt_binlog_index_name,
>> g_log_bin_basename, ".index");
>> + g_relay_log_index=
>> make_replication_log_name(opt_relaylog_index_name,
>> g_relay_log_basename, ".index");
>> +
>
> Please, consider the case that these calls may fail. Maybe:
>
> if (!g_log_bin_basename || !g_relay_log_basename || !g_log_bin_index ||
> !g_relay_log_index)
> exit(1);
>
>> /* call ha_init_key_cache() on all key caches to init them */
>> process_key_caches(&ha_init_key_cache);
>>
>> @@ -5778,9 +5822,13 @@ struct my_option my_long_options[]=
>> &opt_bin_logname,&opt_bin_logname, 0, GET_STR_ALLOC,
>> OPT_ARG, 0, 0, 0, 0, 0, 0},
>> {"log-bin-index", 0,
>> - "File that holds the names for last binary log files.",
>> + "File that holds the names for binary log files.",
>> &opt_binlog_index_name,&opt_binlog_index_name, 0, GET_STR,
>> REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
>> + {"relay-log-index", 0,
>> + "File that holds the names for relay log files.",
>> +&opt_relaylog_index_name,&opt_relaylog_index_name, 0, GET_STR,
>> + REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
>> {"log-isam", OPT_ISAM_LOG, "Log all MyISAM changes to file.",
>> &myisam_log_filename,&myisam_log_filename, 0, GET_STR,
>> OPT_ARG, 0, 0, 0, 0, 0, 0},
>>
>> === added file 'sql/rpl_init.cc'
>> --- a/sql/rpl_init.cc 1970-01-01 00:00:00 +0000
>> +++ b/sql/rpl_init.cc 2010-09-27 17:30:05 +0000
>> @@ -0,0 +1,7 @@
>> +#include "my_global.h"
>> +#include "rpl_init.h"
>> +
>> +const char *g_log_bin_index= 0;
>> +const char *g_relay_log_index= 0;
>> +const char *g_relay_log_basename= 0;
>> +const char *g_log_bin_basename= 0;
>>
>> === added file 'sql/rpl_init.h'
>> --- a/sql/rpl_init.h 1970-01-01 00:00:00 +0000
>> +++ b/sql/rpl_init.h 2010-09-27 17:30:05 +0000
>> @@ -0,0 +1,9 @@
>> +#ifndef RPL_INIT_INCLUDED
>> +#define RPL_INIT_INCLUDED
>> +
>> +extern const char *g_log_bin_index;
>> +extern const char *g_relay_log_index;
>> +extern const char *g_relay_log_basename;
>> +extern const char *g_log_bin_basename;
>> +
>> +#endif /* RPL_INIT_INCLUDED */
>>
>> === modified file 'sql/slave.h'
>> --- a/sql/slave.h 2010-03-31 14:05:33 +0000
>> +++ b/sql/slave.h 2010-09-27 17:30:05 +0000
>> @@ -113,6 +113,7 @@ extern bool use_slave_mask;
>> extern char *slave_load_tmpdir;
>> extern char *master_info_file, *relay_log_info_file;
>> extern char *opt_relay_logname, *opt_relaylog_index_name;
>> +extern char *opt_binlog_index_name;
>> extern my_bool opt_skip_slave_start, opt_reckless_slave;
>> extern my_bool opt_log_slave_updates;
>> extern char *opt_slave_skip_errors;
>>
>> === modified file 'sql/sys_vars.cc'
>> --- a/sql/sys_vars.cc 2010-08-30 14:07:40 +0000
>> +++ b/sql/sys_vars.cc 2010-09-27 17:30:05 +0000
>> @@ -38,6 +38,7 @@
>> #include "rpl_mi.h"
>> #include "transaction.h"
>> #include "mysqld.h"
>> +#include "rpl_init.h"
>> #include "lock.h"
>> #include "sql_time.h" // known_date_time_formats
>> #include "sql_acl.h" // SUPER_ACL,
>> @@ -2897,10 +2898,33 @@ static Sys_var_charptr Sys_relay_log(
>> READ_ONLY GLOBAL_VAR(opt_relay_logname), CMD_LINE(REQUIRED_ARG),
>> IN_FS_CHARSET, DEFAULT(0));
>>
>> +// Uses NO_CMD_LINE since the --relay-log-index option set
>> +// opt_relaylog_index_name variable and computes a value for the
>> +// g_relay_log_index variable.
>> static Sys_var_charptr Sys_relay_log_index(
>> "relay_log_index", "The location and name to use for the file "
>> "that keeps a list of the last relay logs",
>> - READ_ONLY GLOBAL_VAR(opt_relaylog_index_name), CMD_LINE(REQUIRED_ARG),
>> + READ_ONLY GLOBAL_VAR(g_relay_log_index), NO_CMD_LINE,
>> + IN_FS_CHARSET, DEFAULT(0));
>> +
>
> Please, use /* */.
>
>> +// Uses NO_CMD_LINE since the --log-bin-index option set
>> +// opt_binlog_index_name variable and computes a value for the
>> +// g_log_bin_index variable.
>> +static Sys_var_charptr Sys_binlog_index(
>> + "log_bin_index", "File that holds the names for last binary log
>> files.",
>> + READ_ONLY GLOBAL_VAR(g_log_bin_index), NO_CMD_LINE,
>> + IN_FS_CHARSET, DEFAULT(0));
>
> Please, use /* */.
>
>> +
>> +static Sys_var_charptr Sys_relay_log_basename(
>> + "relay_log_basename",
>> + "The full path of the relay log file names, excluding the extension.",
>> + READ_ONLY GLOBAL_VAR(g_relay_log_basename), NO_CMD_LINE,
>> + IN_FS_CHARSET, DEFAULT(0));
>> +
>> +static Sys_var_charptr Sys_log_bin_basename(
>> + "log_bin_basename",
>> + "The full path of the binary log file names, excluding the extension.",
>> + READ_ONLY GLOBAL_VAR(g_log_bin_basename), NO_CMD_LINE,
>> IN_FS_CHARSET, DEFAULT(0));
>>
>> static Sys_var_charptr Sys_relay_log_info_file(
>>
>>
>>
>>
>>
>

Thread
bzr commit into mysql-5.5 branch (mats.kindahl:3087) WL#5465Mats Kindahl27 Sep
Re: bzr commit into mysql-5.5 branch (mats.kindahl:3087) WL#5465Alfranio Correia5 Oct
  • Re: bzr commit into mysql-5.5 branch (mats.kindahl:3087) WL#5465Alfranio Correia5 Oct
Re: bzr commit into mysql-5.5 branch (mats.kindahl:3087) WL#5465Sven Sandberg13 Oct