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. Repository:

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 > > > > > > I don't understand why we have this for loop here iterating over cnxns, >

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. Repository:

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 > > > > > > I don't understand why we have this for loop here iterating over cnxns, >

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 > > > > > > why is this method synchronized? I should have added a comment here. Wil

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

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 clarifica

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, 6:

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. Repository:

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., Hongch

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 > > > > > > Can we extract this to a method to avoid duplication > > Rakesh R wro

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 Rakesh R
> On March 7, 2015, 5:24 a.m., Rakesh R wrote: > > src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java, line > > 493 > > > > > > Can we extract this to a method to avoid duplication adding few more

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 --- src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.jav

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 > > > > > > I prefer to use X509Exception instead of X509Error, can you rename this > > to X509Excep

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 > > > > > > I prefer to use X509Exception instead of X509Error, can you rename this > > to X509Excep

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 > > > > > > can we get rid of these red tabs pls? Yes I did a few other p

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
--- 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. 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:41 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
> On March 6, 2015, 8:39 p.m., Rakesh R wrote: > > src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java, line 356 > > > > > > do we need synchronization here? It's not obvious here. I am going to add some co

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 > > > > > > 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/#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
--- 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-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. Repository:

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. Repository:

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, 11:

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. Repository:

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 > > > > > > i think you are changing the behavior here.. before we

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 --- src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.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-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
> On Feb. 25, 2015, 6:28 p.m., Raul Gutierrez Segales wrote: > > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java, line 1463 > > > > > > when should this be done? why is this needed? i fear this is eve

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 > > > > > > so if any of this fails, we just give up and throw? why not catch it,

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 > > > > > > so if any of this fails, we just give up and throw? why not catch it,

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
> On Feb. 25, 2015, 6:28 p.m., Raul Gutierrez Segales wrote: > > src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java, line 353 > > > > > > so if any of this fails, we just give up and throw? why not catch it,

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 > > > > > > so if any of this fails, we just give up and throw? why not catch it,

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 > > > > > > why throw all of these? i think we should handle them and just throw > > one

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/ZooKeeper.java, line 134 > > > > > > nit: if you grep around for other properties, the convention seems to > > be: zo

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