krishan1390 opened a new pull request, #18582: URL: https://github.com/apache/pinot/pull/18582
## 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 - [x] `./mvnw -pl pinot-controller -am test -Dtest=SegmentDeletionManagerTest,RetentionManagerTest,PinotHelixResourceManagerLastLLCSegmentsTest` - [x] `./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](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
