From 171ab416fb87c0cd8a28c34318922b8636c2621d Mon Sep 17 00:00:00 2001 From: Aibek Bukabayev Date: Fri, 23 Aug 2024 09:09:43 +0000 Subject: [PATCH] PXB-3334 Code cleanups in reduced lock feature made some minor code changes for better readability --- .../innobase/xtrabackup/src/ddl_tracker.cc | 70 +++++----- storage/innobase/xtrabackup/src/xtrabackup.cc | 123 +++++++++++------- 2 files changed, 113 insertions(+), 80 deletions(-) diff --git a/storage/innobase/xtrabackup/src/ddl_tracker.cc b/storage/innobase/xtrabackup/src/ddl_tracker.cc index e0f68f16220..5c071169bd3 100644 --- a/storage/innobase/xtrabackup/src/ddl_tracker.cc +++ b/storage/innobase/xtrabackup/src/ddl_tracker.cc @@ -36,6 +36,10 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA #include "xb0xb.h" // check_if_skip_table #include "xtrabackup.h" // datafiles_iter_t +static std::string EXT_DEL = ".del"; +static std::string EXT_REN = ".ren"; +static std::string EXT_NEW = ".new"; + void ddl_tracker_t::backup_file_op(uint32_t space_id, mlog_id_t type, const byte *buf, ulint len, lsn_t start_lsn) { @@ -125,7 +129,7 @@ void ddl_tracker_t::add_to_recopy_tables(space_id_t space_id, lsn_t start_lsn, 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); + path.erase(0, strlen(Fil_path::DOT_SLASH)); } std::lock_guard lock(m_ddl_tracker_mutex); @@ -219,10 +223,7 @@ void ddl_tracker_t::add_drop_table_from_redo(const space_id_t space_id, } bool ddl_tracker_t::is_missing_table(const std::string &name) { - if (missing_tables.count(name)) { - return true; - } - return false; + return missing_tables.count(name) != 0; } void ddl_tracker_t::add_renamed_table(const space_id_t &space_id, @@ -279,7 +280,7 @@ static void data_copy_thread_func(copy_thread_ctxt_t *ctxt) { continue; } std::string dest_name = node->name; - dest_name.append(".new"); + dest_name.append(EXT_NEW); if (xtrabackup_copy_datafile(node, num, dest_name.c_str())) { xb::error() << "failed to copy datafile " << node->name; *(ctxt->error) = true; @@ -294,8 +295,11 @@ 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 +/** + @param space_id The space id of the current file + @param file_name File name that we will modify + @param ext Extension that we will modify to + @returns .del or .ren file name starting with space_id e.g schema/spaceid.del */ std::string ddl_tracker_t::convert_file_name(space_id_t space_id, std::string file_name, @@ -471,8 +475,9 @@ void ddl_tracker_t::handle_ddl_operations() { * redo_mgr.start and xb_load_tablespaces. This causes we ending up with two * tablespaces with the same spaceID. Remove them from new tables */ for (auto &table : tables_in_backup) { - if (new_tables.find(table.first) != new_tables.end()) { - new_tables.erase(table.first); + space_id_t space_id = table.first; + if (new_tables.find(space_id) != new_tables.end()) { + new_tables.erase(space_id); } } @@ -489,7 +494,7 @@ void ddl_tracker_t::handle_ddl_operations() { // We never create .del for ibdata* ut_ad(!fsp_is_system_tablespace(table)); backup_file_printf( - convert_file_name(table, renames[table].first, ".ibd.del").c_str(), + convert_file_name(table, renames[table].first, EXT_DEL).c_str(), "%s", ""); } string name = tables_in_backup[table]; @@ -498,35 +503,41 @@ void ddl_tracker_t::handle_ddl_operations() { } for (auto &table : drops) { - if (check_if_skip_table(table.second.c_str())) { + space_id_t space_id = table.first; + std::string table_name = table.second; + + if (check_if_skip_table(table_name.c_str())) { continue; } /* Remove from rename */ - renames.erase(table.first); + renames.erase(space_id); /* Remove from new tables and skip drop*/ - if (new_tables.find(table.first) != new_tables.end()) { - new_tables.erase(table.first); + if (new_tables.find(space_id) != new_tables.end()) { + new_tables.erase(space_id); continue; } /* Table not in the backup, nothing to drop, skip drop*/ - if (tables_in_backup.find(table.first) == tables_in_backup.end()) { + if (tables_in_backup.find(space_id) == tables_in_backup.end()) { continue; } // We never create .del for ibdata* - ut_ad(!fsp_is_system_tablespace(table.first)); - backup_file_printf( - convert_file_name(table.first, table.second, ".ibd.del").c_str(), "%s", - ""); + ut_ad(!fsp_is_system_tablespace(space_id)); + backup_file_printf(convert_file_name(space_id, table_name, EXT_DEL).c_str(), + "%s", ""); } for (auto &table : renames) { - if (check_if_skip_table(table.second.second.c_str())) { + space_id_t space_id = table.first; + std::string old_table_name = table.second.first; + std::string new_table_name = table.second.second; + + if (check_if_skip_table(new_table_name.c_str())) { continue; } - if (check_if_skip_table(table.second.first.c_str())) { + if (check_if_skip_table(old_table_name.c_str())) { continue; } /* renamed new table. update new table entry to renamed table name @@ -537,20 +548,20 @@ void ddl_tracker_t::handle_ddl_operations() { 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; + if (new_tables.find(space_id) != new_tables.end() || + is_missing_table(old_table_name)) { + new_tables[space_id] = new_table_name; continue; } /* Table not in the backup, nothing to rename, skip rename*/ - if (tables_in_backup.find(table.first) == tables_in_backup.end()) { + if (tables_in_backup.find(space_id) == 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()); + convert_file_name(space_id, old_table_name, EXT_REN).c_str(), "%s", + new_table_name.c_str()); } for (auto table = new_tables.begin(); table != new_tables.end();) { @@ -613,8 +624,7 @@ void ddl_tracker_t::handle_ddl_operations() { // Create .del files for deleted undo tablespaces for (auto &elem : deleted_undo_files) { backup_file_printf( - convert_file_name(elem.second, elem.first, ".ibu.del").c_str(), "%s", - ""); + convert_file_name(elem.second, elem.first, EXT_DEL).c_str(), "%s", ""); } // Add new undo files to be recopied diff --git a/storage/innobase/xtrabackup/src/xtrabackup.cc b/storage/innobase/xtrabackup/src/xtrabackup.cc index 8706cdaa5a6..3a1d80871bb 100644 --- a/storage/innobase/xtrabackup/src/xtrabackup.cc +++ b/storage/innobase/xtrabackup/src/xtrabackup.cc @@ -520,6 +520,15 @@ extern struct rand_struct sql_rand; extern mysql_mutex_t LOCK_sql_rand; bool xb_generated_redo = false; ddl_tracker_t *ddl_tracker = nullptr; +static std::string EXT_DEL = ".del"; +static std::string EXT_REN = ".ren"; +static std::string EXT_NEW = ".new"; +static std::string EXT_IBD = ".ibd"; +static std::string EXT_IBU = ".ibu"; +static std::string EXT_META = ".meta"; +static std::string EXT_NEW_META = ".new.meta"; +static std::string EXT_DELTA = ".delta"; +static std::string EXT_NEW_DELTA = ".new.delta"; static void check_all_privileges(); static bool validate_options(const char *file, int argc, char **argv); @@ -4877,7 +4886,7 @@ static bool get_meta_path( { size_t len = strlen(delta_path); - if (len <= 6 || strcmp(delta_path + len - 6, ".delta")) { + if (len <= 6 || strcmp(delta_path + len - 6, EXT_DELTA.c_str())) { return false; } memcpy(meta_path, delta_path, len - 6); @@ -5144,7 +5153,7 @@ static pfs_os_file_t xb_delta_open_matching_space( } Fil_path::normalize(real_name); /* Truncate ".ibd" */ - dest_space_name[strlen(dest_space_name) - 4] = '\0'; + dest_space_name[strlen(dest_space_name) - EXT_IBD.length()] = '\0'; /* Create the database directory if it doesn't exist yet */ if (!os_file_create_directory(dest_dir, false)) { @@ -5195,7 +5204,7 @@ static pfs_os_file_t xb_delta_open_matching_space( fil_space_read_name_and_filepath(f_space_id, &space_name, &oldpath); ut_a(res); xb::info() << "Renaming " << dest_space_name << " to " << tmpname - << ".ibu"; + << EXT_IBU; ut_a(os_file_status(oldpath, &exists, &type)); @@ -5250,7 +5259,7 @@ static pfs_os_file_t xb_delta_open_matching_space( ut_a(res); xb::info() << "Renaming " << dest_space_name << " to " << tmpname - << ".ibd"; + << EXT_IBU; ut_a(os_file_status(oldpath, &exists, &type)); @@ -5345,8 +5354,9 @@ static pfs_os_file_t xb_delta_open_matching_space( } /************************************************************************ -Applies a given .delta file to the corresponding data file. -@return true on success */ + * Applies a given .delta file to the corresponding data file. + * @return true on success + */ static bool xtrabackup_apply_delta( const datadir_entry_t &entry, /*!. -This map is later used to delete the .delta and .meta file for a dropped -tablespace (ie. when processing the .del entries in reduced lock) -@return true on success */ + * Scan .meta files and build std::unordered_map. + * This map is later used to delete the .delta and .meta file for a dropped + * tablespace (ie. when processing the .del entries in reduced lock) + * @return true on success + */ static bool xtrabackup_scan_meta( const datadir_entry_t &entry, /*! tablespace with space_id = 10 will be renamed to test/new_name.ibd -@return true on success */ + * Handle DDL for renamed files + * example input: test/10.ren file with content = test/new_name.ibd ; + * result: 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, /*! tablespace with space_id = 10 will be deleted -@return true on success */ + * Handle DDL for deleted files + * example input: test/10.del file + * result: tablespace with space_id=10 will be deleted + * @return true on success + */ static bool prepare_handle_del_files( const datadir_entry_t &entry, /*!name, name)); @@ -5896,8 +5919,8 @@ static bool rm_if_not_found( } /** Make sure that we have a read access to the given datadir entry -@param[in] statinfo entry stat info -@param[out] name entry name */ + * @param[in] statinfo entry stat info + * @param[out] name entry name */ static void check_datadir_enctry_access(const char *name, const struct stat *statinfo) { const char *entry_type = S_ISDIR(statinfo->st_mode) ? "directory" : "file"; @@ -6012,7 +6035,7 @@ bool xb_process_datadir(const char *path, /*!