Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-12-05 Thread Patrick Hunt
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/#review158075 --- Ship it! Latest patch looks good to me - the recent change

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-11-29 Thread Rakesh R
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/ --- (Updated Nov. 29, 2016, 6:26 p.m.) Review request for zookeeper, fpj, Ivan

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-11-29 Thread Rakesh R
> On Nov. 29, 2016, 5:16 p.m., Patrick Hunt wrote: > > src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java, > > line 449 > > > > > > Doesn't this change change the semantics of SID?

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-11-29 Thread Patrick Hunt
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/#review157277 ---

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-11-24 Thread Rakesh R
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/ --- (Updated Nov. 25, 2016, 7:28 a.m.) Review request for zookeeper, fpj, Ivan

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-11-10 Thread Rakesh R
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/ --- (Updated Nov. 10, 2016, 3:38 p.m.) Review request for zookeeper, fpj, Ivan

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-10-28 Thread Eugene Koontz
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/#review154205 ---

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-10-24 Thread Rakesh R
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/ --- (Updated Oct. 25, 2016, 5:05 a.m.) Review request for zookeeper, fpj, Ivan

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-10-05 Thread Michael Han
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/#review151536 --- Ship it! Thanks Rakesh, latest patch looks great to me. -

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-10-04 Thread Rakesh R
> On Sept. 19, 2016, 7:21 p.m., Michael Han wrote: > > src/java/main/org/apache/zookeeper/server/quorum/Learner.java, line 248 > > > > > > This check seems redundant to me - when we are here we either already > >

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-10-04 Thread Rakesh R
> On Sept. 19, 2016, 11:52 p.m., Michael Han wrote: > > src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java, > > line 219 > > > > > > Should we put the check of in progress sid here, or up on the

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-10-04 Thread Rakesh R
> On Sept. 27, 2016, 9:27 p.m., Michael Han wrote: > > src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java, > > line 131 > > > > > > Right, I think the key is sc.isComplete == true does

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-10-04 Thread Rakesh R
> On Sept. 20, 2016, 10:24 p.m., Michael Han wrote: > > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java, line 106 > > > > > > Should we move both ThreadPoolExecutor and inprogressConnections into >

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-10-04 Thread Rakesh R
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/ --- (Updated Oct. 5, 2016, 4:14 a.m.) Review request for zookeeper, fpj, Ivan

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-10-04 Thread Rakesh R
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/ --- (Updated Oct. 5, 2016, 4:10 a.m.) Review request for zookeeper, fpj, Ivan

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-10-04 Thread Rakesh R
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/ --- (Updated Oct. 5, 2016, 4:08 a.m.) Review request for zookeeper, fpj, Ivan

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-09-27 Thread Michael Han
> On Sept. 20, 2016, 10:24 p.m., Michael Han wrote: > > src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java, > > line 77 > > > > > > How about test if 'quorumRequireSasl' flag is set or

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-09-27 Thread Michael Han
> On Sept. 19, 2016, 11:52 p.m., Michael Han wrote: > > src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java, > > line 166 > > > > > > I have a mixed feeling about this change - it makes

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-09-27 Thread Michael Han
> On Sept. 19, 2016, 7:21 p.m., Michael Han wrote: > > src/java/main/org/apache/zookeeper/server/quorum/Learner.java, line 248 > > > > > > This check seems redundant to me - when we are here we either already > >

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-09-27 Thread Michael Han
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/#review150630 ---

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-09-26 Thread Rakesh R
> On Sept. 19, 2016, 11:52 p.m., Michael Han wrote: > > src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java, > > line 117 > > > > > > This inprogressConnections is used to make sure we don't have

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-09-26 Thread Rakesh R
> On Sept. 20, 2016, 10:24 p.m., Michael Han wrote: > > src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java, > > line 77 > > > > > > How about test if 'quorumRequireSasl' flag is set or

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-09-20 Thread Michael Han
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/#review149705 --- src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-09-19 Thread Michael Han
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/#review149566 ---

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-09-19 Thread Michael Han
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/#review149501 --- src/java/main/org/apache/zookeeper/server/quorum/Learner.java

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-09-15 Thread Rakesh R
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/ --- (Updated Sept. 15, 2016, 4:48 p.m.) Review request for zookeeper, fpj, Ivan

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-07-12 Thread Rakesh R
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/ --- (Updated July 12, 2016, 7:28 p.m.) Review request for zookeeper, fpj, Ivan

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-07-12 Thread Rakesh R
> On June 28, 2016, 11:56 p.m., Michael Han wrote: > > src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java, > > line 121 > > > > > > I recommend logging the content of the exception here

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-07-12 Thread Rakesh R
> On June 28, 2016, 8:49 p.m., Michael Han wrote: > > src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuthLearner.java, > > line 34 > > > > > > The return value in the function signature is void, and the

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-06-28 Thread Michael Han
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/#review139902 ---

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-06-28 Thread Michael Han
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/#review139854 ---

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-06-26 Thread Rakesh R
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/ --- (Updated June 26, 2016, 9:13 a.m.) Review request for zookeeper, fpj, Ivan

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-06-26 Thread Rakesh R
> On June 15, 2016, 10:49 p.m., Patrick Hunt wrote: > > src/java/main/org/apache/zookeeper/server/quorum/Leader.java, line 328 > > > > > > See my comment in the constructor for LearnerHandler Done. Removed

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-06-26 Thread Rakesh R
> On June 15, 2016, 11:12 p.m., Patrick Hunt wrote: > > src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthClient.java, > > line 79 > > > > > > why not make this info? it's output infrequently,

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-06-26 Thread Rakesh R
> On June 16, 2016, 12:01 a.m., Patrick Hunt wrote: > > src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java, > > line 230 > > > > > > why is this debug? throwable seems pretty heavy handed? > >

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-06-20 Thread Rakesh R
> On June 16, 2016, 12:01 a.m., Patrick Hunt wrote: > > src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java, > > line 230 > > > > > > why is this debug? throwable seems pretty heavy handed?

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-06-15 Thread Patrick Hunt
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/#review137886 --- I reviewed all of the LOG messages, here are some related

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-06-15 Thread Patrick Hunt
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/#review137875 --- src/java/main/org/apache/zookeeper/server/quorum/auth/README.md

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-06-15 Thread Patrick Hunt
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/#review137866 --- src/java/main/org/apache/zookeeper/server/quorum/Leader.java

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-06-10 Thread Raul Gutierrez Segales
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/#review136981 --- Ship it! Ship It! - Raul Gutierrez Segales On June 5, 2016,

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-06-09 Thread Rakesh R
> On May 13, 2016, 9:25 p.m., Patrick Hunt wrote: > > ivy.xml, line 79 > > > > > > Adding all these is a bit worrisome. On the plus side it's all test > > dependencies. I tried generating the release tar and it

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-06-06 Thread Michael Han
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/#review136362 --- Ship it! Ship It! - Michael Han On June 5, 2016, 7:56 a.m.,

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-06-06 Thread Michael Han
> On May 24, 2016, 12:03 a.m., Michael Han wrote: > > src/java/main/org/apache/zookeeper/server/quorum/Learner.java, line 247 > > > > > > This will throw SaslException when authentication failed. As a result, > >

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-06-05 Thread Rakesh R
> On May 24, 2016, 12:03 a.m., Michael Han wrote: > > src/java/main/org/apache/zookeeper/server/quorum/Learner.java, line 247 > > > > > > This will throw SaslException when authentication failed. As a result, > >

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-06-05 Thread Rakesh R
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/ --- (Updated June 5, 2016, 7:56 a.m.) Review request for zookeeper, fpj, Ivan

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-05-29 Thread Rakesh R
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/ --- (Updated May 30, 2016, 4:13 a.m.) Review request for zookeeper, fpj, Ivan

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-05-28 Thread Rakesh R
> On May 24, 2016, 12:03 a.m., Michael Han wrote: > > src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java, line > > 32 > > > > > > I am tempted to rename this variable to

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-05-28 Thread Rakesh R
> On May 25, 2016, 8:28 p.m., Patrick Hunt wrote: > > src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java, > > line 567 > > > > > > What's the effect of not addressing this todo? Is this

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-05-28 Thread Rakesh R
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/ --- (Updated May 28, 2016, 5:59 p.m.) Review request for zookeeper, fpj, Ivan

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-05-25 Thread Patrick Hunt
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/#review134828 ---

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-05-24 Thread Rakesh R
> On May 24, 2016, 12:03 a.m., Michael Han wrote: > > src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java, line > > 32 > > > > > > I am tempted to rename this variable to

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-05-24 Thread Rakesh R
> On May 23, 2016, 8:02 p.m., Michael Han wrote: > > src/java/main/org/apache/zookeeper/server/quorum/Learner.java, line 202 > > > > > > I think the reason ZOOKEEPER-1506 is reverted as part of this patch is > >

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-05-24 Thread Michael Han
> On May 23, 2016, 8:02 p.m., Michael Han wrote: > > src/java/main/org/apache/zookeeper/server/quorum/Learner.java, line 202 > > > > > > I think the reason ZOOKEEPER-1506 is reverted as part of this patch is > >

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-05-24 Thread Michael Han
> On May 24, 2016, 12:03 a.m., Michael Han wrote: > > src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java, line > > 32 > > > > > > I am tempted to rename this variable to

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-05-24 Thread Rakesh R
> On May 24, 2016, 12:03 a.m., Michael Han wrote: > > src/java/main/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java, line > > 32 > > > > > > I am tempted to rename this variable to

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-05-24 Thread Rakesh R
> On May 23, 2016, 8:02 p.m., Michael Han wrote: > > src/java/main/org/apache/zookeeper/server/quorum/Learner.java, line 202 > > > > > > I think the reason ZOOKEEPER-1506 is reverted as part of this patch is > >

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-05-23 Thread Michael Han
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/#review134426 --- ivy.xml (line 94)

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-05-23 Thread Michael Han
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/#review133626 --- src/java/main/org/apache/zookeeper/server/quorum/Learner.java

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-05-23 Thread Ivan Kelly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/#review134333 --- Ship it! Ship It! - Ivan Kelly On May 20, 2016, 3:10 a.m.,

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-05-19 Thread Rakesh R
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/ --- (Updated May 20, 2016, 3:10 a.m.) Review request for zookeeper, fpj, Ivan

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-05-18 Thread Rakesh R
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/ --- (Updated May 18, 2016, 6:39 a.m.) Review request for zookeeper, fpj, Ivan

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-05-15 Thread Rakesh R
> On May 13, 2016, 10:08 p.m., Patrick Hunt wrote: > > src/java/main/org/apache/zookeeper/common/PathUtils.java, line 111 > > > > > > I'm pretty suspect of this method. I'd like to see it removed or at > > least

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-05-15 Thread Rakesh R
> On May 13, 2016, 9:25 p.m., Patrick Hunt wrote: > > ivy.xml, line 79 > > > > > > Adding all these is a bit worrisome. On the plus side it's all test > > dependencies. I tried generating the release tar and it

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-05-13 Thread Patrick Hunt
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/#review133212 --- src/java/main/org/apache/zookeeper/common/PathUtils.java (line

Re: Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-05-13 Thread Patrick Hunt
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/#review133204 --- ivy.xml (line 79)

Review Request 47354: ZOOKEEPER-1045 : Quorum mutual authentication using SASL mechanism

2016-05-13 Thread Rakesh R
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47354/ --- Review request for zookeeper, fpj, Ivan Kelly, Patrick Hunt, and Raul Gutierrez