[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #8572: Fix the deadlock in ClusterChangeMediator

[GitBox <gi...@apache.org>]

Jackie-Jiang commented on code in PR #8572:
URL: https://github.com/apache/pinot/pull/8572#discussion_r854759946


##########
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 understand that using `notifyAll()` can prevent certain deadlock situations, but in the current code there is only one single thread that can wait on this lock. Do you see a problem using `notify()` in this case?



-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org