[Maria-developers] MDEV-5115 review

Hi Serg, Here is my review of your MDEV-5115 patch. First, a general comment: Another way to fix this would be a smaller patch. It would not attempt to implement anything related to the new event types. Instead, it would just have a small bit of code that allows to read the binary format of the new event types, but it would represent them internally as the old type, just ignoring any of the extra information, which appears to be currently unused outside of NDB. Probably then it would also make sense to not implement the renaming of the old event types. There would just be three extra cases in Log_event::read_log_event(), and a bit of extra code in the *_rows_log_event construstructors to be able to ignore any extra info. However, I do not really have much against your approach of doing a partial merge to get code that is closer to what is in MySQL, if you prefer that. I mention it because we discussed briefly already on IRC whether a smaller patch could be enough. A few detailed comments below. Otherwise, while I would have probably done the minimal patch myself, I am ok with this way also. So ok to push after fixing the few commit and code comments mentioned below. - Kristian.

------------------------------------------------------------ revno: 3921 revision-id: sergii@pisem.net-20131203192301-afokgpslfq0f3xg8 parent: sergii@pisem.net-20131201111624-xe3f1ul6otcyvyom fixes bug: https://mariadb.atlassian.net/browse/MDEV-5115 committer: Sergei Golubchik branch nick: 10.0 timestamp: Tue 2013-12-03 20:23:01 +0100 message: MDEV-5115 RBR from MySQL 5.6 to MariaDB 10.0 does not work

Patially merge WL#5917, to understand v2 row events

"Partially merge WL#5917, which introduces new event types for row-based binlogging. This is needed to be able to replicate from a >=5.6 MySQL master to a MariaDB slave."

=== modified file 'mysql-test/suite/binlog/t/binlog_old_versions.test' --- mysql-test/suite/binlog/t/binlog_old_versions.test 2013-04-09 21:27:41 +0000 +++ mysql-test/suite/binlog/t/binlog_old_versions.test 2013-12-03 19:23:01 +0000 @@ -24,6 +24,17 @@

source include/not_embedded.inc;

+--echo ==== Read binlog with v2 row events ==== + +# Read binlog. +--exec $MYSQL_BINLOG --local-load=$MYSQLTEST_VARDIR/tmp/ suite/binlog/std_data/ver_trunk_row_v2.001 | $MYSQL --local-infile=1 +# Show result. +SELECT * FROM t1 ORDER BY a; +SELECT * FROM t2 ORDER BY a; +SELECT COUNT(*) FROM t3; +# Reset. +DROP TABLE t1, t2, t3; +

--echo ==== Read modern binlog (version 5.1.23) ====

Is this the only test we have of reading the v2 row event types? I think it is not a particularly good test, as it tests mysqlbinlog reading them, while the interesting thing is a slave receiving them from the master. Even though some code is shared between the two there are lots of differences as well. The better test would be to install the pre-generated binlog file manually on a master and have the slave replicate it. However, it is easy to end up spending too much time on setting up these replication tests and getting them to work reliably on all platforms, so maybe not worth it. Maybe a good compromise is to instead ask Elena to test it manually against a real MySQL 5.6 server - after all, that is the actual case we want to work.

=== modified file 'sql/log_event.h' --- sql/log_event.h 2013-11-01 11:00:11 +0000 +++ sql/log_event.h 2013-12-03 19:23:01 +0000

@@ -659,11 +663,11 @@ enum Log_event_type PRE_GA_DELETE_ROWS_EVENT = 22,

/* - These event numbers are used from 5.1.16 and forward + These event numbers are used from 5.1.16 until mysql-trunk-xx

Please change this comment to explain the actual situation. Ie. that they are used in MariaDB, but not in MySQL >= 5.6.

@@ -677,6 +681,20 @@ enum Log_event_type HEARTBEAT_LOG_EVENT= 27,

/* + In some situations, it is necessary to send over ignorable + data to the slave: data that a slave can handle in case there + is code for handling it, but which can be ignored if it is not + recognized. + */ + IGNORABLE_LOG_EVENT= 28, + ROWS_QUERY_LOG_EVENT= 29,

Add a comment that these are MySQL 5.6 events that are unimplemented in MariaDB. (Do we need another MDEV to be able to replicate against a MySQL master that produces ROWS_QUERY_LOG_EVENT? It is quite similar to our ANNOTATE_ROWS_EVENT, IIRC).

+ /* Version 2 of the Row events */ + WRITE_ROWS_EVENT = 30, + UPDATE_ROWS_EVENT = 31, + DELETE_ROWS_EVENT = 32,

Add comment that these are only generated by MySQL 5.6.