Code review comment for lp:~vlad-lesin/percona-server/5.5-mysqlbinlog-replacedb

Revision history for this message
Vlad Lesin (vlad-lesin) wrote :

On 12/20/2013 12:58 PM, Laurynas Biveinis wrote:
> - This needs two associated blueprints, one for 5.5, and one for
> 5.6
Done.

> - Oracle has implemented this functionality in 5.7.1. From the
> user perspective the difference is option syntax, ours:
> --rewrite-db='a->b,c->d'
> theirs:
> --rewrite-db='a->b' --rewrite-db='c->d'
> To ensure smooth migration once we switch to their
> implementation in 5.7, please test both syntaxes in the
> testcase.
Yes, currently both formats are allowed, but if the old
format(comma-separated rules) is used the warning is issued.

> - What should be the interaction between --rewrite-db and
> --database options, i.e. which one should come first? My first
> thought is rewrite-db before database, i.e. first we map, and
> then we filter. This is what the current patch appears to do
> for SBR, but it's reversed for RBR (judging from
> shall_skip_database() and rewrite_db() call order). This would
> also need a testcase.
Yes, you are right, the order of rewrite-db and database filtering was
reversed for RBR. Fixed and the corresponding test is added.

> - Rewrite rules are not applied to CREATE_FILE_EVENT. This
> wouldn't be an issue in itself, but these events are filtered
> against --database, thus, depending on how the previous item is
> resolved, these events might need rewrite-db application too,
> so that they can be filtered properly. This would also need a
> testcase.
I added the code which replaces db name in the event processing, but I
have no idea how to test it as there is the following comment in
log_event.h:

"The events which really update data are Query_log_event,
Execute_load_query_log_event and old Load_log_event and
Execute_load_log_event events (Execute_load_query is used together with
Begin_load_query and Append_block events to replicate LOAD DATA INFILE.
Create_file/Append_block/Execute_load (which includes Load_log_event)
were used to replicate LOAD DATA before the 5.0.3)."

So CREATE_FILE_EVENT were used before 5.0.3.

> - LOAD DATA events (EXECUTE_LOAD_QUERY_EVENT) need test coverage
> too. Probably this can be handled together with the previous
> item.
The corresponding test case is made.

> - Because RBR event DB rewrite requires shifting of
> Table_map_og_event contents around if the DB name length
> changes, please test this case for both shorter and longer
> names.
Done.
> - Since Oracle has implemented this, there are some Oracle
> testcases too. Consider importing binlog_mysqlbinlog_rewrite_db
> binlog_rewrite_db_noleak binlog_rewrite_suppress_use from MySQL
> 5.7. If you choose to, make sure they get the correct bzr file
> ids (i.e. bzr add --file-ids-from=path to 5.7 tree)
Done.

« Back to merge proposal