shuturmurgh commented on code in PR #18486:
URL: https://github.com/apache/pinot/pull/18486#discussion_r4502311509
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/RetentionUtils.java:
##########
@@ -49,18 +69,38 @@ private RetentionUtils() {
/// @return true if the segment is past the retention boundary, false
otherwise
public static boolean isPurgeable(String tableNameWithType,
SegmentZKMetadata segmentZKMetadata, long retentionMs,
long currentTimeMs, boolean useCreationTimeFallback) {
- String segmentName = segmentZKMetadata.getSegmentName();
- long endTimeMs = segmentZKMetadata.getEndTimeMs();
+ return isPurgeableInternal(segmentZKMetadata.getEndTimeMs(),
segmentZKMetadata.getCreationTime(), retentionMs,
+ currentTimeMs, useCreationTimeFallback, tableNameWithType,
segmentZKMetadata.getSegmentName());
+ }
+
+ /// Whether segment file metadata is past the retention window, using end
time from
+ /// {@link #getSegmentMetadataEndTimeMillis(SegmentMetadata)} and
+ /// {@link SegmentMetadata#getIndexCreationTime()} as the optional fallback
clock. Invalid-end paths emit the same
+ /// style of debug/warn lines as the ZK overload, using
+ /// {@link SegmentMetadata#getName()} for the segment in log messages.
+ ///
+ /// - If end time is valid: purgeable when `currentTimeMs - endTimeMs >
retentionMs`.
+ /// - If end time is invalid and `useCreationTimeFallback` is true and index
creation time is valid: purgeable when
+ /// `currentTimeMs - creationTimeMs > retentionMs`.
+ /// - Otherwise: not purgeable (fail-open).
+ public static boolean isPurgeable(String tableNameWithType, SegmentMetadata
segmentMetadata, long retentionMs,
+ long currentTimeMs, boolean useCreationTimeFallback) {
Review Comment:
Done in e167b86. The overload is now `isPurgeable(tableNameWithType,
segmentMetadata, retentionMs, currentTimeMs)`. It reads only the end time and
time unit and never calls `getIndexCreationTime()`.
I also restored the `SegmentZKMetadata` overload to its original body, so
the creation-time fallback used by the retention manager and the minion task
generators is unchanged. The fallback test for the new overload is removed.
--
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]
