From 0d2fa1334c924f18a676928d1d8d5d43abc3933f Mon Sep 17 00:00:00 2001 From: Aibek Bukabayev Date: Thu, 15 Feb 2024 15:32:48 +0600 Subject: [PATCH] modifications --- storage/innobase/fil/fil0fil.cc | 27 ++- .../innobase/xtrabackup/src/ddl_tracker.cc | 62 ++++- storage/innobase/xtrabackup/src/ddl_tracker.h | 10 + storage/innobase/xtrabackup/src/xtrabackup.cc | 223 +++++++++++++----- .../ddl_between_discovery_and_file_open.sh | 62 +++++ .../suites/lockless/rename_then_drop_table.sh | 95 ++++++++ 6 files changed, 402 insertions(+), 77 deletions(-) create mode 100644 storage/innobase/xtrabackup/test/suites/lockless/ddl_between_discovery_and_file_open.sh create mode 100644 storage/innobase/xtrabackup/test/suites/lockless/rename_then_drop_table.sh diff --git a/storage/innobase/fil/fil0fil.cc b/storage/innobase/fil/fil0fil.cc index aa292b536f58..d491f74d7e69 100644 --- a/storage/innobase/fil/fil0fil.cc +++ b/storage/innobase/fil/fil0fil.cc @@ -11593,7 +11593,6 @@ void Tablespace_dirs::open_ibd(const Const_iter &start, const Const_iter &end, size_t thread_id, bool &result) { if (!result) return; - uint8_t max_attempt_retries = (opt_lock_ddl == LOCK_DDL_REDUCED) ? 10 : 1; for (auto it = start; it != end; ++it) { const std::string filename = it->second; const auto &files = m_dirs[it->first]; @@ -11606,17 +11605,19 @@ void Tablespace_dirs::open_ibd(const Const_iter &start, const Const_iter &end, /* cannot use auto [err, space_id] = fil_open_for_xtrabackup() as space_id is unused here and we get unused variable error during compilation */ dberr_t err; - uint8_t attempts = 0; - while (attempts < max_attempt_retries) { - attempts++; - std::tie(err, std::ignore) = fil_open_for_xtrabackup( - phy_filename, filename.substr(0, filename.length() - 4)); - /* PXB-2275 - Allow DB_INVALID_ENCRYPTION_META as we will test it in the - end of the backup */ - if (err == DB_SUCCESS || err == DB_INVALID_ENCRYPTION_META) break; - - if (attempts == max_attempt_retries) result = false; - } + std::tie(err, std::ignore) = fil_open_for_xtrabackup( + phy_filename, filename.substr(0, filename.length() - 4)); + + /* Allow deleted tables between disovery and file open when + LOCK_DDL_REDUCED, they will be handled by ddl_tracker */ + if (err == DB_CANNOT_OPEN_FILE && opt_lock_ddl == LOCK_DDL_REDUCED) { + ddl_tracker->add_missing_table(phy_filename); + } else + /* PXB-2275 - Allow DB_INVALID_ENCRYPTION_META as we will test it in + the end of the backup */ + if (err != DB_SUCCESS && err != DB_INVALID_ENCRYPTION_META) { + result = false; + } } } @@ -11999,6 +12000,8 @@ dberr_t Tablespace_dirs::scan(bool populate_fil_cache) { err = DB_SUCCESS; } + debug_sync_point("xtrabackup_suspend_between_file_discovery_and_open"); + if (err == DB_SUCCESS && populate_fil_cache) { bool result = true; std::function diff --git a/storage/innobase/xtrabackup/src/ddl_tracker.cc b/storage/innobase/xtrabackup/src/ddl_tracker.cc index 80057522ab29..8151d90788fb 100644 --- a/storage/innobase/xtrabackup/src/ddl_tracker.cc +++ b/storage/innobase/xtrabackup/src/ddl_tracker.cc @@ -121,6 +121,21 @@ void ddl_tracker_t::add_table(const space_id_t &space_id, std::string name) { tables_in_backup[space_id] = name; } +void ddl_tracker_t::add_missing_table(std::string path) { + Fil_path::normalize(path); + if (Fil_path::has_prefix(path, Fil_path::DOT_SLASH)) { + path.erase(0, 2); + } + missing_tables.insert(path); +} + +bool ddl_tracker_t::is_missing_table(const std::string &name) { + if (missing_tables.count(name)) { + return true; + } + return false; +} + /* ======== Data copying thread context ======== */ typedef struct { @@ -167,6 +182,16 @@ static void data_copy_thread_func(copy_thread_ctxt_t *ctxt) { my_thread_end(); } +/* returns .del or .ren file name starting with space_id + like schema/spaceid.ibd.del +*/ +std::string ddl_tracker_t::convert_file_name(space_id_t space_id, + std::string file_name, + std::string ext) { + auto sep_pos = file_name.find_last_of(Fil_path::SEPARATOR); + return file_name.substr(0, sep_pos + 1) + std::to_string(space_id) + ext; +} + void ddl_tracker_t::handle_ddl_operations() { xb::info() << "DDL tracking : handling DDL operations"; @@ -197,7 +222,9 @@ void ddl_tracker_t::handle_ddl_operations() { for (auto &table : recopy_tables) { if (tables_in_backup.find(table) != tables_in_backup.end()) { if (renames.find(table) != renames.end()) { - backup_file_printf((renames[table].first + ".del").c_str(), "%s", ""); + backup_file_printf( + convert_file_name(table, renames[table].first, ".ibd.del").c_str(), + "%s", ""); } string name = tables_in_backup[table]; new_tables[table] = name; @@ -216,7 +243,15 @@ void ddl_tracker_t::handle_ddl_operations() { new_tables.erase(table.first); continue; } - backup_file_printf((table.second + ".del").c_str(), "%s", ""); + + /* Table not in the backup, nothing to drop, skip drop*/ + if (tables_in_backup.find(table.first) == tables_in_backup.end()) { + continue; + } + + backup_file_printf( + convert_file_name(table.first, table.second, ".ibd.del").c_str(), "%s", + ""); } for (auto &table : renames) { @@ -226,13 +261,28 @@ void ddl_tracker_t::handle_ddl_operations() { if (check_if_skip_table(table.second.first.c_str())) { continue; } - /* renamed new table. update new table entry to renamed table name */ - if (new_tables.find(table.first) != new_tables.end()) { + /* renamed new table. update new table entry to renamed table name + or if table is missing and renamed, add the renamed table to the new_table + list. for example: 1. t1.ibd is discovered + 2. t1.ibd renamed to t2.ibd + 3. t2.ibd is opened and loaded to cache to copy + 4. t1.ibd is missing now + so we should add t2.ibd to new_tables and skip .ren file so that we don't + try to rename t1.ibd to t2.idb where t1.ibd is missing */ + if (new_tables.find(table.first) != new_tables.end() || + is_missing_table(table.second.first)) { new_tables[table.first] = table.second.second; continue; } - backup_file_printf((table.second.first + ".ren").c_str(), "%s", - table.second.second.c_str()); + + /* Table not in the backup, nothing to rename, skip rename*/ + if (tables_in_backup.find(table.first) == tables_in_backup.end()) { + continue; + } + + backup_file_printf( + convert_file_name(table.first, table.second.first, ".ibd.ren").c_str(), + "%s", table.second.second.c_str()); } fil_close_all_files(); diff --git a/storage/innobase/xtrabackup/src/ddl_tracker.h b/storage/innobase/xtrabackup/src/ddl_tracker.h index 36f6e3c4e6a9..10dd30a29037 100644 --- a/storage/innobase/xtrabackup/src/ddl_tracker.h +++ b/storage/innobase/xtrabackup/src/ddl_tracker.h @@ -35,6 +35,8 @@ class ddl_tracker_t { space_id_to_name_t drops; /* For DDL operation found in redo log, */ std::unordered_map> renames; + /** Tables that have been deleted between discovery and file open */ + std::unordered_set missing_tables; public: /** Add a new table in the DDL tracker table list. @@ -52,5 +54,13 @@ class ddl_tracker_t { ulint len, lsn_t start_lsn); /** Function responsible to generate files based on DDL operations */ void handle_ddl_operations(); + /** Note that a table has been deleted between disovery and file open + @param[in] path missing table name with path. */ + void add_missing_table(std::string path); + /** Check if table is in missing list + @param[in] name tablespace name */ + bool is_missing_table(const std::string &name); + std::string convert_file_name(space_id_t space_id, std::string file_name, + std::string ext); }; #endif // DDL_TRACKER_H diff --git a/storage/innobase/xtrabackup/src/xtrabackup.cc b/storage/innobase/xtrabackup/src/xtrabackup.cc index ce1fdc1efa0e..b9c501123108 100644 --- a/storage/innobase/xtrabackup/src/xtrabackup.cc +++ b/storage/innobase/xtrabackup/src/xtrabackup.cc @@ -5486,59 +5486,141 @@ static std::string read_file_as_string(const std::string file) { fclose(f); return std::string(content, len); } -/* Handle DDL for renamed files */ + +/** +Handle DDL for renamed files +example input: test/10.ibd.ren file with content = test/new_name.ibd ; + -> tablespace with space_id = 10 will be renamed to test/new_name.ibd +@return true on success */ static bool prepare_handle_ren_files( const datadir_entry_t &entry, /*!id, &space_name, &oldpath); + + if (!res || !os_file_exists(oldpath)) { + xb::error() << "prepare_handle_ren_files: Tablespace " << fil_space->name + << " not found."; + ut::free(oldpath); + ut::free(space_name); + return false; + } + + strncpy(tmpname, dest_space_name, sizeof(tmpname) - 1); + + xb::info() << "prepare_handle_ren_files: renaming " << fil_space->name + << " to " << dest_space_name; + + if (!fil_rename_tablespace(fil_space->id, oldpath, tmpname, NULL)) { + xb::error() << "prepare_handle_ren_files: Cannot rename " + << fil_space->name << " to " << dest_space_name; + ut::free(oldpath); + ut::free(space_name); + return false; + } + // rename .delta .meta files as well + if (xtrabackup_incremental) { + std::string from_path = + entry.datadir + OS_PATH_SEPARATOR + std::string{space_name}; + ; + std::string to_path = entry.datadir + ren_file_content; + + rename_force(from_path + ".ibd.delta", to_path + ".delta"); + rename_force(from_path + ".ibd.meta", to_path + ".meta"); + } + // delete the .ren file, we don't need it anymore + os_file_delete(0, ren_path.c_str()); + ut::free(oldpath); + ut::free(space_name); + + return true; } - os_file_delete(0, ren_file.c_str()); - return true; + + xb::error() << "prepare_handle_ren_files(): failed to handle " << ren_path; + return false; } -/* Handle DDL for deleted files */ + +/** +Handle DDL for deleted files +example input: test/10.ibd.del file + -> tablespace with space_id = 10 will be deleted +@return true on success */ static bool prepare_handle_del_files( const datadir_entry_t &entry, /*!id, &space_name, &path); + + if (!res || !os_file_exists(path)) { + xb::error() << "prepare_handle_del_files: Tablespace " << fil_space->name + << " not found."; + ut::free(path); + ut::free(space_name); + return false; + } + + xb::info() << "prepare_handle_del_files: deleting " << fil_space->name; + + dberr_t err = fil_delete_tablespace(fil_space->id, BUF_REMOVE_NONE); + if (err != DB_SUCCESS) { + xb::error() << "prepare_handle_del_files: Cannot delete " + << fil_space->name; + ut::free(path); + ut::free(space_name); + return false; + } + + os_file_delete(0, del_file.c_str()); + if (xtrabackup_incremental) { + std::string del_path = + entry.datadir + OS_PATH_SEPARATOR + std::string{space_name}; + delete_force(del_path + ".ibd.delta"); + delete_force(del_path + ".ibd.meta"); + } + return true; + } else { + // if table was already deleted then return true + os_file_delete(0, del_file.c_str()); + return true; + } } /************************************************************************ Callback to handle datadir entry. Deletes entry if it has no matching @@ -6619,27 +6701,6 @@ static void xtrabackup_prepare_func(int argc, char **argv) { backup_redo_log_flushed_lsn = incremental_flushed_lsn; } - /* Handle DDL files produced by DDL tracking during backup */ - xb_process_datadir( - xtrabackup_incremental_dir ? xtrabackup_incremental_dir : ".", ".del", - prepare_handle_del_files, NULL); - xb_process_datadir( - xtrabackup_incremental_dir ? xtrabackup_incremental_dir : ".", ".ren", - prepare_handle_ren_files, NULL); - if (xtrabackup_incremental_dir) { - /** This is the new file, this might be less than the original .ibd because - * we are copying the file while there are still dirty pages in the BP. - * Those changes will later be conciliated via redo log*/ - xb_process_datadir(xtrabackup_incremental_dir, ".new.meta", - prepare_handle_new_files, NULL); - xb_process_datadir(xtrabackup_incremental_dir, ".new.delta", - prepare_handle_new_files, NULL); - xb_process_datadir(xtrabackup_incremental_dir, ".new", - prepare_handle_new_files, NULL); - } else { - xb_process_datadir(".", ".new", prepare_handle_new_files, NULL); - } - init_mysql_environment(); my_thread_init(); THD *thd = create_internal_thd(); @@ -6715,6 +6776,50 @@ static void xtrabackup_prepare_func(int argc, char **argv) { Tablespace_map::instance().deserialize("./"); + /* Handle `RENAME/DELETE` DDL files produced by DDL tracking during backup */ + err = xb_data_files_init(); + if (err != DB_SUCCESS) { + xb::error() << "xb_data_files_init() failed " + << "with error code " << err; + goto error_cleanup; + } + + if (!xb_process_datadir( + xtrabackup_incremental_dir ? xtrabackup_incremental_dir : ".", ".ren", + prepare_handle_ren_files, NULL)) { + xb_data_files_close(); + goto error_cleanup; + } + if (!xb_process_datadir( + xtrabackup_incremental_dir ? xtrabackup_incremental_dir : ".", ".del", + prepare_handle_del_files, NULL)) { + xb_data_files_close(); + goto error_cleanup; + } + + xb_data_files_close(); + fil_close(); + innodb_free_param(); + + /* Handle `CREATE` DDL files produced by DDL tracking during backup */ + if (xtrabackup_incremental_dir) { + /** This is the new file, this might be less than the original .ibd because + * we are copying the file while there are still dirty pages in the BP. + * Those changes will later be conciliated via redo log*/ + xb_process_datadir(xtrabackup_incremental_dir, ".new.meta", + prepare_handle_new_files, NULL); + xb_process_datadir(xtrabackup_incremental_dir, ".new.delta", + prepare_handle_new_files, NULL); + xb_process_datadir(xtrabackup_incremental_dir, ".new", + prepare_handle_new_files, NULL); + } else { + xb_process_datadir(".", ".new", prepare_handle_new_files, NULL); + } + + if (innodb_init_param()) { + goto error_cleanup; + } + if (xtrabackup_incremental) { Tablespace_map::instance().deserialize(xtrabackup_incremental_dir); err = xb_data_files_init(); diff --git a/storage/innobase/xtrabackup/test/suites/lockless/ddl_between_discovery_and_file_open.sh b/storage/innobase/xtrabackup/test/suites/lockless/ddl_between_discovery_and_file_open.sh new file mode 100644 index 000000000000..fdd2192703c4 --- /dev/null +++ b/storage/innobase/xtrabackup/test/suites/lockless/ddl_between_discovery_and_file_open.sh @@ -0,0 +1,62 @@ +############################################################################### +# PXB-3220: Tolerate file deletion/rename between discovery and file open +############################################################################### + +. inc/common.sh + +require_debug_pxb_version + +start_server + +$MYSQL $MYSQL_ARGS -Ns -e "CREATE TABLE test.drop_table (id INT PRIMARY KEY AUTO_INCREMENT); INSERT INTO test.drop_table VALUES(1);" test +$MYSQL $MYSQL_ARGS -Ns -e "CREATE TABLE test.rename_table (id INT PRIMARY KEY AUTO_INCREMENT); INSERT INTO test.rename_table VALUES(1);" test +$MYSQL $MYSQL_ARGS -Ns -e "CREATE TABLE test.alter_rename_table (id INT PRIMARY KEY AUTO_INCREMENT); INSERT INTO test.alter_rename_table VALUES(1)" test +$MYSQL $MYSQL_ARGS -Ns -e "CREATE TABLE test.alter_drop_column_table (id INT PRIMARY KEY AUTO_INCREMENT, name VARCHAR(50)); INSERT INTO test.alter_drop_column_table VALUES(1, 'test')" test + +innodb_wait_for_flush_all + +xtrabackup --backup --target-dir=$topdir/backup \ + --debug-sync="xtrabackup_suspend_between_file_discovery_and_open" --lock-ddl=REDUCED \ + 2> >( tee $topdir/backup.log)& + +job_pid=$! +pid_file=$topdir/backup/xtrabackup_debug_sync +wait_for_xb_to_suspend $pid_file +xb_pid=`cat $pid_file` +echo "backup pid is $job_pid" + +# Delete table +$MYSQL $MYSQL_ARGS -Ns -e "DROP TABLE test.drop_table;" test +# Rename table and generate redo +$MYSQL $MYSQL_ARGS -Ns -e "RENAME TABLE test.rename_table TO test.new_rename_table; INSERT INTO test.new_rename_table VALUES(2);" test +# Alter table rename and generate redo +$MYSQL $MYSQL_ARGS -Ns -e "ALTER TABLE test.alter_rename_table RENAME test.new_alter_rename_table; INSERT INTO test.new_alter_rename_table VALUES(2);" test +# Alter table drop column and generate redo +$MYSQL $MYSQL_ARGS -Ns -e "ALTER TABLE test.alter_drop_column_table DROP COLUMN name; INSERT INTO test.alter_drop_column_table VALUES(2);" test + +# Resume the xtrabackup process +vlog "Resuming xtrabackup" +kill -SIGCONT $xb_pid +run_cmd wait $job_pid + +if ! egrep -q "DDL tracking : LSN: [0-9]* delete table ID: [0-9]* Name: test/drop_table.ibd" $topdir/backup.log ; then + die "xtrabackup did not handle delete table DDL" +fi + +if ! egrep -q "DDL tracking : LSN: [0-9]* rename table ID: [0-9]* From: test/rename_table.ibd To: test/new_rename_table.ibd" $topdir/backup.log ; then + die "xtrabackup did not handle rename table DDL" +fi + +if ! egrep -q "DDL tracking : LSN: [0-9]* rename table ID: [0-9]* From: test/alter_rename_table.ibd To: test/new_alter_rename_table.ibd" $topdir/backup.log ; then + die "xtrabackup did not handle alter table rename DDL" +fi + +xtrabackup --prepare --target-dir=$topdir/backup +record_db_state test +stop_server +rm -rf $mysql_datadir/* +xtrabackup --copy-back --target-dir=$topdir/backup +start_server +verify_db_state test +stop_server +rm -rf $mysql_datadir $topdir/backup diff --git a/storage/innobase/xtrabackup/test/suites/lockless/rename_then_drop_table.sh b/storage/innobase/xtrabackup/test/suites/lockless/rename_then_drop_table.sh new file mode 100644 index 000000000000..a2ce6cc3bf2c --- /dev/null +++ b/storage/innobase/xtrabackup/test/suites/lockless/rename_then_drop_table.sh @@ -0,0 +1,95 @@ +######################################################################################### +# PXB-3227: Rename table and then drop table, make sure that original table was deleted +######################################################################################### + +. inc/common.sh + +require_debug_pxb_version +start_server + +$MYSQL $MYSQL_ARGS -Ns -e "CREATE TABLE test.original_table (id INT PRIMARY KEY AUTO_INCREMENT); INSERT INTO test.original_table VALUES(1);" test +innodb_wait_for_flush_all + +xtrabackup --backup --target-dir=$topdir/backup \ + --debug-sync="xtrabackup_suspend_at_start" --lock-ddl=REDUCED \ + 2> >( tee $topdir/backup.log)& + +job_pid=$! +pid_file=$topdir/backup/xtrabackup_debug_sync +wait_for_xb_to_suspend $pid_file +xb_pid=`cat $pid_file` +echo "backup pid is $job_pid" + +# Rename table and generate redo +$MYSQL $MYSQL_ARGS -Ns -e "RENAME TABLE test.original_table TO test.renamed_table; INSERT INTO test.renamed_table VALUES (2);" test +# Drop table +$MYSQL $MYSQL_ARGS -Ns -e "DROP TABLE test.renamed_table;" test + +# Resume the xtrabackup process +vlog "Resuming xtrabackup" +kill -SIGCONT $xb_pid +run_cmd wait $job_pid + +xtrabackup --prepare --target-dir=$topdir/backup +record_db_state test +stop_server +rm -rf $mysql_datadir/* +xtrabackup --copy-back --target-dir=$topdir/backup +start_server +verify_db_state test + +if [ -f $mysql_datadir/test/original_table.ibd ] ; then + die 'dropped original_table.ibd file has been copied to the datadir' +fi + +stop_server +rm -rf $topdir/backup + +######################################################################################################### +# PXB-3229: Make sure backup does not fail with `File exists` error trying to create .del file twice +######################################################################################################### + +start_server + +$MYSQL $MYSQL_ARGS -Ns -e "CREATE TABLE test.t1 (id INT PRIMARY KEY AUTO_INCREMENT); INSERT INTO test.t1 VALUES(1);" test +$MYSQL $MYSQL_ARGS -Ns -e "CREATE TABLE test.t2 (id INT PRIMARY KEY AUTO_INCREMENT); INSERT INTO test.t2 VALUES(1);" test +innodb_wait_for_flush_all + +xtrabackup --backup --target-dir=$topdir/backup \ + --debug-sync="xtrabackup_suspend_at_start" --lock-ddl=REDUCED \ + 2> >( tee $topdir/backup.log)& + +job_pid=$! +pid_file=$topdir/backup/xtrabackup_debug_sync +wait_for_xb_to_suspend $pid_file +xb_pid=`cat $pid_file` +echo "backup pid is $job_pid" + +# Rename table and generate redo +$MYSQL $MYSQL_ARGS -Ns -e "RENAME TABLE test.t1 TO test.t3; INSERT INTO test.t3 VALUES (2);" test +# Drop table +$MYSQL $MYSQL_ARGS -Ns -e "DROP TABLE test.t3;" test +# Rename table and generate redo +$MYSQL $MYSQL_ARGS -Ns -e "RENAME TABLE test.t2 TO test.t3; INSERT INTO test.t3 VALUES (2);" test +# Drop table +$MYSQL $MYSQL_ARGS -Ns -e "DROP TABLE test.t3;" test + +# Resume the xtrabackup process +vlog "Resuming xtrabackup" +kill -SIGCONT $xb_pid +run_cmd wait $job_pid + +xtrabackup --prepare --target-dir=$topdir/backup +record_db_state test +stop_server +rm -rf $mysql_datadir/* +xtrabackup --copy-back --target-dir=$topdir/backup +start_server +verify_db_state test +stop_server +rm -rf $mysql_datadir $topdir/backup + + + + +