List:Commits« Previous MessageNext Message »
From:Luís Soares Date:October 27 2010 12:22pm
Subject:Re: bzr push into mysql-next-mr branch (andrei.elkin:3184 to 3185)
Bug#57589
View as plain text  
Hi Andrei,

   Nice Work.
   Please find my review comments below.

STATUS
------

   Not approved.

REQUIRED CHANGES
----------------

   RC1. In the first hunk, it seems that goto err; is missing. Please
        add it.

   RC2. Please, run the error/warning message text through Jon. I am a
        bit sad that we have this message hardcoded in the text. But,
        AFAIK, we don't support localization for this kind of
        thing... or do we ?

REQUESTS
--------

   R1. Since binlog_format is also changeable when binary log is not
       active, it's only fair that we allow this too for checksums.

       Also, without this hunk I would get a SIGSEV when setting
       binlog_checksum= CRC32 without binary log enabled.

       Good then...

SUGGESTIONS
-----------
  n/a

DETAILS
-------
  n/a

On 10/27/2010 11:24 AM, Andrei Elkin wrote:
>   3185 Andrei Elkin	2010-10-27
>        Bug #57589 SHOW SLAVE STATUS doesnt show err:1665 on NM-OS when slave can't
> handle checksum
>
>        OS can't connect to the checksumming-ON NM and an error goes out to the
>        error log (which is good), but OS can't stop trying to reconnect
>        constantly (which is not).
>
>        While practically this scenario must be pretty rare it's possible
>        to fix the issue a rather nice way.
>        Master sends back to checksum-unaware OS the
> ER_MASTER_FATAL_ERROR_READING_BINLOG
>        critical error accompanied with a verbose clarification mentioning
>        the checksum situation like in the following snippet of the error log from the
> patch testing:
>
>        101026 20:15:45 [ERROR] Error reading packet from server: Slave can not handle
> replication events with the checksum that master is configured to log (
> server_errno=1236)
>        101026 20:15:45 [ERROR] Slave I/O: Got fatal error 1236 from master when
> reading data from binary log: 'Slave can not handle replication events with the checksum
> that master is configured to log', Error_code: 1236
>
>        Master also logs a warning
>
>        101026 20:15:33 [Warning] Configured to log replication events with checksum
> Master rejects sending them to Slave that can not handle it.
>
>
>        Additional fixes for wl#2540 targeting sysvar suite.
>        Each system var must have a test file in there. The tests are adeed.
>       @ mysql-test/suite/sys_vars/r/all_vars.result
>          results are changed.
>       @ sql/rpl_master.cc
>          In case of checksumming-ON NM ->  OS replication
>          Master sends back to checksum-unaware OS the
> ER_MASTER_FATAL_ERROR_READING_BINLOG
>          critical error accompanied with a verbose clarification.
>          Master also logs a warning.
>       @ sql/rpl_slave.cc
>          Unneeded anymore piece of codes is removed.
>       @ sql/share/errmsg-utf8.txt
>          Unneeded error is removed.
>       @ sql/sys_vars.cc
>          In case of binlog is not open, binlog_checksum changes anyway when a new
> value is set.
>
>      modified:
>        mysql-test/suite/sys_vars/r/all_vars.result
>        sql/rpl_master.cc
>        sql/rpl_slave.cc
>        sql/share/errmsg-utf8.txt
>        sql/sys_vars.cc
>   3184 Andrei Elkin	2010-10-26
>        wl#2540
>
>        fixing error constants shifted after next-mr ->  wl2540 merge. Meaningful
> changes are only to rpl_checksum_cache where sync_master was corrected to be played on the
> right connection
>
>      modified:
>        mysql-test/r/explain.result
>        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_checksum_cache.test
>        mysql-test/suite/sys_vars/r/all_vars.result
>        mysql-test/suite/sys_vars/r/max_binlog_cache_size_basic.result
> === modified file 'mysql-test/suite/sys_vars/r/all_vars.result'
> --- a/mysql-test/suite/sys_vars/r/all_vars.result	2010-10-26 16:06:43 +0000
> +++ b/mysql-test/suite/sys_vars/r/all_vars.result	2010-10-27 10:23:24 +0000
> @@ -14,17 +14,11 @@ INNODB_MONITOR_COUNTER_RESET
>   INNODB_MONITOR_COUNTER_RESET_ALL
>   INNODB_MONITOR_COUNTER_ON
>   INNODB_MONITOR_COUNTER_OFF
> -BINLOG_CHECKSUM
> -MASTER_VERIFY_CHECKSUM
>   INNODB_FILE_FORMAT_MAX
> -SLAVE_SQL_VERIFY_CHECKSUM
>   INNODB_MONITOR_COUNTER_RESET
>   INNODB_MONITOR_COUNTER_RESET_ALL
>   INNODB_MONITOR_COUNTER_ON
>   INNODB_MONITOR_COUNTER_OFF
> -BINLOG_CHECKSUM
> -MASTER_VERIFY_CHECKSUM
>   INNODB_FILE_FORMAT_MAX
> -SLAVE_SQL_VERIFY_CHECKSUM
>   drop table t1;
>   drop table t2;
>
> === modified file 'sql/rpl_master.cc'
> --- a/sql/rpl_master.cc	2010-10-25 19:02:24 +0000
> +++ b/sql/rpl_master.cc	2010-10-27 10:23:24 +0000
> @@ -815,9 +815,12 @@ impossible position";
>                current_checksum_alg != BINLOG_CHECKSUM_ALG_OFF&&
>                current_checksum_alg != BINLOG_CHECKSUM_ALG_UNDEF)
>            {
> -           my_errno= ER_SLAVE_IS_NOT_CHECKSUM_CAPABLE;
> -           errmsg= ER(my_errno);
> -           goto err;
> +           my_errno= ER_MASTER_FATAL_ERROR_READING_BINLOG;
> +           errmsg= "Slave can not handle replication events with the checksum "
> +             "that master is configured to log";
> +           sql_print_warning("Configured to log replication events with "
> +                             "checksum Master rejects sending them to Slave "
> +                             "that can not handle it.");
>            }
>            binlog_can_be_corrupted= test((*packet)[FLAGS_OFFSET+ev_offset]&
>                                          LOG_EVENT_BINLOG_IN_USE_F);
> @@ -912,9 +915,13 @@ impossible position";
>               current_checksum_alg != BINLOG_CHECKSUM_ALG_OFF&&
>               current_checksum_alg != BINLOG_CHECKSUM_ALG_UNDEF)
>           {
> -          my_errno= ER_SLAVE_IS_NOT_CHECKSUM_CAPABLE;
> -          errmsg= ER(my_errno);
> -           goto err;
> +          my_errno= ER_MASTER_FATAL_ERROR_READING_BINLOG;
> +          errmsg= "Slave can not handle replication events with the checksum "
> +            "that master is configured to log";
> +          sql_print_warning("Configured to log replication events with "
> +                            "checksum Master rejects sending them to Slave "
> +                            "that can not handle it.");
> +          goto err;
>           }
>           binlog_can_be_corrupted= test((*packet)[FLAGS_OFFSET+ev_offset]&
>                                         LOG_EVENT_BINLOG_IN_USE_F);
>
> === modified file 'sql/rpl_slave.cc'
> --- a/sql/rpl_slave.cc	2010-10-25 19:02:24 +0000
> +++ b/sql/rpl_slave.cc	2010-10-27 10:23:24 +0000
> @@ -3359,11 +3359,6 @@ Stopping slave I/O thread due to out-of-
>             mi->report(ERROR_LEVEL, ER_OUT_OF_RESOURCES,
>                        "%s", ER(ER_OUT_OF_RESOURCES));
>             goto err;
> -        case ER_SLAVE_IS_NOT_CHECKSUM_CAPABLE:
> -          mi->report(ERROR_LEVEL, ER_SLAVE_IS_NOT_CHECKSUM_CAPABLE,
> -                     ER(ER_SLAVE_IS_NOT_CHECKSUM_CAPABLE),
> -                     mysql_error_number, mysql_error(mysql));
> -          goto err;
>           }
>           if (try_to_reconnect(thd, mysql, mi,&retry_count, suppress_warnings,
>                                reconnect_messages[SLAVE_RECON_ACT_EVENT]))
>
> === modified file 'sql/share/errmsg-utf8.txt'
> --- a/sql/share/errmsg-utf8.txt	2010-10-25 19:02:24 +0000
> +++ b/sql/share/errmsg-utf8.txt	2010-10-27 10:23:24 +0000
> @@ -6391,8 +6391,6 @@ ER_NETWORK_READ_EVENT_CHECKSUM_FAILURE
>     eng "Replication event checksum verification failed while reading from network."
>   ER_BINLOG_READ_EVENT_CHECKSUM_FAILURE
>     eng "Replication event checksum verification failed while reading from a log
> file."
> -ER_SLAVE_IS_NOT_CHECKSUM_CAPABLE
> -  eng "Slave can not handle replication events with the checksum that master is
> configured to log."
>   ER_WARN_INDEX_NOT_APPLICABLE
>     eng "Cannot use %-.64s access on index '%-.64s' due to type or collation
> conversion on field '%-.64s'"
>   ER_BINLOG_CACHE_SIZE_GREATER_THAN_MAX
>
> === modified file 'sql/sys_vars.cc'
> --- a/sql/sys_vars.cc	2010-10-25 19:02:24 +0000
> +++ b/sql/sys_vars.cc	2010-10-27 10:23:24 +0000
> @@ -1897,6 +1897,10 @@ bool Sys_var_enum_binlog_checksum::globa
>         mysql_bin_log.checksum_alg_reset= (uint8)
> var->save_result.ulonglong_value;
>       mysql_bin_log.rotate_and_purge(flags);
>     }
> +  else
> +  {
> +    binlog_checksum_options= var->save_result.ulonglong_value;
> +  }
>     DBUG_ASSERT((ulong) binlog_checksum_options ==
> var->save_result.ulonglong_value);
>     DBUG_ASSERT(mysql_bin_log.checksum_alg_reset == BINLOG_CHECKSUM_ALG_UNDEF);
>     mysql_mutex_unlock(mysql_bin_log.get_log_lock());
>
>
>
>
>
e
Thread
bzr push into mysql-next-mr branch (andrei.elkin:3184 to 3185) Bug#57589Andrei Elkin27 Oct
  • Re: bzr push into mysql-next-mr branch (andrei.elkin:3184 to 3185)Bug#57589Luís Soares27 Oct
Re: bzr push into mysql-next-mr branch (andrei.elkin:3184 to 3185)Bug#57589Luís Soares27 Oct