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(
>
>
>
>
>
