hadoop-zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Flavio Junqueira <...@yahoo-inc.com>
Subject Re: Incorrect implementation of QuorumCnxManager.haveDelivered()?
Date Thu, 26 Mar 2009 10:11:45 GMT
Hi Raghu, I'm moving this thread to the zookeeper-dev list. I think  
this discussion is more appropriate to that list.

With respect to the implementation of haveDelivered, please check  the  
posts on jira, issue 275 (https://issues.apache.org/jira/browse/ZOOKEEPER-275 
) for a scenario that makes it necessary, and let me know if it is  
still not clear after you check it out or you simply don't agree.

About the queue of messages, it was our intention originally to have  
all quorum communication going through the same channel (leader  
election and broadcast), and we thought about using QuorumCnxManager  
for that. We haven't had to time to make that change mainly because it  
would be a major change it seems. So, we have a more general  
implementation that queues messages to be delivered to other peers.  
This is not supposed to be a problem, though, because it simply  
implements a channel that eventually delivers messages.


On Mar 26, 2009, at 3:28 AM, raghul@yahoo.com wrote:

> Hello,
> I am a ZooKeeper newbie, so pardon me if I am repeating questions  
> that have been raised before.
> I believe the implementation of QuorumCnxManager.haveDelivered() is  
> incorrect. If I understand correctly, queueSendMap contains a queue  
> of messages for each peer to which the local peer is trying to send  
> election messages. When FastLeaderElection notices a timeout while  
> polling for inbound messages, it checks to see if all the messages  
> have been delivered by calling this function. So shouldn't this  
> function actually check each queue in the hash map and return true  
> if all of them are empty? This method is rather returning true the  
> if just one of the queues is empty?
>    /**
>     * Check if all queues are empty, indicating that all messages  
> have been delivered.
>     */
>    boolean haveDelivered() {
>        for (ArrayBlockingQueue<ByteBuffer> queue :  
> queueSendMap.values()) {
>            LOG.debug("Queue size: " + queue.size());
>            if (queue.size() == 0)
>                return true;
>        }
>        return false;
>    }
> Also, could someone expain the reason behind maitaining a queue of  
> messages for each peer in queueSendMap? Why do we need a per peer  
> queue here? Since this is used during election, the local peer is  
> not sending more than one message at a time to the remote peer. So  
> the hash map needs to store just one message per remote peer?
> -Raghu

View raw message