Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-13 Thread Hongchao Deng
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 14, 2015, 2:38 a.m.) Review request for zookeeper.

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-13 Thread Hongchao Deng
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

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-13 Thread Hongchao Deng
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 14, 2015, 2:59 a.m.) Review request for zookeeper.

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-13 Thread fpj
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

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-12 Thread Raul Gutierrez Segales
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review75917 --- Ship it! Ship It! - Raul Gutierrez Segales On March 10, 2015,

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-12 Thread Hongchao Deng
--- 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.

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-12 Thread Hongchao Deng
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. //

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-12 Thread fpj
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review76070 --- It looks pretty good, I just have a few small comments and

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-10 Thread Hongchao Deng
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 10, 2015, 6:04 p.m.) Review request for zookeeper.

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-09 Thread Rakesh R
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review75737 --- Ship it! Ship It! - Rakesh R On March 7, 2015, 4:02 p.m.,

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-07 Thread Hongchao Deng
On March 7, 2015, 5:24 a.m., Rakesh R wrote: src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java, line 493 https://reviews.apache.org/r/31277/diff/14/?file=888101#file888101line493 Can we extract this to a method to avoid duplication Rakesh R wrote:

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-07 Thread Hongchao Deng
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 7, 2015, 4:02 p.m.) Review request for zookeeper. Repository:

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-06 Thread Hongchao Deng
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 7, 2015, 1:04 a.m.) Review request for zookeeper. Repository:

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-06 Thread Hongchao Deng
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 6, 2015, 10:48 p.m.) Review request for zookeeper.

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-06 Thread Hongchao Deng
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 6, 2015, 10:41 p.m.) Review request for zookeeper.

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-06 Thread Hongchao Deng
On March 6, 2015, 8:39 p.m., Rakesh R wrote: src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java, line 356 https://reviews.apache.org/r/31277/diff/11/?file=886448#file886448line356 do we need synchronization here? It's not obvious here. I am going to add some comments.

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-06 Thread Hongchao Deng
On March 7, 2015, 1:08 a.m., Raul Gutierrez Segales wrote: src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java, line 849 https://reviews.apache.org/r/31277/diff/12-14/?file=887835#file887835line849 can we get rid of these red tabs pls? Yes I did a few other pushes to

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-06 Thread Raul Gutierrez Segales
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review75593 --- src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-06 Thread Hongchao Deng
On March 6, 2015, 8:39 p.m., Rakesh R wrote: src/java/main/org/apache/zookeeper/common/X509Error.java, line 21 https://reviews.apache.org/r/31277/diff/11/?file=886450#file886450line21 I prefer to use X509Exception instead of X509Error, can you rename this to X509Exception?

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-06 Thread Hongchao Deng
On March 6, 2015, 8:44 p.m., Rakesh R wrote: src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml, line 618 https://reviews.apache.org/r/31277/diff/11/?file=886447#file886447line618 Netty usage is pluggable. SSL feature will be enabled when user user plugged-in

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-06 Thread Rakesh R
--- 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

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-06 Thread Rakesh R
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review75542 --- src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-06 Thread Rakesh R
On March 6, 2015, 8:39 p.m., Rakesh R wrote: src/java/main/org/apache/zookeeper/common/X509Error.java, line 21 https://reviews.apache.org/r/31277/diff/11/?file=886450#file886450line21 I prefer to use X509Exception instead of X509Error, can you rename this to X509Exception?

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-06 Thread Rakesh R
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review75616 ---

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-06 Thread Rakesh R
On March 7, 2015, 5:24 a.m., Rakesh R wrote: src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java, line 493 https://reviews.apache.org/r/31277/diff/14/?file=888101#file888101line493 Can we extract this to a method to avoid duplication adding few more to the above

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-05 Thread Hongchao Deng
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 5, 2015, 10:37 p.m.) Review request for zookeeper.

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-05 Thread Hongchao Deng
--- 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.

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-03 Thread Raul Gutierrez Segales
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review75084 --- Ship it! Ship It! - Raul Gutierrez Segales On March 2, 2015,

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-02 Thread Raul Gutierrez Segales
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review74846 ---

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-02 Thread Hongchao Deng
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 2, 2015, 11:22 p.m.) Review request for zookeeper.

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-02 Thread Hongchao Deng
On March 2, 2015, 11:04 p.m., Raul Gutierrez Segales wrote: src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java, line 311 https://reviews.apache.org/r/31277/diff/8/?file=882265#file882265line311 i think you are changing the behavior here.. before we'd throw

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-02 Thread Hongchao Deng
On Feb. 25, 2015, 6:28 p.m., Raul Gutierrez Segales wrote: src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java, line 1463 https://reviews.apache.org/r/31277/diff/5/?file=875863#file875863line1463 when should this be done? why is this needed? i fear this is even more

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-02 Thread Raul Gutierrez Segales
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review74766 --- src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-03-02 Thread Hongchao Deng
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated March 2, 2015, 6:44 p.m.) Review request for zookeeper. Repository:

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-02-27 Thread Hongchao Deng
On Feb. 25, 2015, 6:28 p.m., Raul Gutierrez Segales wrote: src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java, line 353 https://reviews.apache.org/r/31277/diff/5/?file=875851#file875851line353 so if any of this fails, we just give up and throw? why not catch it, report

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-02-26 Thread Hongchao Deng
On Feb. 25, 2015, 6:28 p.m., Raul Gutierrez Segales wrote: src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java, line 353 https://reviews.apache.org/r/31277/diff/5/?file=875851#file875851line353 so if any of this fails, we just give up and throw? why not catch it, report

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-02-26 Thread Hongchao Deng
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated Feb. 26, 2015, 7:34 p.m.) Review request for zookeeper. Repository:

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-02-25 Thread Hongchao Deng
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated Feb. 25, 2015, 5:35 p.m.) Review request for zookeeper. Repository:

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-02-25 Thread Hongchao Deng
On Feb. 25, 2015, 6:28 p.m., Raul Gutierrez Segales wrote: src/java/main/org/apache/zookeeper/ZooKeeper.java, line 134 https://reviews.apache.org/r/31277/diff/5/?file=875852#file875852line134 nit: if you grep around for other properties, the convention seems to be:

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-02-25 Thread Hongchao Deng
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated Feb. 25, 2015, 7:18 p.m.) Review request for zookeeper. Repository:

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-02-25 Thread Hongchao Deng
On Feb. 25, 2015, 6:28 p.m., Raul Gutierrez Segales wrote: src/java/main/org/apache/zookeeper/common/X509Util.java, line 57 https://reviews.apache.org/r/31277/diff/5/?file=875853#file875853line57 why throw all of these? i think we should handle them and just throw one (probably

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-02-25 Thread Raul Gutierrez Segales
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review74038 --- src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-02-25 Thread Raul Gutierrez Segales
On Feb. 25, 2015, 6:28 p.m., Raul Gutierrez Segales wrote: src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java, line 353 https://reviews.apache.org/r/31277/diff/5/?file=875851#file875851line353 so if any of this fails, we just give up and throw? why not catch it, report

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-02-25 Thread Hongchao Deng
On Feb. 25, 2015, 6:28 p.m., Raul Gutierrez Segales wrote: src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java, line 353 https://reviews.apache.org/r/31277/diff/5/?file=875851#file875851line353 so if any of this fails, we just give up and throw? why not catch it, report

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-02-24 Thread Hongchao Deng
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated Feb. 25, 2015, 6:21 a.m.) Review request for zookeeper. Repository:

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-02-22 Thread Hongchao Deng
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated Feb. 22, 2015, 7:24 p.m.) Review request for zookeeper. Repository:

Re: Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-02-22 Thread Hongchao Deng
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- (Updated Feb. 22, 2015, 7:21 p.m.) Review request for zookeeper. Repository:

Review Request 31277: ZOOKEEPER-2125: SSL on Netty client-server communication

2015-02-21 Thread Hongchao Deng
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/ --- Review request for zookeeper. Repository: zookeeper-git Description ---