From 8ef2313edc99ca06f3f69dec8d3328494f6f10b9 Mon Sep 17 00:00:00 2001 From: Willem Kaufmann Date: Thu, 25 Jul 2024 19:21:21 -0400 Subject: [PATCH] archival: fix `purger::collect_manifest_paths()` Before, the `purger` would push back what it assumed was the spillover manifest file by default to its list of `collected_manifests`. In the case of ABS, `_api.list_objects` might actually return the directory itself as a `Blob`. This would lead to the `purger` attempting to download the directory as if it were a manifest, which would always fail. This would completely block the `purger` from progressing and deleting other partitions in the deleted topic, as it would retry the same doomed manifest download. Correct the logic in `collect_manifest_paths()` by checking the path for `manifest.bin`, which should be contained within the spillover filename (e.g `.../5_21/manifest.bin.10.11.0.1.999.1000`). (cherry picked from commit e3574ffbbfde3eb09b12786d6270a2fba4c85937) --- src/v/archival/purger.cc | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/v/archival/purger.cc b/src/v/archival/purger.cc index 9a7da7687b703..d97d5f4ad95f2 100644 --- a/src/v/archival/purger.cc +++ b/src/v/archival/purger.cc @@ -225,7 +225,23 @@ purger::collect_manifest_paths( continue; } - collected.spillover.push_back(std::move(item.key)); + // The spillover manifest path is of the form + // "{prefix}/{manifest.bin().x.x.x.x.x.x}" Find the index of the last + // '/' in the path, so we can check just the filename (starting from the + // first character after '/'). + const size_t filename_idx = path.rfind('/'); + if (filename_idx == std::string_view::npos) { + continue; + } + + // File should start with "manifest.bin()", but it should have + // additional spillover components as well. + std::string_view file = path.substr(filename_idx + 1); + if ( + file.starts_with(cloud_storage::partition_manifest::filename()) + && !file.ends_with(cloud_storage::partition_manifest::filename())) { + collected.spillover.push_back(std::move(item.key)); + } } co_return collected;