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