Hi!

...

...

...

...

"knielsen" == knielsen <knielsen@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