cryptoe commented on code in PR #15180:
URL: https://github.com/apache/druid/pull/15180#discussion_r1368700110


##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/MSQWorkerTaskLauncher.java:
##########
@@ -348,6 +351,68 @@ public boolean isTaskLatest(String taskId)
     }
   }
 
+  public static class WorkerStats
+  {
+    String workerId;
+    TaskState state;
+    long duration;
+
+    /**
+     * For JSON deserialization only
+     */
+    public WorkerStats()
+    {
+    }
+
+    public WorkerStats(String workerId, TaskState state, long duration)
+    {
+      this.workerId = workerId;
+      this.state = state;
+      this.duration = duration;
+    }
+
+    @JsonProperty()
+    public String getWorkerId()
+    {
+      return workerId;
+    }
+
+    @JsonProperty()
+    public TaskState getState()
+    {
+      return state;
+    }
+
+    @JsonProperty("durationMs")
+    public long getDuration()
+    {
+      return duration;
+    }
+  }
+
+  public Map<Integer, List<WorkerStats>> getWorkerStats()
+  {
+    final Map<Integer, List<WorkerStats>> workerStats = new TreeMap<>();
+
+    for (Map.Entry<String, TaskTracker> taskEntry : taskTrackers.entrySet()) {

Review Comment:
   Lets use a concurrentHashMap ?
    
   ```
   Returns a synchronized (thread-safe) map backed by the specified map. In 
order to guarantee serial access, it is critical that all access to the backing 
map is accomplished through the returned map.
   It is imperative that the user manually synchronize on the returned map when 
traversing any of its collection views via Iterator, Spliterator or Stream:
      Map m = Collections.synchronizedMap(new HashMap());
          ...
      Set s = m.keySet();  // Needn't be in synchronized block
          ...
      synchronized (m) {  // Synchronizing on m, not s!
          Iterator i = s.iterator(); // Must be in synchronized block
          while (i.hasNext())
              foo(i.next());
      }
     
     ```
   Since its every easy to use s.iteratator() no ?
   



##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/MSQWorkerTaskLauncher.java:
##########
@@ -111,7 +115,7 @@ private enum State
   // Mutable state accessible only to the main loop. LinkedHashMap since order 
of key set matters. Tasks are added

Review Comment:
   Lets update the comments as well. 
   



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