Details
-
Improvement
-
Status: Resolved
-
Minor
-
Resolution: Fixed
-
3.6.0
Description
// Access to ipMap or to any Set contained in the map needs to be // protected with synchronized (ipMap) { ... } private final Map<InetAddress, Set<NettyServerCnxn>> ipMap = new HashMap<>(); private void addCnxn(NettyServerCnxn cnxn) { cnxns.add(cnxn); synchronized (ipMap){ InetAddress addr = ((InetSocketAddress)cnxn.getChannel().remoteAddress()).getAddress(); Set<NettyServerCnxn> s = ipMap.get(addr); if (s == null) { s = new HashSet<>(); ipMap.put(addr, s); } s.add(cnxn); } }
This can be done better (less code, less contention) with Java 8 Map API. Although, as I look at this, the only thing this is used for is a count of the number of connections from each address. Maybe this should just store a count instead of a collection.
Also note that an exclusive lock is required with each interaction of the table. By moving to a ConcurrentHashMap:
Retrieval operations (including get) generally do not block, so may overlap with update operations (including put and remove).
https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ConcurrentHashMap.html
Removing this lock should improve ZK's performance for highly concurrent client workloads, especially since its Async Netty operations, unless of course there are other locks elsewhere.