swaminathanmanish opened a new pull request, #18257:
URL: https://github.com/apache/pinot/pull/18257
## Problem
REFRESH (non-APPEND) offline tables have their segments deleted via
`manageSegmentLineageCleanupForTable`, which previously called
`deleteSegments(tableNameWithType, segmentsToDelete)` — the no-arg overload
that ignores `deletedSegmentsRetentionPeriod` from the table config and always
falls back to the cluster-level default (7 days).
This means operators could not configure immediate deletion for REFRESH
tables even though the infrastructure (`retentionMs <= 0` → skip
`Deleted_Segments/` staging and delete directly) already supported it.
A secondary issue: the cluster-level config
`controller.deleted.segments.retentionInDays` only accepted integer days,
making sub-day values impossible to express.
## Changes
**`RetentionManager.java`**
- `manageSegmentLineageCleanupForTable` now passes
`deletedSegmentsRetentionPeriod` from the table config to `deleteSegments`, so
REFRESH tables honour their per-table staging retention.
- Setting `deletedSegmentsRetentionPeriod: "0d"` on a REFRESH table now
causes superseded segments to be deleted immediately instead of being staged
for the cluster default.
**`ControllerConf.java`**
- Adds `controller.deleted.segments.retentionPeriod` config key accepting a
period string (`"7d"`, `"12h"`, `"0d"`).
- New `getDeletedSegmentsRetentionMs()` getter: prefers the period key,
falls back to `retentionInDays × 86400000` for backward compatibility.
**`SegmentDeletionManager.java`**
- Constructor parameter changed from `int deletedSegmentsRetentionInDays` to
`long deletedSegmentsRetentionMs` to accept the already-resolved value
directly, removing the forced day-granularity conversion.
**`PinotHelixResourceManager.java`**
- Field and constructor parameter updated to `long
deletedSegmentsRetentionMs`.
- `ControllerConf` constructor delegates to
`getDeletedSegmentsRetentionMs()` instead of
`getDeletedSegmentsRetentionInDays()`.
**`SegmentDeletionManagerTest.java`**
- Updated 3 constructor calls from `7` (int days) to
`TimeUnit.DAYS.toMillis(7)` (long ms).
## Usage
**Immediate deletion for all tables (cluster-level):**
```properties
controller.deleted.segments.retentionPeriod=0d
```
**12-hour staging window (cluster-level):**
```properties
controller.deleted.segments.retentionPeriod=12h
```
**Per-table immediate deletion for a REFRESH table:**
```json
"segmentsConfig": {
"deletedSegmentsRetentionPeriod": "0d"
}
```
## Backward Compatibility
- `controller.deleted.segments.retentionInDays` continues to work unchanged;
the new key only takes effect when explicitly set.
- No behavior change for APPEND tables or realtime tables.
- `null` `deletedSegmentsRetentionPeriod` on a table config falls through to
the cluster default as before.
--
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]
