-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27244/#review63604
-----------------------------------------------------------


Looks better, Hongchao, thanks. I have a few more comments below, hope you 
don't mind.


src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java
<https://reviews.apache.org/r/27244/#comment105881>

    Is the reason to have workSemaphore to block in doTransport when there 
isn't anything in the outgoing queue? If so, we might be better off with a 
blocking queue rather than a list as we have right now.



src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java
<https://reviews.apache.org/r/27244/#comment105880>

    I'm not sure why you're not using the ChannelFuture returned here. See this 
connect() method in bookkeeper for an example 
https://github.com/apache/bookkeeper/blob/branch-4.0/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java#L112



src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java
<https://reviews.apache.org/r/27244/#comment105876>

    Could you elaborate on what you're trying to do with this NIOLock? I'm not 
sure what you mean with race cocnditions in the comment.


- fpj


On Nov. 24, 2014, 7:01 p.m., Hongchao Deng wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27244/
> -----------------------------------------------------------
> 
> (Updated Nov. 24, 2014, 7:01 p.m.)
> 
> 
> Review request for zookeeper.
> 
> 
> Repository: zookeeper-git
> 
> 
> Description
> -------
> 
> ZOOKEEPER-2069
> 
> 
> Diffs
> -----
> 
>   src/java/main/org/apache/zookeeper/ClientCnxn.java b4ece07 
>   src/java/main/org/apache/zookeeper/ClientCnxnSocket.java 5ca0ba7 
>   src/java/main/org/apache/zookeeper/ClientCnxnSocketNIO.java adb27ee 
>   src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION 
>   src/java/test/org/apache/zookeeper/test/NettyNettySuiteBase.java 
> PRE-CREATION 
>   src/java/test/org/apache/zookeeper/test/NettyNettySuiteHammerTest.java 
> PRE-CREATION 
>   src/java/test/org/apache/zookeeper/test/NettyNettySuiteTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/27244/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Hongchao Deng
> 
>

Reply via email to