pri1712 commented on code in PR #18256:
URL: https://github.com/apache/pinot/pull/18256#discussion_r3315909740


##########
pinot-core/src/main/java/org/apache/pinot/core/periodictask/PeriodicTaskScheduler.java:
##########
@@ -57,41 +67,92 @@ public void init(List<PeriodicTask> periodicTasks) {
 
   /**
    * Starts scheduling periodic tasks.
+   * It uses the cron expression if provided, if not, it falls back to the 
default fixed delay scheduling.
+   *
    */
   public synchronized void start() {
     if (_executorService != null) {
       LOGGER.warn("Periodic task scheduler already started");
+      return;
     }
+    Preconditions.checkState(_periodicTasks != null,
+        "Periodic task scheduler has not been initialized");
 
     if (_periodicTasks.isEmpty()) {
       LOGGER.warn("No periodic task scheduled");
-    } else {
-      Collection<PeriodicTask> periodicTasks = _periodicTasks.values();
-      LOGGER.info("Starting periodic task scheduler with tasks: {}", 
periodicTasks);
-      _executorService = 
Executors.newScheduledThreadPool(_periodicTasks.size());
+      return;
+    }
+
+    Collection<PeriodicTask> periodicTasks = _periodicTasks.values();
+    LOGGER.info("Starting periodic task scheduler with tasks: {}", 
periodicTasks);
+
+    for (PeriodicTask task : periodicTasks) {
+      String cron = task.getCronExpression();
+      if (cron != null && !cron.trim().isEmpty() && 
!CronExpression.isValidExpression(cron)) {
+        throw new IllegalArgumentException(
+            String.format("Invalid CRON expression '%s' for task '%s'. Halting 
controller startup.",
+                cron, task.getTaskName())
+        );
+      }
+    }
+
+    boolean hasCronTasks = periodicTasks.stream()
+        .map(PeriodicTask::getCronExpression)
+        .anyMatch(cron -> cron != null && !cron.trim().isEmpty());
+
+    if (hasCronTasks) {
+      try {
+        try {
+          _scheduler = new StdSchedulerFactory().getScheduler();

Review Comment:
   created a dedicated named scheduler for controller tasks



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