Add named-blob cleanup container exclusion list

[zichengl]

Summary

Adds an opt-in exclusion list to the named-blob stale-data cleanup runner so operators can exempt specific containers from cleanup. New config frontend.named.blob.cleanup.excluded.containers is a comma-separated list of fully-qualified accountName/containerName entries. Excluded containers are skipped by NamedBlobsCleanupRunner, so their superseded (stale) named-blob versions are retained instead of deleted. Defaults to empty, so there is no behavior change until an operator opts a container in.

Motivation: when named blobs are enabled, re-uploading the same name supersedes the previous version, and the cleanup runner eventually deletes the stale version(s). A consumer that has pinned an internal blob id (instead of reading by name) then gets 410 Gone once that pinned version is cleaned up. Reading by blob name is the durable fix; this exclusion list is a temporary, reversible safety valve that retains stale versions for specific containers while such consumers migrate.

Behavior:

• NamedBlobsCleanupRunner skips any non-DISABLED container whose accountName/containerName is in the list.
• Fail-safe: if the parent account cannot be resolved, the container is treated as not excluded, so an account-lookup miss can never silently suppress cleanup across all containers.
• When the list is empty the runner does no extra account lookups (short-circuits), so the default path is unchanged.

Testing Done

• ./gradlew :ambry-frontend:test --tests com.github.ambry.frontend.NamedBlobsCleanupRunnerTest — 6/6 pass, including new tests:
  • testExcludedContainersAreSkipped — an excluded container is never pulled/deleted; a sibling container in the same account still is.
  • testContainerProcessedWhenAccountUnresolvedDespiteExclusionList — an unresolved account leaves the container processed (fail-safe).
• ./gradlew :ambry-api:test --tests com.github.ambry.config.FrontendConfigTest — pass, including testNamedBlobCleanupExcludedContainers (default empty + comma-separated parsing).

Risk

• Additive and safe-by-default: an empty list preserves current behavior. The feature can only ever cause the cleaner to delete less, never more.
• Fully reversible: remove entries (or clear the property) to resume cleanup.
• Possible follow-up: support account-level entries (whole-account exclusion) in addition to per-container.

[satishkotha]

nit: check if containers with special characters parsed correctly (There are containers with whitespace allowed historically)

[zichengl]

Good catch. Utils.splitString splits only on , and filters empty segments without trimming, so whitespace inside a name is preserved, and the runner matches accountName/containerName verbatim — names with spaces resolve correctly. I documented the exact-match (whitespace-significant) semantics on the config field, and added testExcludedContainerWithWhitespaceInNameIsSkipped which parses "my account/my container" via Utils.splitString (exactly as FrontendConfig does) and asserts the container is skipped. See 21ab1fd.

[zichengl]

Digging in more, documenting "don't pad with spaces" isn't enough — it's a silent footgun. Utils.splitString(value, ",") splits on comma and drops empties but doesn't trim, so a list written the normal way — a/b, c/d — keeps a leading space on every entry after the first (" c/d"). That never matches accountName/containerName, so the container just keeps getting cleaned: the exact data loss this list exists to prevent, and it fails quietly.

So I switched to trimming each entry (and dropping blanks). Trimming only removes the outer padding, so whitespace inside a name is still significant — my account/my container matches verbatim, and the historical whitespace case you flagged still works. The one case you can't express this way is a name that literally begins or ends with a space; I called that out on the config field as out of scope for this list.

Added FrontendConfigTest coverage for the space-padded / blank-entry list and the internal-whitespace name. c7943f2

[codecov-commenter]

Codecov Report

❌ Patch coverage is 36.00000% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.82%. Comparing base (52ba813) to head (ef87010).
⚠️ Report is 410 commits behind head on master.

Files with missing lines	Patch %	Lines
...github/ambry/frontend/NamedBlobsCleanupRunner.java	16.66%	14 Missing and 1 partial ⚠️
...n/java/com/github/ambry/config/FrontendConfig.java	83.33%	1 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             master    #3286       +/-   ##
=============================================
- Coverage     64.24%   50.82%   -13.42%     
+ Complexity    10398     8706     -1692     
=============================================
  Files           840      938       +98     
  Lines         71755    80491     +8736     
  Branches       8611     9689     +1078     
=============================================
- Hits          46099    40913     -5186     
- Misses        23004    36195    +13191     
- Partials       2652     3383      +731     

☔ 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.

[satishkotha]

do we need this additional look up on account here? does container have accountName in it?

[zichengl]

Container carries only getParentAccountId() (the numeric id), not the account name, so to match on a human-readable accountName/containerName we do need to resolve the name. A couple of things keep the cost down:

• It's guarded by excludedContainers.isEmpty() and returns before any lookup, so in the normal case (no exclusions configured) there's zero overhead.
• When a list is set, getAccountById is an in-memory AccountService lookup, not a round trip.

I went with names rather than ids because this is a hand-edited safety-valve config and account-id/container-id is opaque to operators. If you'd rather drop the lookup entirely, I can key on parentAccountId + "/" + containerId (both already on Container), which also happens to be rename-stable — just less readable in the config. Happy to switch if you prefer that trade.

[zichengl]

Container only carries the numeric parent account id (getParentAccountId()) — it doesn't expose the account name. So to match the name-based accountName/containerName key we do have to resolve it via accountService.getAccountById(id).getName(); the name simply isn't on the container.

It's intentionally cheap, though:

• short-circuited by excludedContainers.isEmpty(), so there are zero extra lookups in the normal (no-exclusions) case;
• getAccountById is an in-memory cache hit, and the runner already holds accountService;
• a null account is treated as not excluded (fail-safe).

We could drop the lookup by keying the list on the numeric identity the container already has (parentAccountId/containerName, or parentAccountId:containerId = Container.toString()), but the list is human-maintained by name (sourced from the tracking sheet), so I kept it name-based for operability. Happy to switch to id-based — or iterate getAllAccounts() so the Account (and its name) is in hand without the per-container lookup — if you prefer.