shuturmurgh commented on code in PR #18486:
URL: https://github.com/apache/pinot/pull/18486#discussion_r4432679870
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/RetentionUtils.java:
##########
@@ -69,4 +109,43 @@ public static boolean isPurgeable(String tableNameWithType,
SegmentZKMetadata se
}
return false;
}
+
+ /// Parses {@link
SegmentsValidationAndRetentionConfig#getRetentionTimeUnit()} and
+ /// {@link SegmentsValidationAndRetentionConfig#getRetentionTimeValue()}
into table data retention duration in
+ /// milliseconds (same interpretation as controller time-based retention).
+ ///
+ /// @return millis, or empty if unset or not parseable
+ public static OptionalLong parseTableDataRetentionMillis(
Review Comment:
Agreed. `TableConfigRetentionUtils.buildRetentionStrategy` now delegates to
`RetentionUtils.parseTableDataRetentionMillis`, so the upload check and the
purge path share one parser (75d3a2c). The resulting boundary is the same as
before, and the existing `RetentionManagerTest` cases pass unchanged. I also
added `TableConfigRetentionUtilsTest`. It covers the configured window,
lowercase units, invalid or missing retention, and propagation of the
creation-time fallback flag.
While doing this I also dropped the helper's parse-failure log to DEBUG, so
a misconfigured table logs one warning per retention cycle instead of two.
--
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]