----------------------------------------------------------- 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 > >