swaminathanmanish commented on code in PR #15118:
URL: https://github.com/apache/pinot/pull/15118#discussion_r1977412212


##########
pinot-minion/src/main/java/org/apache/pinot/minion/event/MinionProgressObserver.java:
##########
@@ -62,14 +64,29 @@ public synchronized void notifyTaskStart(PinotTaskConfig 
pinotTaskConfig) {
    */
   @Override
   public synchronized void notifyProgress(PinotTaskConfig pinotTaskConfig, 
@Nullable Object progress) {
-    if (LOGGER.isDebugEnabled()) {
-      LOGGER.debug("Update progress: {} for task: {}", progress, 
pinotTaskConfig.getTaskId());
-    }
+    String progressMessage = null;
+    MinionTaskBaseObserverStats.StatusEntry statusEntry = null;
     _taskProgressStats.setCurrentState(MinionTaskState.IN_PROGRESS.name());
-    addStatus(new MinionTaskBaseObserverStats.StatusEntry.Builder()
-        .withTs(System.currentTimeMillis())
-        .withStatus((progress == null) ? "" : progress.toString())
-        .build());
+    if (progress instanceof MinionTaskBaseObserverStats.StatusEntry) {

Review Comment:
   Are these checks (instance of StatusEntry Vs ObserverStats Vs toString) just 
for backwards compatibility ?



##########
pinot-spi/src/main/java/org/apache/pinot/spi/tasks/MinionTaskBaseObserverStats.java:
##########
@@ -34,8 +38,11 @@
  */
 public class MinionTaskBaseObserverStats {
   protected String _taskId;
+  protected String _currentStage;
   protected String _currentState;
   protected long _startTimestamp;
+  protected long _endTimestamp;
+  protected Map<String, Timer> _stageTimes = new HashMap<>();

Review Comment:
   This need not necessarily be used by all minion tasks? Could you add 
documentation on what this captures/how its supposed to be used. 
   



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