----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review75525 -----------------------------------------------------------
src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java <https://reviews.apache.org/r/31277/#comment122666> please move sslEngine,sslContext inside the method initSSL(pipeline) as local variables? src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java <https://reviews.apache.org/r/31277/#comment122662> do we need synchronization here? src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java <https://reviews.apache.org/r/31277/#comment122667> I could see new ZKClientPipelineFactory() is always created and not required to do null checks here, isn't it? src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java <https://reviews.apache.org/r/31277/#comment122657> this log wouldn't help much, probably you can try include info like pipeline.getChannel() details. Or can make this DEBUG level priority. src/java/main/org/apache/zookeeper/common/X509Error.java <https://reviews.apache.org/r/31277/#comment122659> I prefer to use X509Exception instead of X509Error, can you rename this to X509Exception? src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java <https://reviews.apache.org/r/31277/#comment122665> same as above, do we need synchronization here? src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java <https://reviews.apache.org/r/31277/#comment122664> this log wouldn't help much, probably you can try include info like pipeline.getChannel() details. Or can make this DEBUG level priority. src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java <https://reviews.apache.org/r/31277/#comment122714> FinalRequestProcessor also has the logic of #closeSession(). I couldn't see the closure of session from secureCnxnFactory in FinalRequestProcessor. Please add the same logic of #closeSession() in FinalRequestProcessor as well. Refer: FinalRequestProcessor#processRequest() if (request.type == OpCode.closeSession) { - Rakesh R On March 6, 2015, 12:17 a.m., Hongchao Deng wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31277/ > ----------------------------------------------------------- > > (Updated March 6, 2015, 12:17 a.m.) > > > Review request for zookeeper. > > > Repository: zookeeper-git > > > Description > ------- > > ZOOKEEPER-2125: SSL on Netty client-server communication > > > Diffs > ----- > > src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml > 202051f1f7f517b1e1a3c561c0008449ab3c48a6 > src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION > src/java/main/org/apache/zookeeper/ZooKeeper.java > dd13cc9ba5096312b06999a03ae0057cd3677623 > src/java/main/org/apache/zookeeper/common/X509Error.java PRE-CREATION > src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION > src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java > acabb33f6c7a000706763ccba94cbaf5aaaca08e > src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java > 41268805fe16244aeea4db3f35f13a6987b30187 > src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java > 14037722c569d560acef56de0b5a7ae13464128c > src/java/main/org/apache/zookeeper/server/ServerConfig.java > f2b8463e871739319bdf40be1f014d5ad0af5602 > src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java > 30a0ed390bb7473ddb36757da97bc7d5f4281887 > src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java > 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 > src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java > b756d349abeb1fc69534100c3633db4c1c18e031 > src/java/main/org/apache/zookeeper/server/quorum/Leader.java > 20589045752a7ba4ae9c9090055a4fcbe86a8eda > src/java/main/org/apache/zookeeper/server/quorum/Learner.java > 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java > 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java > badc8df1f05dea4be337bc8312d7ac22f6c77dc3 > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java > d17c58d59e0131a78adde1becb5c23ce8c7a16a7 > > src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java > 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f > src/java/test/data/ssl/README.md PRE-CREATION > src/java/test/data/ssl/testKeyStore.jks PRE-CREATION > src/java/test/data/ssl/testTrustStore.jks PRE-CREATION > src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java > 6ab19b1eb137c8b13b8ad031d474e213267da1ea > src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java > 6ce058e48d17410d89d8348ee659dd7752bfd578 > src/java/test/org/apache/zookeeper/test/ReconfigTest.java > 8b238ee7463508122010208ebc3e786caa2cf1b1 > src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION > > Diff: https://reviews.apache.org/r/31277/diff/ > > > Testing > ------- > > > Thanks, > > Hongchao Deng > >
