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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
