Skip to content

Commit

Permalink
archival: fix purger::collect_manifest_paths()
Browse files Browse the repository at this point in the history
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 e3574ff)
  • Loading branch information
WillemKauf authored and vbotbuildovich committed Jul 26, 2024
1 parent 1b39d98 commit 368492b
Showing 1 changed file with 17 additions and 1 deletion.
18 changes: 17 additions & 1 deletion src/v/archival/purger.cc
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,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;
Expand Down

0 comments on commit 368492b

Please sign in to comment.