maria-developers team mailing list archive

[Kristian Nielsen <knielsen@xxxxxxxxxxxxxxx>]

Monty, thanks for review!

I followed all your suggestions I believe and will push to 5.5 when current
merge is over.

 - Kristian.

Michael Widenius <monty@xxxxxxxxxxxx> writes:

> We agree to change the name of replicate-ignore-do-not-replicate
> variable to just 'replicate' with a default value of 1.

Yes (in the end I chose @@skip_replication and
@@replicate_events_marked_with_skip as we discussed).

> I have now added to MariaDB 5.3 the following new comment syntax to allow one
> to do MariaDB specific executable comments:
>
> /* !M##### */

Ok, I've made a ToDo note to use it when I merge this WL to MariaDB-5.5.

> Move the print_do_not_replicate_statement to just before
> exlq->print(result_file, print_event_info, fname);

Done

> 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.

Yes, agree, done.

> 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)

Yes, I need to adjust this appropriately when merging into the appropriate
tree.

> 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.

Done.

> 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
> ...

Indeed, this is a bug! Fixed.

> knielsen> +    uint16 flags= uint2korr(&((*packet)[FLAGS_OFFSET+1]));
>
> Please add a comment to explain the +1 above

Done.

 - Kristian.