From c58d2c941c49550a85d610b1ad7b51cf0d4cd8db Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Fri, 22 Jan 2021 16:03:07 +1100 Subject: MDEV-20939: Race condition between mysqldump import and InnoDB persistent statistics calculation mysqldump --system=stats and --system=timezones by default used ordinary INSERT statements populate EITS, innodb stats, and timezone tables. As these all have primary keys it could result in conflict. The behavior desired with --system= is to replace the tables. As such we assume --replace for the purposes of stats and timezone tables there if --insert-ignore isn't specified. --- client/mysqldump.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'client/mysqldump.c') diff --git a/client/mysqldump.c b/client/mysqldump.c index 96bfd754d42..2eaec829867 100644 --- a/client/mysqldump.c +++ b/client/mysqldump.c @@ -4640,7 +4640,7 @@ static int dump_all_servers() static int dump_all_stats() { - my_bool prev_no_create_info; + my_bool prev_no_create_info, prev_opt_replace_into; if (mysql_select_db(mysql, "mysql")) { @@ -4648,6 +4648,8 @@ static int dump_all_stats() return 1; /* If --force */ } fprintf(md_result_file,"\nUSE mysql;\n"); + prev_opt_replace_into= opt_replace_into; + opt_replace_into|= !opt_ignore; prev_no_create_info= opt_no_create_info; opt_no_create_info= 1; /* don't overwrite recreate tables */ /* EITS added in 10.0.1 */ @@ -4666,6 +4668,7 @@ static int dump_all_stats() dump_table("innodb_table_stats", "mysql", NULL, 0); } opt_no_create_info= prev_no_create_info; + opt_replace_into= prev_opt_replace_into; return 0; } @@ -4676,12 +4679,14 @@ static int dump_all_stats() static int dump_all_timezones() { - my_bool opt_prev_no_create_info; + my_bool opt_prev_no_create_info, opt_prev_replace_into; if (mysql_select_db(mysql, "mysql")) { DB_error(mysql, "when selecting the database"); return 1; /* If --force */ } + opt_prev_replace_into= opt_replace_into; + opt_replace_into|= !opt_ignore; opt_prev_no_create_info= opt_no_create_info; opt_no_create_info= 1; fprintf(md_result_file,"\nUSE mysql;\n"); @@ -4691,6 +4696,7 @@ static int dump_all_timezones() dump_table("time_zone_transition", "mysql", NULL, 0); dump_table("time_zone_transition_type", "mysql", NULL, 0); opt_no_create_info= opt_prev_no_create_info; + opt_replace_into= opt_prev_replace_into; return 0; } -- cgit v1.2.1 From c207f04eccf2a1f35e6bac2b8146f6c5e0643857 Mon Sep 17 00:00:00 2001 From: Oleksandr Byelkin Date: Wed, 13 Jan 2021 18:36:48 +0100 Subject: MDEV-21785: sequences used as default by other table not dumped in right order by mysqldump Dump sequences first. This atch made to keep it small and to keep number of queries to the server the same. Order of tables in a dump can not be changed (except sequences first) because mysql_list_tables uses SHOW TABLES and I used SHOW FULL TABLES. --- client/mysqldump.c | 79 ++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 62 insertions(+), 17 deletions(-) (limited to 'client/mysqldump.c') diff --git a/client/mysqldump.c b/client/mysqldump.c index a964f96437d..e8601e5f7d5 100644 --- a/client/mysqldump.c +++ b/client/mysqldump.c @@ -42,6 +42,11 @@ /* on merge conflict, bump to a higher version again */ #define DUMP_VERSION "10.19" +/** + First mysql version supporting sequences. +*/ +#define FIRST_SEQUENCE_VERSION 100300 + #include #include #include @@ -92,6 +97,11 @@ /* Max length GTID position that we will output. */ #define MAX_GTID_LENGTH 1024 +/* Dump sequence/tables control */ +#define DUMP_TABLE_ALL -1 +#define DUMP_TABLE_TABLE 0 +#define DUMP_TABLE_SEQUENCE 1 + static my_bool ignore_table_data(const uchar *hash_key, size_t len); static void add_load_option(DYNAMIC_STRING *str, const char *option, const char *option_value); @@ -3876,14 +3886,6 @@ static void dump_table(const char *table, const char *db, const uchar *hash_key, MYSQL_ROW row; DBUG_ENTER("dump_table"); - /* - Check does table has a sequence structure and if has apply different sql queries - */ - if (check_if_ignore_table(table, table_type) & IGNORE_SEQUENCE_TABLE) - { - get_sequence_structure(table, db); - DBUG_VOID_RETURN; - } /* Make sure you get the create table info before the following check for --no-data flag below. Otherwise, the create table info won't be printed. @@ -4368,18 +4370,36 @@ err: } /* dump_table */ -static char *getTableName(int reset) +static char *getTableName(int reset, int want_sequences) { MYSQL_ROW row; if (!get_table_name_result) { - if (!(get_table_name_result= mysql_list_tables(mysql,NullS))) - return(NULL); + if (mysql_get_server_version(mysql) >= FIRST_SEQUENCE_VERSION) + { + const char *query= "SHOW FULL TABLES"; + if (mysql_query_with_error_report(mysql, 0, query)) + return (NULL); + + if (!(get_table_name_result= mysql_store_result(mysql))) + return (NULL); + } + else + { + if (!(get_table_name_result= mysql_list_tables(mysql,NullS))) + return(NULL); + } } if ((row= mysql_fetch_row(get_table_name_result))) - return((char*) row[0]); + { + if (want_sequences != DUMP_TABLE_ALL) + while (row && MY_TEST(strcmp(row[1], "SEQUENCE")) == want_sequences) + row= mysql_fetch_row(get_table_name_result); + if (row) + return((char*) row[0]); + } if (reset) mysql_data_seek(get_table_name_result,0); /* We want to read again */ else @@ -5312,7 +5332,7 @@ static int dump_all_tables_in_db(char *database) { DYNAMIC_STRING query; init_dynamic_string_checked(&query, "LOCK TABLES ", 256, 1024); - for (numrows= 0 ; (table= getTableName(1)) ; ) + for (numrows= 0 ; (table= getTableName(1, DUMP_TABLE_ALL)) ; ) { char *end= strmov(afterdot, table); if (include_table((uchar*) hash_key,end - hash_key)) @@ -5346,7 +5366,19 @@ static int dump_all_tables_in_db(char *database) DBUG_RETURN(1); } } - while ((table= getTableName(0))) + + if (mysql_get_server_version(mysql) >= FIRST_SEQUENCE_VERSION && + !opt_no_create_info) + { + // First process sequences + while ((table= getTableName(1, DUMP_TABLE_SEQUENCE))) + { + char *end= strmov(afterdot, table); + if (include_table((uchar*) hash_key, end - hash_key)) + get_sequence_structure(table, database); + } + } + while ((table= getTableName(0, DUMP_TABLE_TABLE))) { char *end= strmov(afterdot, table); if (include_table((uchar*) hash_key, end - hash_key)) @@ -5495,7 +5527,7 @@ static my_bool dump_all_views_in_db(char *database) { DYNAMIC_STRING query; init_dynamic_string_checked(&query, "LOCK TABLES ", 256, 1024); - for (numrows= 0 ; (table= getTableName(1)); ) + for (numrows= 0 ; (table= getTableName(1, DUMP_TABLE_TABLE)); ) { char *end= strmov(afterdot, table); if (include_table((uchar*) hash_key,end - hash_key)) @@ -5518,7 +5550,7 @@ static my_bool dump_all_views_in_db(char *database) else verbose_msg("-- dump_all_views_in_db : logs flushed successfully!\n"); } - while ((table= getTableName(0))) + while ((table= getTableName(0, DUMP_TABLE_TABLE))) { char *end= strmov(afterdot, table); if (include_table((uchar*) hash_key, end - hash_key)) @@ -5648,7 +5680,7 @@ static int get_sys_var_lower_case_table_names() static int dump_selected_tables(char *db, char **table_names, int tables) { - char table_buff[NAME_LEN*2+3]; + char table_buff[NAME_LEN*2+3], table_type[NAME_LEN]; DYNAMIC_STRING lock_tables_query; char **dump_tables, **pos, **end; int lower_case_table_names; @@ -5745,9 +5777,22 @@ static int dump_selected_tables(char *db, char **table_names, int tables) DBUG_RETURN(1); } } + + if (mysql_get_server_version(mysql) >= FIRST_SEQUENCE_VERSION) + { + /* Dump Sequence first */ + for (pos= dump_tables; pos < end; pos++) + { + DBUG_PRINT("info",("Dumping sequence(?) %s", *pos)); + if (check_if_ignore_table(*pos, table_type) & IGNORE_SEQUENCE_TABLE) + get_sequence_structure(*pos, db); + } + } /* Dump each selected table */ for (pos= dump_tables; pos < end; pos++) { + if (check_if_ignore_table(*pos, table_type) & IGNORE_SEQUENCE_TABLE) + continue; DBUG_PRINT("info",("Dumping table %s", *pos)); dump_table(*pos, db, NULL, 0); if (opt_dump_triggers && -- cgit v1.2.1 From cbc75e9948acd849956aa78e1d31ed4ec350c35f Mon Sep 17 00:00:00 2001 From: Rucha Deodhar Date: Thu, 21 Jan 2021 11:34:05 +0530 Subject: MDEV-20939: Race condition between mysqldump import and InnoDB persistent statistics calculation Analysis: When --replace or --insert-ignore is not given, dumping of mysql.innodb_index_stats and mysql.innodb_table_stats will result into race condition. Fix: Check if these options are present with --system=stats (because dumping under --system=stats is safe). Otherwise, dump only structure, ignoring data because innodb will recalculate data anyway. --- client/mysqldump.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'client/mysqldump.c') diff --git a/client/mysqldump.c b/client/mysqldump.c index 2eaec829867..ecca380777f 100644 --- a/client/mysqldump.c +++ b/client/mysqldump.c @@ -1052,6 +1052,20 @@ static int get_options(int *argc, char ***argv) if ((ho_error= handle_options(argc, argv, my_long_options, get_one_option))) return(ho_error); + /* + Dumping under --system=stats with --replace or --inser-ignore is safe and will not + retult into race condition. Otherwise dump only structure and ignore data by default + while dumping. + */ + if (!(opt_system & OPT_SYSTEM_STATS) && !(opt_ignore || opt_replace_into)) + { + if (my_hash_insert(&ignore_data, + (uchar*) my_strdup("mysql.innodb_index_stats", MYF(MY_WME))) || + my_hash_insert(&ignore_data, + (uchar*) my_strdup("mysql.innodb_table_stats", MYF(MY_WME)))) + return(EX_EOM); + } + if (opt_system & OPT_SYSTEM_ALL) opt_system|= ~0; -- cgit v1.2.1