> On March 12, 2015, 9:47 p.m., fpj wrote: > > src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java, line > > 279 > > <https://reviews.apache.org/r/31277/diff/16/?file=890434#file890434line279> > > > > why is this method synchronized?
I should have added a comment here. Will do it. // The synchronized is to prevent the race on shared variable "sslEngine". // Basically we only need to create it once. > On March 12, 2015, 9:47 p.m., fpj wrote: > > src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml, line 621 > > <https://reviews.apache.org/r/31277/diff/16/?file=890427#file890427line621> > > > > What if we change the text here to simply: the port to listen on for > > secure client connections using SSL. Fixed. > On March 12, 2015, 9:47 p.m., fpj wrote: > > src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml, line 1057 > > <https://reviews.apache.org/r/31277/diff/16/?file=890427#file890427line1057> > > > > Don't you need to change this bit as well? Fixed. > On March 12, 2015, 9:47 p.m., fpj wrote: > > src/java/test/org/apache/zookeeper/test/SSLTest.java, line 97 > > <https://reviews.apache.org/r/31277/diff/16/?file=890452#file890452line97> > > > > It might be a good idea to do a CLIENT_COUNT for clarity. This is the server. It's trying connect to every server to test it. > On March 12, 2015, 9:47 p.m., fpj wrote: > > src/java/test/org/apache/zookeeper/test/SSLTest.java, line 115 > > <https://reviews.apache.org/r/31277/diff/16/?file=890452#file890452line115> > > > > Should we run an operation just to make sure the wiring on the server > > side is fine? Fixed. > On March 12, 2015, 9:47 p.m., fpj wrote: > > src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java, line > > 486 > > <https://reviews.apache.org/r/31277/diff/16/?file=890432#file890432line486> > > > > I don't understand why we have this for loop here iterating over cnxns, > > why can't we simply call closeSession on the sessionId? I'm trying to keep the original logic here. What if all ServerCnxnFactories are null (the comments said we are just playing with diff)? Then it's possible that we won't return here and go the the switch block. This is a left problem. Might be great if you can review it again. - Hongchao ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review76070 ----------------------------------------------------------- On March 13, 2015, 12:12 a.m., Hongchao Deng wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31277/ > ----------------------------------------------------------- > > (Updated March 13, 2015, 12:12 a.m.) > > > Review request for zookeeper. > > > Repository: zookeeper-git > > > Description > ------- > > ZOOKEEPER-2125: SSL on Netty client-server communication > > > Diffs > ----- > > build.xml bb5ff4f70dc7cf93d42b67b80885c13fd4c4f13c > 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/X509Exception.java PRE-CREATION > src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION > src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java > a97be4a5452006fbd85d355c0dcb16276cbf1c59 > 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 > >
