PrachiKhobragade commented on code in PR #8572:
URL: https://github.com/apache/pinot/pull/8572#discussion_r855616042


##########
pinot-broker/src/main/java/org/apache/pinot/broker/broker/helix/ClusterChangeMediator.java:
##########
@@ -210,21 +206,21 @@ public void onLiveInstanceChange(List<LiveInstance> 
liveInstances, NotificationC
    *
    * @param changeType Type of the change
    */
-  private synchronized void enqueueChange(ChangeType changeType) {
+  private void enqueueChange(ChangeType changeType) {
     // Do not enqueue or process changes if already stopped
-    if (_stopped) {
+    if (!_running) {
+      LOGGER.warn("ClusterChangeMediator already stopped, skipping enqueuing 
the {} change", changeType);
       return;
     }
     if (_clusterChangeHandlingThread.isAlive()) {
-      LOGGER.info("Enqueue {} change", changeType);
-      synchronized (_lastChangeTimeMap) {
-        if (!_lastChangeTimeMap.containsKey(changeType)) {
-          _lastChangeTimeMap.put(changeType, System.currentTimeMillis());
+      LOGGER.info("Enqueueing {} change", changeType);
+      if (_lastChangeTimeMap.putIfAbsent(changeType, 
System.currentTimeMillis()) == null) {
+        synchronized (_lastChangeTimeMap) {
           _lastChangeTimeMap.notify();

Review Comment:
   I don't think notifyAll will be required in single threaded case. Thanks for 
clarifying.
   



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