From e71924a48ef002351f3e3487e7d1a9cd819662be Mon Sep 17 00:00:00 2001 From: Satya Bodapati Date: Mon, 22 Apr 2024 21:57:51 +0100 Subject: [PATCH] PXB-3120 : Assertion failure: Dir_Walker::is_directory Problem: -------- InnoDB assumes directories or files do not disappear. It is true for the engine because, it is in the startup mode and no opeartions are allowed at this point of time. Analysis: --------- With lock-ddl=RECUCED, tables can be dropped concurrently when pxb does *.ibd scan or subdirectories can disappear too. Fix: ---- Handle walk_posix() for missing files/directories. The scan should continue and skip these deleted files or directories. --- storage/innobase/include/os0file.h | 4 ++ storage/innobase/os/os0file.cc | 47 +++++++++++++++++-- storage/innobase/srv/srv0tmp.cc | 3 +- .../innobase/xtrabackup/src/backup_copy.cc | 4 +- .../ddl_between_discovery_and_file_open.sh | 38 +++++++++++++++ 5 files changed, 90 insertions(+), 6 deletions(-) diff --git a/storage/innobase/include/os0file.h b/storage/innobase/include/os0file.h index 3b69d61ab77b..786d83a42313 100644 --- a/storage/innobase/include/os0file.h +++ b/storage/innobase/include/os0file.h @@ -1790,10 +1790,14 @@ class Dir_Walker { public: using Path = std::string; +#ifdef XTRABACKUP + static std::tuple is_directory(const Path &path); +#else /** Check if the path is a directory. The file/directory must exist. @param[in] path The path to check @return true if it is a directory */ static bool is_directory(const Path &path); +#endif /* XTRABACKUP */ /** Depth first traversal of the directory starting from basedir @param[in] basedir Start scanning from this directory diff --git a/storage/innobase/os/os0file.cc b/storage/innobase/os/os0file.cc index 5783c297fda3..a537c44f5f7a 100644 --- a/storage/innobase/os/os0file.cc +++ b/storage/innobase/os/os0file.cc @@ -3642,13 +3642,24 @@ void Dir_Walker::walk_posix(const Path &basedir, bool recursive, Function &&f) { DIR *parent = opendir(current.m_path.c_str()); if (parent == nullptr) { - ib::info(ER_IB_MSG_784) << "Failed to walk directory" - << " '" << current.m_path << "'"; + if (opt_lock_ddl != LOCK_DDL_REDUCED) { + ib::info(ER_IB_MSG_784) << "Failed to walk directory" + << " '" << current.m_path << "'"; + } + continue; + } + auto [exists, file_type] = is_directory(current.m_path); + + if (!exists) { + if (opt_lock_ddl != LOCK_DDL_REDUCED) { + ib::info(ER_IB_MSG_784) << "Failed to walk directory" + << " '" << current.m_path << "'"; + } continue; } - if (!is_directory(current.m_path)) { + if (file_type == OS_FILE_TYPE_FILE) { f(current.m_path, current.m_depth); } @@ -3680,7 +3691,24 @@ void Dir_Walker::walk_posix(const Path &basedir, bool recursive, Function &&f) { continue; } - if (is_directory(path) && recursive) { + DBUG_EXECUTE_IF( + "dir_walker_dbug", if (strncmp(dirent->d_name, "drop_table.ibd", + strlen("drop_table.ibd")) == 0) { + *const_cast(&xtrabackup_debug_sync) = "dir_walker"; + + debug_sync_point("dir_walker"); + }); + + auto [exists, file_type] = is_directory(path); + + if (!exists) { + if (opt_lock_ddl != LOCK_DDL_REDUCED) { + ib::info() << "Path: " << path << " is missing "; + } + continue; + } + + if (file_type == OS_FILE_TYPE_DIR && recursive) { directories.push(Entry(path, current.m_depth + 1)); } else { f(path, current.m_depth + 1); @@ -7918,6 +7946,16 @@ void os_file_set_umask(ulint umask) { os_innodb_umask = umask; } @return the umask to use for file creation. */ ulint os_file_get_umask() { return (os_innodb_umask); } +#ifdef XTRABACKUP +std::tuple Dir_Walker::is_directory(const Path &path) { + os_file_type_t type; + bool exists; + + os_file_status(path.c_str(), &exists, &type); + + return {exists, type}; +} +#else /** Check if the path is a directory. The file/directory must exist. @param[in] path The path to check @return true if it is a directory */ @@ -7937,6 +7975,7 @@ bool Dir_Walker::is_directory(const Path &path) { return (false); } +#endif /* XTRABACKUP */ dberr_t os_file_write_retry(IORequest &type, const char *name, pfs_os_file_t file, const void *buf, diff --git a/storage/innobase/srv/srv0tmp.cc b/storage/innobase/srv/srv0tmp.cc index f5b3e54a9fa5..2649e696b4aa 100644 --- a/storage/innobase/srv/srv0tmp.cc +++ b/storage/innobase/srv/srv0tmp.cc @@ -316,7 +316,8 @@ void Tablespace_pool::delete_old_pool(bool create_new_db) { Dir_Walker::walk(temp_tbsp_dir.c_str(), false, [&](const std::string &path) { /* If it is a file and the suffix matches ".ibt", then delete it */ - if (!Dir_Walker::is_directory(path) && path.size() >= 4 && + auto [exists, file_type] = Dir_Walker::is_directory(path); + if (exists && file_type == OS_FILE_TYPE_FILE && path.size() >= 4 && (path.compare(path.length() - 4, 4, DOT_IBT) == 0)) { os_file_delete_if_exists(innodb_temp_file_key, path.c_str(), nullptr); } diff --git a/storage/innobase/xtrabackup/src/backup_copy.cc b/storage/innobase/xtrabackup/src/backup_copy.cc index bd643dc6ccb2..23ee85d0ac4f 100644 --- a/storage/innobase/xtrabackup/src/backup_copy.cc +++ b/storage/innobase/xtrabackup/src/backup_copy.cc @@ -1749,7 +1749,9 @@ bool binlog_file_location::find_binlog(const std::string &dir, error = false; Dir_Walker::walk(dir, false, [&](const std::string &path) mutable { - if (ends_with(path.c_str(), ".index") && !Dir_Walker::is_directory(path)) { + auto [exists, file_type] = Dir_Walker::is_directory(path); + if (ends_with(path.c_str(), ".index") && exists && + file_type == OS_FILE_TYPE_FILE) { std::ifstream f_index(path); if (f_index.fail()) { xb::error() << "cannot read " << SQUOTE(path.c_str()); 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 index 46fab35dc795..b6364fdf4264 100644 --- 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 @@ -98,6 +98,44 @@ xtrabackup --copy-back --target-dir=$BACKUP_DIR start_server verify_db_state test +echo +echo PXB-3120 : Assertion failure: Dir_Walker::is_directory +echo + +$MYSQL $MYSQL_ARGS -Ns -e "CREATE TABLE test.drop_table (id INT PRIMARY KEY AUTO_INCREMENT); INSERT INTO test.drop_table VALUES(1);" test + +innodb_wait_for_flush_all + +XB_ERROR_LOG=$topdir/backup.log +BACKUP_DIR=$topdir/backup +rm -rf $BACKUP_DIR +xtrabackup_background --backup --target-dir=$BACKUP_DIR --debug="d,dir_walker_dbug" --lock-ddl=REDUCED + +XB_PID=$! +pid_file=$BACKUP_DIR/xtrabackup_debug_sync +wait_for_xb_to_suspend $pid_file + +# Delete table +$MYSQL $MYSQL_ARGS -Ns -e "DROP TABLE test.drop_table;" test + +# Resume the xtrabackup process +vlog "Resuming xtrabackup" +kill -SIGCONT $XB_PID +run_cmd wait $XB_PID + +if ! egrep -q "DDL tracking : LSN: [0-9]* delete table ID: [0-9]* Name: test/drop_table.ibd" $XB_ERROR_LOG ; then + die "xtrabackup did not handle delete table DDL" +fi + +xtrabackup --prepare --target-dir=$BACKUP_DIR +record_db_state test + +stop_server +rm -rf $mysql_datadir/* +xtrabackup --copy-back --target-dir=$BACKUP_DIR +start_server +verify_db_state test + stop_server rm -rf $mysql_datadir $BACKUP_DIR rm $XB_ERROR_LOG