Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v24.1.x] archival: fix purger::collect_manifest_paths() #22541

Merged

Conversation

vbotbuildovich
Copy link
Collaborator

Backport of PR #22335

@vbotbuildovich vbotbuildovich added this to the v24.1.x-next milestone Jul 26, 2024
@vbotbuildovich vbotbuildovich added the kind/backport PRs targeting a stable branch label Jul 26, 2024
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)
@WillemKauf WillemKauf force-pushed the backport-pr-22335-v24.1.x-48 branch from 368492b to 15fc60d Compare July 27, 2024 01:49
@WillemKauf
Copy link
Contributor

Backport build failed due to missing partition_manifest::filename().

Force push to amend:

@@ -242,9 +242,10 @@ purger::collect_manifest_paths(
        // File should start with "manifest.bin()", but it should have
        // additional spillover components as well.
        std::string_view file = path.substr(filename_idx + 1);
+       static const ss::sstring partition_manifest_filename = "manifest.bin";
        if (
-         file.starts_with(cloud_storage::partition_manifest::filename())
-         && !file.ends_with(cloud_storage::partition_manifest::filename())) {
+         file.starts_with(partition_manifest_filename)
+        && !file.ends_with(partition_manifest_filename)) {
            collected.spillover.push_back(std::move(item.key));
        }
    }

@WillemKauf
Copy link
Contributor

CI failure is #21626

@WillemKauf WillemKauf merged commit c74f61f into redpanda-data:v24.1.x Jul 27, 2024
17 checks passed
@BenPope BenPope modified the milestones: v24.1.x-next, v24.1.14 Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/redpanda kind/backport PRs targeting a stable branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants