From a8eb932260897c14e3caf881e35386d9c4dff2fb Mon Sep 17 00:00:00 2001 From: Satya Bodapati Date: Fri, 5 Jul 2024 19:15:45 +0100 Subject: [PATCH] PXB-3320 : prepare_handle_del_files() fails to delete the .meta and .delta files for deleted tablespaces in incremental backup directory Problem: -------- 1. take full backup with --lock-ddl=reduced 2. create table t1(a INT), lets say space_id 10 3. start incremental backup and pause before backup_start() function (we take Bakcup lock here) 4. incremental backup copied t1.ibd.meta and t1.ibd.delta by this time 5. DROP TABLE t1 6. resume the incremental backup. 10.del file is created 7. prepare the full backup with --apply-log-only 8. prepare incremental backup incremental backup prepare first processes the .del files. before this all tabelspaces are loaded via .ibd scan since there is no t1.ibd in backup directory( it is only present as meta and delta file) in incremental backup directory, space_id with 10 is not in cache. Hence prepare_handle_del_files() will not delete the files related to space_id 10. We end up with orphan .ibd or .ibu files. Server ignore orphan .ibd But if the tablespace is undo tablespace, orphan .ibu are not ignored by server. Server discovers them via *.ibu scan. This can lead to assertion failures. --- storage/innobase/xtrabackup/src/xtrabackup.cc | 53 ++++++++++++------- .../suites/lockless/incremental_delete.sh | 52 ++++++++++++++++++ 2 files changed, 87 insertions(+), 18 deletions(-) create mode 100644 storage/innobase/xtrabackup/test/suites/lockless/incremental_delete.sh diff --git a/storage/innobase/xtrabackup/src/xtrabackup.cc b/storage/innobase/xtrabackup/src/xtrabackup.cc index ebe2fa193ec..65b56e413b3 100644 --- a/storage/innobase/xtrabackup/src/xtrabackup.cc +++ b/storage/innobase/xtrabackup/src/xtrabackup.cc @@ -5539,6 +5539,18 @@ static bool xtrabackup_apply_delta( return false; } +// Function to check if a string ends with another string +static bool ends_with(const string &full_string, const string &ext) { + // Check if the ending string is longer than the full + // string + if (ext.size() > full_string.size()) return false; + + // Compare the ending of the full string with the target + // ending + return full_string.compare(full_string.size() - ext.size(), ext.size(), + ext) == 0; +} + /************************************************************************ Scan .meta files and build std::unordered_map. This map is later used to delete the .delta and .meta file for a dropped @@ -5559,6 +5571,12 @@ static bool xtrabackup_scan_meta( ut_a(xtrabackup_incremental); + // We dont want to add .new.meta to meta_map, as they are meant to be replace + // the old version of the file. + if (ends_with(entry.file_name, ".new.meta")) { + return true; + } + if (!xb_read_delta_metadata(meta_path.c_str(), &info)) { goto error; } @@ -5790,28 +5808,27 @@ static bool prepare_handle_del_files( ut::free(space_name); return false; } + } - os_file_delete(0, del_file.c_str()); - if (xtrabackup_incremental) { - auto [exists, meta_file] = is_in_meta_map(fil_space->id); - if (exists) { - // create .delta path from .meta - std::string delta_file = - meta_file.substr(0, strlen(meta_file.c_str()) - strlen(".meta")) + - ".delta"; - xb::info() << "Deleting incremental meta file: " << meta_file; - delete_force(meta_file); - xb::info() << "Deleting incremental delta file: " << delta_file; - delete_force(delta_file); - } + if (xtrabackup_incremental) { + auto [exists, meta_file] = is_in_meta_map(space_id); + if (exists) { + // create .delta path from .meta + std::string delta_file = + meta_file.substr(0, strlen(meta_file.c_str()) - strlen(".meta")) + + ".delta"; + xb::info() << "Deleting incremental meta file: " << meta_file; + delete_force(meta_file); + xb::info() << "Deleting incremental delta file: " << delta_file; + delete_force(delta_file); } - return true; - } else { - // if table was already deleted then return true - os_file_delete(0, del_file.c_str()); - return true; } + + xb::info() << "prepare_handle_del_files: deleting " << del_file.c_str(); + os_file_delete(0, del_file.c_str()); + return true; } + /************************************************************************ Callback to handle datadir entry. Deletes entry if it has no matching fil_space in fil_system directory. diff --git a/storage/innobase/xtrabackup/test/suites/lockless/incremental_delete.sh b/storage/innobase/xtrabackup/test/suites/lockless/incremental_delete.sh new file mode 100644 index 00000000000..ca9a9e87a23 --- /dev/null +++ b/storage/innobase/xtrabackup/test/suites/lockless/incremental_delete.sh @@ -0,0 +1,52 @@ +############################################################################### +# PXB-3320: prepare_handle_del_files() fails to delete the .meta and .delta files +# for deleted tablespaces in incremental backup directory +############################################################################### + +. inc/common.sh + +require_debug_pxb_version +start_server + +innodb_wait_for_flush_all + +xtrabackup --backup --target-dir=$topdir/backup_base --lock-ddl=REDUCED + +innodb_wait_for_flush_all + +$MYSQL $MYSQL_ARGS -Ns -e "CREATE TABLE t2(a INT)" test + +xtrabackup --backup --target-dir=$topdir/backup_inc --incremental-basedir=$topdir/backup_base \ + --debug-sync="ddl_tracker_before_lock_ddl" --lock-ddl=REDUCED \ + 2> >( tee $topdir/backup_inc.log)& + +job_pid=$! +pid_file=$topdir/backup_inc/xtrabackup_debug_sync +wait_for_xb_to_suspend $pid_file +xb_pid=`cat $pid_file` +echo "backup pid is $job_pid" + +# Generate redo on table than delete it +$MYSQL $MYSQL_ARGS -Ns -e "INSERT INTO test.t2 VALUES (1); DROP TABLE test.t2;" test + +# Resume the xtrabackup process +vlog "Resuming xtrabackup" +kill -SIGCONT $xb_pid +run_cmd wait $job_pid + +xtrabackup --prepare --apply-log-only --target-dir=$topdir/backup_base +xtrabackup --prepare --target-dir=$topdir/backup_base --incremental-dir=$topdir/backup_inc + +# Ensure that t2.ibd shouldn't be present +FILE=$topdir/backup_base/test/t2.ibd +[ ! -f $FILE ] || die "$FILE exists. It should have been deleted by prepare. Server has dropped the table during incremental" + +record_db_state test +stop_server +rm -rf $mysql_datadir/* +xtrabackup --copy-back --target-dir=$topdir/backup_base +start_server +verify_db_state test +rm -rf $topdir/backup_base +rm -rf $topdir/backup_inc +rm $topdir/backup_inc.log