From 80f7a219187f3181117e997a74a4b3ce5242a8b0 Mon Sep 17 00:00:00 2001 From: Satya Bodapati Date: Thu, 22 Jan 2026 11:53:34 +0000 Subject: [PATCH] PXB-3643: Improve xbcloud delete to support Hierarchical Namespaces https://perconadev.atlassian.net/browse/PXB-3643 Problem: -------- On Azure platform, buckets with HNS (Hierarchical Namespaces) enabled, xbcloud delete operation failed. Analysis: --------- When HNS is enabled on Azure, it is no longer a flatnamespace, meanining just deleting objects as files is not sufficient. Deleting a dir before deleting all the files in directory caused delete failure. Refactor xbcloud delete to work with HNS-style directories by listing files and directories separately. Introduce a partitioned listing API with a default implementation and add Azure-specific directory detection using ResourceType. Refactor Azure listing parsing logic, and introduce a separate listin logic only for delete. With this new listing logic, xbcloud will receive information about files and directories. xbcloud will delete all files first and then delete the directories. --- .../xtrabackup/src/xbcloud/CMakeLists.txt | 1 + .../innobase/xtrabackup/src/xbcloud/azure.cc | 74 ++++++++++++++++--- .../innobase/xtrabackup/src/xbcloud/azure.h | 31 ++++++++ .../xtrabackup/src/xbcloud/object_store.cc | 41 ++++++++++ .../xtrabackup/src/xbcloud/object_store.h | 15 ++++ .../xtrabackup/src/xbcloud/xbcloud.cc | 47 ++++++++++-- 6 files changed, 190 insertions(+), 19 deletions(-) create mode 100644 storage/innobase/xtrabackup/src/xbcloud/object_store.cc diff --git a/storage/innobase/xtrabackup/src/xbcloud/CMakeLists.txt b/storage/innobase/xtrabackup/src/xbcloud/CMakeLists.txt index bfc15c2b8741..bc955bab8e0b 100644 --- a/storage/innobase/xtrabackup/src/xbcloud/CMakeLists.txt +++ b/storage/innobase/xtrabackup/src/xbcloud/CMakeLists.txt @@ -45,6 +45,7 @@ MYSQL_ADD_EXECUTABLE(xbcloud xbcloud.cc ../xbstream_read.cc http.cc + object_store.cc azure.cc s3.cc s3_ec2.cc diff --git a/storage/innobase/xtrabackup/src/xbcloud/azure.cc b/storage/innobase/xtrabackup/src/xbcloud/azure.cc index 5b8890ecaea2..127626190b0f 100644 --- a/storage/innobase/xtrabackup/src/xbcloud/azure.cc +++ b/storage/innobase/xtrabackup/src/xbcloud/azure.cc @@ -40,7 +40,7 @@ const std::string AZURE_DATE_HEADER = "x-ms-date"; const std::string AZURE_VERSION_HEADER = "x-ms-version"; const std::string AZURE_BLOB_TYPE_HEADER = "x-ms-blob-type"; const std::string AZURE_STORAGE_CLASS_HEADER = "x-ms-access-tier"; -const std::string AZURE_VERSION_DATE = "2020-06-12"; +const std::string AZURE_VERSION_DATE = "2020-10-02"; const std::string AZURE_DEVELOPMENT_HOST = "127.0.0.1:10000"; const std::string AZURE_HOST = ".blob.core.windows.net"; @@ -531,9 +531,12 @@ Azure_client::Azure_client(const Http_client *client, storage_account, access_key, development_storage, storage_class)); } -bool Azure_client::list_objects_with_prefix(const std::string &container, - const std::string &prefix, - std::vector &objects) { +// Common helper function for listing objects - handles pagination and XML +// parsing +template +bool Azure_client::list_objects_common(const std::string &container, + const std::string &prefix, + ProcessBlob &&process_blob) { bool truncated = true; std::string next_marker; @@ -602,15 +605,9 @@ bool Azure_client::list_objects_with_prefix(const std::string &container, auto node = blobs_node->first_node("Blob"); while (node != nullptr) { - auto name = node->first_node("Name"); - if (name == nullptr) { - msg_ts( - "%s: Failed to parse list container result. Cannot find object " - "name.\n", - my_progname); - return false; + if (!process_blob(node)) { + return false; // Processing failed } - objects.push_back(name->value()); node = node->next_sibling("Blob"); } } @@ -618,4 +615,57 @@ bool Azure_client::list_objects_with_prefix(const std::string &container, return true; } +bool Azure_client::list_objects_with_prefix(const std::string &container, + const std::string &prefix, + std::vector &objects) { + return list_objects_common( + container, prefix, [&objects](rapidxml::xml_node<> *node) { + auto name = node->first_node("Name"); + if (name == nullptr) { + msg_ts( + "%s: Failed to parse list container result. Cannot find object " + "name.\n", + my_progname); + return false; + } + objects.push_back(name->value()); + return true; + }); +} + +bool Azure_client::list_objects_files_and_dirs(const std::string &container, + const std::string &prefix, + std::vector &files, + std::vector &dirs) { + return list_objects_common( + container, prefix, [&](rapidxml::xml_node<> *node) { + auto name = node->first_node("Name"); + if (name == nullptr) { + msg_ts( + "%s: Failed to parse list container result. Cannot find object " + "name.\n", + my_progname); + return false; + } + + // HNS returns directories explicitly via the ResourceType property. + bool is_directory = false; + auto properties_node = node->first_node("Properties"); + if (properties_node) { + auto type_node = properties_node->first_node("ResourceType"); + if (type_node && type_node->value() && + strcmp(type_node->value(), "directory") == 0) { + is_directory = true; + } + } + + if (is_directory) { + dirs.push_back(name->value()); + } else { + files.push_back(name->value()); + } + return true; + }); +} + } // namespace xbcloud diff --git a/storage/innobase/xtrabackup/src/xbcloud/azure.h b/storage/innobase/xtrabackup/src/xbcloud/azure.h index 62cbba94db30..c09615fcbedf 100644 --- a/storage/innobase/xtrabackup/src/xbcloud/azure.h +++ b/storage/innobase/xtrabackup/src/xbcloud/azure.h @@ -107,6 +107,14 @@ class Azure_client { Event_handler *h, Azure_client::async_download_callback_t callback, CURLcode rc, const Http_connection *conn, ulong count); + // Common helper function for listing objects - handles pagination and XML + // parsing ProcessBlob is a callable that takes (rapidxml::xml_node<>* node) + // and returns bool Returns false to stop processing, true to continue + template + bool list_objects_common(const std::string &container, + const std::string &prefix, + ProcessBlob &&process_blob); + public: Azure_client(const Http_client *client, const std::string &storage_account, const std::string &access_key, bool development_storage, @@ -147,6 +155,22 @@ class Azure_client { const std::string &prefix, std::vector &objects); + /** + * List objects under a prefix and split them into files and directories. + * + * For HNS-enabled containers, directory entries are returned explicitly. + * + * @param container Container name. + * @param prefix Prefix to list. + * @param files Output list of file objects. + * @param dirs Output list of directory objects. + * @return true on success, false on error. + */ + bool list_objects_files_and_dirs(const std::string &container, + const std::string &prefix, + std::vector &files, + std::vector &dirs); + ulong get_max_retries() { return max_retries; } ulong get_max_backoff() { return max_backoff; } @@ -235,6 +259,13 @@ class Azure_object_store : public Object_store { bool &success) override { return azure_client.download_object(container, name, success); } + virtual bool list_objects_files_and_dirs( + const std::string &container, const std::string &directory, + std::vector &files, + std::vector &dirs) override { + return azure_client.list_objects_files_and_dirs(container, directory + "/", + files, dirs); + } }; } // namespace xbcloud diff --git a/storage/innobase/xtrabackup/src/xbcloud/object_store.cc b/storage/innobase/xtrabackup/src/xbcloud/object_store.cc new file mode 100644 index 000000000000..ca5d81d095dc --- /dev/null +++ b/storage/innobase/xtrabackup/src/xbcloud/object_store.cc @@ -0,0 +1,41 @@ +/****************************************************** +Copyright (c) 2026 Percona LLC and/or its affiliates. + +Object Store interface. + +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; version 2 of the License. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program; if not, write to the Free Software +Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA + +*******************************************************/ + +#include "xbcloud/object_store.h" + +namespace xbcloud { + +/** + * Default implementation that lists objects and treats all as files. + * + * @param container Container/bucket name. + * @param directory Directory prefix to list. + * @param files Output list of file objects. + * @param dirs Output list of directory objects (unused here). + * @return true on success, false on error. + */ +bool Object_store::list_objects_files_and_dirs(const std::string &container, + const std::string &directory, + std::vector &files, + std::vector &dirs) { + return list_objects_in_directory(container, directory, files); +} + +} // namespace xbcloud diff --git a/storage/innobase/xtrabackup/src/xbcloud/object_store.h b/storage/innobase/xtrabackup/src/xbcloud/object_store.h index d135c5bd551c..651f2b8cb27d 100644 --- a/storage/innobase/xtrabackup/src/xbcloud/object_store.h +++ b/storage/innobase/xtrabackup/src/xbcloud/object_store.h @@ -56,6 +56,21 @@ class Object_store { virtual Http_buffer download_object(const std::string &container, const std::string &name, bool &success) = 0; + /** + * List objects under a directory prefix and split them into files and dirs. + * + * Default implementation treats all returned objects as files. + * + * @param container Container/bucket name. + * @param directory Directory prefix to list. + * @param files Output list of file objects. + * @param dirs Output list of directory objects (may be empty). + * @return true on success, false on error. + */ + virtual bool list_objects_files_and_dirs(const std::string &container, + const std::string &directory, + std::vector &files, + std::vector &dirs); virtual ~Object_store() {} }; diff --git a/storage/innobase/xtrabackup/src/xbcloud/xbcloud.cc b/storage/innobase/xtrabackup/src/xbcloud/xbcloud.cc index 5883ac8da145..bd5e19bfe2f5 100644 --- a/storage/innobase/xtrabackup/src/xbcloud/xbcloud.cc +++ b/storage/innobase/xtrabackup/src/xbcloud/xbcloud.cc @@ -1045,16 +1045,27 @@ bool chunk_name_to_file_name(const std::string &chunk_name, return true; } +/** + * Delete a backup directory from object storage. + * + * @param store Object store implementation. + * @param container Container/bucket name. + * @param backup_name Backup directory to delete. + * @return true on success, false on error. + */ bool xbcloud_delete(Object_store *store, const std::string &container, const std::string &backup_name) { - std::vector object_list; + std::vector files; + std::vector dirs; - if (!store->list_objects_in_directory(container, backup_name, object_list)) { + // First pass lists files/dirs to validate existence and drive deletion. + if (!store->list_objects_files_and_dirs(container, backup_name, files, + dirs)) { msg_ts("%s: Delete failed. Cannot list %s.\n", my_progname, backup_name.c_str()); return false; } - if (object_list.empty()) { + if (files.empty() && dirs.empty()) { msg_ts("%s: error: backup named %s doesn't exists!\n", my_progname, backup_name.c_str()); return false; @@ -1068,7 +1079,7 @@ bool xbcloud_delete(Object_store *store, const std::string &container, auto thread = h.run(); bool error = false; - for (const auto &obj : object_list) { + for (const auto &obj : files) { std::string file_name; my_off_t idx; if (error) break; @@ -1090,6 +1101,8 @@ bool xbcloud_delete(Object_store *store, const std::string &container, } }, std::placeholders::_1, obj, &error))) { + h.stop(); + thread.join(); return false; } } @@ -1099,11 +1112,31 @@ bool xbcloud_delete(Object_store *store, const std::string &container, if (error) { msg_ts("%s: Delete failed.\n", my_progname); - } else { - msg_ts("%s: Delete completed.\n", my_progname); + return false; + } + + if (!dirs.empty()) { + std::sort(dirs.begin(), dirs.end(), std::greater()); + for (const auto &d : dirs) { + msg_ts("%s: Deleting directory %s.\n", my_progname, d.c_str()); + if (!store->delete_object(container, d)) { + msg_ts("%s: Delete failed. Cannot delete directory %s.\n", my_progname, + d.c_str()); + return false; + } + } + + // Delete the root directory of the backup + msg_ts("%s: Deleting directory %s.\n", my_progname, backup_name.c_str()); + if (!store->delete_object(container, backup_name)) { + msg_ts("%s: Warning: Failed to delete root directory %s.\n", my_progname, + backup_name.c_str()); + } } - return !error; + msg_ts("%s: Delete completed.\n", my_progname); + + return true; } void download_func(download_thread_ctxt_t &cntx) {