maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #09473
Re: [Commits] ca24c9d: MDEV-9383: Server fails to read master.info after upgrade 10.0 -> 10.1
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@xxxxxxxxxxxxxxx> 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)
> {
> 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@xxxxxxxxxxx
> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits
Follow ups
