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, [email protected] wrote: > At http://bazaar.launchpad.net/~maria-captains/maria/10.0 > > ------------------------------------------------------------ > revno: 4245 > revision-id: [email protected] > parent: [email protected] > committer: [email protected] > 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 > [email protected] > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp

