Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Plain View
Zookeeper, mail # dev - Re: Review Request: patch for ZOOKEEPER-1560: Zookeeper client hangs on creation of large nodes


+
Skye Wanderman-Milne 2012-10-25, 22:38
+
Skye Wanderman-Milne 2012-10-25, 22:51
+
Skye Wanderman-Milne 2012-10-25, 23:33
+
Skye Wanderman-Milne 2012-10-26, 00:53
+
Ted Yu 2012-10-25, 22:55
+
Patrick Hunt 2012-10-31, 04:58
+
Patrick Hunt 2012-10-25, 21:36
+
Skye Wanderman-Milne 2012-10-25, 23:08
+
Patrick Hunt 2012-10-25, 21:23
+
Patrick Hunt 2012-10-25, 23:42
+
Skye Wanderman-Milne 2012-10-25, 23:11
+
Skye Wanderman-Milne 2012-10-25, 23:08
Copy link to this message
-
Re: Review Request: patch for ZOOKEEPER-1560: Zookeeper client hangs on creation of large nodes
Patrick Hunt 2012-10-26, 05:50


> On Oct. 25, 2012, 9:23 p.m., Patrick Hunt wrote:
> > src/java/main/org/apache/zookeeper/ClientCnxnSocketNIO.java, lines 133-134
> > <https://reviews.apache.org/r/7730/diff/1/?file=179601#file179601line133>
> >
> >     why are we synchronizing this? it's already protected by outgoingQueue, is this not enough? (it was only protected by outgoingQueue in 3.3 branch, why did it change?)
>
> Skye Wanderman-Milne wrote:
>     pendingQueue is not always protected by outgoingQueue (see ClientCnxn.SendThread.readResponse, SendThread.cleanup). It's likely that these functions are never called concurrently with doIO but I'll have to look into it, and maybe we should document somewhere that pendingQueue should be protected by outgoingQueue (since right now it's not). I'll also look into what's going on in 3.3.
>
> Skye Wanderman-Milne wrote:
>     In 3.3 the other references to pendingQueue (readResponse, cleanup) are synced on itself (and not outgoingQueue), so it looks like not syncing in doIO was an oversight and we should keep the synchronized block.
>
> Patrick Hunt wrote:
>     Ok. In that case can you verify that there are no lock ordering issues that might cause a deadlock?
>
> Skye Wanderman-Milne wrote:
>     I don't see any potential deadlocks -- this is actually the only instance where both locks are held at the same time.
>    
>     It also appears pendingQueue is only used within the sendThread (except for ClientCnxn.toString), so I'm tempted to remove all synchronization on it completely.

Ok, that's what I wanted to verify.
- Patrick
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7730/#review12792
-----------------------------------------------------------
On Oct. 25, 2012, 10:51 p.m., Skye Wanderman-Milne wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7730/
> -----------------------------------------------------------
>
> (Updated Oct. 25, 2012, 10:51 p.m.)
>
>
> Review request for zookeeper, Patrick Hunt and Ted Yu.
>
>
> Description
> -------
>
> see ZOOKEEPER-1560 JIRA
>
>
> This addresses bug ZOOKEEPER-1560.
>     https://issues.apache.org/jira/browse/ZOOKEEPER-1560
>
>
> Diffs
> -----
>
>   src/java/main/org/apache/zookeeper/ClientCnxnSocketNIO.java 70d8538
>
> Diff: https://reviews.apache.org/r/7730/diff/
>
>
> Testing
> -------
>
> unit tests (including testLargeNodeData from ZOOKEEPER-1560 JIRA)
>
>
> Thanks,
>
> Skye Wanderman-Milne
>
>

+
Skye Wanderman-Milne 2012-10-26, 00:59
+
Nikita Vetoshkin 2012-10-25, 07:46