Skip to content

Return HTTP 410 for undelete on permanently deleted blob#3236

Draft
rcshaw wants to merge 1 commit intolinkedin:masterfrom
rcshaw:undelete-410-on-permanently-deleted
Draft

Return HTTP 410 for undelete on permanently deleted blob#3236
rcshaw wants to merge 1 commit intolinkedin:masterfrom
rcshaw:undelete-410-on-permanently-deleted

Conversation

@rcshaw
Copy link
Copy Markdown

@rcshaw rcshaw commented Apr 30, 2026

Summary

Add a case for BlobDeletedPermanently in UndeleteOperation.processServerError, mapping it to RouterErrorCode.BlobDeleted so the frontend returns HTTP 410 Gone. Previously this server error code had no case in the switch and fell through to the default UnexpectedInternalError, surfacing as HTTP 500. A 410 (or any 4xx) is the correct semantic since the blob has been compacted and the undelete request can never succeed; clients should not retry.

Testing Done

Added blobDeletedPermanentlyTest in UndeleteManagerTest. Tests pass ./gradlew :ambry-router:test --tests "com.github.ambry.router.UndeleteManagerTest"

Add a case for BlobDeletedPermanently in UndeleteOperation.processServerError, mapping it to RouterErrorCode.BlobDeleted (HTTP 410 Gone).
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 30, 2026

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 52.25%. Comparing base (52ba813) to head (fb0ba99).
⚠️ Report is 376 commits behind head on master.

Files with missing lines Patch % Lines
...ava/com/github/ambry/router/UndeleteOperation.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             master    #3236       +/-   ##
=============================================
- Coverage     64.24%   52.25%   -11.99%     
+ Complexity    10398     8911     -1487     
=============================================
  Files           840      931       +91     
  Lines         71755    82074    +10319     
  Branches       8611    10436     +1825     
=============================================
- Hits          46099    42889     -3210     
- Misses        23004    35809    +12805     
- Partials       2652     3376      +724     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@snalli snalli changed the base branch from master to snalli/clustermap-test-flakes May 2, 2026 00:32
@snalli
Copy link
Copy Markdown
Contributor

snalli commented May 2, 2026

Bouncing to retrigger CI on the new base (snalli/clustermap-test-flakes).

@snalli snalli closed this May 2, 2026
@snalli
Copy link
Copy Markdown
Contributor

snalli commented May 2, 2026

Reopened — CI should now run with the parallelized unit-test matrix + clustermap deflake fixes inherited from the new base.

@snalli snalli reopened this May 2, 2026
@snalli snalli changed the base branch from snalli/clustermap-test-flakes to master May 2, 2026 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants