Hi Kristian, MDEV-6344 looks fine. But I don't feel confident reviewing MDEV-6336. Regards, Sergey On Mon, Jun 16, 2014 at 11:38:04AM +0200, knielsen@knielsen-hq.org wrote:

At http://bazaar.launchpad.net/~maria-captains/maria/10.0

------------------------------------------------------------ revno: 4245 revision-id: knielsen@knielsen-hq.org-20140616093803-gub60pda6t77ch3k parent: igor@askmonty.org-20140610223256-nqs0kkp7i45qzyvx committer: knielsen@knielsen-hq.org branch nick: work-10.0 timestamp: Mon 2014-06-16 11:38:03 +0200 message: MDEV-6336: mysqldump --master-data does not work with GTID setups MDEV-6344: mysqldump issues FLUSH TABLES, which gets written into binlog and replicated

Add a --gtid option (for compatibility, the original behaviour is preserved when --gtid is not used).

With --gtid, --master-data and --dump-slave output the GTID position (the old-style file/offset position is still output, but commented out). Also, a CHANGE MASTER TO master_use_gtid=slave_pos is output to ensure a provisioned slave is configured in GTID, as requested.

Without --gtid, the GTID position is still output, if available, but commented out.

Also fix MDEV-6344, to avoid FLUSH TABLES getting into the binlog. Otherwise a mysqldump on a slave server will silently inject a GTID which does not exist on the master, which is highly undesirable. === modified file 'client/client_priv.h' --- a/client/client_priv.h 2013-09-14 01:09:36 +0000 +++ b/client/client_priv.h 2014-06-16 09:38:03 +0000 @@ -92,6 +92,7 @@ enum options_client OPT_REPORT_PROGRESS, OPT_SKIP_ANNOTATE_ROWS_EVENTS, OPT_SSL_CRL, OPT_SSL_CRLPATH, + OPT_USE_GTID, OPT_MAX_CLIENT_OPTION /* should be always the last */ };

=== modified file 'client/mysqldump.c' --- a/client/mysqldump.c 2014-05-09 10:35:11 +0000 +++ b/client/mysqldump.c 2014-06-16 09:38:03 +0000 @@ -134,6 +134,7 @@ static ulong opt_compatible_mode= 0; #define MYSQL_OPT_SLAVE_DATA_COMMENTED_SQL 2 static uint opt_mysql_port= 0, opt_master_data; static uint opt_slave_data; +static uint opt_use_gtid; static uint my_end_arg; static char * opt_mysql_unix_port=0; static int first_error=0; @@ -346,6 +347,13 @@ static struct my_option my_long_options[ {"force", 'f', "Continue even if we get an SQL error.", &ignore_errors, &ignore_errors, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0}, + {"gtid", OPT_USE_GTID, "Used together with --master-data=1 or --dump-slave=1." + "When enabled, the output from those options will set the GTID position " + "instead of the binlog file and offset; the file/offset will appear only as " + "a comment. When disabled, the GTID position will still appear in the " + "output, but only commented.", + &opt_use_gtid, &opt_use_gtid, 0, GET_BOOL, NO_ARG, + 0, 0, 0, 0, 0, 0}, {"help", '?', "Display this help message and exit.", 0, 0, 0, GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0}, {"hex-blob", OPT_HEXBLOB, "Dump binary strings (BINARY, " @@ -1177,7 +1185,7 @@ check_consistent_binlog_pos(char *binlog

if (mysql_query_with_error_report(mysql, &res, "SHOW STATUS LIKE 'binlog_snapshot_%'")) - return 1; + return 0;

found= 0; while ((row= mysql_fetch_row(res))) @@ -1200,6 +1208,90 @@ check_consistent_binlog_pos(char *binlog return (found == 2); }

+ +/* + Get the GTID position corresponding to a given old-style binlog position. + This uses BINLOG_GTID_POS(). The advantage is that the GTID position can + be obtained completely non-blocking in this way (without the need for + FLUSH TABLES WITH READ LOCK), as the old-style position can be obtained + with START TRANSACTION WITH CONSISTENT SNAPSHOT. + + Returns 0 if ok, non-zero if error. +*/ +static int +get_binlog_gtid_pos(char *binlog_pos_file, char *binlog_pos_offset, + char *out_gtid_pos) +{ + DYNAMIC_STRING query; + MYSQL_RES *res; + MYSQL_ROW row; + int err; + char file_buf[FN_REFLEN*2+1], offset_buf[LONGLONG_LEN*2+1]; + size_t len_pos_file= strlen(binlog_pos_file); + size_t len_pos_offset= strlen(binlog_pos_offset); + + if (len_pos_file >= FN_REFLEN || len_pos_offset > LONGLONG_LEN) + return 0; + mysql_real_escape_string(mysql, file_buf, binlog_pos_file, len_pos_file); + mysql_real_escape_string(mysql, offset_buf, binlog_pos_offset, len_pos_offset); + init_dynamic_string_checked(&query, "SELECT BINLOG_GTID_POS('", 256, 1024); + dynstr_append_checked(&query, file_buf); + dynstr_append_checked(&query, "', '"); + dynstr_append_checked(&query, offset_buf); + dynstr_append_checked(&query, "')"); + + err= mysql_query_with_error_report(mysql, &res, query.str); + dynstr_free(&query); + if (err) + return err; + + err= 1; + if ((row= mysql_fetch_row(res))) + { + strmake(out_gtid_pos, row[0], FN_REFLEN-1); + err= 0; + } + mysql_free_result(res); + + return err; +} + + +/* + Get the GTID position on a master or slave. + The parameter MASTER is non-zero to get the position on a master + (@@gtid_binlog_pos) or zero for a slave (@@gtid_slave_pos). + + This uses the @@gtid_binlog_pos or @@gtid_slave_pos, so requires FLUSH TABLES + WITH READ LOCK or similar to be consistent. + + Returns 0 if ok, non-zero for error. +*/ +static int +get_gtid_pos(char *out_gtid_pos, int master) +{ + MYSQL_RES *res; + MYSQL_ROW row; + int found; + + if (mysql_query_with_error_report(mysql, &res, + (master ? + "SELECT @@GLOBAL.gtid_binlog_pos" : + "SELECT @@GLOBAL.gtid_slave_pos"))) + return 1; + + found= 0; + if ((row= mysql_fetch_row(res))) + { + strmake(out_gtid_pos, row[0], FN_REFLEN-1); + found++; + } + mysql_free_result(res); + + return (found != 1); +} + + static char *my_case_str(const char *str, uint str_len, const char *token, @@ -4799,12 +4891,14 @@ static int dump_selected_tables(char *db } /* dump_selected_tables */

-static int do_show_master_status(MYSQL *mysql_con, int consistent_binlog_pos) +static int do_show_master_status(MYSQL *mysql_con, int consistent_binlog_pos, + int have_mariadb_gtid, int use_gtid) { MYSQL_ROW row; MYSQL_RES *UNINIT_VAR(master); char binlog_pos_file[FN_REFLEN]; char binlog_pos_offset[LONGLONG_LEN+1]; + char gtid_pos[FN_REFLEN]; char *file, *offset; const char *comment_prefix= (opt_master_data == MYSQL_OPT_MASTER_DATA_COMMENTED_SQL) ? "-- " : ""; @@ -4815,6 +4909,9 @@ static int do_show_master_status(MYSQL * return 1; file= binlog_pos_file; offset= binlog_pos_offset; + if (have_mariadb_gtid && + get_binlog_gtid_pos(binlog_pos_file, binlog_pos_offset, gtid_pos)) + return 1; } else { @@ -4844,6 +4941,9 @@ static int do_show_master_status(MYSQL * return 0; } } + + if (have_mariadb_gtid && get_gtid_pos(gtid_pos, 1)) + return 1; }

/* SHOW MASTER STATUS reports file and position */ @@ -4852,7 +4952,19 @@ static int do_show_master_status(MYSQL * "recovery from\n--\n\n"); fprintf(md_result_file, "%sCHANGE MASTER TO MASTER_LOG_FILE='%s', MASTER_LOG_POS=%s;\n", - comment_prefix, file, offset); + (use_gtid ? "-- " : comment_prefix), file, offset); + if (have_mariadb_gtid) + { + print_comment(md_result_file, 0, + "\n--\n-- GTID to start replication from\n--\n\n"); + if (use_gtid) + fprintf(md_result_file, + "%sCHANGE MASTER TO MASTER_USE_GTID=slave_pos;\n", + comment_prefix); + fprintf(md_result_file, + "%sSET GLOBAL gtid_slave_pos='%s';\n", + (!use_gtid ? "-- " : comment_prefix), gtid_pos); + } check_io(md_result_file);

if (!consistent_binlog_pos) @@ -4922,12 +5034,16 @@ static int add_slave_statements(void) return(0); }

-static int do_show_slave_status(MYSQL *mysql_con) +static int do_show_slave_status(MYSQL *mysql_con, int use_gtid, + int have_mariadb_gtid) { MYSQL_RES *UNINIT_VAR(slave); MYSQL_ROW row; const char *comment_prefix= (opt_slave_data == MYSQL_OPT_SLAVE_DATA_COMMENTED_SQL) ? "-- " : ""; + const char *gtid_comment_prefix= (use_gtid ? comment_prefix : "-- "); + const char *nogtid_comment_prefix= (!use_gtid ? comment_prefix : "-- "); + int set_gtid_done= 0;

if (mysql_query_with_error_report(mysql_con, &slave, multi_source ? @@ -4945,8 +5061,30 @@ static int do_show_slave_status(MYSQL *m

while ((row= mysql_fetch_row(slave))) { + if (multi_source && !set_gtid_done) + { + char gtid_pos[FN_REFLEN]; + if (have_mariadb_gtid && get_gtid_pos(gtid_pos, 0)) + return 1; + if (opt_comments) + fprintf(md_result_file, "\n--\n-- Gtid position to start replication " + "from\n--\n\n"); + fprintf(md_result_file, "%sSET GLOBAL gtid_slave_pos='%s';\n", + gtid_comment_prefix, gtid_pos); + set_gtid_done= 1; + } if (row[9 + multi_source] && row[21 + multi_source]) { + if (use_gtid) + { + if (multi_source) + fprintf(md_result_file, "%sCHANGE MASTER '%.80s' TO " + "MASTER_USE_GTID=slave_pos;\n", gtid_comment_prefix, row[0]); + else + fprintf(md_result_file, "%sCHANGE MASTER TO " + "MASTER_USE_GTID=slave_pos;\n", gtid_comment_prefix); + } + /* SHOW MASTER STATUS reports file and position */ if (opt_comments) fprintf(md_result_file, @@ -4955,9 +5093,9 @@ static int do_show_slave_status(MYSQL *m

if (multi_source) fprintf(md_result_file, "%sCHANGE MASTER '%.80s' TO ", - comment_prefix, row[0]); + nogtid_comment_prefix, row[0]); else - fprintf(md_result_file, "%sCHANGE MASTER TO ", comment_prefix); + fprintf(md_result_file, "%sCHANGE MASTER TO ", nogtid_comment_prefix);

if (opt_include_master_host_port) { @@ -5030,12 +5168,13 @@ static int do_flush_tables_read_lock(MYS FLUSH TABLES is to lower the probability of a stage where both mysqldump and most client connections are stalled. Of course, if a second long update starts between the two FLUSHes, we have that bad stall. + + We use the LOCAL option, as we do not want the FLUSH TABLES replicated to + other servers. */ return - ( mysql_query_with_error_report(mysql_con, 0, - ((opt_master_data != 0) ? - "FLUSH /*!40101 LOCAL */ TABLES" : - "FLUSH TABLES")) || + ( mysql_query_with_error_report(mysql_con, 0, + "FLUSH /*!40101 LOCAL */ TABLES") || mysql_query_with_error_report(mysql_con, 0, "FLUSH TABLES WITH READ LOCK") ); } @@ -5656,6 +5795,7 @@ int main(int argc, char **argv) char bin_log_name[FN_REFLEN]; int exit_code; int consistent_binlog_pos= 0; + int have_mariadb_gtid= 0; MY_INIT(argv[0]);

sf_leaking_memory=1; /* don't report memory leaks on early exits */ @@ -5696,7 +5836,10 @@ int main(int argc, char **argv)

/* Check if the server support multi source */ if (mysql_get_server_version(mysql) >= 100000) + { multi_source= 2; + have_mariadb_gtid= 1; + }

if (opt_slave_data && do_stop_slave_sql(mysql)) goto err; @@ -5743,9 +5886,11 @@ int main(int argc, char **argv) /* Add 'STOP SLAVE to beginning of dump */ if (opt_slave_apply && add_stop_slave()) goto err; - if (opt_master_data && do_show_master_status(mysql, consistent_binlog_pos)) + if (opt_master_data && do_show_master_status(mysql, consistent_binlog_pos, + have_mariadb_gtid, opt_use_gtid)) goto err; - if (opt_slave_data && do_show_slave_status(mysql)) + if (opt_slave_data && do_show_slave_status(mysql, opt_use_gtid, + have_mariadb_gtid)) goto err; if (opt_single_transaction && do_unlock_tables(mysql)) /* unlock but no commit! */ goto err;

=== modified file 'mysql-test/r/rpl_mysqldump_slave.result' --- a/mysql-test/r/rpl_mysqldump_slave.result 2014-02-26 14:28:07 +0000 +++ b/mysql-test/r/rpl_mysqldump_slave.result 2014-06-16 09:38:03 +0000 @@ -4,18 +4,59 @@ include/master-slave.inc # New --dump-slave, --apply-slave-statements functionality # use test; +-- SET GLOBAL gtid_slave_pos=''; CHANGE MASTER '' TO MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START; STOP ALL SLAVES; +-- SET GLOBAL gtid_slave_pos=''; CHANGE MASTER '' TO MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START; START ALL SLAVES; STOP ALL SLAVES; +-- SET GLOBAL gtid_slave_pos=''; CHANGE MASTER '' TO MASTER_HOST='127.0.0.1', MASTER_PORT=MASTER_MYPORT, MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START; START ALL SLAVES; start slave; Warnings: Note 1254 Slave is already running +-- SET GLOBAL gtid_slave_pos=''; CHANGE MASTER '' TO MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START; start slave; Warnings: Note 1254 Slave is already running +*** Test mysqldump --dump-slave GTID functionality. +SET gtid_seq_no = 1000; +CREATE TABLE t1 (a INT PRIMARY KEY); +DROP TABLE t1; +CREATE TABLE t2 (a INT PRIMARY KEY); +DROP TABLE t2; + +1. --dump-slave=1 + +SET GLOBAL gtid_slave_pos='0-1-1001'; +CHANGE MASTER '' TO MASTER_USE_GTID=slave_pos; +-- CHANGE MASTER '' TO MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START; + +2. --dump-slave=2 + +-- SET GLOBAL gtid_slave_pos='0-1-1001'; +-- CHANGE MASTER '' TO MASTER_USE_GTID=slave_pos; +-- CHANGE MASTER '' TO MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START; +*** Test mysqldump --master-data GTID functionality. + +1. --master-data=1 + +-- CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START; +CHANGE MASTER TO MASTER_USE_GTID=slave_pos; +SET GLOBAL gtid_slave_pos='0-2-1003'; + +2. --master-data=2 + +-- CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START; +-- CHANGE MASTER TO MASTER_USE_GTID=slave_pos; +-- SET GLOBAL gtid_slave_pos='0-2-1003'; + +3. --master-data --single-transaction + +-- CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START; +CHANGE MASTER TO MASTER_USE_GTID=slave_pos; +SET GLOBAL gtid_slave_pos='0-2-1003'; include/rpl_end.inc

=== modified file 'mysql-test/t/rpl_mysqldump_slave.test' --- a/mysql-test/t/rpl_mysqldump_slave.test 2014-02-26 14:28:07 +0000 +++ b/mysql-test/t/rpl_mysqldump_slave.test 2014-06-16 09:38:03 +0000 @@ -36,4 +36,53 @@ start slave; --exec $MYSQL_DUMP_SLAVE --compact --dump-slave no_such_db start slave;

+ +--echo *** Test mysqldump --dump-slave GTID functionality. + +--connection master +SET gtid_seq_no = 1000; +CREATE TABLE t1 (a INT PRIMARY KEY); +DROP TABLE t1; +--sync_slave_with_master + +--connection slave +# Inject a local transaction on the slave to check that this is not considered +# for --dump-slave. +CREATE TABLE t2 (a INT PRIMARY KEY); +DROP TABLE t2; + +--echo +--echo 1. --dump-slave=1 +--echo +--replace_regex /MASTER_LOG_POS=[0-9]+/MASTER_LOG_POS=BINLOG_START/ +--exec $MYSQL_DUMP_SLAVE --compact --dump-slave=1 --gtid test + +--echo +--echo 2. --dump-slave=2 +--echo +--replace_regex /MASTER_LOG_POS=[0-9]+/MASTER_LOG_POS=BINLOG_START/ +--exec $MYSQL_DUMP_SLAVE --compact --dump-slave=2 --gtid test + + +--echo *** Test mysqldump --master-data GTID functionality. +--echo +--echo 1. --master-data=1 +--echo +--replace_regex /MASTER_LOG_POS=[0-9]+/MASTER_LOG_POS=BINLOG_START/ +--exec $MYSQL_DUMP_SLAVE --compact --master-data=1 --gtid test + +--echo +--echo 2. --master-data=2 +--echo +--replace_regex /MASTER_LOG_POS=[0-9]+/MASTER_LOG_POS=BINLOG_START/ +--exec $MYSQL_DUMP_SLAVE --compact --master-data=2 --gtid test + +--echo +--echo 3. --master-data --single-transaction +--echo +--replace_regex /MASTER_LOG_POS=[0-9]+/MASTER_LOG_POS=BINLOG_START/ +--exec $MYSQL_DUMP_SLAVE --compact --master-data --single-transaction --gtid test + + + --source include/rpl_end.inc

_______________________________________________ commits mailing list commits@mariadb.org https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits