maria-developers team mailing list archive

[Sergey Vojtovich <svoj@xxxxxxxxxxx>]

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@xxxxxxxxxxxxxxx wrote:
> At http://bazaar.launchpad.net/~maria-captains/maria/10.0
> 
> ------------------------------------------------------------
> revno: 4245
> revision-id: knielsen@xxxxxxxxxxxxxxx-20140616093803-gub60pda6t77ch3k
> parent: igor@xxxxxxxxxxxx-20140610223256-nqs0kkp7i45qzyvx
> committer: knielsen@xxxxxxxxxxxxxxx
> 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@xxxxxxxxxxx
> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits