Jackie-Jiang commented on code in PR #18001:
URL: https://github.com/apache/pinot/pull/18001#discussion_r3003321103


##########
pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java:
##########
@@ -687,72 +640,73 @@ public int getRetentionControllerFrequencyInSeconds() {
     return 
Optional.ofNullable(getProperty(ControllerPeriodicTasksConf.RETENTION_MANAGER_FREQUENCY_PERIOD))
         .filter(period -> isValidPeriodWithLogging(
             ControllerPeriodicTasksConf.RETENTION_MANAGER_FREQUENCY_PERIOD, 
period))
-        .map(period -> (int) convertPeriodToSeconds(period)).orElseGet(
-            () -> 
getProperty(ControllerPeriodicTasksConf.DEPRECATED_RETENTION_MANAGER_FREQUENCY_IN_SECONDS,
-                
ControllerPeriodicTasksConf.DEFAULT_RETENTION_MANAGER_FREQUENCY_IN_SECONDS));
+        .map(period -> (int) convertPeriodToSeconds(period))
+        .orElse((int) 
convertPeriodToSeconds(ControllerPeriodicTasksConf.DEFAULT_RETENTION_MANAGER_FREQUENCY_PERIOD));
   }
 
   public void setRetentionControllerFrequencyInSeconds(int 
retentionFrequencyInSeconds) {
-    
setProperty(ControllerPeriodicTasksConf.DEPRECATED_RETENTION_MANAGER_FREQUENCY_IN_SECONDS,
-        Integer.toString(retentionFrequencyInSeconds));
+    setProperty(ControllerPeriodicTasksConf.RETENTION_MANAGER_FREQUENCY_PERIOD,
+        Long.toString(retentionFrequencyInSeconds) + "s");
   }
 
   /**
-   * Returns 
<code>controller.offline.segment.interval.checker.frequencyPeriod</code>, or
-   * <code>controller.offline.segment.interval.checker.frequencyPeriod</code> 
or the segment level
-   * validation interval, in the order of decreasing preference from left to 
right. Falls-back to
-   * the next config only if the current config is missing. This is done in 
order to retain the
-   * current behavior, wherein the offline validation tasks were done at 
segment level validation
-   * interval frequency The default value is the new 
DEFAULT_OFFLINE_SEGMENT_INTERVAL_CHECKER_FREQUENCY_IN_SECONDS
+   * Returns the offline segment interval checker frequency in seconds.
+   * Reads {@code controller.offline.segment.interval.checker.frequencyPeriod} 
as a period string (e.g. "24h"),
+   * falling back to {@code 
DEFAULT_OFFLINE_SEGMENT_INTERVAL_CHECKER_FREQUENCY_PERIOD} if absent or invalid.
    *
-   * @return the supplied config in seconds
+   * @return the configured frequency in seconds
    */
   public int getOfflineSegmentIntervalCheckerFrequencyInSeconds() {
     return Optional.ofNullable(
             
getProperty(ControllerPeriodicTasksConf.OFFLINE_SEGMENT_INTERVAL_CHECKER_FREQUENCY_PERIOD))
         .filter(period -> isValidPeriodWithLogging(
             
ControllerPeriodicTasksConf.OFFLINE_SEGMENT_INTERVAL_CHECKER_FREQUENCY_PERIOD, 
period))
-        .map(period -> (int) convertPeriodToSeconds(period)).orElseGet(() -> 
getProperty(
-            
ControllerPeriodicTasksConf.DEPRECATED_OFFLINE_SEGMENT_INTERVAL_CHECKER_FREQUENCY_IN_SECONDS,
-            
ControllerPeriodicTasksConf.DEFAULT_OFFLINE_SEGMENT_INTERVAL_CHECKER_FREQUENCY_IN_SECONDS));
+        .map(period -> (int) convertPeriodToSeconds(period))
+        .orElse(
+        (int) convertPeriodToSeconds(
+            
ControllerPeriodicTasksConf.DEFAULT_OFFLINE_SEGMENT_INTERVAL_CHECKER_FREQUENCY_PERIOD));

Review Comment:
   Directly use it as the default value of the property



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

Reply via email to