Skip to content

Add opt-in all-version purge to named-blob cleanup - #3305

Merged
zichengl merged 2 commits into
linkedin:masterfrom
zichengl:8845012+zichengl/cleaner-purge-all-versions
Oct 14, 2026
Merged

zichengl merged 2 commits into
linkedin:masterfrom
zichengl:8845012+zichengl/cleaner-purge-all-versions

Conversation

@zichengl

@zichengl zichengl commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  1. Add an opt-in all-version purge mode to the named-blob cleaner, including latest READY and recent IN_PROGRESS versions. Existing constructors keep purge disabled.
  2. Use ascending primary-key order for MySQL purge pages, retaining an inclusive last-name cursor so a single name can span multiple pages.
  3. Add a caller-owned eligibility predicate, bounded first-page dry-run preview, counters, and failure-safe paging. Exclusions remain authoritative and container lifecycle status is unchanged.

Durability risk: high when enabled. Purge intentionally deletes live/latest versions. Callers must explicitly approve each container, fence source reads/writes, and verify that the source data is no longer needed. Eligibility is checked before each page and delete, but revocation cannot cancel an already-issued deletion. Deletion or metadata failures stop purge; subsequent runs restart the sweep. Repeated versions after metadata cleanup also stop the sweep, and purge never skips names after query kills.

This draft proposes only the generic OSS mechanism. Deployment configuration, migration/completeness checks, composite-database delegation, and verification of composite-router delete routing remain integration responsibilities. No rollout is included. Expired/already-deleted mappings retain their existing cleanup paths. Primary-key ordering avoids the mixed-order filesort but does not guarantee bounded latency when many index entries are filtered.

Please review the API/runner policy first, then the MySQL paging behavior.

Testing Done

Local validation uses JDK 17; CI uses JDK 11.

Command Result
./gradlew --no-daemon --console=plain -PdisableShipkit :ambry-frontend:test --tests com.github.ambry.frontend.NamedBlobsCleanupRunnerTest :ambry-named-mysql:test --tests com.github.ambry.named.MySqlNamedBlobDbTest :ambry-named-mysql:compileIntTestJava Passed: 24 cleaner tests and 19 database unit tests; integration tests compile.
git diff --check Passed.
./gradlew --no-daemon --console=plain -PdisableShipkit :ambry-named-mysql:intTest --tests 'com.github.ambry.named.MySqlNamedBlobDbIntegrationTest.testPurge*' Not run locally after the fix: local MySQL is unavailable. Live SQL validation is covered by the CI command below.
./gradlew --scan --warning-mode=summary intTest -x :ambry-server:intTest codeCoverageReport Passed in CI. Named-blob integration suite: 110 tests, 102 passed, 8 skipped, 0 failed; purge paging and EXPLAIN pass in both hard-delete configurations.

Full local build/full local test suite not run.

Add caller-owned purge eligibility and dry-run preview without changing container lifecycle or enabling purge by default. Scan MySQL purge pages in primary-key order, wait for metadata cleanup before paging, and stop on deletion failures or repeated versions. Keep deployment and migration-specific policy outside the generic cleaner.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@codecov-commenter

codecov-commenter commented Oct 9, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 28.35821% with 48 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.70%. Comparing base (52ba813) to head (7105f05).
⚠️ Report is 424 commits behind head on master.

Files with missing lines Patch % Lines
...github/ambry/frontend/NamedBlobsCleanupRunner.java 18.75% 39 Missing ⚠️
.../main/java/com/github/ambry/named/NamedBlobDb.java 0.00% 5 Missing ⚠️
.../java/com/github/ambry/named/MySqlNamedBlobDb.java 71.42% 1 Missing and 3 partials ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             master    #3305       +/-   ##
=============================================
- Coverage     64.24%   50.70%   -13.54%     
+ Complexity    10398     8711     -1687     
=============================================
  Files           840      939       +99     
  Lines         71755    80881     +9126     
  Branches       8611     9768     +1157     
=============================================
- Hits          46099    41014     -5085     
- Misses        23004    36465    +13461     
- Partials       2652     3402      +750     

☔ View full report in Codecov by Harness.
📢 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.

Metadata cleanup timestamps use System.currentTimeMillis(), whereas named blob GET evaluates deletion against the injected MockTime. Reset the shared test clock at the start and after cleanup so the Deleted assertion does not depend on test ordering or elapsed suite time. Preserve all existing purge paging and deletion assertions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@zichengl
zichengl marked this pull request as ready for review October 13, 2026 17:00
@zichengl
zichengl merged commit 6e959c1 into linkedin:master Oct 14, 2026
11 checks passed
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