From a5d4fc7c3f082ada11ca61304d01f1c68e72835d Mon Sep 17 00:00:00 2001 From: Andrzej Jarzabek Date: Thu, 21 May 2026 12:49:25 +0200 Subject: [PATCH 1/6] Incorporate changes from code review. --- storage/maria/ma_backup.cc | 60 ++++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 25 deletions(-) diff --git a/storage/maria/ma_backup.cc b/storage/maria/ma_backup.cc index 9d3e0c1a3d82e..a3a68afd7336e 100644 --- a/storage/maria/ma_backup.cc +++ b/storage/maria/ma_backup.cc @@ -124,8 +124,8 @@ namespace #ifndef _WIN32 const int datadir_fd; #endif - static const std::vector data_exts; - static const std::string log_file_prefix; + static const LEX_CSTRING data_exts[]; + static const LEX_CSTRING log_file_prefix; using dir_name = std::string; using dir_contents = std::vector; using database_dir = std::pair; @@ -258,25 +258,30 @@ namespace int copy_file(const std::string &path) const noexcept { + return copy_file(path.c_str()); + } + + int copy_file(const char*path) const noexcept + { #ifndef _WIN32 int ret_val = 0; - int src_fd = openat(datadir_fd, path.c_str(), O_RDONLY); + int src_fd = openat(datadir_fd, path, O_RDONLY); if (src_fd < 0) { - my_error(ER_CANT_OPEN_FILE, MYF(0), path.c_str(), errno); + my_error(ER_CANT_OPEN_FILE, MYF(0), path, errno); return 1; } - int tgt_fd = openat(target.fd, path.c_str(), + int tgt_fd = openat(target.fd, path, O_CREAT | O_EXCL | O_WRONLY, 0777); if (tgt_fd < 0) { - my_error(ER_CANT_CREATE_FILE, MYF(0), path.c_str(), errno); + my_error(ER_CANT_CREATE_FILE, MYF(0), path, errno); ret_val = 1; goto finish; } if (copy_entire_file(src_fd, tgt_fd) != 0) { - my_error(ER_ERROR_ON_WRITE, MYF(0), path.c_str(), errno); + my_error(ER_ERROR_ON_WRITE, MYF(0), path, errno); ret_val = 1; } close(tgt_fd); @@ -298,31 +303,22 @@ namespace } - static bool is_db_file(const char* file_name) noexcept - { - for (const std::string& ext : data_exts) - { - if (ends_with(file_name, ext)) - return true; - } - /* As a stop-gap db/opt files are also copied here, this should be done in SQL layer. */ - return !strcmp(file_name, "db.opt"); - } + static bool is_db_file(const char* file_name) noexcept; - static bool ends_with(const char* str, const std::string& suffix) noexcept + static bool ends_with(const char* str, const LEX_CSTRING& suffix) noexcept { size_t str_len = strlen(str); - size_t suffix_len = suffix.size(); + size_t suffix_len = suffix.length; if (str_len < suffix_len) return false; return memcmp(str + str_len - suffix_len, - suffix.data(), + suffix.str, suffix_len) == 0; } - static bool begins_with(const char* str, const std::string& prefix) noexcept + static bool begins_with(const char* str, const LEX_CSTRING& prefix) noexcept { - return strncmp(str, prefix.data(), prefix.size()) == 0; + return strncmp(str, prefix.str, prefix.length) == 0; } #ifdef _WIN32 @@ -336,9 +332,23 @@ namespace /* TODO: .frm failes are not Aria-specific, .MYD and .MYI are MyISAM files; they are copied here as a stop-gap */ - const std::vector - Aria_backup::data_exts {".MAD"s, ".MAI"s, "MYD"s, "MYI"s, "frm"s}; - const std::string Aria_backup::log_file_prefix {"aria_log."}; + const LEX_CSTRING Aria_backup::data_exts[] {{C_STRING_WITH_LEN(".MAD")}, + {C_STRING_WITH_LEN(".MAI")}, + {C_STRING_WITH_LEN(".MYD")}, + {C_STRING_WITH_LEN(".MYI")}, + {C_STRING_WITH_LEN(".frm")}}; + const LEX_CSTRING Aria_backup::log_file_prefix {C_STRING_WITH_LEN("aria_log.")}; + + bool Aria_backup::is_db_file(const char* file_name) noexcept + { + for (const LEX_CSTRING& ext : data_exts) + { + if (ends_with(file_name, ext)) + return true; + } + /* As a stop-gap db/opt files are also copied here, this should be done in SQL layer. */ + return !strcmp(file_name, "db.opt"); + } std::unique_ptr aria_backup; } From f7a3fb3b8b40071a4bc6dabc9ad9058f6b737326 Mon Sep 17 00:00:00 2001 From: Andrzej Jarzabek Date: Thu, 28 May 2026 14:31:32 +0200 Subject: [PATCH 2/6] Change BACKUP SERVER plugin API to allow reduced blocking in Aria backup. (blocking itself not changed yet) --- sql/handler.h | 31 +++++++-- sql/sql_backup.cc | 76 +++++++++++++++++++---- storage/innobase/handler/backup_innodb.cc | 31 ++++++++- storage/innobase/handler/backup_innodb.h | 11 +++- storage/innobase/handler/ha_innodb.cc | 1 + storage/maria/ha_maria.cc | 1 + storage/maria/ma_backup.cc | 20 ++++-- storage/maria/ma_backup.h | 12 +++- 8 files changed, 158 insertions(+), 25 deletions(-) diff --git a/sql/handler.h b/sql/handler.h index e9a2f7e164e08..ea697ae20517b 100644 --- a/sql/handler.h +++ b/sql/handler.h @@ -1517,6 +1517,17 @@ struct backup_target #endif }; +/** BACKUP SERVER processing phase. Phases must occur in this order. */ +enum backup_proc_phase { + BACKUP_PROC_PHASE_BACKUP_LOCKED, /* concurrent backup guaranteed not to run */ + BACKUP_PROC_PHASE_DDL_LOCKED, /* DDL can't happen concurrently */ + BACKUP_PROC_PHASE_COMMIT_LOCKED}; /* Commits are blocked */ + +/** BACKUP SERVER ending phase. Phases must occur in this order. */ +enum backup_end_phase { + BACKUP_END_PHASE_COMMIT_LOCKED, /* Commits are blocked */ + BACKUP_END_PHASE_BACKUP_LOCKED}; /* Only concurrent backups blocked */ + /* handlerton is a singleton structure - one instance per storage engine - to provide access to storage engine functionality that works on the @@ -1920,7 +1931,7 @@ struct handlerton : public transaction_participant void (*end_backup)(void); /** - Start of BACKUP SERVER: collect all files to be backed up + Start of BACKUP SERVER @param thd current session @param target backup target @return error code @@ -1928,20 +1939,32 @@ struct handlerton : public transaction_participant */ int (*backup_start)(THD *thd, backup_target target); /** - Process a file that was collected in backup_start(). + Called when a given phase of backup processing starts. + @param thd current session + @param phase the processing phase which started + @return error code + @retval 0 on completion + @note All subsequent calls to backup_step until next call to + backup_start_proc_phase or backup_end are in that phase + */ + int (*backup_start_proc_phase)(THD *thd, backup_proc_phase phase); + /** + Process a unit (file, table etc.) in current phase. Engines should + define their units in such a way that they may be processed concurrently. @param thd current session - @return number of files remaining, or negative on error + @return number of units remaining in processing phase, or negative on error @retval 0 on completion */ int (*backup_step)(THD *thd); /** Finish copying and determine the logical time of the backup snapshot. @param thd current sesssion + @param phase which phase of the end stage to run @param abort whether BACKUP SERVER was aborted @return error code @retval 0 on success */ - int (*backup_end)(THD *thd, bool abort); + int (*backup_end)(THD *thd, backup_end_phase phase, bool abort); /** Clean up after any backup_end(). @param thd the parameter on which backup_end() was invoked diff --git a/sql/sql_backup.cc b/sql/sql_backup.cc index 224345c5d6ec6..3efd6a956a320 100644 --- a/sql/sql_backup.cc +++ b/sql/sql_backup.cc @@ -194,11 +194,26 @@ static my_bool backup_start(THD *thd, plugin_ref plugin, void *dst) noexcept return false; } +static my_bool backup_start_proc_phase(THD *thd, plugin_ref plugin, void *arg) noexcept +{ + handlerton *hton= plugin_hton(plugin); + if (hton->backup_start_proc_phase) + return hton->backup_start_proc_phase(thd, *static_cast(arg)); + return false; +} + +struct backup_end_args +{ + backup_end_phase phase; + bool abort; +}; + static my_bool backup_end(THD *thd, plugin_ref plugin, void *arg) noexcept { + backup_end_args *args= static_cast(arg); handlerton *hton= plugin_hton(plugin); if (hton->backup_end) - return hton->backup_end(thd, arg != nullptr); + return hton->backup_end(thd, args->phase, args->abort); return false; } @@ -221,6 +236,32 @@ static my_bool backup_finalize(THD *thd, plugin_ref plugin, void *dst) noexcept return 0; } +static bool start_proc_phase(THD *thd, backup_proc_phase phase) +{ + return plugin_foreach_with_mask(thd, backup_start_proc_phase, + MYSQL_STORAGE_ENGINE_PLUGIN, + PLUGIN_IS_DELETED|PLUGIN_IS_READY, + &phase); +} + +static bool run_backup_processing(THD *thd) +{ + /* The backup_step may be invoked in multiple concurrent threads. + At the time backup_end is invoked, all backup_step will have to complete. */ + return plugin_foreach_with_mask(thd, backup_step, + MYSQL_STORAGE_ENGINE_PLUGIN, + PLUGIN_IS_DELETED|PLUGIN_IS_READY, nullptr); +} + +static bool run_backup_end_phase(THD *thd, backup_end_phase phase, bool failed) +{ + backup_end_args args {phase, failed}; + return plugin_foreach_with_mask(thd, backup_end, MYSQL_STORAGE_ENGINE_PLUGIN, + PLUGIN_IS_DELETED|PLUGIN_IS_READY, + &args) || + failed; +} + bool Sql_cmd_backup::execute(THD *thd) { if (check_global_access(thd, RELOAD_ACL) || @@ -267,20 +308,29 @@ bool Sql_cmd_backup::execute(THD *thd) MYSQL_STORAGE_ENGINE_PLUGIN, PLUGIN_IS_DELETED|PLUGIN_IS_READY, &dir); - /* The backup_step may be invoked in multiple concurrent threads. - At the time backup_end is invoked, all backup_step will have to complete. */ if (!fail) - fail= plugin_foreach_with_mask(thd, backup_step, - MYSQL_STORAGE_ENGINE_PLUGIN, - PLUGIN_IS_DELETED|PLUGIN_IS_READY, nullptr); + { + fail= thd->mdl_context.upgrade_shared_lock(mdl_request.ticket, + MDL_BACKUP_WAIT_DDL, + thd->variables.lock_wait_timeout) || + start_proc_phase(thd, BACKUP_PROC_PHASE_BACKUP_LOCKED); + fail= fail || run_backup_processing(thd); + } - fail= - thd->mdl_context.upgrade_shared_lock(mdl_request.ticket, - MDL_BACKUP_WAIT_COMMIT, - thd->variables.lock_wait_timeout) || - plugin_foreach_with_mask(thd, backup_end, MYSQL_STORAGE_ENGINE_PLUGIN, - PLUGIN_IS_DELETED|PLUGIN_IS_READY, - reinterpret_cast(fail)) || fail; + if (!fail) + { + fail= thd->mdl_context.upgrade_shared_lock(mdl_request.ticket, + MDL_BACKUP_WAIT_COMMIT, + thd->variables.lock_wait_timeout) || + start_proc_phase(thd, BACKUP_PROC_PHASE_COMMIT_LOCKED); + fail= fail || run_backup_processing(thd); + } + + fail= run_backup_end_phase(thd, BACKUP_END_PHASE_COMMIT_LOCKED, fail) || fail; + + mdl_request.ticket->downgrade_lock(MDL_BACKUP_START); + + fail= run_backup_end_phase(thd, BACKUP_END_PHASE_BACKUP_LOCKED, fail) || fail; /* The final part must not interfere with the use of the server datadir. Release the locks. */ diff --git a/storage/innobase/handler/backup_innodb.cc b/storage/innobase/handler/backup_innodb.cc index 6c473c47c9fdd..3461987d41e8a 100644 --- a/storage/innobase/handler/backup_innodb.cc +++ b/storage/innobase/handler/backup_innodb.cc @@ -54,6 +54,8 @@ class InnoDB_backup backup_context &context() const noexcept { ut_ad(log_sys.latch_have_any()); ut_ad(trx); return trx->lock.backup; } + /** Calls to step() should be ignored */ + bool ignore_step=false; public: /** Start of BACKUP SERVER: collect all files to be backed up @@ -139,6 +141,13 @@ class InnoDB_backup return fail; } + void stop_processing() noexcept + { + log_sys.latch.wr_lock(); + ignore_step= true; + log_sys.latch.wr_unlock(); + } + /** Process a file that was collected at init(). This may be invoked from multiple concurrent threads. @@ -153,6 +162,11 @@ class InnoDB_backup log_sys.latch.wr_lock(); backup_context &ctx{context()}; ut_ad(ctx.max_first_lsn); + if (ignore_step) + { + log_sys.latch.wr_unlock(); + return 0; + } size_t size{queue.size()}; if (!logs.empty()) { @@ -772,14 +786,27 @@ int innodb_backup_start(THD *thd, backup_target target) noexcept return innodb_backup.init(thd, target); } +int innodb_backup_start_proc_phase(THD *thd, backup_proc_phase phase) noexcept +{ + /* This logic relies of the fact that Backup-locked is the first + processing phase and innodb_backup is initially set to process steps. + If this should change this code may need to be adjusted, possibly + adding a start_processing() method. */ + if (phase!=BACKUP_PROC_PHASE_BACKUP_LOCKED) + innodb_backup.stop_processing(); + return 0; +} + int innodb_backup_step(THD *thd) noexcept { return innodb_backup.step(thd); } -int innodb_backup_end(THD *thd, bool abort) noexcept +int innodb_backup_end(THD *thd, backup_end_phase phase, bool abort) noexcept { - return innodb_backup.end(thd, abort); + if (phase==BACKUP_END_PHASE_BACKUP_LOCKED) + return innodb_backup.end(thd, abort); + return 0; } int innodb_backup_finalize(THD *thd, backup_target target) noexcept diff --git a/storage/innobase/handler/backup_innodb.h b/storage/innobase/handler/backup_innodb.h index 8d5f81ca35898..80c70cc8dae60 100644 --- a/storage/innobase/handler/backup_innodb.h +++ b/storage/innobase/handler/backup_innodb.h @@ -22,6 +22,15 @@ */ int innodb_backup_start(THD *thd, backup_target target) noexcept; +/** + Start backup processing phase + @param thd current session + @param phase the phase which is started + @return error code + @retval 0 on success +*/ +int innodb_backup_start_proc_phase(THD *thd, backup_proc_phase phase) noexcept; + /** Process a file that was collected in backup_start(). @param thd current session @@ -37,7 +46,7 @@ int innodb_backup_step(THD *thd) noexcept; @return error code @retval 0 on success */ -int innodb_backup_end(THD *thd, bool abort) noexcept; +int innodb_backup_end(THD *thd, backup_end_phase phase, bool abort) noexcept; /** Clean up after innodb_backup_end(). diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 5c28d270d13ff..2582813d61f83 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -4144,6 +4144,7 @@ static int innodb_init(void* p) innobase_hton->update_optimizer_costs= innobase_update_optimizer_costs; innobase_hton->backup_start = innodb_backup_start; + innobase_hton->backup_start_proc_phase = innodb_backup_start_proc_phase; innobase_hton->backup_step = innodb_backup_step; innobase_hton->backup_end = innodb_backup_end; innobase_hton->backup_finalize = innodb_backup_finalize; diff --git a/storage/maria/ha_maria.cc b/storage/maria/ha_maria.cc index 624a7f0a043d3..9d9bde948282f 100644 --- a/storage/maria/ha_maria.cc +++ b/storage/maria/ha_maria.cc @@ -3944,6 +3944,7 @@ static int ha_maria_init(void *p) maria_hton->end_backup= maria_end_backup; maria_hton->update_optimizer_costs= aria_update_optimizer_costs; maria_hton->backup_start= aria_backup_start; + maria_hton->backup_start_proc_phase= aria_backup_start_proc_phase; maria_hton->backup_step= aria_backup_step; maria_hton->backup_end= aria_backup_end; diff --git a/storage/maria/ma_backup.cc b/storage/maria/ma_backup.cc index a3a68afd7336e..8782b5cc4fdbb 100644 --- a/storage/maria/ma_backup.cc +++ b/storage/maria/ma_backup.cc @@ -359,14 +359,26 @@ int aria_backup_start(THD *thd, backup_target target) noexcept return !aria_backup->is_initialized(); } +int aria_backup_start_proc_phase(THD *thd, backup_proc_phase phase) noexcept +{ + return 0; +} + int aria_backup_step(THD *thd) noexcept { return 0; } -int aria_backup_end(THD *thd, bool abort) noexcept +int aria_backup_end(THD *thd, backup_end_phase phase, bool abort) noexcept { - int ret_val= aria_backup->end(thd, abort); - aria_backup.reset(); - return ret_val; + switch (phase) + { + case BACKUP_END_PHASE_COMMIT_LOCKED: + return aria_backup->end(thd, abort); + case BACKUP_END_PHASE_BACKUP_LOCKED: + aria_backup.reset(); + return 0; + default: + return 0; + } } diff --git a/storage/maria/ma_backup.h b/storage/maria/ma_backup.h index 3bec606648dac..8ac2719062042 100644 --- a/storage/maria/ma_backup.h +++ b/storage/maria/ma_backup.h @@ -29,6 +29,15 @@ */ int aria_backup_start(THD *thd, backup_target target) noexcept; +/** + Start a new processing phase. + @param thd current session + @param phase the processing phase which started + @return error code + @retval 0 on completion +*/ +int aria_backup_start_proc_phase(THD *thd, backup_proc_phase phase) noexcept; + /** Process a file that was collected in backup_start(). @param thd current session @@ -40,8 +49,9 @@ int aria_backup_step(THD *thd) noexcept; /** Finish copying and determine the logical time of the backup snapshot. @param thd current session + @param phase phase of end operation @param abort whether BACKUP SERVER was aborted @return error code @retval 0 on success */ -int aria_backup_end(THD *thd, bool abort) noexcept; +int aria_backup_end(THD *thd, backup_end_phase phase, bool abort) noexcept; From 4698cc24854a696c65c18377264299d2422c77bf Mon Sep 17 00:00:00 2001 From: Andrzej Jarzabek Date: Fri, 29 May 2026 11:14:57 +0200 Subject: [PATCH 3/6] Copy "safe" Aria files under DDL lock only to allow concurrent DML. --- sql/sql_backup.cc | 32 +-- storage/maria/ma_backup.cc | 402 +++++++++++++++++++++++++++---------- 2 files changed, 317 insertions(+), 117 deletions(-) diff --git a/sql/sql_backup.cc b/sql/sql_backup.cc index 3efd6a956a320..ced8efd852970 100644 --- a/sql/sql_backup.cc +++ b/sql/sql_backup.cc @@ -262,6 +262,18 @@ static bool run_backup_end_phase(THD *thd, backup_end_phase phase, bool failed) failed; } +static bool upgrade_and_process(THD *thd, + MDL_request& mdl_request, + enum_mdl_type lock_level, + backup_proc_phase phase) +{ + bool fail= thd->mdl_context.upgrade_shared_lock(mdl_request.ticket, + lock_level, + thd->variables.lock_wait_timeout) || + start_proc_phase(thd, phase); + return fail || run_backup_processing(thd); +} + bool Sql_cmd_backup::execute(THD *thd) { if (check_global_access(thd, RELOAD_ACL) || @@ -310,20 +322,12 @@ bool Sql_cmd_backup::execute(THD *thd) if (!fail) { - fail= thd->mdl_context.upgrade_shared_lock(mdl_request.ticket, - MDL_BACKUP_WAIT_DDL, - thd->variables.lock_wait_timeout) || - start_proc_phase(thd, BACKUP_PROC_PHASE_BACKUP_LOCKED); - fail= fail || run_backup_processing(thd); - } - - if (!fail) - { - fail= thd->mdl_context.upgrade_shared_lock(mdl_request.ticket, - MDL_BACKUP_WAIT_COMMIT, - thd->variables.lock_wait_timeout) || - start_proc_phase(thd, BACKUP_PROC_PHASE_COMMIT_LOCKED); - fail= fail || run_backup_processing(thd); + fail= start_proc_phase(thd, BACKUP_PROC_PHASE_BACKUP_LOCKED) || + run_backup_processing(thd) || + upgrade_and_process(thd, mdl_request, MDL_BACKUP_WAIT_DDL, + BACKUP_PROC_PHASE_DDL_LOCKED) || + upgrade_and_process(thd, mdl_request, MDL_BACKUP_WAIT_COMMIT, + BACKUP_PROC_PHASE_COMMIT_LOCKED); } fail= run_backup_end_phase(thd, BACKUP_END_PHASE_COMMIT_LOCKED, fail) || fail; diff --git a/storage/maria/ma_backup.cc b/storage/maria/ma_backup.cc index 8782b5cc4fdbb..7e32d6fec8f8e 100644 --- a/storage/maria/ma_backup.cc +++ b/storage/maria/ma_backup.cc @@ -15,11 +15,15 @@ #include "maria_def.h" #include "ma_backup.h" +#include #include #include #include #include #include +#include +#include +#include #ifdef __APPLE__ #include @@ -33,6 +37,7 @@ */ using namespace std::string_literals; + namespace { class Source_dir @@ -107,66 +112,98 @@ namespace #endif // _WIN32 } - int end(THD *thd, bool abort) noexcept + bool start_copy_dml_safe() noexcept + { + ddl_locked= true; + if (scan_dbdirs()) + return true; + flatten_table_lists(); + return ensure_target_dirs(); + } + + bool start_copy_unsafe() noexcept + { + if (scan_logs()) + return true; + unsafe_copy_phase = true; + return false; + } + + int step() noexcept + { + if (!ddl_locked) + return 0; + return unsafe_copy_phase? + unsafe_copy_step(): + dml_safe_copy_step(); + } + + int end(bool /*abort*/) noexcept { - int ret_val = 0; - if (!abort) { - if (int err= perform_backup() != 0) - { - ret_val= err; - }; - } translog_enable_purge(); - return ret_val; + return 0; } private: backup_target target; #ifndef _WIN32 const int datadir_fd; #endif - static const LEX_CSTRING data_exts[]; + /* All file suffixes are 4 characters long (dot and 3 letter extension) */ + static constexpr size_t suffix_len= 4; + static const char* data_ext; + static const char* index_ext; static const LEX_CSTRING log_file_prefix; + static const LEX_CSTRING tmp_file_prefix; + static const char* misc_exts[]; + static const char* control_file_name; using dir_name = std::string; using dir_contents = std::vector; using database_dir = std::pair; - std::vector database_dirs; + using database_dirs = std::vector; + /* Transactional tables with checksum */ + database_dirs dml_safe_tables; + /* All other Aria tables */ + database_dirs unsafe_tables; + /* Aria log files */ std::vector log_files; - bool have_control_file = false; + std::vector misc_files; + /* directories in which misc files are */ + std::vector misc_dirs; - int perform_backup() noexcept - { - if (scan_datadir()) - return 1; - if (copy_databases()) - return 1; - if (copy_control_file()) - return 1; - if(translog_flush(translog_get_horizon())) - return 1; - if (copy_logs()) - return 1; - return 0; - } - - int scan_datadir() noexcept + bool have_control_file = false; + bool safe_files_copied = false; + + /* Refer to a string stored elsewhere */ + using dir_ref= std::string_view; + using tablename_ref= std::string_view; + using table_ref= std::pair; + using table_list= std::vector; + + /* Flattened versions of dml_safe_tables and unsafe_tables. */ + table_list dml_safe_table_list; + table_list unsafe_tables_list; + size_t dml_safe_tables_copied= 0; + size_t unsafe_tables_copied= 0; + size_t log_files_copied= 0; + size_t misc_files_copied= 0; + bool ddl_locked= false; + bool unsafe_copy_phase = false; + + int scan_dbdirs() noexcept { const char* base_dir = maria_data_root; Source_dir datadir(base_dir, MYF(MY_WANT_STAT)); if (datadir.is_error()) return 1; - datadir.for_each([this](const fileinfo &fi) + int error = datadir.for_each([this](const fileinfo &fi) { if (fi.mystat->st_mode & S_IFDIR) { - if (scan_database_dir(fi.name) != 0) - return 1; - } else if (begins_with(fi.name, log_file_prefix)) - log_files.emplace_back(fi.name); - else if (strcmp(fi.name, "aria_log_control") == 0) - have_control_file = true; + return scan_database_dir(fi.name); + } return 0; }); - return 0; + return error; } int scan_database_dir(const char* dir_name) noexcept @@ -176,32 +213,111 @@ namespace Source_dir db_dir(dir_path.c_str(), MYF(0)); if (db_dir.is_error()) return 1; - std::vector files_to_backup; - db_dir.for_each([&files_to_backup](const fileinfo &fi) + dir_contents safe; + dir_contents unsafe; + int error= db_dir.for_each([this, &safe, &unsafe, dir_name] + (const fileinfo &fi) { - if (is_db_file(fi.name)) - files_to_backup.emplace_back(fi.name); + const char* filename= fi.name; + size_t filename_len = strlen(filename); + if (filename_len >= suffix_len) + { + const char* suffix = filename + filename_len - suffix_len; + if(match_suffix(suffix, index_ext)) + { + if (!is_tmp_table(filename)) + { + auto is_safe = is_safe_table(dir_name, filename); + if (std::holds_alternative(is_safe)) + { + std::string table_name(filename, filename_len - suffix_len); + if (std::get(is_safe)) + safe.push_back(std::move(table_name)); + else + unsafe.push_back(std::move(table_name)); + } + else + { + return std::get(is_safe); + } + } + } + else if (match_misc_ext(suffix) || !strcmp(filename, "db.opt")) + { + if(misc_dirs.empty() || misc_dirs.back() != dir_name) + misc_dirs.emplace_back(dir_name); + misc_files.push_back(std::string(dir_name) + "/" + filename); + } + } return 0; }); - if (!files_to_backup.empty()) - database_dirs.emplace_back(dir_name, std::move(files_to_backup)); - return 0; + if(!error) + { + if (!safe.empty()) + dml_safe_tables.emplace_back(dir_name, std::move(safe)); + if (!unsafe.empty()) + unsafe_tables.emplace_back(dir_name, std::move(unsafe)); + } + return error; } - int copy_databases() noexcept + static bool is_tmp_table(const char* filename) noexcept { - for (const database_dir& dir : database_dirs) + return begins_with(filename, tmp_file_prefix); + } + + void flatten_table_lists() noexcept + { + flatten_table_list(dml_safe_tables, dml_safe_table_list); + flatten_table_list(unsafe_tables, unsafe_tables_list); + } + + static void flatten_table_list(const database_dirs& dirs, table_list& list) noexcept + { + for (const database_dir& dir : dirs) { - const char* dir_name = dir.first.c_str(); - if (ensure_target_subdir(dir_name) != 0) - { - my_error(ER_CANT_CREATE_FILE, MYF(0), dir_name, errno); - return 1; - } - if (copy_database(dir) != 0) - return 1; + for (const std::string& table : dir.second) + list.emplace_back(dir.first, table); } - return 0; + } + + int scan_logs() noexcept + { + const char* base_dir = maria_data_root; + Source_dir datadir(base_dir, MYF(0)); + if (datadir.is_error()) + return 1; + int error = datadir.for_each([this](const fileinfo &fi) + { + if (begins_with(fi.name, log_file_prefix)) + log_files.emplace_back(fi.name); + else if (strcmp(fi.name, "aria_log_control") == 0) + have_control_file = true; + return 0; + }); + return error; + } + + bool ensure_target_dirs() noexcept + { + using string = std::string; + std::vector dirs; + for (const database_dir &dir : dml_safe_tables) + dirs.push_back(&dir.first); + for (const database_dir &dir : unsafe_tables) + dirs.push_back(&dir.first); + for (const string &dir: misc_dirs) + dirs.push_back(&dir); + std::sort(dirs.begin(), dirs.end(), + [](const string *a, const string *b) { return *a < *b; }); + auto dirs_end = std::unique(dirs.begin(), dirs.end(), + [](const string *a, const string *b) { return *a == *b; }); + for (auto it = dirs.begin(); it != dirs_end; ++it) + { + if (ensure_target_subdir((*it)->c_str())) + return true; + } + return false; } /* @@ -228,32 +344,114 @@ namespace return 0; } - int copy_database(const database_dir& dir) noexcept + /* Returns result or error code. */ + std::variant is_safe_table(const char* dir_name, const char* myi_file_name) { - for (const std::string& file : dir.second) + ARIA_TABLE_CAPABILITIES cap; +#ifndef _WIN32 + std::string path= std::string(dir_name) + "/" + myi_file_name; + File fd= openat(datadir_fd, path.c_str(), O_RDONLY); + if (fd < 0) { - std::string file_path= dir.first + "/" + file; - if (copy_file(file_path) != 0) - return 1; + my_errno= errno; + my_error(ER_CANT_OPEN_FILE, MYF(0), path.c_str(), errno); } - return 0; +#else + std::string path= std::string(maria_data_root) + "/" + + dir_name + "/" + myi_file_name; + File fd= my_open(path.c_str(), O_RDONLY, MYF(MY_WME)); +#endif + if (fd < 0) + { + return my_errno; + } + std::variant result; + mysql_mutex_lock(&THR_LOCK_maria); + int fail = aria_get_capabilities(fd, myi_file_name, &cap); + if (fail) + { + my_error(ER_FILE_CORRUPT, MYF(0), path.c_str()); + result= fail; + goto end; + } + result = cap.transactional && cap.checksum; + aria_free_capabilities(&cap); +end: + mysql_mutex_unlock(&THR_LOCK_maria); + close(fd); + return result; } - int copy_control_file() noexcept + int dml_safe_copy_step() noexcept { - if (!have_control_file) + if (dml_safe_tables_copied == dml_safe_table_list.size()) return 0; - return copy_file("aria_log_control"); + const table_ref& table = dml_safe_table_list[dml_safe_tables_copied]; + if (copy_table(table) != 0) + return -1; + ++dml_safe_tables_copied; + return dml_safe_table_list.size() - dml_safe_tables_copied; } - int copy_logs() noexcept + int unsafe_copy_step() noexcept { - for (const std::string& file : log_files) + if (have_control_file) { - if (copy_file(file) != 0) - return 1; + if (copy_control_file() != 0) + return -1; + have_control_file = false; } - return 0; + else if(log_files_copied < log_files.size()) + { + if (copy_file(log_files[log_files_copied]) != 0) + return -1; + ++log_files_copied; + } + else if (unsafe_tables_copied < unsafe_tables_list.size()) + { + const table_ref& table = unsafe_tables_list[unsafe_tables_copied]; + if (copy_table(table) != 0) + return -1; + ++unsafe_tables_copied; + } + else if (misc_files_copied < misc_files.size()) + { + if (copy_file(misc_files[misc_files_copied]) != 0) + return -1; + ++misc_files_copied; + } + assert(!have_control_file); + return + log_files.size() - log_files_copied + + unsafe_tables_list.size() - unsafe_tables_copied + + misc_files.size() - misc_files_copied; + } + + int copy_table(const table_ref& table) noexcept + { + dir_ref dir_name = table.first; + tablename_ref table_name = table.second; + std::string index_path; + index_path.reserve(dir_name.size() + table_name.size() + 5); + index_path= dir_name; + index_path += "/"; + size_t dir_part_length = index_path.size(); + index_path.resize(index_path.size() + table_name.size()); + std::copy(table_name.begin(), table_name.end(), + index_path.begin() + dir_part_length); + std::string data_path; + data_path.reserve(dir_name.size() + table_name.size() + 5); + data_path= index_path; + index_path+= index_ext; + data_path+= data_ext; + return copy_file(index_path) || copy_file(data_path); + } + + int copy_control_file() noexcept + { + if (!have_control_file) + return 0; + return copy_file(control_file_name); } int copy_file(const std::string &path) const noexcept @@ -302,17 +500,12 @@ namespace #endif } - - static bool is_db_file(const char* file_name) noexcept; - - static bool ends_with(const char* str, const LEX_CSTRING& suffix) noexcept + /* Match if suffix is one of the "other" extensions we need to copy */ + static bool match_misc_ext(const char* suffix) noexcept; + static bool match_suffix(const char* suffix1, const char* suffix2) noexcept { - size_t str_len = strlen(str); - size_t suffix_len = suffix.length; - if (str_len < suffix_len) - return false; - return memcmp(str + str_len - suffix_len, - suffix.str, + return memcmp(suffix1, + suffix2, suffix_len) == 0; } @@ -330,29 +523,31 @@ namespace #endif }; + + const LEX_CSTRING Aria_backup::log_file_prefix {C_STRING_WITH_LEN("aria_log.")}; + const LEX_CSTRING Aria_backup::tmp_file_prefix {C_STRING_WITH_LEN("#sql")}; + const char* Aria_backup::data_ext (".MAD"); + const char* Aria_backup::index_ext (".MAI"); + const char* Aria_backup::control_file_name {"aria_log_control"}; + + /* TODO: .frm failes are not Aria-specific, .MYD and .MYI are MyISAM files; they are copied here as a stop-gap */ - const LEX_CSTRING Aria_backup::data_exts[] {{C_STRING_WITH_LEN(".MAD")}, - {C_STRING_WITH_LEN(".MAI")}, - {C_STRING_WITH_LEN(".MYD")}, - {C_STRING_WITH_LEN(".MYI")}, - {C_STRING_WITH_LEN(".frm")}}; - const LEX_CSTRING Aria_backup::log_file_prefix {C_STRING_WITH_LEN("aria_log.")}; + const char* Aria_backup::misc_exts[] {".MYD", ".MYI", ".frm"}; - bool Aria_backup::is_db_file(const char* file_name) noexcept + bool Aria_backup::match_misc_ext(const char* suffix) noexcept { - for (const LEX_CSTRING& ext : data_exts) - { - if (ends_with(file_name, ext)) - return true; - } - /* As a stop-gap db/opt files are also copied here, this should be done in SQL layer. */ - return !strcmp(file_name, "db.opt"); + return std::find_if(std::begin(misc_exts), std::end(misc_exts), + [suffix](const char* ext) { + return match_suffix(suffix, ext); + }) != std::end(misc_exts); } std::unique_ptr aria_backup; } + + int aria_backup_start(THD *thd, backup_target target) noexcept { aria_backup= std::make_unique(thd, target); @@ -361,24 +556,25 @@ int aria_backup_start(THD *thd, backup_target target) noexcept int aria_backup_start_proc_phase(THD *thd, backup_proc_phase phase) noexcept { - return 0; + switch (phase) + { + case BACKUP_PROC_PHASE_DDL_LOCKED: + return aria_backup->start_copy_dml_safe(); + case BACKUP_PROC_PHASE_COMMIT_LOCKED: + return aria_backup->start_copy_unsafe(); + default: + return 0; + } } int aria_backup_step(THD *thd) noexcept { - return 0; + return aria_backup->step(); } int aria_backup_end(THD *thd, backup_end_phase phase, bool abort) noexcept { - switch (phase) - { - case BACKUP_END_PHASE_COMMIT_LOCKED: - return aria_backup->end(thd, abort); - case BACKUP_END_PHASE_BACKUP_LOCKED: + if (phase == BACKUP_END_PHASE_BACKUP_LOCKED) aria_backup.reset(); - return 0; - default: - return 0; - } + return 0; } From 824afeb8e37becf2b95e82d7a3584df1a5588621 Mon Sep 17 00:00:00 2001 From: Andrzej Jarzabek Date: Mon, 1 Jun 2026 14:18:25 +0200 Subject: [PATCH 4/6] Add phase information to backup_stem to simplify implementation --- sql/handler.h | 9 +- sql/sql_backup.cc | 13 +-- storage/innobase/handler/backup_innodb.cc | 29 +----- storage/innobase/handler/backup_innodb.h | 12 +-- storage/innobase/handler/ha_innodb.cc | 1 - storage/maria/ma_backup.cc | 105 +++++++++++----------- storage/maria/ma_backup.h | 5 +- 7 files changed, 71 insertions(+), 103 deletions(-) diff --git a/sql/handler.h b/sql/handler.h index ea697ae20517b..200fc21fa3e75 100644 --- a/sql/handler.h +++ b/sql/handler.h @@ -1519,9 +1519,9 @@ struct backup_target /** BACKUP SERVER processing phase. Phases must occur in this order. */ enum backup_proc_phase { - BACKUP_PROC_PHASE_BACKUP_LOCKED, /* concurrent backup guaranteed not to run */ - BACKUP_PROC_PHASE_DDL_LOCKED, /* DDL can't happen concurrently */ - BACKUP_PROC_PHASE_COMMIT_LOCKED}; /* Commits are blocked */ + BACKUP_PROC_PHASE_BACKUP_LOCKED = 0, /* concurrent backup guaranteed not to run */ + BACKUP_PROC_PHASE_DDL_LOCKED, /* DDL can't happen concurrently */ + BACKUP_PROC_PHASE_COMMIT_LOCKED}; /* Commits are blocked */ /** BACKUP SERVER ending phase. Phases must occur in this order. */ enum backup_end_phase { @@ -1952,10 +1952,11 @@ struct handlerton : public transaction_participant Process a unit (file, table etc.) in current phase. Engines should define their units in such a way that they may be processed concurrently. @param thd current session + @param phase the processing phase for this step @return number of units remaining in processing phase, or negative on error @retval 0 on completion */ - int (*backup_step)(THD *thd); + int (*backup_step)(THD *thd, backup_proc_phase phase); /** Finish copying and determine the logical time of the backup snapshot. @param thd current sesssion diff --git a/sql/sql_backup.cc b/sql/sql_backup.cc index ced8efd852970..af759a23f6734 100644 --- a/sql/sql_backup.cc +++ b/sql/sql_backup.cc @@ -217,12 +217,12 @@ static my_bool backup_end(THD *thd, plugin_ref plugin, void *arg) noexcept return false; } -static my_bool backup_step(THD *thd, plugin_ref plugin, void *) noexcept +static my_bool backup_step(THD *thd, plugin_ref plugin, void *arg) noexcept { handlerton *hton= plugin_hton(plugin); int res= 0; if (hton->backup_step) - while ((res= hton->backup_step(thd))) + while ((res= hton->backup_step(thd, *static_cast(arg)))) if (res < 0) break; return res != 0; @@ -244,13 +244,14 @@ static bool start_proc_phase(THD *thd, backup_proc_phase phase) &phase); } -static bool run_backup_processing(THD *thd) +static bool run_backup_processing(THD *thd, backup_proc_phase phase) { /* The backup_step may be invoked in multiple concurrent threads. At the time backup_end is invoked, all backup_step will have to complete. */ return plugin_foreach_with_mask(thd, backup_step, MYSQL_STORAGE_ENGINE_PLUGIN, - PLUGIN_IS_DELETED|PLUGIN_IS_READY, nullptr); + PLUGIN_IS_DELETED|PLUGIN_IS_READY, + &phase); } static bool run_backup_end_phase(THD *thd, backup_end_phase phase, bool failed) @@ -271,7 +272,7 @@ static bool upgrade_and_process(THD *thd, lock_level, thd->variables.lock_wait_timeout) || start_proc_phase(thd, phase); - return fail || run_backup_processing(thd); + return fail || run_backup_processing(thd, phase); } bool Sql_cmd_backup::execute(THD *thd) @@ -323,7 +324,7 @@ bool Sql_cmd_backup::execute(THD *thd) if (!fail) { fail= start_proc_phase(thd, BACKUP_PROC_PHASE_BACKUP_LOCKED) || - run_backup_processing(thd) || + run_backup_processing(thd, BACKUP_PROC_PHASE_BACKUP_LOCKED) || upgrade_and_process(thd, mdl_request, MDL_BACKUP_WAIT_DDL, BACKUP_PROC_PHASE_DDL_LOCKED) || upgrade_and_process(thd, mdl_request, MDL_BACKUP_WAIT_COMMIT, diff --git a/storage/innobase/handler/backup_innodb.cc b/storage/innobase/handler/backup_innodb.cc index 3461987d41e8a..c4002f3488ce9 100644 --- a/storage/innobase/handler/backup_innodb.cc +++ b/storage/innobase/handler/backup_innodb.cc @@ -54,8 +54,6 @@ class InnoDB_backup backup_context &context() const noexcept { ut_ad(log_sys.latch_have_any()); ut_ad(trx); return trx->lock.backup; } - /** Calls to step() should be ignored */ - bool ignore_step=false; public: /** Start of BACKUP SERVER: collect all files to be backed up @@ -141,13 +139,6 @@ class InnoDB_backup return fail; } - void stop_processing() noexcept - { - log_sys.latch.wr_lock(); - ignore_step= true; - log_sys.latch.wr_unlock(); - } - /** Process a file that was collected at init(). This may be invoked from multiple concurrent threads. @@ -162,11 +153,6 @@ class InnoDB_backup log_sys.latch.wr_lock(); backup_context &ctx{context()}; ut_ad(ctx.max_first_lsn); - if (ignore_step) - { - log_sys.latch.wr_unlock(); - return 0; - } size_t size{queue.size()}; if (!logs.empty()) { @@ -786,22 +772,13 @@ int innodb_backup_start(THD *thd, backup_target target) noexcept return innodb_backup.init(thd, target); } -int innodb_backup_start_proc_phase(THD *thd, backup_proc_phase phase) noexcept +int innodb_backup_step(THD *thd, backup_proc_phase phase) noexcept { - /* This logic relies of the fact that Backup-locked is the first - processing phase and innodb_backup is initially set to process steps. - If this should change this code may need to be adjusted, possibly - adding a start_processing() method. */ - if (phase!=BACKUP_PROC_PHASE_BACKUP_LOCKED) - innodb_backup.stop_processing(); + if (phase==BACKUP_PROC_PHASE_BACKUP_LOCKED) + return innodb_backup.step(thd); return 0; } -int innodb_backup_step(THD *thd) noexcept -{ - return innodb_backup.step(thd); -} - int innodb_backup_end(THD *thd, backup_end_phase phase, bool abort) noexcept { if (phase==BACKUP_END_PHASE_BACKUP_LOCKED) diff --git a/storage/innobase/handler/backup_innodb.h b/storage/innobase/handler/backup_innodb.h index 80c70cc8dae60..41b198b523941 100644 --- a/storage/innobase/handler/backup_innodb.h +++ b/storage/innobase/handler/backup_innodb.h @@ -22,22 +22,14 @@ */ int innodb_backup_start(THD *thd, backup_target target) noexcept; -/** - Start backup processing phase - @param thd current session - @param phase the phase which is started - @return error code - @retval 0 on success -*/ -int innodb_backup_start_proc_phase(THD *thd, backup_proc_phase phase) noexcept; - /** Process a file that was collected in backup_start(). @param thd current session + @param phase current processing phase @return number of files remaining, or negative on error @retval 0 on completion */ -int innodb_backup_step(THD *thd) noexcept; +int innodb_backup_step(THD *thd, backup_proc_phase phase) noexcept; /** Finish copying and determine the logical time of the backup snapshot. diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 2582813d61f83..5c28d270d13ff 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -4144,7 +4144,6 @@ static int innodb_init(void* p) innobase_hton->update_optimizer_costs= innobase_update_optimizer_costs; innobase_hton->backup_start = innodb_backup_start; - innobase_hton->backup_start_proc_phase = innodb_backup_start_proc_phase; innobase_hton->backup_step = innodb_backup_step; innobase_hton->backup_end = innodb_backup_end; innobase_hton->backup_finalize = innodb_backup_finalize; diff --git a/storage/maria/ma_backup.cc b/storage/maria/ma_backup.cc index 7e32d6fec8f8e..b1cdff7093bba 100644 --- a/storage/maria/ma_backup.cc +++ b/storage/maria/ma_backup.cc @@ -114,7 +114,6 @@ namespace bool start_copy_dml_safe() noexcept { - ddl_locked= true; if (scan_dbdirs()) return true; flatten_table_lists(); @@ -129,13 +128,49 @@ namespace return false; } - int step() noexcept + int dml_safe_copy_step() noexcept { - if (!ddl_locked) + if (dml_safe_tables_copied == dml_safe_table_list.size()) return 0; - return unsafe_copy_phase? - unsafe_copy_step(): - dml_safe_copy_step(); + const table_ref& table = dml_safe_table_list[dml_safe_tables_copied]; + if (copy_table(table) != 0) + return -1; + ++dml_safe_tables_copied; + return dml_safe_table_list.size() - dml_safe_tables_copied; + } + + int unsafe_copy_step() noexcept + { + if (have_control_file) + { + if (copy_control_file() != 0) + return -1; + have_control_file = false; + } + else if(log_files_copied < log_files.size()) + { + if (copy_file(log_files[log_files_copied]) != 0) + return -1; + ++log_files_copied; + } + else if (unsafe_tables_copied < unsafe_tables_list.size()) + { + const table_ref& table = unsafe_tables_list[unsafe_tables_copied]; + if (copy_table(table) != 0) + return -1; + ++unsafe_tables_copied; + } + else if (misc_files_copied < misc_files.size()) + { + if (copy_file(misc_files[misc_files_copied]) != 0) + return -1; + ++misc_files_copied; + } + assert(!have_control_file); + return + log_files.size() - log_files_copied + + unsafe_tables_list.size() - unsafe_tables_copied + + misc_files.size() - misc_files_copied; } int end(bool /*abort*/) noexcept @@ -186,7 +221,6 @@ namespace size_t unsafe_tables_copied= 0; size_t log_files_copied= 0; size_t misc_files_copied= 0; - bool ddl_locked= false; bool unsafe_copy_phase = false; int scan_dbdirs() noexcept @@ -382,51 +416,6 @@ namespace return result; } - int dml_safe_copy_step() noexcept - { - if (dml_safe_tables_copied == dml_safe_table_list.size()) - return 0; - const table_ref& table = dml_safe_table_list[dml_safe_tables_copied]; - if (copy_table(table) != 0) - return -1; - ++dml_safe_tables_copied; - return dml_safe_table_list.size() - dml_safe_tables_copied; - } - - int unsafe_copy_step() noexcept - { - if (have_control_file) - { - if (copy_control_file() != 0) - return -1; - have_control_file = false; - } - else if(log_files_copied < log_files.size()) - { - if (copy_file(log_files[log_files_copied]) != 0) - return -1; - ++log_files_copied; - } - else if (unsafe_tables_copied < unsafe_tables_list.size()) - { - const table_ref& table = unsafe_tables_list[unsafe_tables_copied]; - if (copy_table(table) != 0) - return -1; - ++unsafe_tables_copied; - } - else if (misc_files_copied < misc_files.size()) - { - if (copy_file(misc_files[misc_files_copied]) != 0) - return -1; - ++misc_files_copied; - } - assert(!have_control_file); - return - log_files.size() - log_files_copied + - unsafe_tables_list.size() - unsafe_tables_copied + - misc_files.size() - misc_files_copied; - } - int copy_table(const table_ref& table) noexcept { dir_ref dir_name = table.first; @@ -567,9 +556,17 @@ int aria_backup_start_proc_phase(THD *thd, backup_proc_phase phase) noexcept } } -int aria_backup_step(THD *thd) noexcept +int aria_backup_step(THD *thd, backup_proc_phase phase) noexcept { - return aria_backup->step(); + switch (phase) + { + case BACKUP_PROC_PHASE_DDL_LOCKED: + return aria_backup->dml_safe_copy_step(); + case BACKUP_PROC_PHASE_COMMIT_LOCKED: + return aria_backup->unsafe_copy_step(); + default: + return 0; + } } int aria_backup_end(THD *thd, backup_end_phase phase, bool abort) noexcept diff --git a/storage/maria/ma_backup.h b/storage/maria/ma_backup.h index 8ac2719062042..7725a9e0d0edd 100644 --- a/storage/maria/ma_backup.h +++ b/storage/maria/ma_backup.h @@ -39,12 +39,13 @@ int aria_backup_start(THD *thd, backup_target target) noexcept; int aria_backup_start_proc_phase(THD *thd, backup_proc_phase phase) noexcept; /** - Process a file that was collected in backup_start(). + Processing step (processes 1 table). @param thd current session + @param phase current processing phase @return number of files remaining, or negative on error @retval 0 on completion */ -int aria_backup_step(THD *thd) noexcept; +int aria_backup_step(THD *thd, backup_proc_phase phase) noexcept; /** Finish copying and determine the logical time of the backup snapshot. From 1b0e54bda1fde1b8e18015104d1755b06b0aeaa9 Mon Sep 17 00:00:00 2001 From: Andrzej Jarzabek Date: Wed, 27 May 2026 07:39:03 +0200 Subject: [PATCH 5/6] Remove InnoDB table from test. --- mysql-test/main/backup_server_restore.result | 9 --------- mysql-test/main/backup_server_restore.test | 4 ---- 2 files changed, 13 deletions(-) diff --git a/mysql-test/main/backup_server_restore.result b/mysql-test/main/backup_server_restore.result index 83bf230bc8619..19c6887b56ee0 100644 --- a/mysql-test/main/backup_server_restore.result +++ b/mysql-test/main/backup_server_restore.result @@ -1,6 +1,4 @@ Prepare database -CREATE TABLE tinno (i INTEGER) ENGINE=InnoDB; -INSERT INTO tinno VALUES (1), (2), (3), (4); CREATE TABLE tariatr (i INTEGER) ENGINE=Aria TRANSACTIONAL=1; INSERT INTO tariatr VALUES (2), (3), (5), (7); CREATE TABLE tariant (i INTEGER) ENGINE=Aria TRANSACTIONAL=0; @@ -10,12 +8,6 @@ BACKUP SERVER TO '$MYSQLTEST_VARDIR/some_directory'; Restore the database # restart: --datadir=MYSQLTEST_VARDIR/some_directory Check contents after restore -SELECT * FROM tinno; -i -1 -2 -3 -4 SELECT * FROM tariatr; i 2 @@ -36,6 +28,5 @@ Note 1034 Table is fixed Restart database in original data directory # restart Clean up -DROP TABLE tinno; DROP TABLE tariatr; DROP TABLE tariant; diff --git a/mysql-test/main/backup_server_restore.test b/mysql-test/main/backup_server_restore.test index ef1f5af27a68b..dc633d7529696 100644 --- a/mysql-test/main/backup_server_restore.test +++ b/mysql-test/main/backup_server_restore.test @@ -2,8 +2,6 @@ --source include/have_innodb.inc --echo Prepare database -CREATE TABLE tinno (i INTEGER) ENGINE=InnoDB; -INSERT INTO tinno VALUES (1), (2), (3), (4); CREATE TABLE tariatr (i INTEGER) ENGINE=Aria TRANSACTIONAL=1; INSERT INTO tariatr VALUES (2), (3), (5), (7); CREATE TABLE tariant (i INTEGER) ENGINE=Aria TRANSACTIONAL=0; @@ -23,7 +21,6 @@ call mtr.add_suppression("Checking table: "); --source include/restart_mysqld.inc --echo Check contents after restore -SELECT * FROM tinno; SELECT * FROM tariatr; SELECT * FROM tariant; @@ -32,7 +29,6 @@ SELECT * FROM tariant; --source include/restart_mysqld.inc --echo Clean up -DROP TABLE tinno; DROP TABLE tariatr; DROP TABLE tariant; --rmdir $MYSQLTEST_VARDIR/some_directory From 8ecd31a4a86fed772692bbe37d1c807abfcbd8c0 Mon Sep 17 00:00:00 2001 From: Andrzej Jarzabek Date: Tue, 2 Jun 2026 12:48:41 +0200 Subject: [PATCH 6/6] Make Aria processing step thread-safe. --- storage/maria/ma_backup.cc | 119 ++++++++++++++++++++++++------------- 1 file changed, 77 insertions(+), 42 deletions(-) diff --git a/storage/maria/ma_backup.cc b/storage/maria/ma_backup.cc index b1cdff7093bba..5b669441f6117 100644 --- a/storage/maria/ma_backup.cc +++ b/storage/maria/ma_backup.cc @@ -17,6 +17,7 @@ #include "ma_backup.h" #include #include +#include #include #include #include @@ -36,8 +37,6 @@ BACKUP SERVER support for Aria engine */ -using namespace std::string_literals; - namespace { class Source_dir @@ -73,9 +72,6 @@ namespace private: MY_DIR *dir_info {nullptr}; }; - - - /** Backup state; protected by log_sys.latch */ class Aria_backup { public: @@ -124,53 +120,92 @@ namespace { if (scan_logs()) return true; - unsafe_copy_phase = true; return false; } + /* Copy an Aria table that is safe to be copied while concurrent DML + is in progress. */ int dml_safe_copy_step() noexcept { - if (dml_safe_tables_copied == dml_safe_table_list.size()) + size_t idx= dml_safe_tables_copied.fetch_add(1, std::memory_order_relaxed); + if (idx >= dml_safe_table_list.size()) return 0; - const table_ref& table = dml_safe_table_list[dml_safe_tables_copied]; - if (copy_table(table) != 0) + if (copy_table(dml_safe_table_list[idx]) != 0) return -1; - ++dml_safe_tables_copied; - return dml_safe_table_list.size() - dml_safe_tables_copied; + return dml_safe_table_list.size() - idx - 1; } + /* Copy an entity that is not safe to copy if there are concurrent + writes to it. One entity is copied, of the first category that has + any remaning entities to be copied. Returns the total number of + entities to be copied in all categories. Categories in order: + - log control file + - log files + - Aria tables + - other ("miscellaneous") files + */ int unsafe_copy_step() noexcept { + int remaining= 0; + bool step_complete= false; + /* If control file is always the first file copied and there is only + one, it is never included in the "steps remaining" calculation. + Should the order be changed, the calculation needs to be updated for + the control file as well. */ if (have_control_file) { - if (copy_control_file() != 0) - return -1; - have_control_file = false; - } - else if(log_files_copied < log_files.size()) - { - if (copy_file(log_files[log_files_copied]) != 0) - return -1; - ++log_files_copied; - } - else if (unsafe_tables_copied < unsafe_tables_list.size()) - { - const table_ref& table = unsafe_tables_list[unsafe_tables_copied]; - if (copy_table(table) != 0) - return -1; - ++unsafe_tables_copied; + bool already_copied= control_file_copied.exchange(true); + if (!already_copied) + { + if (copy_control_file() != 0) + return -1; + step_complete= true; + } } - else if (misc_files_copied < misc_files.size()) + + /* if no entity was copied before, and there are entities of a given + kind remaining to be copied, copy an entity. In each case update + 'remaining' with the number of remainig copies of that kind. */ + auto copy_from_list_step= [&step_complete, &remaining] ( + auto &list, std::atomic &copied, auto copy_action) { - if (copy_file(misc_files[misc_files_copied]) != 0) - return -1; - ++misc_files_copied; - } - assert(!have_control_file); - return - log_files.size() - log_files_copied + - unsafe_tables_list.size() - unsafe_tables_copied + - misc_files.size() - misc_files_copied; + if(!step_complete) + { + size_t idx= copied.fetch_add(1, std::memory_order_relaxed); + if (idx < list.size()) + { + if (copy_action(list[idx]) != 0) + return 1; + step_complete= true; + remaining+= list.size() - idx - 1; + } + } + else + { + size_t current_copied= copied.load(std::memory_order_relaxed); + if (current_copied < list.size()) + remaining+= list.size() - current_copied; + } + return 0; + }; + + if (copy_from_list_step(log_files, log_files_copied, + [this](const std::string &filename) { + return copy_file(filename); + }) != 0) + return -1; + if (copy_from_list_step(unsafe_tables_list, unsafe_tables_copied, + [this](const table_ref &table) { + return copy_table(table); + }) != 0) + return -1; + if (copy_from_list_step(misc_files, misc_files_copied, + [this](const std::string &path) { + return copy_file(path); + }) != 0) + return -1; + + return remaining; } int end(bool /*abort*/) noexcept @@ -217,11 +252,11 @@ namespace /* Flattened versions of dml_safe_tables and unsafe_tables. */ table_list dml_safe_table_list; table_list unsafe_tables_list; - size_t dml_safe_tables_copied= 0; - size_t unsafe_tables_copied= 0; - size_t log_files_copied= 0; - size_t misc_files_copied= 0; - bool unsafe_copy_phase = false; + std::atomic dml_safe_tables_copied {0}; + std::atomic unsafe_tables_copied {0}; + std::atomic log_files_copied {0}; + std::atomic misc_files_copied {0}; + std::atomic control_file_copied {false}; int scan_dbdirs() noexcept {