zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [zookeeper] BELUGABEHR commented on issue #880: ZOOKEEPER-3340: Improve Queue Usage in QuorumCnxManager.java
Date Sat, 13 Apr 2019 15:02:23 GMT
BELUGABEHR commented on issue #880: ZOOKEEPER-3340: Improve Queue Usage in QuorumCnxManager.java
URL: https://github.com/apache/zookeeper/pull/880#issuecomment-482817330
 
 
   @lvfangmin Thank you for the review.  So, this proposed PR 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:
   
   ```
       public void addToRecvQueue(Message msg) {
           synchronized(recvQLock) {
               if (recvQueue.remainingCapacity() == 0) {
                   try {
   ```
   
   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](https://github.com/apache/zookeeper/blob/master/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java#L1270-L1285).
 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 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message