9aman commented on code in PR #15142:
URL: https://github.com/apache/pinot/pull/15142#discussion_r1984783168
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/retention/strategy/RetentionStrategy.java:
##########
@@ -34,4 +34,14 @@ public interface RetentionStrategy {
* @return Whether the segment should be purged
*/
boolean isPurgeable(String tableNameWithType, SegmentZKMetadata
segmentZKMetadata);
-}
+
+ /**
+ * Returns whether the segment should be purged based on end time.
+ *
+ * @param segmentName The name of the segment to check
+ * @param tableNameWithType Table name with type
+ * @param endTimeMs The end time of the segment in milliseconds
+ * @return Whether the segment should be purged
+ */
+ boolean isPurgeable(String segmentName, String tableNameWithType, long
endTimeMs);
Review Comment:
On a second pass I think it's fine to have this.
One can pass the endTimeMs that needs to be compared to
System.currentTimeMillis() - retentionMS i.e.
System.currentTimeMillis() - retentionMS > endTimeMs.
It's a more generic implementation of the
```
public boolean isPurgeable(String tableNameWithType, SegmentZKMetadata
segmentZKMetadata)
```
Where one only needs to rely on endTime present in segmentZKMetadata
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
