Hi Kristian! Ok to push. (see a suggestion inline) Best, Nirbhay On Thu, Apr 7, 2016 at 8:55 AM, Kristian Nielsen <knielsen@knielsen-hq.org> wrote:
Hi Nirbhay,
Do you want to review this patch for MDEV-9383? It concerns your code added for do_domain_ids=(...).
- Kristian.
Kristian Nielsen <knielsen@knielsen-hq.org> writes:
revision-id: ca24c9d1671e90e43daa483af32fc19891d1a646 (mariadb-10.1.13-8-gca24c9d) parent(s): 4b6a3518e4dc9088d1f42cd9bc487d06137d2760 committer: Kristian Nielsen timestamp: 2016-04-07 14:44:29 +0200 message:
MDEV-9383: Server fails to read master.info after upgrade 10.0 -> 10.1
In some cases, MariaDB 10.0 could write a master.info file that was read incorrectly by 10.1 and could cause server to fail to start after an upgrade.
(If writing a new master.info file that is shorter than the old, extra junk may remain at the end of the file. This is handled properly in 10.1 with an END_MARKER line, but this line is not written by 10.0. The fix here is to make 10.1 robust at reading the master.info files written by 10.0).
Fix several things around reading master.info and read_mi_key_from_file():
- read_mi_key_from_file() did not distinguish between a line with and without an eqals '=' sign.
- If a line was empty, read_mi_key_from_file() would incorrectly return the key from the previous call.
- An extra using_gtid=X line left-over by MariaDB 10.0 might incorrectly be read and overwrite the correct value.
- Fix incorrect usage of strncmp() which should be strcmp().
- Add test cases.
--- mysql-test/std_data/bad2_master.info | 35 +++++ mysql-test/std_data/bad3_master.info | 37 +++++ mysql-test/std_data/bad4_master.info | 35 +++++ mysql-test/std_data/bad5_master.info | 35 +++++ mysql-test/std_data/bad6_master.info | 36 +++++ mysql-test/std_data/bad_master.info | 35 +++++ .../suite/rpl/r/rpl_upgrade_master_info.result | 88 +++++++++++ .../suite/rpl/t/rpl_upgrade_master_info.test | 163 +++++++++++++++++++++ sql/rpl_mi.cc | 81 +++++----- 9 files changed, 512 insertions(+), 33 deletions(-)
diff --git a/mysql-test/std_data/bad2_master.info b/mysql-test/std_data/ bad2_master.info new file mode 100644 index 0000000..6172256 --- /dev/null +++ b/mysql-test/std_data/bad2_master.info @@ -0,0 +1,35 @@ +33 +mysql-bin.000001 +4 +127.0.0.1 +root + +3310 +60 +0 + + + + + +0 +1800.000 + +0 + +0 + + + + + + + + + + + + + +using_gtid=1 +=0 diff --git a/mysql-test/std_data/bad3_master.info b/mysql-test/std_data/ bad3_master.info new file mode 100644 index 0000000..6e632cd --- /dev/null +++ b/mysql-test/std_data/bad3_master.info @@ -0,0 +1,37 @@ +33 +mysql-bin.000001 +4 +127.0.0.1 +root + +3310 +60 +0 + + + + + +0 +1800.000 + +0 + +0 + + + + + + + + + + + + + +using_gtid=1 + + +0 diff --git a/mysql-test/std_data/bad4_master.info b/mysql-test/std_data/ bad4_master.info new file mode 100644 index 0000000..87572ef --- /dev/null +++ b/mysql-test/std_data/bad4_master.info @@ -0,0 +1,35 @@ +33 +mysql-bin.000001 +4 +127.0.0.1 +root + +3310 +60 +0 + + + + + +0 +1800.000 + +0 + +0 + + + + + + + + + + + + + +using_gtid=1 +d=1 diff --git a/mysql-test/std_data/bad5_master.info b/mysql-test/std_data/ bad5_master.info new file mode 100644 index 0000000..4ea8113 --- /dev/null +++ b/mysql-test/std_data/bad5_master.info @@ -0,0 +1,35 @@ +33 +mysql-bin.000001 +4 +127.0.0.1 +root + +3310 +60 +0 + + + + + +0 +1800.000 + +0 + +0 + + + + + + + + + + + + + +using_gtid=1 +using_gtid diff --git a/mysql-test/std_data/bad6_master.info b/mysql-test/std_data/ bad6_master.info new file mode 100644 index 0000000..0f48f48 --- /dev/null +++ b/mysql-test/std_data/bad6_master.info @@ -0,0 +1,36 @@ +33 +mysql-bin.000001 +4 +127.0.0.1 +root + +3310 +60 +0 + + + + + +0 +1800.000 + +0 + +0 + + + + + + + + + + + + + +using_gtid=1 +END_MARKER +do_domain_ids=20 Hulubulu!!?! diff --git a/mysql-test/std_data/bad_master.info b/mysql-test/std_data/ bad_master.info new file mode 100644 index 0000000..1541fdf --- /dev/null +++ b/mysql-test/std_data/bad_master.info @@ -0,0 +1,35 @@ +33 +mysql-bin.000001 +4 +127.0.0.1 +root + +3310 +60 +0 + + + + + +0 +1800.000 + +0 + +0 + + + + + + + + + + + + + +using_gtid=1 + diff --git a/mysql-test/suite/rpl/r/rpl_upgrade_master_info.result b/mysql-test/suite/rpl/r/rpl_upgrade_master_info.result new file mode 100644 index 0000000..f966f18 --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_upgrade_master_info.result @@ -0,0 +1,88 @@ +include/master-slave.inc +[connection master] +*** MDEV-9383: Server fails to read master.info after upgrade 10.0 -> 10.1 *** +include/stop_slave.inc +CHANGE MASTER TO master_use_gtid=CURRENT_POS; +include/rpl_stop_server.inc [server_number=2] +include/rpl_start_server.inc [server_number=2] +CREATE TABLE t1 (a INT PRIMARY KEY); +INSERT INTO t1 VALUES (1); +include/save_master_gtid.inc +CHANGE MASTER TO master_host='127.0.0.1', master_port=SERVER_MYPORT_1; +include/start_slave.inc +include/sync_with_master_gtid.inc +SELECT * FROM t1; +a +1 +include/stop_slave.inc +include/rpl_stop_server.inc [server_number=2] +include/rpl_start_server.inc [server_number=2] +INSERT INTO t1 VALUES (2); +include/save_master_gtid.inc +CHANGE MASTER TO master_host='127.0.0.1', master_port=SERVER_MYPORT_1; +include/start_slave.inc +include/sync_with_master_gtid.inc +SELECT * FROM t1 ORDER BY a; +a +1 +2 +include/stop_slave.inc +include/rpl_stop_server.inc [server_number=2] +include/rpl_start_server.inc [server_number=2] +INSERT INTO t1 VALUES (3); +include/save_master_gtid.inc +CHANGE MASTER TO master_host='127.0.0.1', master_port=SERVER_MYPORT_1; +include/start_slave.inc +include/sync_with_master_gtid.inc +SELECT * FROM t1 ORDER BY a; +a +1 +2 +3 +include/stop_slave.inc +include/rpl_stop_server.inc [server_number=2] +include/rpl_start_server.inc [server_number=2] +INSERT INTO t1 VALUES (4); +include/save_master_gtid.inc +CHANGE MASTER TO master_host='127.0.0.1', master_port=SERVER_MYPORT_1; +include/start_slave.inc +include/sync_with_master_gtid.inc +SELECT * FROM t1 ORDER BY a; +a +1 +2 +3 +4 +include/stop_slave.inc +include/rpl_stop_server.inc [server_number=2] +include/rpl_start_server.inc [server_number=2] +INSERT INTO t1 VALUES (5); +include/save_master_gtid.inc +CHANGE MASTER TO master_host='127.0.0.1', master_port=SERVER_MYPORT_1; +include/start_slave.inc +include/sync_with_master_gtid.inc +SELECT * FROM t1 ORDER BY a; +a +1 +2 +3 +4 +5 +include/stop_slave.inc +include/rpl_stop_server.inc [server_number=2] +include/rpl_start_server.inc [server_number=2] +INSERT INTO t1 VALUES (6); +include/save_master_gtid.inc +CHANGE MASTER TO master_host='127.0.0.1', master_port=SERVER_MYPORT_1; +include/start_slave.inc +include/sync_with_master_gtid.inc +SELECT * FROM t1 ORDER BY a; +a +1 +2 +3 +4 +5 +6 +DROP TABLE t1; +include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_upgrade_master_info.test b/mysql-test/suite/rpl/t/rpl_upgrade_master_info.test new file mode 100644 index 0000000..e81e7c0 --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_upgrade_master_info.test @@ -0,0 +1,163 @@ +--source include/master-slave.inc + +--echo *** MDEV-9383: Server fails to read master.info after upgrade 10.0 -> 10.1 *** + +--connection slave +--source include/stop_slave.inc +CHANGE MASTER TO master_use_gtid=CURRENT_POS; +--let $datadir= `SELECT @@datadir` + +--let $rpl_server_number= 2 +--source include/rpl_stop_server.inc + +--remove_file $datadir/master.info +--copy_file $MYSQL_TEST_DIR/std_data/bad_master.info $datadir/ master.info + +--let $rpl_server_number= 2 +--source include/rpl_start_server.inc + +--source include/wait_until_connected_again.inc + +--connection master +CREATE TABLE t1 (a INT PRIMARY KEY); +INSERT INTO t1 VALUES (1); +--source include/save_master_gtid.inc + +--connection slave +# Fix the port after we replaced master.info. +--replace_result $SERVER_MYPORT_1 SERVER_MYPORT_1 +eval CHANGE MASTER TO master_host='127.0.0.1', master_port=$SERVER_MYPORT_1; +--source include/start_slave.inc +--source include/sync_with_master_gtid.inc +SELECT * FROM t1; + +--source include/stop_slave.inc + +--let $rpl_server_number= 2 +--source include/rpl_stop_server.inc + +--remove_file $datadir/master.info +--copy_file $MYSQL_TEST_DIR/std_data/bad2_master.info $datadir/ master.info + +--let $rpl_server_number= 2 +--source include/rpl_start_server.inc + +--source include/wait_until_connected_again.inc + +--connection master +INSERT INTO t1 VALUES (2); +--source include/save_master_gtid.inc + +--connection slave +# Fix the port after we replaced master.info. +--replace_result $SERVER_MYPORT_1 SERVER_MYPORT_1 +eval CHANGE MASTER TO master_host='127.0.0.1', master_port=$SERVER_MYPORT_1; +--source include/start_slave.inc +--source include/sync_with_master_gtid.inc +SELECT * FROM t1 ORDER BY a; + +--source include/stop_slave.inc + +--let $rpl_server_number= 2 +--source include/rpl_stop_server.inc + +--remove_file $datadir/master.info +--copy_file $MYSQL_TEST_DIR/std_data/bad3_master.info $datadir/ master.info + +--let $rpl_server_number= 2 +--source include/rpl_start_server.inc + +--source include/wait_until_connected_again.inc + +--connection master +INSERT INTO t1 VALUES (3); +--source include/save_master_gtid.inc + +--connection slave +# Fix the port after we replaced master.info. +--replace_result $SERVER_MYPORT_1 SERVER_MYPORT_1 +eval CHANGE MASTER TO master_host='127.0.0.1', master_port=$SERVER_MYPORT_1; +--source include/start_slave.inc +--source include/sync_with_master_gtid.inc +SELECT * FROM t1 ORDER BY a; + +--source include/stop_slave.inc + +--let $rpl_server_number= 2 +--source include/rpl_stop_server.inc + +--remove_file $datadir/master.info +--copy_file $MYSQL_TEST_DIR/std_data/bad4_master.info $datadir/ master.info + +--let $rpl_server_number= 2 +--source include/rpl_start_server.inc + +--source include/wait_until_connected_again.inc + +--connection master +INSERT INTO t1 VALUES (4); +--source include/save_master_gtid.inc + +--connection slave +# Fix the port after we replaced master.info. +--replace_result $SERVER_MYPORT_1 SERVER_MYPORT_1 +eval CHANGE MASTER TO master_host='127.0.0.1', master_port=$SERVER_MYPORT_1; +--source include/start_slave.inc +--source include/sync_with_master_gtid.inc +SELECT * FROM t1 ORDER BY a; + +--source include/stop_slave.inc + +--let $rpl_server_number= 2 +--source include/rpl_stop_server.inc + +--remove_file $datadir/master.info +--copy_file $MYSQL_TEST_DIR/std_data/bad5_master.info $datadir/ master.info + +--let $rpl_server_number= 2 +--source include/rpl_start_server.inc + +--source include/wait_until_connected_again.inc + +--connection master +INSERT INTO t1 VALUES (5); +--source include/save_master_gtid.inc + +--connection slave +# Fix the port after we replaced master.info. +--replace_result $SERVER_MYPORT_1 SERVER_MYPORT_1 +eval CHANGE MASTER TO master_host='127.0.0.1', master_port=$SERVER_MYPORT_1; +--source include/start_slave.inc +--source include/sync_with_master_gtid.inc +SELECT * FROM t1 ORDER BY a; + +--source include/stop_slave.inc + +--let $rpl_server_number= 2 +--source include/rpl_stop_server.inc + +--remove_file $datadir/master.info +--copy_file $MYSQL_TEST_DIR/std_data/bad6_master.info $datadir/ master.info + +--let $rpl_server_number= 2 +--source include/rpl_start_server.inc + +--source include/wait_until_connected_again.inc + +--connection master +INSERT INTO t1 VALUES (6); +--source include/save_master_gtid.inc + +--connection slave +# Fix the port after we replaced master.info. +--replace_result $SERVER_MYPORT_1 SERVER_MYPORT_1 +eval CHANGE MASTER TO master_host='127.0.0.1', master_port=$SERVER_MYPORT_1; +--source include/start_slave.inc +--source include/sync_with_master_gtid.inc +SELECT * FROM t1 ORDER BY a; + + +# Cleanup +--connection master +DROP TABLE t1; +--source include/rpl_end.inc diff --git a/sql/rpl_mi.cc b/sql/rpl_mi.cc index df72134..706824c 100644 --- a/sql/rpl_mi.cc +++ b/sql/rpl_mi.cc @@ -205,43 +205,56 @@ void init_master_log_pos(Master_info* mi)
/** Parses the IO_CACHE for "key=" and returns the "key". + If no '=' found, returns the whole line (for END_MARKER).
@param key [OUT] Key buffer @param max_size [IN] Maximum buffer size @param f [IN] IO_CACHE file + @param found_equal [OUT] Set true if a '=' was found.
@retval 0 Either "key=" or '\n' found @retval 1 EOF */ -static int read_mi_key_from_file(char *key, int max_size, IO_CACHE *f) +static int read_mi_key_from_file(char *key, int max_size, IO_CACHE *f, + bool *found_equal)
^^ Looks like this line is not properly intended.
{ int i= 0, c; - char *last_p;
DBUG_ENTER("read_key_from_file");
- while (((c= my_b_get(f)) != '\n') && (c != my_b_EOF)) + *found_equal= false; + if (max_size <= 0) + DBUG_RETURN(1); + for (;;) { - last_p= key + i; - - if (i < max_size) + if (i >= max_size-1) { - if (c == '=') - { - /* We found '=', replace it by 0 and return. */ - *last_p= 0; - DBUG_RETURN(0); - } - else - *last_p= c; + key[i] = '\0'; + DBUG_RETURN(0); + } + c= my_b_get(f); + if (c == my_b_EOF) + { + DBUG_RETURN(1); + } + else if (c == '\n') + { + key[i]= '\0'; + DBUG_RETURN(0); + } + else if (c == '=') + { + key[i]= '\0'; + *found_equal= true; + DBUG_RETURN(0); + } + else + { + key[i]= c; + ++i; } - ++i; } - - if (c == my_b_EOF) - DBUG_RETURN(1); - - DBUG_RETURN(0); + /* NotReached */ }
enum { @@ -539,6 +552,10 @@ file '%s')", fname); if (lines >= LINE_FOR_LAST_MYSQL_FUTURE) { uint i; + bool got_eq; + bool seen_using_gtid= false; + bool seen_do_domain_ids=false, seen_ignore_domain_ids=false; + /* Skip lines used by / reserved for MySQL >= 5.6. */ for (i= LINE_FOR_FIRST_MYSQL_5_6; i <= LINE_FOR_LAST_MYSQL_FUTURE; ++i) { @@ -551,11 +568,12 @@ file '%s')", fname); for "key=" and returns the "key" if found. The "value" can then the parsed on case by case basis. The "unknown" lines would be ignored to facilitate downgrades. + 10.0 does not have the END_MARKER before any left-overs at the end + of the file. So ignore any but the first occurrence of a key. */ - while (!read_mi_key_from_file(buf, sizeof(buf), &mi->file)) + while (!read_mi_key_from_file(buf, sizeof(buf), &mi->file, &got_eq)) { - /* using_gtid */ - if (!strncmp(buf, STRING_WITH_LEN("using_gtid"))) + if (got_eq && !seen_using_gtid && !strcmp(buf, "using_gtid")) { int val; if (!init_intvar_from_file(&val, &mi->file, 0)) @@ -566,15 +584,13 @@ file '%s')", fname); mi->using_gtid= Master_info::USE_GTID_SLAVE_POS; else mi->using_gtid= Master_info::USE_GTID_NO; - continue; + seen_using_gtid= true; } else { sql_print_error("Failed to initialize master info using_gtid"); goto errwithmsg; } } - - /* DO_DOMAIN_IDS */ - if (!strncmp(buf, STRING_WITH_LEN("do_domain_ids"))) + else if (got_eq && !seen_do_domain_ids && !strcmp(buf, "do_domain_ids")) { if (mi->domain_id_filter.init_ids(&mi->file,
Domain_id_filter::DO_DOMAIN_IDS))
@@ -582,11 +598,10 @@ file '%s')", fname); sql_print_error("Failed to initialize master info do_domain_ids"); goto errwithmsg; } - continue; + seen_do_domain_ids= true; } - - /* IGNORE_DOMAIN_IDS */ - if (!strncmp(buf, STRING_WITH_LEN("ignore_domain_ids"))) + else if (got_eq && !seen_ignore_domain_ids && + !strcmp(buf, "ignore_domain_ids")) { if (mi->domain_id_filter.init_ids(&mi->file,
Domain_id_filter::IGNORE_DOMAIN_IDS))
@@ -595,9 +610,9 @@ file '%s')", fname); "ignore_domain_ids"); goto errwithmsg; } - continue; + seen_ignore_domain_ids= true; } - else if (!strncmp(buf, STRING_WITH_LEN("END_MARKER"))) + else if (!got_eq && !strcmp(buf, "END_MARKER")) { /* Guard agaist extra left-overs at the end of file, in case a later _______________________________________________ commits mailing list commits@mariadb.org https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits