jackjlli commented on a change in pull request #3819: Refactor periodic task
URL: https://github.com/apache/incubator-pinot/pull/3819#discussion_r255814222
 
 

 ##########
 File path: 
pinot-core/src/main/java/org/apache/pinot/core/periodictask/BasePeriodicTask.java
 ##########
 @@ -42,9 +61,125 @@ public long getInitialDelayInSeconds() {
     return _initialDelayInSeconds;
   }
 
+  /**
+   * Returns the status of the {@code started} flag. This flag will be set 
after calling {@link #start()}, and reset
+   * after calling {@link #stop()}.
+   */
+  public final boolean isStarted() {
+    return _started;
+  }
+
+  /**
+   * Returns the status of the {@code running} flag. This flag will be set 
during the task execution.
+   */
+  public final boolean isRunning() {
+    return _running;
+  }
+
+  /**
+   * {@inheritDoc}
+   * <p>
+   * This method sets {@code started} flag to true.
+   */
   @Override
-  public String getTaskName() {
-    return _taskName;
+  public final synchronized void start() {
+    if (_started) {
+      LOGGER.warn("Task: {} is already started", _taskName);
+      return;
+    }
+    _started = true;
+
+    try {
+      startTask();
+    } catch (Exception e) {
+      LOGGER.error("Caught exception while setting up task: {}", _taskName, e);
+    }
+  }
+
+  /**
+   * Can be override for extra task setups.
+   */
+  protected void startTask() {
+  }
+
+  /**
+   * {@inheritDoc}
+   * <p>
+   * During the task execution, the {@code running} flag will be set.
+   */
+  @Override
+  public final void run() {
+    _running = true;
+
+    if (_started) {
+      long startTime = System.currentTimeMillis();
+      LOGGER.info("Start running task: {}", _taskName);
+      try {
+        runTask();
+      } catch (Exception e) {
+        LOGGER.error("Caught exception while running task: {}", _taskName, e);
+      }
+      LOGGER.info("Finish running task: {} in {}ms", _taskName, 
System.currentTimeMillis() - startTime);
+    } else {
+      LOGGER.warn("Task: {} is skipped because it is not started or already 
stopped", _taskName);
+    }
+
+    _running = false;
+  }
+
+  /**
+   * Executes the task. This method should early terminate if {@link}
 
 Review comment:
   Please add the condition when it should early terminate.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@pinot.apache.org
For additional commands, e-mail: dev-h...@pinot.apache.org

Reply via email to