Hi Alfranio,
After applying your patch, the following test will crash after running
several times:
source include/master-slave.inc;
source include/have_debug.inc;
source include/have_binlog_format_statement.inc;
connection master;
create table t1 (a int);
connection slave;
source include/stop_slave.inc;
set global binlog_format='row';
source include/start_slave.inc;
connection master;
send insert into t1 values (1);
connection slave;
set RUNNING_SLAVE binlog_format='statement';
connection master;
reap;
sync_slave_with_master;
Allow another session to change slave session variables is very
risky, besides crash, it can also cause data inconsistency, take
binlog_format for example, if its value is 'row' before slave SQL thread
start applying an statement query event, and then all the updates of
rows will be logged in row format, and then after all rows are updated,
some other thread changes binlog_format to 'statement', then there is a
possibility that the same statement will also be logged in statement
format. We also use session variables to guarantee that the statement
will get the same result on slave as on master, such TIMESTAMP,
INSERT_ID, etc, if we allow such session variables to be changed by
another session, then that will break all these statements.
Based on this and other comments on the bug report, I strongly suggest
not to support this feature, and just close the bug as not a bug or
won't fix.
Even if this feature is really needed, I'd suggest to open a WL for
that.
Alfranio Correia wrote:
> #At
> file:///home/acorreia/workspace.sun/repository.mysql/bzrwork/bug-40146/mysql-5.1-bugteam/
> based on revid:alexey.kopytov@stripped
>
> 2841 Alfranio Correia 2009-03-18
> BUG#40146 log_slave_updates uses MIXED mode for binlog regardless of
> binlog_format
>
> It was not possible to check or change a SQL Thread's session variable. In
> particular,
> to change binlog_format, one needed to change its global value, stop and
> restart the
> replication. This approach had two drawbacks. It required to globally change a
> value
> of a variable thus affecting not only the SQL Thread but any new thread.
> Besides, it
> was necessary to stop the replication for a few seconds to inherit the new
> value.
>
> This patch fixes this issues by introducing the following commands:
> SET RUNNING_SLAVE <variable> = <value>;
> SHOW RUNNING_SLAVE like <variable>;
>
> The variants of the commend as described in the manual are also accepted. The
> user
> issuing the command and the user on behalf of which the SQL Thread is running
> must
> have the appropriate privileges to successfully execute this command. In
> particular,
> to change the binlog_format both need SUPER privilege.
>
> @sql/lex:
> introduces a new token.
> @sql/mysql_priv.h
> introduces a new type.
> @sql/set_var.cc and @sql/set_var.h
> checks when the new type is used and instead of changing
> information on the current thread, the SQL Thread has
> its information changed.
> @sql/sql_lex.cc
> updates a comment.
> @sql/slq_show.cc
> checks when the new type is used and instead of retrieving
> information on the current thread, the SQL Thread has
> its information retrieved.
> @sql/sql_yacc.yy
> changes the grammar.
> modified:
> sql/lex.h
> sql/mysql_priv.h
> sql/set_var.cc
> sql/set_var.h
> sql/sql_lex.cc
> sql/sql_show.cc
> sql/sql_yacc.yy
>
> === modified file 'sql/lex.h'
> --- a/sql/lex.h 2008-02-12 09:43:38 +0000
> +++ b/sql/lex.h 2009-03-18 17:00:18 +0000
> @@ -478,6 +478,7 @@ static SYMBOL symbols[] = {
> { "SIGNED", SYM(SIGNED_SYM)},
> { "SIMPLE", SYM(SIMPLE_SYM)},
> { "SLAVE", SYM(SLAVE)},
> + { "RUNNING_SLAVE", SYM(SLAVE_SESSION_SYM)},
> { "SNAPSHOT", SYM(SNAPSHOT_SYM)},
> { "SMALLINT", SYM(SMALLINT)},
> { "SOCKET", SYM(SOCKET_SYM)},
>
> === modified file 'sql/mysql_priv.h'
> --- a/sql/mysql_priv.h 2009-03-13 08:51:25 +0000
> +++ b/sql/mysql_priv.h 2009-03-18 17:00:18 +0000
> @@ -741,7 +741,7 @@ class user_var_entry;
> class Security_context;
> enum enum_var_type
> {
> - OPT_DEFAULT= 0, OPT_SESSION, OPT_GLOBAL
> + OPT_DEFAULT= 0, OPT_SESSION, OPT_GLOBAL, OPT_SLAVE_SESSION
> };
> class sys_var;
> #ifdef MYSQL_SERVER
>
> === modified file 'sql/set_var.cc'
> --- a/sql/set_var.cc 2009-03-11 22:32:53 +0000
> +++ b/sql/set_var.cc 2009-03-18 17:00:18 +0000
> @@ -55,6 +55,7 @@
> #include <mysql.h>
> #include "slave.h"
> #include "rpl_mi.h"
> +#include "rpl_rli.h"
> #include <my_getopt.h>
> #include <thr_alarm.h>
> #include <myisam.h>
> @@ -1128,7 +1129,6 @@ static void fix_query_cache_size(THD *th
> Note: query_cache_size is a global variable reflecting the
> requested cache size. See also query_cache_size_arg
> */
> -
> if (query_cache_size != new_cache_size)
> push_warning_printf(current_thd, MYSQL_ERROR::WARN_LEVEL_WARN,
> ER_WARN_QC_RESIZE, ER(ER_WARN_QC_RESIZE),
> @@ -1253,38 +1253,45 @@ bool sys_var_thd_binlog_format::check(TH
> }
>
>
> -bool sys_var_thd_binlog_format::is_readonly() const
> +bool sys_var_thd_binlog_format::is_readonly(THD* thd) const
> {
> - /*
> - Under certain circumstances, the variable is read-only (unchangeable):
> - */
> - THD *thd= current_thd;
> - /*
> - If RBR and open temporary tables, their CREATE TABLE may not be in the
> - binlog, so we can't toggle to SBR in this connection.
> - The test below will also prevent SET GLOBAL, well it was not easy to test
> - if global or not here.
> - And this test will also prevent switching from RBR to RBR (a no-op which
> - should not happen too often).
> -
> - If we don't have row-based replication compiled in, the variable
> - is always read-only.
> - */
> - if ((thd->variables.binlog_format == BINLOG_FORMAT_ROW) &&
> - thd->temporary_tables)
> +#ifdef HAVE_REPLICATION
> + if (thd->slave_thread)
> {
> - my_error(ER_TEMP_TABLE_PREVENTS_SWITCH_OUT_OF_RBR, MYF(0));
> - return 1;
> + /*
> + There is no need to check for this if you want to change the SQL Thread.
> + */
> }
> - /*
> - if in a stored function/trigger, it's too late to change mode
> - */
> - if (thd->in_sub_stmt)
> + else
> +#endif
> {
> - my_error(ER_STORED_FUNCTION_PREVENTS_SWITCH_BINLOG_FORMAT, MYF(0));
> - return 1;
> + /*
> + If RBR and open temporary tables, their CREATE TABLE may not be in the
> + binlog, so we can't toggle to SBR in this connection.
> + The test below will also prevent SET GLOBAL, well it was not easy to test
> + if global or not here.
> + And this test will also prevent switching from RBR to RBR (a no-op which
> + should not happen too often).
> +
> + If we don't have row-based replication compiled in, the variable
> + is always read-only.
> + */
> + if ((thd->variables.binlog_format == BINLOG_FORMAT_ROW) &&
> + thd->temporary_tables)
> + {
> + my_error(ER_TEMP_TABLE_PREVENTS_SWITCH_OUT_OF_RBR, MYF(0));
> + return 1;
> + }
> + /*
> + if in a stored function/trigger, it's too late to change mode
> + */
> + if (thd->in_sub_stmt)
> + {
> + my_error(ER_STORED_FUNCTION_PREVENTS_SWITCH_BINLOG_FORMAT, MYF(0));
> + return 1;
> + }
> }
> - return sys_var_thd_enum::is_readonly();
> + return sys_var_thd_enum::is_readonly(thd);
> }
>
>
> @@ -3432,23 +3439,73 @@ sys_var *intern_find_sys_var(const char
> int sql_set_variables(THD *thd, List<set_var_base> *var_list)
> {
> int error;
> + THD *chg_thd= thd;
> List_iterator_fast<set_var_base> it(*var_list);
> +
> DBUG_ENTER("sql_set_variables");
>
> set_var_base *var;
> +#ifdef HAVE_REPLICATION
> + Relay_log_info *rli= NULL;
> + bool has_slave_session= FALSE;
> + while ((var= it++))
> + {
> + if (var->type == OPT_SLAVE_SESSION)
> + {
> + has_slave_session= TRUE;
> + break;
> + }
> + }
> + if (has_slave_session)
> + {
> + pthread_mutex_lock(&LOCK_active_mi);
> + if (active_mi)
> + {
> + rli= &active_mi->rli;
> + pthread_mutex_lock(&rli->run_lock);
> + if (!rli->sql_thd)
> + {
> + error= -1;
> + my_error(ER_SLAVE_NOT_RUNNING,MYF(0));
> + goto err;
> + }
> + chg_thd= rli->sql_thd;
> + }
> + else
> + {
> + error= 1;
> + has_slave_session= FALSE;
> + pthread_mutex_unlock(&LOCK_active_mi);
> + my_error(ER_SLAVE_NOT_RUNNING,MYF(0));
> + goto err;
> + }
> + }
> + it.rewind();
> +#endif
> while ((var=it++))
> {
> - if ((error= var->check(thd)))
> + /*
> + Make sure that both threads have the right permission.
> + */
> + error= var->check(chg_thd);
> + error= (error ? error: (chg_thd != thd ? var->check(thd): error));
> + if (error)
> goto err;
> }
> if (!(error= test(thd->is_error())))
> {
> it.rewind();
> while ((var= it++))
> - error|= var->update(thd); // Returns 0, -1 or 1
> + error|= var->update(chg_thd); // Returns 0, -1 or 1
> }
> -
> err:
> +#ifdef HAVE_REPLICATION
> + if (has_slave_session)
> + {
> + pthread_mutex_unlock(&rli->run_lock);
> + pthread_mutex_unlock(&LOCK_active_mi);
> + }
> +#endif
> free_underlaid_joins(thd, &thd->lex->select_lex);
> DBUG_RETURN(error);
> }
> @@ -3489,7 +3546,7 @@ bool not_all_support_one_shot(List<set_v
>
> int set_var::check(THD *thd)
> {
> - if (var->is_readonly())
> + if (var->is_readonly(thd))
> {
> my_error(ER_INCORRECT_GLOBAL_LOCAL_VAR, MYF(0), var->name, "read only");
> return -1;
>
> === modified file 'sql/set_var.h'
> --- a/sql/set_var.h 2008-12-02 11:21:05 +0000
> +++ b/sql/set_var.h 2009-03-18 17:00:18 +0000
> @@ -115,7 +115,7 @@ public:
> virtual bool check_default(enum_var_type type)
> { return option_limits == 0; }
> virtual bool is_struct() { return 0; }
> - virtual bool is_readonly() const { return 0; }
> + virtual bool is_readonly(THD*) const { return 0; }
> CHARSET_INFO *charset(THD *thd);
> virtual sys_var_pluginvar *cast_pluginvar() { return 0; }
>
> @@ -231,7 +231,7 @@ public:
> my_bool *value_arg)
> :sys_var_bool_ptr(chain, name_arg, value_arg)
> {}
> - bool is_readonly() const { return 1; }
> + bool is_readonly(THD*) const { return 1; }
> };
>
>
> @@ -297,7 +297,7 @@ public:
> return 1;
> }
> bool check_default(enum_var_type type) { return 1; }
> - bool is_readonly() const { return 1; }
> + bool is_readonly(THD*) const { return 1; }
> };
>
>
> @@ -338,7 +338,7 @@ public:
> return 1;
> }
> bool check_default(enum_var_type type) { return 1; }
> - bool is_readonly() const { return 1; }
> + bool is_readonly(THD*) const { return 1; }
> };
>
>
> @@ -387,7 +387,7 @@ public:
> bool update(THD *thd, set_var *var) { return 1; }
> SHOW_TYPE show_type() { return SHOW_CHAR; }
> bool check_update_type(Item_result type) { return 1; }
> - bool is_readonly() const { return 1; }
> + bool is_readonly(THD*) const { return 1; }
> uchar *value_ptr(THD *thd, enum_var_type type, LEX_STRING *base);
> };
>
> @@ -959,7 +959,7 @@ public:
> return (*value_ptr_func)(thd);
> }
> SHOW_TYPE show_type() { return show_type_value; }
> - bool is_readonly() const { return 1; }
> + bool is_readonly(THD*) const { return 1; }
> };
>
>
> @@ -1000,7 +1000,7 @@ public:
> return ptr;
> }
> SHOW_TYPE show_type() { return show_type_value; }
> - bool is_readonly() const { return 1; }
> + bool is_readonly(THD*) const { return 1; }
> };
>
>
> @@ -1037,7 +1037,7 @@ public:
> bool check_type(enum_var_type type) { return type != OPT_GLOBAL; }
> bool check_update_type(Item_result type) { return 1; }
> SHOW_TYPE show_type() { return SHOW_CHAR; }
> - bool is_readonly() const { return 1; }
> + bool is_readonly(THD*) const { return 1; }
> };
>
>
> @@ -1240,7 +1240,7 @@ public:
> fix_binlog_format_after_update)
> {};
> bool check(THD *thd, set_var *var);
> - bool is_readonly() const;
> + bool is_readonly(THD* thd) const;
> };
>
> /****************************************************************************
> @@ -1250,7 +1250,8 @@ public:
> class set_var_base :public Sql_alloc
> {
> public:
> - set_var_base() {}
> + enum_var_type type;
> + set_var_base(enum_var_type type_arg= OPT_DEFAULT): type(type_arg) {}
> virtual ~set_var_base() {}
> virtual int check(THD *thd)=0; /* To check privileges etc. */
> virtual int update(THD *thd)=0; /* To set the value */
> @@ -1267,7 +1268,6 @@ class set_var :public set_var_base
> public:
> sys_var *var;
> Item *value;
> - enum_var_type type;
> union
> {
> CHARSET_INFO *charset;
> @@ -1282,7 +1282,7 @@ public:
>
> set_var(enum_var_type type_arg, sys_var *var_arg,
> const LEX_STRING *base_name_arg, Item *value_arg)
> - :var(var_arg), type(type_arg), base(*base_name_arg)
> + :set_var_base(type_arg), var(var_arg), base(*base_name_arg)
> {
> /*
> If the set value is a field, change it to a string to allow things like
>
> === modified file 'sql/sql_lex.cc'
> --- a/sql/sql_lex.cc 2009-03-05 14:22:33 +0000
> +++ b/sql/sql_lex.cc 2009-03-18 17:00:18 +0000
> @@ -1385,7 +1385,7 @@ int MYSQLlex(void *arg, void *yythd)
> /*
> We come here when we have found two '@' in a row.
> We should now be able to handle:
> - [(global | local | session) .]variable_name
> + [(global | local | session | slave_session ) .]variable_name
> */
>
> for (result_state= 0; ident_map[c= lip->yyGet()]; result_state|= c);
>
> === modified file 'sql/sql_show.cc'
> --- a/sql/sql_show.cc 2009-03-05 11:15:47 +0000
> +++ b/sql/sql_show.cc 2009-03-18 17:00:18 +0000
> @@ -19,6 +19,11 @@
> #include "mysql_priv.h"
> #include "sql_select.h" // For select_describe
> #include "sql_show.h"
> +#ifdef HAVE_REPLICATION
> +#include "slave.h"
> +#include "rpl_mi.h"
> +#include "rpl_rli.h"
> +#endif
> #include "repl_failsafe.h"
> #include "sp.h"
> #include "sp_head.h"
> @@ -2098,7 +2103,7 @@ inline void make_upper(char *buf)
> *buf= my_toupper(system_charset_info, *buf);
> }
>
> -static bool show_status_array(THD *thd, const char *wild,
> +static bool show_status_array(THD *thd, THD *chg_thd, const char *wild,
> SHOW_VAR *variables,
> enum enum_var_type value_type,
> struct system_status_var *status_var,
> @@ -2145,12 +2150,12 @@ static bool show_status_array(THD *thd,
> Repeat as necessary, if new var is again SHOW_FUNC
> */
> for (var=variables; var->type == SHOW_FUNC; var= &tmp)
> - ((mysql_show_var_func)(var->value))(thd, &tmp, buff);
> + ((mysql_show_var_func)(var->value))(chg_thd, &tmp, buff);
>
> SHOW_TYPE show_type=var->type;
> if (show_type == SHOW_ARRAY)
> {
> - show_status_array(thd, wild, (SHOW_VAR *) var->value, value_type,
> + show_status_array(thd, chg_thd, wild, (SHOW_VAR *) var->value, value_type,
> status_var, name_buffer, table, ucase_names, partial_cond);
> }
> else
> @@ -2168,8 +2173,8 @@ static bool show_status_array(THD *thd,
> {
> sys_var *var= ((sys_var *) value);
> show_type= var->show_type();
> - value= (char*) var->value_ptr(thd, value_type, &null_lex_str);
> - charset= var->charset(thd);
> + value= (char*) var->value_ptr(chg_thd, value_type, &null_lex_str);
> + charset= var->charset(chg_thd);
> }
>
> pos= end= buff;
> @@ -5319,6 +5324,7 @@ int fill_variables(THD *thd, TABLE_LIST
> {
> DBUG_ENTER("fill_variables");
> int res= 0;
> + THD* chg_thd= thd;
> LEX *lex= thd->lex;
> const char *wild= lex->wild ? lex->wild->ptr() : NullS;
> enum enum_schema_tables schema_table_idx=
> @@ -5331,10 +5337,47 @@ int fill_variables(THD *thd, TABLE_LIST
> schema_table_idx == SCH_GLOBAL_VARIABLES)
> option_type= OPT_GLOBAL;
>
> +#ifdef HAVE_REPLICATION
> + Relay_log_info *rli= NULL;
> + bool has_slave_session= FALSE;
> + if (lex->option_type == OPT_SLAVE_SESSION && !thd->slave_thread)
> + {
> + pthread_mutex_lock(&LOCK_active_mi);
> + if (active_mi)
> + {
> + rli= &active_mi->rli;
> + pthread_mutex_lock(&rli->run_lock);
> + has_slave_session= TRUE;
> + if (!rli->sql_thd)
> + {
> + res= 1;
> + my_error(ER_SLAVE_NOT_RUNNING,MYF(0));
> + goto err;
> + }
> + chg_thd= rli->sql_thd;
> + }
> + else
> + {
> + res= 1;
> + has_slave_session= FALSE;
> + pthread_mutex_unlock(&LOCK_active_mi);
> + my_error(ER_SLAVE_NOT_RUNNING,MYF(0));
> + goto err;
> + }
> + }
> +#endif
> rw_rdlock(&LOCK_system_variables_hash);
> - res= show_status_array(thd, wild, enumerate_sys_vars(thd, sorted_vars),
> + res= show_status_array(thd, chg_thd, wild, enumerate_sys_vars(chg_thd,
> sorted_vars),
> option_type, NULL, "", tables->table, upper_case_names,
> cond);
> rw_unlock(&LOCK_system_variables_hash);
> +err:
> +#ifdef HAVE_REPLICATION
> + if (has_slave_session)
> + {
> + pthread_mutex_unlock(&rli->run_lock);
> + pthread_mutex_unlock(&LOCK_active_mi);
> + }
> +#endif
> DBUG_RETURN(res);
> }
>
> @@ -5373,7 +5416,7 @@ int fill_status(THD *thd, TABLE_LIST *ta
> pthread_mutex_lock(&LOCK_status);
> if (option_type == OPT_GLOBAL)
> calc_sum_of_all_status(&tmp);
> - res= show_status_array(thd, wild,
> + res= show_status_array(thd, thd, wild,
> (SHOW_VAR *)all_status_vars.buffer,
> option_type, tmp1, "", tables->table,
> upper_case_names, cond);
>
> === modified file 'sql/sql_yacc.yy'
> --- a/sql/sql_yacc.yy 2009-03-05 14:22:33 +0000
> +++ b/sql/sql_yacc.yy 2009-03-18 17:00:18 +0000
> @@ -510,10 +510,10 @@ bool my_yyoverflow(short **a, YYSTYPE **
>
> %pure_parser /* We have threads */
> /*
> - Currently there are 169 shift/reduce conflicts.
> + Currently there are 172 shift/reduce conflicts.
> We should not introduce new conflicts any more.
> */
> -%expect 169
> +%expect 172
>
> /*
> Comments for TOKENS.
> @@ -979,6 +979,7 @@ bool my_yyoverflow(short **a, YYSTYPE **
> %token SIGNED_SYM
> %token SIMPLE_SYM /* SQL-2003-N */
> %token SLAVE
> +%token SLAVE_SESSION_SYM
> %token SMALLINT /* SQL-2003-R */
> %token SNAPSHOT_SYM
> %token SOCKET_SYM
> @@ -11594,6 +11595,7 @@ keyword_sp:
> | SIMPLE_SYM {}
> | SHARE_SYM {}
> | SHUTDOWN {}
> + | SLAVE_SESSION_SYM {}
> | SNAPSHOT_SYM {}
> | SOUNDS_SYM {}
> | SOURCE_SYM {}
> @@ -11762,6 +11764,7 @@ option_type:
> | GLOBAL_SYM { $$=OPT_GLOBAL; }
> | LOCAL_SYM { $$=OPT_SESSION; }
> | SESSION_SYM { $$=OPT_SESSION; }
> + | SLAVE_SESSION_SYM { $$=OPT_SLAVE_SESSION; }
> ;
>
> option_type2:
> @@ -11774,6 +11777,7 @@ opt_var_type:
> | GLOBAL_SYM { $$=OPT_GLOBAL; }
> | LOCAL_SYM { $$=OPT_SESSION; }
> | SESSION_SYM { $$=OPT_SESSION; }
> + | SLAVE_SESSION_SYM { $$=OPT_SLAVE_SESSION; }
> ;
>
> opt_var_ident_type:
> @@ -11781,6 +11785,7 @@ opt_var_ident_type:
> | GLOBAL_SYM '.' { $$=OPT_GLOBAL; }
> | LOCAL_SYM '.' { $$=OPT_SESSION; }
> | SESSION_SYM '.' { $$=OPT_SESSION; }
> + | SLAVE_SESSION_SYM { $$=OPT_SLAVE_SESSION; }
> ;
>
> ext_option_value:
>
>
