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
