[GitHub] [nifi] markap14 commented on a change in pull request #5390: NIFI-9217 - avoid deadlock on cluster operation

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

markap14 commented on a change in pull request #5390:
URL: https://github.com/apache/nifi/pull/5390#discussion_r708611876



##########
File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/leader/election/CuratorLeaderElectionManager.java
##########
@@ -64,10 +64,10 @@
 
     private volatile boolean stopped = true;
 
-    private final Map<String, LeaderRole> leaderRoles = new HashMap<>();
-    private final Map<String, RegisteredRole> registeredRoles = new HashMap<>();
+    private final ConcurrentMap<String, LeaderRole> leaderRoles = new ConcurrentHashMap<>();
+    private final ConcurrentMap<String, RegisteredRole> registeredRoles = new ConcurrentHashMap<>();
 
-    private final Map<String, TimedBuffer<TimestampedLong>> leaderChanges = new HashMap<>();
+    private final ConcurrentMap<String, TimedBuffer<TimestampedLong>> leaderChanges = new ConcurrentHashMap<>();

Review comment:
       @exceptionfactory I was considering commenting the same thing. But then I decided not to because I think there's some value in calling it a `ConcurrentMap`. Similarly, if you instantiated `TreeSet` but didn't use any of the methods in `SortedSet`, only those in `Set` would you declare it as `Set` or `SortedSet`? I think there's some value in declaring that a `SortedSet` just because the declaration makes it clearer of your expectations. Similarly here, it makes it more clear that the declared `Map` is expected to be thread safe.
   
   That said, I don't have a problem with it one way or the other but would tend to lean slightly toward `ConcurrentMap`.




-- 
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: issues-unsubscribe@nifi.apache.org

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