[ https://issues.apache.org/jira/browse/ZOOKEEPER-3340?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Andor Molnar resolved ZOOKEEPER-3340. ------------------------------------- Resolution: Fixed Issue resolved by pull request 880 [https://github.com/apache/zookeeper/pull/880] > Introduce CircularBlockingQueue in QuorumCnxManager.java > -------------------------------------------------------- > > Key: ZOOKEEPER-3340 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3340 > Project: ZooKeeper > Issue Type: Improvement > Components: server > Reporter: David Mollitor > Assignee: David Mollitor > Priority: Major > Labels: pull-request-available > Fix For: 3.6.0 > > Time Spent: 3h 40m > Remaining Estimate: 0h > > I was recently profiling a on a ZK Quorum Leader in a low-volume environment > and noticed that most of its time was spent in > {{QuorumCnxManager#RecvWorker}}. Nothing wrong with that, but it did draw my > attention to it. I noticed that {{Queue}} interactions are a bit... verbose. > I would like to propose that we streamline this area of the code. > > [https://github.com/apache/zookeeper/blob/master/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java#L1291-L1309] > This proposed JIRA should not be viewed simply as 'ArrayBlockingQueue' v.s. > 'CircularBlockingQueue'. > One of the things that this PR does is remove the need for double-locking. > For example in addToRecvQueue the following condition exists: > {code} > public void addToRecvQueue(Message msg) { > synchronized(recvQLock) { > if (recvQueue.remainingCapacity() == 0) { > try { > {code} > From here it can be observed that there are two locks obtained: {{recvQLock}} > and the one internal to {{recvQueue}}. This is required because there are > multiple interactions that this Manager wants to do on the queue in a > serialized way. The {{CircularBlockingQueue}} performs all of those actions > on behalf of the caller, but it does it internal to the queue, under a single > lock,... the one internal to {{CircularBlockingQueue}}. > The current code also has some race-conditions that are simply ignored when > they happen. The race conditions are detailed nicely in the code comments > here. However, the changes in this PR directly deal with, and eliminate, > these race conditions altogether since all actions that work against the > {{CircularBlockingQueue}} all occur within its internal locks. This greatly > simplifies the code and removes the need for new folks to learn this nuance > of "why is the code doing this." -- This message was sent by Atlassian Jira (v8.3.4#803005)