> On Dec. 11, 2014, 10:53 p.m., fpj wrote: > > build.xml, line 42 > > <https://reviews.apache.org/r/27244/diff/28/?file=789546#file789546line42> > > > > Is this accidental? There is an issue open to change it to 1.7. If it > > is necessary, we can make this change here, but otherwise leave it for the > > other patch.
In order to get a blocking outgoing queue in with minimum change (as stated in your last comment), I choose to use LinkedBlockingDeque, which is required by JDK 6. Changing to 1.7 seems fine with me :) > On Dec. 11, 2014, 10:53 p.m., fpj wrote: > > src/java/main/org/apache/zookeeper/ClientCnxn.java, line 1372 > > <https://reviews.apache.org/r/27244/diff/28/?file=789547#file789547line1372> > > > > If these SASL changes aren't strictly necessary for this patch, we > > should do them in a different jira. This is commented by Raul @rgs to change.. I usually didn't change name myself. > On Dec. 11, 2014, 10:53 p.m., fpj wrote: > > src/java/main/org/apache/zookeeper/ClientCnxnSocketNIO.java, line 68 > > <https://reviews.apache.org/r/27244/diff/28/?file=789549#file789549line68> > > > > Why does pendingQueue need to be made concurrent? I will convert it back. > On Dec. 11, 2014, 10:53 p.m., fpj wrote: > > src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java, line 70 > > <https://reviews.apache.org/r/27244/diff/28/?file=789550#file789550line70> > > > > I see, the previous sasl changes are necessary here so that we can > > proceed only if authentication suceeds. Please confirm. I change enableWrite(), enableReadWriteOnly(), WakeupCnxn() interfaces: 1. enable read, write is renaming only. Because in netty it doesn't make sense to say enable write. It's more natural to say SASL completed and what we are doing here. 2. I split WakeupCnxn() into two because they are for two purposes: one is for notifying packet has been added to outgoingQueue, the other is for notifying connection is intentionally closed. They are different in Netty. - Hongchao ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27244/#review64803 ----------------------------------------------------------- On Dec. 11, 2014, 6:11 p.m., Hongchao Deng wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27244/ > ----------------------------------------------------------- > > (Updated Dec. 11, 2014, 6:11 p.m.) > > > Review request for zookeeper. > > > Repository: zookeeper-git > > > Description > ------- > > ZOOKEEPER-2069 > > > Diffs > ----- > > build.xml bb5ff4f > 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/main/org/apache/zookeeper/ZooKeeperTestable.java 775d1a2 > src/java/main/org/apache/zookeeper/client/ZooKeeperSaslClient.java dbc1080 > src/java/test/org/apache/zookeeper/test/ClientTest.java dbe595c > 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 > ------- > > 1. use LinkedBlockingDeque. > > > Thanks, > > Hongchao Deng > >
