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

Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote :

    - This needs two associated blueprints, one for 5.5, and one for
      5.6

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

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

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

    - LOAD DATA events (EXECUTE_LOAD_QUERY_EVENT) need test coverage
      too. Probably this can be handled together with the previous
      item.

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

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

review: Needs Fixing

« Back to merge proposal