List:Commits« Previous MessageNext Message »
From:Sven Sandberg Date:September 6 2010 12:03pm
Subject:Re: bzr commit into mysql-trunk-bugfixing branch (alfranio.correia:3257)
Bug#55377
View as plain text  
Hi Alfranio,

Patch is getting better, but please also fix this:
  (1) Use an appropriate error code (not heartbeat out of range because 
this is not heartbeat-related)
  (2) Save the error message in sql/share/errmsg-utf8.txt (not 
hard-coded message because it won't get translated)

/Sven


Alfranio Correia wrote:
> #At
> file:///home/acorreia/workspace.sun/repository.mysql.new/bzrwork/bug-55377/mysql-trunk-bugfixing/
> based on revid:marc.alff@stripped
> 
>  3257 Alfranio Correia	2010-09-03
>       BUG#55377 max_binlog_cache_size does not work as specified
>       
>       The binary log is created with a maximum size defined by
>       max (max_binlog_cache_size, binlog_cache_size) and not by
>       the max_binlog_cache_size as one expects. 
>       
>       To fix this behavior, we issue a warning message when the
>       binlog_cache_size is greater than the max_binlog_cache_size
>       and set binlog_cache_size to max_binlog_cache_size.
>      @ mysql-test/extra/rpl_tests/rpl_binlog_max_cache_size.test
>         Suppressed the warning message that is issued when the binlog_cache_size
>         is greater than the max_binlog_cache_size. Besides, the binlog_cache_size's
>         value is restored as it is changed when the binary log is openned.
>      @ mysql-test/suite/rpl/r/rpl_mixed_binlog_max_cache_size.result
>         Updated the result file.
>      @ mysql-test/suite/rpl/r/rpl_row_binlog_max_cache_size.result
>         Updated the result file.
>      @ mysql-test/suite/rpl/r/rpl_stm_binlog_max_cache_size.result
>         Updated the result file.
>      @ mysql-test/suite/rpl/t/rpl_mixed_binlog_max_cache_size-master.opt
>         Set the binlog_cache_size's value to be greater than the
> max_binlog_cache_size.
>      @ mysql-test/suite/rpl/t/rpl_row_binlog_max_cache_size-master.opt
>         Set the binlog_cache_size's value to be greater than the
> max_binlog_cache_size.
>      @ mysql-test/suite/rpl/t/rpl_stm_binlog_max_cache_size-master.opt
>         Set the binlog_cache_size's value to be greater than the
> max_binlog_cache_size.
>      @ sql/binlog.cc
>         Added the body of function to check if binlog_cache_size <=
> max_binlog_cache_size.
>      @ sql/binlog.h
>         Added the definition of function to check if binlog_cache_size <=
> max_binlog_cache_size.
>      @ sql/mysqld.cc
>         Checked if binlog_cache_size <= max_binlog_cache_size during startup.
>      @ sql/sys_vars.cc
>         Checked if binlog_cache_size <= max_binlog_cache_size after calling
> "SET".
> 
>     modified:
>       mysql-test/extra/rpl_tests/rpl_binlog_max_cache_size.test
>       mysql-test/suite/rpl/r/rpl_mixed_binlog_max_cache_size.result
>       mysql-test/suite/rpl/r/rpl_row_binlog_max_cache_size.result
>       mysql-test/suite/rpl/r/rpl_stm_binlog_max_cache_size.result
>       mysql-test/suite/rpl/t/rpl_mixed_binlog_max_cache_size-master.opt
>       mysql-test/suite/rpl/t/rpl_row_binlog_max_cache_size-master.opt
>       mysql-test/suite/rpl/t/rpl_stm_binlog_max_cache_size-master.opt
>       sql/binlog.cc
>       sql/binlog.h
>       sql/mysqld.cc
>       sql/sys_vars.cc
> === modified file 'mysql-test/extra/rpl_tests/rpl_binlog_max_cache_size.test'
> --- a/mysql-test/extra/rpl_tests/rpl_binlog_max_cache_size.test	2010-06-30 20:56:21
> +0000
> +++ b/mysql-test/extra/rpl_tests/rpl_binlog_max_cache_size.test	2010-09-03 17:12:32
> +0000
> @@ -1,11 +1,13 @@
> 
> ########################################################################################
> -#    This test verifies if the binlog is not corrupted when the cache buffer is not
> -#    big enough to accommodate the changes and is divided in five steps:
> +#    This test does two things. First it verifies if the binlog is not corrupted
> when
> +#    the cache buffer is not big enough to accommodate the changes and is divided
> in
> +#    five steps:
>  #
>  #    1 - Single Statements:
> -#    1.1 - Single statement on transactional table.
> -#    1.2 - Single statement on non-transactional table. 
> -#    1.3 - Single statement on both transactional and non-transactional tables.
> +#      1.1 - Single statement on transactional table.
> +#      1.2 - Single statement on non-transactional table. 
> +#      1.3 - Single statement on both transactional and non-transactional tables.
> +#
>  #    In both 1.2 and 1.3, an incident event is logged to notify the user that the
>  #    master and slave are diverging.
>  #
> @@ -19,10 +21,14 @@
>  #    table. In this case, a failure means that the statement does not get into
>  #    the cache and an incident event is logged to notify the user that the master
>  #    and slave are diverging.
> -#    
> +#
> +#    Then it checks what happens when we try to set BINLOG_CACHE_SIZE to a value
> that
> +#    is greater than MAX_BINLOG_CACHE_SIZE and vice-versa. In both cases, We expect
> +#    that BINLOG_CACHE_SIZE will be set to MAX_BINLOG_CACHE_SIZE. (BUG#55377)
> 
> ########################################################################################
>  
>  call mtr.add_suppression("Unsafe statement written to the binary log using statement
> format since BINLOG_FORMAT = STATEMENT");
> +call mtr.add_suppression("Option binlog_cache_size ");
>  
>  CREATE TABLE t1(a INT PRIMARY KEY, data VARCHAR(30000)) ENGINE=Innodb;
>  CREATE TABLE t2(a INT PRIMARY KEY, data VARCHAR(30000)) ENGINE=MyIsam;
> @@ -374,10 +380,29 @@ let $diff_statement= SELECT * FROM t1;
>  --source include/diff_master_slave.inc
>  
>  --echo
> ########################################################################################
> +--echo #                       CHECK MAX_BINLOG_CACHE_SIZE and BINLOG_CACHE_SIZE
> +--echo
> ########################################################################################
> +#
> +# Checking what happens when we try to set BINLOG_CACHE_SIZE to a value that is
> +# greater than MAX_BINLOG_CACHE_SIZE and vice-versa. In both cases, We expect
> +# that BINLOG_CACHE_SIZE will be set to MAX_BINLOG_CACHE_SIZE. (BUG#55377)
> +# 
> +connection master;
> +
> +SET @@GLOBAL.MAX_BINLOG_CACHE_SIZE= 8192;
> +SHOW VARIABLES LIKE 'MAX_BINLOG_CACHE_SIZE';
> +SHOW VARIABLES LIKE 'BINLOG_CACHE_SIZE';
> +SET @@GLOBAL.BINLOG_CACHE_SIZE= 16384;
> +SHOW VARIABLES LIKE 'MAX_BINLOG_CACHE_SIZE';
> +SHOW VARIABLES LIKE 'BINLOG_CACHE_SIZE';
> +SET @@GLOBAL.MAX_BINLOG_CACHE_SIZE= 4096;
> +SHOW VARIABLES LIKE 'MAX_BINLOG_CACHE_SIZE';
> +SHOW VARIABLES LIKE 'BINLOG_CACHE_SIZE';
> +
> +--echo
> ########################################################################################
>  --echo #                                        CLEAN
>  --echo
> ########################################################################################
>  
> -connection master;
>  DROP TABLE t1;
>  DROP TABLE t2;
>  DROP TABLE t3;
> 
> === modified file 'mysql-test/suite/rpl/r/rpl_mixed_binlog_max_cache_size.result'
> --- a/mysql-test/suite/rpl/r/rpl_mixed_binlog_max_cache_size.result	2010-06-30
> 15:25:13 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_mixed_binlog_max_cache_size.result	2010-09-03
> 17:12:32 +0000
> @@ -5,6 +5,7 @@ reset slave;
>  drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
>  start slave;
>  call mtr.add_suppression("Unsafe statement written to the binary log using statement
> format since BINLOG_FORMAT = STATEMENT");
> +call mtr.add_suppression("Option binlog_cache_size ");
>  CREATE TABLE t1(a INT PRIMARY KEY, data VARCHAR(30000)) ENGINE=Innodb;
>  CREATE TABLE t2(a INT PRIMARY KEY, data VARCHAR(30000)) ENGINE=MyIsam;
>  CREATE TABLE t3(a INT PRIMARY KEY, data VARCHAR(30000)) ENGINE=Innodb;
> @@ -120,6 +121,34 @@ Got one of the listed errors
>  COMMIT;
>  source include/diff_master_slave.inc;
> 
> ########################################################################################
> +#                       CHECK MAX_BINLOG_CACHE_SIZE and BINLOG_CACHE_SIZE
>
> +########################################################################################
> +SET @@GLOBAL.MAX_BINLOG_CACHE_SIZE= 8192;
> +SHOW VARIABLES LIKE 'MAX_BINLOG_CACHE_SIZE';
> +Variable_name	Value
> +max_binlog_cache_size	8192
> +SHOW VARIABLES LIKE 'BINLOG_CACHE_SIZE';
> +Variable_name	Value
> +binlog_cache_size	4096
> +SET @@GLOBAL.BINLOG_CACHE_SIZE= 16384;
> +Warnings:
> +Warning	1624	Option binlog_cache_size (16384) is greater than max_binlog_cache_size
> (8192). So setting binlog_cache_size equal to max_binlog_cache_size.
> +SHOW VARIABLES LIKE 'MAX_BINLOG_CACHE_SIZE';
> +Variable_name	Value
> +max_binlog_cache_size	8192
> +SHOW VARIABLES LIKE 'BINLOG_CACHE_SIZE';
> +Variable_name	Value
> +binlog_cache_size	8192
> +SET @@GLOBAL.MAX_BINLOG_CACHE_SIZE= 4096;
> +Warnings:
> +Warning	1624	Option binlog_cache_size (8192) is greater than max_binlog_cache_size
> (4096). So setting binlog_cache_size equal to max_binlog_cache_size.
> +SHOW VARIABLES LIKE 'MAX_BINLOG_CACHE_SIZE';
> +Variable_name	Value
> +max_binlog_cache_size	4096
> +SHOW VARIABLES LIKE 'BINLOG_CACHE_SIZE';
> +Variable_name	Value
> +binlog_cache_size	4096
>
> +########################################################################################
>  #                                        CLEAN
> 
> ########################################################################################
>  DROP TABLE t1;
> 
> === modified file 'mysql-test/suite/rpl/r/rpl_row_binlog_max_cache_size.result'
> --- a/mysql-test/suite/rpl/r/rpl_row_binlog_max_cache_size.result	2010-04-28 12:47:49
> +0000
> +++ b/mysql-test/suite/rpl/r/rpl_row_binlog_max_cache_size.result	2010-09-03 17:12:32
> +0000
> @@ -5,6 +5,7 @@ reset slave;
>  drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
>  start slave;
>  call mtr.add_suppression("Unsafe statement written to the binary log using statement
> format since BINLOG_FORMAT = STATEMENT");
> +call mtr.add_suppression("Option binlog_cache_size ");
>  CREATE TABLE t1(a INT PRIMARY KEY, data VARCHAR(30000)) ENGINE=Innodb;
>  CREATE TABLE t2(a INT PRIMARY KEY, data VARCHAR(30000)) ENGINE=MyIsam;
>  CREATE TABLE t3(a INT PRIMARY KEY, data VARCHAR(30000)) ENGINE=Innodb;
> @@ -123,6 +124,34 @@ Got one of the listed errors
>  COMMIT;
>  source include/diff_master_slave.inc;
> 
> ########################################################################################
> +#                       CHECK MAX_BINLOG_CACHE_SIZE and BINLOG_CACHE_SIZE
>
> +########################################################################################
> +SET @@GLOBAL.MAX_BINLOG_CACHE_SIZE= 8192;
> +SHOW VARIABLES LIKE 'MAX_BINLOG_CACHE_SIZE';
> +Variable_name	Value
> +max_binlog_cache_size	8192
> +SHOW VARIABLES LIKE 'BINLOG_CACHE_SIZE';
> +Variable_name	Value
> +binlog_cache_size	4096
> +SET @@GLOBAL.BINLOG_CACHE_SIZE= 16384;
> +Warnings:
> +Warning	1624	Option binlog_cache_size (16384) is greater than max_binlog_cache_size
> (8192). So setting binlog_cache_size equal to max_binlog_cache_size.
> +SHOW VARIABLES LIKE 'MAX_BINLOG_CACHE_SIZE';
> +Variable_name	Value
> +max_binlog_cache_size	8192
> +SHOW VARIABLES LIKE 'BINLOG_CACHE_SIZE';
> +Variable_name	Value
> +binlog_cache_size	8192
> +SET @@GLOBAL.MAX_BINLOG_CACHE_SIZE= 4096;
> +Warnings:
> +Warning	1624	Option binlog_cache_size (8192) is greater than max_binlog_cache_size
> (4096). So setting binlog_cache_size equal to max_binlog_cache_size.
> +SHOW VARIABLES LIKE 'MAX_BINLOG_CACHE_SIZE';
> +Variable_name	Value
> +max_binlog_cache_size	4096
> +SHOW VARIABLES LIKE 'BINLOG_CACHE_SIZE';
> +Variable_name	Value
> +binlog_cache_size	4096
>
> +########################################################################################
>  #                                        CLEAN
> 
> ########################################################################################
>  DROP TABLE t1;
> 
> === modified file 'mysql-test/suite/rpl/r/rpl_stm_binlog_max_cache_size.result'
> --- a/mysql-test/suite/rpl/r/rpl_stm_binlog_max_cache_size.result	2010-06-30 20:56:21
> +0000
> +++ b/mysql-test/suite/rpl/r/rpl_stm_binlog_max_cache_size.result	2010-09-03 17:12:32
> +0000
> @@ -5,6 +5,7 @@ reset slave;
>  drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
>  start slave;
>  call mtr.add_suppression("Unsafe statement written to the binary log using statement
> format since BINLOG_FORMAT = STATEMENT");
> +call mtr.add_suppression("Option binlog_cache_size ");
>  CREATE TABLE t1(a INT PRIMARY KEY, data VARCHAR(30000)) ENGINE=Innodb;
>  CREATE TABLE t2(a INT PRIMARY KEY, data VARCHAR(30000)) ENGINE=MyIsam;
>  CREATE TABLE t3(a INT PRIMARY KEY, data VARCHAR(30000)) ENGINE=Innodb;
> @@ -120,6 +121,34 @@ Got one of the listed errors
>  COMMIT;
>  source include/diff_master_slave.inc;
> 
> ########################################################################################
> +#                       CHECK MAX_BINLOG_CACHE_SIZE and BINLOG_CACHE_SIZE
>
> +########################################################################################
> +SET @@GLOBAL.MAX_BINLOG_CACHE_SIZE= 8192;
> +SHOW VARIABLES LIKE 'MAX_BINLOG_CACHE_SIZE';
> +Variable_name	Value
> +max_binlog_cache_size	8192
> +SHOW VARIABLES LIKE 'BINLOG_CACHE_SIZE';
> +Variable_name	Value
> +binlog_cache_size	4096
> +SET @@GLOBAL.BINLOG_CACHE_SIZE= 16384;
> +Warnings:
> +Warning	1624	Option binlog_cache_size (16384) is greater than max_binlog_cache_size
> (8192). So setting binlog_cache_size equal to max_binlog_cache_size.
> +SHOW VARIABLES LIKE 'MAX_BINLOG_CACHE_SIZE';
> +Variable_name	Value
> +max_binlog_cache_size	8192
> +SHOW VARIABLES LIKE 'BINLOG_CACHE_SIZE';
> +Variable_name	Value
> +binlog_cache_size	8192
> +SET @@GLOBAL.MAX_BINLOG_CACHE_SIZE= 4096;
> +Warnings:
> +Warning	1624	Option binlog_cache_size (8192) is greater than max_binlog_cache_size
> (4096). So setting binlog_cache_size equal to max_binlog_cache_size.
> +SHOW VARIABLES LIKE 'MAX_BINLOG_CACHE_SIZE';
> +Variable_name	Value
> +max_binlog_cache_size	4096
> +SHOW VARIABLES LIKE 'BINLOG_CACHE_SIZE';
> +Variable_name	Value
> +binlog_cache_size	4096
>
> +########################################################################################
>  #                                        CLEAN
> 
> ########################################################################################
>  DROP TABLE t1;
> 
> === modified file
> 'mysql-test/suite/rpl/t/rpl_mixed_binlog_max_cache_size-master.opt'
> --- a/mysql-test/suite/rpl/t/rpl_mixed_binlog_max_cache_size-master.opt	2009-11-03
> 19:02:56 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_mixed_binlog_max_cache_size-master.opt	2010-09-03
> 17:12:32 +0000
> @@ -1 +1 @@
> ---binlog_cache_size=4096 --max_binlog_cache_size=7680
> +--binlog_cache_size=15360 --max_binlog_cache_size=7680
> --default-storage-engine=MyISAM
> 
> === modified file 'mysql-test/suite/rpl/t/rpl_row_binlog_max_cache_size-master.opt'
> --- a/mysql-test/suite/rpl/t/rpl_row_binlog_max_cache_size-master.opt	2010-06-17
> 20:51:35 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_row_binlog_max_cache_size-master.opt	2010-09-03
> 17:12:32 +0000
> @@ -1 +1 @@
> ---binlog_cache_size=4096 --max_binlog_cache_size=7680
> --default-storage-engine=MyISAM
> +--binlog_cache_size=15360 --max_binlog_cache_size=7680
> --default-storage-engine=MyISAM
> 
> === modified file 'mysql-test/suite/rpl/t/rpl_stm_binlog_max_cache_size-master.opt'
> --- a/mysql-test/suite/rpl/t/rpl_stm_binlog_max_cache_size-master.opt	2009-11-03
> 19:02:56 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_stm_binlog_max_cache_size-master.opt	2010-09-03
> 17:12:32 +0000
> @@ -1 +1 @@
> ---binlog_cache_size=4096 --max_binlog_cache_size=7680
> +--binlog_cache_size=15360 --max_binlog_cache_size=7680
> --default-storage-engine=MyISAM
> 
> === modified file 'sql/binlog.cc'
> --- a/sql/binlog.cc	2010-08-20 03:37:42 +0000
> +++ b/sql/binlog.cc	2010-09-03 17:12:32 +0000
> @@ -242,6 +242,33 @@ private:
>    binlog_cache_mngr(const binlog_cache_mngr& info);
>  };
>  
> +/**
> +  Checks if the BINLOG_CACHE_SIZE's value is greater than MAX_BINLOG_CACHE_SIZE.
> +  If this happens, the BINLOG_CACHE_SIZE is set to MAX_BINLOG_CACHE_SIZE.
> +*/
> +void check_binlog_cache_size(THD *thd)
> +{
> +  const char *msg= "Option binlog_cache_size (%lu) is greater than " \
> +                   "max_binlog_cache_size (%lu). So setting " \
> +                   "binlog_cache_size equal to max_binlog_cache_size.";
> +
> +  if (binlog_cache_size > max_binlog_cache_size)
> +  {
> +    if (thd)
> +    {
> +      push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN,
> +                          ER_SLAVE_HEARTBEAT_VALUE_OUT_OF_RANGE, msg,
> +                          (ulong) binlog_cache_size, (ulong)
> max_binlog_cache_size);
> +    }
> +    else
> +    {
> +      sql_print_warning(msg, (ulong) binlog_cache_size,
> +                        (ulong) max_binlog_cache_size);
> +    }
> +    binlog_cache_size= max_binlog_cache_size;
> +  }
> +}
> +
>   /*
>    Save position of binary log transaction cache.
>  
> 
> === modified file 'sql/binlog.h'
> --- a/sql/binlog.h	2010-08-26 14:29:22 +0000
> +++ b/sql/binlog.h	2010-09-03 17:12:32 +0000
> @@ -260,5 +260,6 @@ int check_binlog_magic(IO_CACHE* log, co
>  bool purge_master_logs(THD* thd, const char* to_log);
>  bool purge_master_logs_before_date(THD* thd, time_t purge_time);
>  bool show_binlog_events(THD *thd, MYSQL_BIN_LOG *binary_log);
> +void check_binlog_cache_size(THD *thd);
>  
>  #endif /* BINLOG_H_INCLUDED */
> 
> === modified file 'sql/mysqld.cc'
> --- a/sql/mysqld.cc	2010-09-01 13:05:01 +0000
> +++ b/sql/mysqld.cc	2010-09-03 17:12:32 +0000
> @@ -4662,6 +4662,8 @@ int mysqld_main(int argc, char **argv)
>    if (opt_bootstrap) /* If running with bootstrap, do not start replication. */
>      opt_skip_slave_start= 1;
>  
> +  check_binlog_cache_size(NULL);
> +
>    binlog_unsafe_map_init();
>    /*
>      init_slave() must be called after the thread keys are created.
> 
> === modified file 'sql/sys_vars.cc'
> --- a/sql/sys_vars.cc	2010-09-01 13:05:01 +0000
> +++ b/sql/sys_vars.cc	2010-09-03 17:12:32 +0000
> @@ -235,6 +235,12 @@ static Sys_var_charptr Sys_basedir(
>         READ_ONLY GLOBAL_VAR(mysql_home_ptr), CMD_LINE(REQUIRED_ARG, 'b'),
>         IN_FS_CHARSET, DEFAULT(0));
>  
> +static bool fix_binlog_cache_size(sys_var *self, THD *thd, enum_var_type type)
> +{
> +  check_binlog_cache_size(thd);
> +  return false;
> +}
> +
>  static Sys_var_ulong Sys_binlog_cache_size(
>         "binlog_cache_size", "The size of the cache to "
>         "hold the SQL statements for the binary log during a "
> @@ -242,7 +248,9 @@ static Sys_var_ulong Sys_binlog_cache_si
>         "transactions you can increase this to get more performance",
>         GLOBAL_VAR(binlog_cache_size),
>         CMD_LINE(REQUIRED_ARG),
> -       VALID_RANGE(IO_SIZE, ULONG_MAX), DEFAULT(32768), BLOCK_SIZE(IO_SIZE));
> +       VALID_RANGE(IO_SIZE, ULONG_MAX), DEFAULT(32768), BLOCK_SIZE(IO_SIZE),
> +       NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(0),
> +       ON_UPDATE(fix_binlog_cache_size));
>  
>  static bool check_has_super(sys_var *self, THD *thd, set_var *var)
>  {
> @@ -1036,7 +1044,9 @@ static Sys_var_ulonglong Sys_max_binlog_
>         GLOBAL_VAR(max_binlog_cache_size), CMD_LINE(REQUIRED_ARG),
>         VALID_RANGE(IO_SIZE, ULONGLONG_MAX),
>         DEFAULT((ULONGLONG_MAX/IO_SIZE)*IO_SIZE),
> -       BLOCK_SIZE(IO_SIZE));
> +       BLOCK_SIZE(IO_SIZE),
> +       NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(0),
> +       ON_UPDATE(fix_binlog_cache_size));
>  
>  static bool fix_max_binlog_size(sys_var *self, THD *thd, enum_var_type type)
>  {
> 
> 
> 
> ------------------------------------------------------------------------
> 
> 

Thread
bzr commit into mysql-trunk-bugfixing branch (alfranio.correia:3257)Bug#55377Alfranio Correia3 Sep
  • Re: bzr commit into mysql-trunk-bugfixing branch (alfranio.correia:3257)Bug#55377Sven Sandberg6 Sep