Controller cleanups: SegmentDeletionManager fallback, LLC filter, retention strategy extraction, mock util

[krishan1390]

Summary

Four small, independent controller-side improvements grouped into one PR. Each lands as its own commit so reviewers can read them concern-by-concern and each is independently revertable. Please do not squash on merge — preserve per-commit granularity.

1. SegmentDeletionManager: fall back to recursive remove when batch ZK remove leaves a non-empty znode. The batch ZK remove API is non-recursive, so it silently fails to delete segment znodes that have accumulated children. On batch-remove failure, fall back to the single-path _propertyStore.remove(...) API, which handles non-empty nodes by deleting recursively. Skip when the znode is already absent.

2. PinotHelixResourceManager#getLastLLCCompletedSegments: skip non-LLC-named segments. A realtime table can contain uploaded (non-LLC-named) segments in DONE state alongside the LLC ones. LLCSegmentName.of(name) returns null for those, causing an NPE on the next field access. Filter them out. Includes a regression test (PinotHelixResourceManagerLastLLCSegmentsTest) that reproduces the NPE without the fix.

3. Extract retention strategy construction into TableConfigRetentionUtils. Pull the TimeRetentionStrategy construction (with its parse / invalid-config handling) out of RetentionManager into a static helper. Returning null for absent or malformed config preserves the existing "skip retention" behavior, and the warn-log shape is preserved (single message for both null/empty and parse-error paths).

4. Add a partition-aware SegmentMetadataMockUtils.mockSegmentMetadata overload. Additive test helper that returns a mock backed by a MurmurPartitionFunction. Purely test-ergonomics.

Test plan

• ./mvnw -pl pinot-controller -am test -Dtest=SegmentDeletionManagerTest,RetentionManagerTest,PinotHelixResourceManagerLastLLCSegmentsTest
• ./mvnw spotless:apply checkstyle:check license:check -pl pinot-controller
• Manual: drop a non-LLC DONE segment into a realtime table and confirm retention no longer NPEs.

🤖 Generated with Claude Code

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.77%. Comparing base (0a8ac02) to head (28efa97).

❗ There is a different number of reports uploaded between BASE (0a8ac02) and HEAD (28efa97). Click for more details.

HEAD has 4 uploads less than BASE

Flag	BASE (0a8ac02)	HEAD (28efa97)
java-21	5	4
unittests	2	1
temurin	5	4
unittests2	1	0

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #18582      +/-   ##
============================================
- Coverage     64.27%   56.77%   -7.50%     
+ Complexity     1137        7    -1130     
============================================
  Files          3335     2567     -768     
  Lines        205898   148958   -56940     
  Branches      32129    24099    -8030     
============================================
- Hits         132333    84568   -47765     
+ Misses        62931    57203    -5728     
+ Partials      10634     7187    -3447     

Flag	Coverage Δ
custom-integration1	100.00% <ø> (ø)
integration	100.00% <ø> (ø)
integration1	100.00% <ø> (ø)
integration2	0.00% <ø> (ø)
java-21	56.77% <ø> (-7.50%)	⬇️
temurin	56.77% <ø> (-7.50%)	⬇️
unittests	56.77% <ø> (-7.50%)	⬇️
unittests1	56.77% <ø> (+0.01%)	⬆️
unittests2	?

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.