Jackie-Jiang commented on a change in pull request #8176:
URL: https://github.com/apache/pinot/pull/8176#discussion_r809571707



##########
File path: 
pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/retention/RetentionManagerTest.java
##########
@@ -197,6 +200,15 @@ public Object answer(InvocationOnMock invocationOnMock)
         return null;
       }
     }).when(resourceManager).deleteSegments(anyString(), 
ArgumentMatchers.anyList());
+
+    // fake segment lineage.
+    SegmentLineage segmentLineage = new SegmentLineage(REALTIME_TABLE_NAME);

Review comment:
       Why do we need to mock a segment lineage? Is this change related?

##########
File path: 
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/SegmentDeletionManager.java
##########
@@ -278,4 +311,36 @@ public void removeAgedDeletedSegments(int retentionInDays) 
{
       LOGGER.info("dataDir is not configured, won't delete any expired 
segments from deleted directory.");
     }
   }
+
+  private String getDeletedSegmentFileName(String fileName, long 
deletedSegmentsRetentionMs) {
+    return fileName + RETENTION_UNTIL_SEPARATOR + 
RETENTION_DATE_FORMAT.format(new Date(
+        System.currentTimeMillis() + deletedSegmentsRetentionMs));
+  }
+
+  private long getDeletionTimeMsFromFile(String targetFile, long 
lastModifiedTime) {
+    String[] split = StringUtils.splitByWholeSeparator(targetFile, 
RETENTION_UNTIL_SEPARATOR);
+    if (split.length == 2) {
+      try {
+        return RETENTION_DATE_FORMAT.parse(split[1]).getTime();
+      } catch (Exception e) {
+        LOGGER.warn("No retention suffix found for file: {}", targetFile);
+      }
+    }
+    LOGGER.info("Fallback to using default cluster retention config: {} ms", 
_defaultDeletedSegmentsRetentionMs);
+    return lastModifiedTime + _defaultDeletedSegmentsRetentionMs;
+  }
+
+  private static Long getRetentionMsFromTableConfig(@Nullable TableConfig 
tableConfig) {

Review comment:
       (minor) Annotate the return as `@Nullable`




-- 
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

Reply via email to