jineshparakh commented on code in PR #17741:
URL: https://github.com/apache/pinot/pull/17741#discussion_r2842084771


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotTaskManager.java:
##########
@@ -1061,6 +1115,129 @@ protected void nonLeaderCleanup(List<String> 
tableNamesWithType) {
     }
   }
 
+  @Override
+  public void onChange(Set<String> changedConfigs, Map<String, String> 
clusterConfigs) {
+    // Parent implementation is a no-op; no need to call super.onChange().
+    // This needed to be revisited if parent behavior changes
+    if (clusterConfigs == null) {
+      return;
+    }
+
+    if (changedConfigs == null
+        || 
changedConfigs.contains(ControllerConf.ControllerPeriodicTasksConf.PINOT_TASK_EXPIRE_TIME_MS))
 {
+      String taskExpireStr = clusterConfigs.get(
+          
ControllerConf.ControllerPeriodicTasksConf.PINOT_TASK_EXPIRE_TIME_MS);
+      if (taskExpireStr != null) {
+        try {
+          long parsed = Long.parseLong(taskExpireStr);
+          if (parsed <= 0) {
+            LOGGER.warn("Ignoring non-positive value {} for {}", parsed,
+                
ControllerConf.ControllerPeriodicTasksConf.PINOT_TASK_EXPIRE_TIME_MS);
+          } else {
+            _helixTaskResourceManager.setTaskExpireTimeMs(parsed);
+          }
+        } catch (NumberFormatException e) {
+          LOGGER.warn("Invalid value for {}: {}",
+              
ControllerConf.ControllerPeriodicTasksConf.PINOT_TASK_EXPIRE_TIME_MS, 
taskExpireStr);
+        }
+      }
+    }
+
+    if (changedConfigs == null

Review Comment:
   Done



##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotTaskManager.java:
##########
@@ -1061,6 +1115,129 @@ protected void nonLeaderCleanup(List<String> 
tableNamesWithType) {
     }
   }
 
+  @Override
+  public void onChange(Set<String> changedConfigs, Map<String, String> 
clusterConfigs) {
+    // Parent implementation is a no-op; no need to call super.onChange().
+    // This needed to be revisited if parent behavior changes
+    if (clusterConfigs == null) {
+      return;
+    }
+
+    if (changedConfigs == null

Review Comment:
   Have added it as a defensive check to prevent gets from a NULL map.



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