Hi!
>>>>> "knielsen" == knielsen <knielsen(a)knielsen-hq.org> writes:
<cut>
We agree to change the name of replicate-ignore-do-not-replicate
variable to just 'replicate' with a default value of 1.
(Double negatives are hard, especially if you test them with !
in the code..)
knielsen> MWL#234: Support for marking binlog events to not be replicated, and for telling slaves not to replicate events with such mark
knielsen> === modified file 'client/mysqlbinlog.cc'
knielsen> --- a/client/mysqlbinlog.cc 2011-05-03 16:10:10 +0000
knielsen> +++ b/client/mysqlbinlog.cc 2011-08-11 09:38:52 +0000
knielsen> @@ -671,6 +671,31 @@ print_use_stmt(PRINT_EVENT_INFO* pinfo,
knielsen> /**
knielsen> + Print "SET do_not_replicate=..." statement when needed.
knielsen> +
knielsen> + Not all servers support this (only MariaDB from some version on). So we
knielsen> + mark the SET to only execute from the version of MariaDB that supports it,
knielsen> + and also only output it if we actually see events with the flag set, to not
knielsen> + get spurious errors on MySQL@Oracle servers of higher version that do not
knielsen> + support the flag.
knielsen> +
knielsen> + So we start out assuming @@do_not_replicate is 0, and only output a SET
knielsen> + statement when it changes.
knielsen> +*/
I have now added to MariaDB 5.3 the following new comment syntax to allow one
to do MariaDB specific executable comments:
/* !M##### */
<cut>
knielsen> @@ -927,6 +958,7 @@ Exit_status process_event(PRINT_EVENT_IN
knielsen> if (!shall_skip_database(exlq->db))
knielsen> {
knielsen> print_use_stmt(print_event_info, exlq);
knielsen> + print_do_not_replicate_statement(print_event_info, ev);
knielsen> if (fname)
knielsen> {
Move the print_do_not_replicate_statement to just before
exlq->print(result_file, print_event_info, fname);
knielsen> convert_path_to_forward_slashes(fname);
knielsen> @@ -1030,6 +1062,12 @@ Exit_status process_event(PRINT_EVENT_IN
knielsen> }
knielsen> /* FALL THROUGH */
knielsen> }
knielsen> + case INTVAR_EVENT:
knielsen> + case RAND_EVENT:
knielsen> + case USER_VAR_EVENT:
knielsen> + case XID_EVENT:
knielsen> + print_do_not_replicate_statement(print_event_info, ev);
knielsen> + /* Fall through ... */
knielsen> default:
knielsen> ev->print(result_file, print_event_info);
knielsen> }
Would it not be safer to have
print_do_not_replicate_statement(print_event_info, ev);
after the default so that we don't miss anything.
(Having a few sporadic 'do_not_replicate=#' events is better than
missing something.
knielsen> === modified file 'configure.in'
knielsen> --- a/configure.in 2011-06-11 11:28:37 +0000
knielsen> +++ b/configure.in 2011-08-11 09:38:52 +0000
knielsen> @@ -13,7 +13,7 @@ dnl When changing the major version numb
knielsen> dnl statement in mysqlbinlog::check_master_version(). You may also need
knielsen> dnl to update version.c in ndb.
knielsen> -AC_INIT([MariaDB Server], [5.2.7-MariaDB], [], [mysql])
knielsen> +AC_INIT([MariaDB Server], [5.4.0-MariaDB], [], [mysql])
5.4, that was intersting ;)
(Should probably not be part of the patch you send the customer)
knielsen> --- a/sql/mysql_priv.h 2011-07-21 10:15:09 +0000
<cut>
knielsen> @@ -2064,6 +2065,7 @@ extern my_bool opt_old_style_user_limits
knielsen> extern uint opt_crash_binlog_innodb;
knielsen> extern char *shared_memory_base_name, *mysqld_unix_port;
knielsen> extern my_bool opt_enable_shared_memory;
knielsen> +extern my_bool opt_replicate_ignore_do_not_replicate;
Not a perfect name (as we both agree upon :)
<cut>
knielsen> +++ b/sql/mysqld.cc 2011-08-11 09:38:52 +0000
knielsen> @@ -553,6 +553,8 @@ uint opt_large_page_size= 0;
knielsen> uint opt_debug_sync_timeout= 0;
knielsen> #endif /* defined(ENABLED_DEBUG_SYNC) */
knielsen> my_bool opt_old_style_user_limits= 0, trust_function_creators= 0;
knielsen> +my_bool opt_replicate_ignore_do_not_replicate;
knielsen> +
knielsen> /*
knielsen> True if there is at least one per-hour limit for some user, so we should
knielsen> check them before each query (and possibly reset counters when hour is
knielsen> @@ -6085,7 +6087,8 @@ enum options_mysqld
knielsen> OPT_IGNORE_BUILTIN_INNODB,
knielsen> OPT_BINLOG_DIRECT_NON_TRANS_UPDATE,
knielsen> OPT_DEFAULT_CHARACTER_SET_OLD,
knielsen> - OPT_MAX_LONG_DATA_SIZE
knielsen> + OPT_MAX_LONG_DATA_SIZE,
knielsen> + OPT_REPLICATE_IGNORE_DO_NOT_REPLICATE
Add the option somewhere close to other replication options.
Benefits:
- Only one row changed
- Will not conflict if MySQL add more options or if we add more
options in later MariaDB versions.
knielsen> === modified file 'sql/slave.cc'
knielsen> --- a/sql/slave.cc 2011-05-03 16:10:10 +0000
knielsen> +++ b/sql/slave.cc 2011-08-11 09:38:52 +0000
knielsen> @@ -1176,6 +1176,38 @@ when it try to get the value of TIME_ZON
knielsen> }
knielsen> }
knielsen> + /*
knielsen> + Request the master to filter away events with the @@do_not_replicate flag
knielsen> + set, if we are running with --replicate-ignore-do_not_replicate=1.
knielsen> + */
knielsen> + if (opt_replicate_ignore_do_not_replicate)
knielsen> + {
knielsen> + if (!mysql_real_query(mysql, STRING_WITH_LEN("SET do_not_replicate=1")))
knielsen> + {
knielsen> + err_code= mysql_errno(mysql);
knielsen> + if (is_network_error(err_code))
knielsen> + {
knielsen> + mi->report(ERROR_LEVEL, err_code,
knielsen> + "Setting master-side filtering of @@do_not_replicate failed "
knielsen> + "with error: %s", mysql_error(mysql));
knielsen> + goto network_err;
knielsen> + }
knielsen> + else if (err_code == ER_UNKNOWN_SYSTEM_VARIABLE)
knielsen> + {
knielsen> + /*
knielsen> + The master is older than the slave and does not support the
knielsen> + @@do_not_replicate feature.
knielsen> + This is not a problem, as such master will not generate events with
knielsen> + the @@do_not_replicate flag set in the first place. We will still
knielsen> + do slave-side filtering of such events though, to handle the (rare)
knielsen> + case of downgrading a master and receiving old events generated from
knielsen> + before the downgrade with the @@do_not_replicate flag set.
knielsen> + */
knielsen> + DBUG_PRINT("info", ("Old master does not support master-side filtering "
knielsen> + "of @@do_not_replicate events."));
knielsen> + }
knielsen> + }
knielsen> + }
Wouldn't it be better to do:
if (err_code == ER_UNKNOWN_SYSTEM_VARIABLE)
...
else
...
Now you don't print an error if there was some other unknown problem
with setting the variable.
What other errors can we get other than ER_UNKNOWN_SYSTEM_VARIABLE
that would allow us to continue ?
knielsen> err:
knielsen> if (errmsg)
knielsen> {
knielsen> @@ -2114,6 +2146,8 @@ int apply_event_and_update_pos(Log_event
knielsen> thd->lex->current_select= 0;
knielsen> if (!ev->when)
knielsen> ev->when= my_time(0);
knielsen> + thd->options= (thd->options & ~OPTION_DO_NOT_REPLICATE) |
knielsen> + (ev->flags & LOG_EVENT_DO_NOT_REPLICATE_F ? OPTION_DO_NOT_REPLICATE : 0);
knielsen> ev->thd = thd; // because up to this point, ev->thd == 0
knielsen> int reason= ev->shall_skip(rli);
knielsen> @@ -3582,6 +3616,7 @@ static int queue_event(Master_info* mi,c
knielsen> {
knielsen> int error= 0;
knielsen> ulong inc_pos;
knielsen> + ulong event_pos;
knielsen> Relay_log_info *rli= &mi->rli;
knielsen> pthread_mutex_t *log_lock= rli->relay_log.get_log_lock();
knielsen> DBUG_ENTER("queue_event");
knielsen> @@ -3667,6 +3702,23 @@ static int queue_event(Master_info* mi,c
knielsen> }
knielsen> /*
knielsen> + If we filter events master-side (eg. @@do_not_replicate), we will see holes
knielsen> + in the event positions from the master. If we see such a hole, adjust
knielsen> + mi->master_log_pos accordingly so we maintain the correct position (for
knielsen> + reconnect, MASTER_POS_WAIT(), etc.)
knielsen> + */
knielsen> + if (inc_pos > 0 &&
knielsen> + event_len >= LOG_POS_OFFSET+4 &&
knielsen> + (event_pos= uint4korr(buf+LOG_POS_OFFSET)) > mi->master_log_pos + inc_pos)
knielsen> + {
knielsen> + inc_pos= event_pos - mi->master_log_pos;
knielsen> + DBUG_PRINT("info", ("Adjust master_log_pos %lu->%lu to account for "
knielsen> + "master-side filtering",
knielsen> + (unsigned long)(mi->master_log_pos + inc_pos),
knielsen> + event_pos));
knielsen> + }
<irc discussion about>
is 'inc_pos= event_pos - mi->master_log_pos' really secure and should
we add
DBUG_ASSERT(mi->master_log_pos == uint4korr(buf+LOG_POS_OFFSET) ||
inc_len == 0)
after
mi->master_log_pos+= inc_pos;
to ensure the correctness.
For the moment we decided to not do this.
<cut>
knielsen> +++ b/sql/sql_repl.cc 2011-08-11 09:38:52 +0000
knielsen> @@ -338,6 +338,41 @@ Increase max_allowed_packet on master";
knielsen> /*
knielsen> + Helper function for mysql_binlog_send() to write an event down the slave
knielsen> + connection.
knielsen> +
knielsen> + Returns NULL on success, error message string on error.
knielsen> +*/
knielsen> +static const char *
knielsen> +send_event_to_slave(THD *thd, NET *net, String* const packet)
knielsen> +{
knielsen> + thd_proc_info(thd, "Sending binlog event to slave");
knielsen> +
knielsen> + /*
knielsen> + Skip events with the @@do_not_replicate flag set, if slave requested
knielsen> + skipping of such events.
knielsen> + */
knielsen> + if (thd->options & OPTION_DO_NOT_REPLICATE)
knielsen> + {
knielsen> + uint16 flags= uint2korr(&((*packet)[FLAGS_OFFSET+1]));
Please add a comment to explain the +1 above
<cut>
Looks good!
Regards,
Monty