You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by Yasuhito Fukuda <fu...@po.ntts.co.jp> on 2015/06/08 11:44:16 UTC
Review Request 35204: ZOOKEEPER-2193: reconfig command completes even
if parameter is wrong obviously
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35204/
-----------------------------------------------------------
Review request for zookeeper.
Bugs: ZOOKEEPER-2193
https://issues.apache.org/jira/browse/ZOOKEEPER-2193
Repository: zookeeper-git
Description
-------
See ZOOKEEPER-2193
Diffs
-----
src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 73fa4e67a09806c90a6037d9a170179cb806b896
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java f15f831701f9c8514db5003ebd550cd3880b48c7
Diff: https://reviews.apache.org/r/35204/diff/
Testing
-------
Thanks,
Yasuhito Fukuda
Re: Review Request 35204: ZOOKEEPER-2193: reconfig command completes
even if parameter is wrong obviously
Posted by Yasuhito Fukuda <fu...@po.ntts.co.jp>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35204/
-----------------------------------------------------------
(Updated 6月 23, 2015, 1:39 p.m.)
Review request for zookeeper.
Bugs: ZOOKEEPER-2193
https://issues.apache.org/jira/browse/ZOOKEEPER-2193
Repository: zookeeper-git
Description
-------
See ZOOKEEPER-2193
Diffs (updated)
-----
src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java eb045de19c9eeb632e5f2b98c5465abcaead7740
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java f15f831701f9c8514db5003ebd550cd3880b48c7
Diff: https://reviews.apache.org/r/35204/diff/
Testing
-------
Thanks,
Yasuhito Fukuda
Re: Review Request 35204: ZOOKEEPER-2193: reconfig command completes
even if parameter is wrong obviously
Posted by Raul Gutierrez Segales <rg...@itevenworks.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35204/#review88715
-----------------------------------------------------------
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java (line 311)
<https://reviews.apache.org/r/35204/#comment141279>
if my is null, this will generate a NullPointerException.
- Raul Gutierrez Segales
On June 19, 2015, 7:33 a.m., Yasuhito Fukuda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35204/
> -----------------------------------------------------------
>
> (Updated June 19, 2015, 7:33 a.m.)
>
>
> Review request for zookeeper.
>
>
> Bugs: ZOOKEEPER-2193
> https://issues.apache.org/jira/browse/ZOOKEEPER-2193
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> See ZOOKEEPER-2193
>
>
> Diffs
> -----
>
> src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java eb045de19c9eeb632e5f2b98c5465abcaead7740
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java f15f831701f9c8514db5003ebd550cd3880b48c7
>
> Diff: https://reviews.apache.org/r/35204/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Yasuhito Fukuda
>
>
Re: Review Request 35204: ZOOKEEPER-2193: reconfig command completes
even if parameter is wrong obviously
Posted by Raul Gutierrez Segales <rg...@itevenworks.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35204/#review88716
-----------------------------------------------------------
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java (line 311)
<https://reviews.apache.org/r/35204/#comment141280>
ohh, i am wrong - the null pointers are filtered out in excludedSpecialAddress
- Raul Gutierrez Segales
On June 19, 2015, 7:33 a.m., Yasuhito Fukuda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35204/
> -----------------------------------------------------------
>
> (Updated June 19, 2015, 7:33 a.m.)
>
>
> Review request for zookeeper.
>
>
> Bugs: ZOOKEEPER-2193
> https://issues.apache.org/jira/browse/ZOOKEEPER-2193
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> See ZOOKEEPER-2193
>
>
> Diffs
> -----
>
> src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java eb045de19c9eeb632e5f2b98c5465abcaead7740
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java f15f831701f9c8514db5003ebd550cd3880b48c7
>
> Diff: https://reviews.apache.org/r/35204/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Yasuhito Fukuda
>
>
Re: Review Request 35204: ZOOKEEPER-2193: reconfig command completes
even if parameter is wrong obviously
Posted by Yasuhito Fukuda <fu...@po.ntts.co.jp>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35204/
-----------------------------------------------------------
(Updated 6月 19, 2015, 4:33 p.m.)
Review request for zookeeper.
Bugs: ZOOKEEPER-2193
https://issues.apache.org/jira/browse/ZOOKEEPER-2193
Repository: zookeeper-git
Description
-------
See ZOOKEEPER-2193
Diffs (updated)
-----
src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java eb045de19c9eeb632e5f2b98c5465abcaead7740
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java f15f831701f9c8514db5003ebd550cd3880b48c7
Diff: https://reviews.apache.org/r/35204/diff/
Testing
-------
Thanks,
Yasuhito Fukuda
Re: Review Request 35204: ZOOKEEPER-2193: reconfig command completes
even if parameter is wrong obviously
Posted by Yasuhito Fukuda <fu...@po.ntts.co.jp>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35204/
-----------------------------------------------------------
(Updated 6月 11, 2015, 4:59 p.m.)
Review request for zookeeper.
Bugs: ZOOKEEPER-2193
https://issues.apache.org/jira/browse/ZOOKEEPER-2193
Repository: zookeeper-git
Description
-------
See ZOOKEEPER-2193
Diffs (updated)
-----
src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 73fa4e67a09806c90a6037d9a170179cb806b896
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java f15f831701f9c8514db5003ebd550cd3880b48c7
Diff: https://reviews.apache.org/r/35204/diff/
Testing
-------
Thanks,
Yasuhito Fukuda
Re: Review Request 35204: ZOOKEEPER-2193: reconfig command completes
even if parameter is wrong obviously
Posted by Yasuhito Fukuda <fu...@po.ntts.co.jp>.
> On 6月 10, 2015, 11:17 a.m., Raul Gutierrez Segales wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java, line 128
> > <https://reviews.apache.org/r/35204/diff/2/?file=981274#file981274line128>
> >
> > this'll override the default type which is Participant:
> >
> > ```java
> > public LearnerType type = LearnerType.PARTICIPANT;
> > ```
> >
> > so this should pass LearnerType.Participant instead of null.
I modified null to LearnerType.PARTICIPANT.
> On 6月 10, 2015, 11:17 a.m., Raul Gutierrez Segales wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java, line 236
> > <https://reviews.apache.org/r/35204/diff/2/?file=981274#file981274line236>
> >
> > to avoid duplicating this code you could probably call the constructor from below here and just move the setType call to after the constructor, i.e.:
> >
> > ```java
> > this(sid, addr, clientAddr, electionAddr, Participant);
> >
> > if (serverParts.length == 4) {
> > setType(serverParts[3]);
> > }
> > ```
Constructor call must be the first statement in a constructor, so I created setMyAddrs() to avoid duplication of code.
> On 6月 10, 2015, 11:17 a.m., Raul Gutierrez Segales wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java, line 330
> > <https://reviews.apache.org/r/35204/diff/2/?file=981274#file981274line330>
> >
> > this method doesn't have to be public.
I modified public to private.
> On 6月 10, 2015, 11:17 a.m., Raul Gutierrez Segales wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java, line 337
> > <https://reviews.apache.org/r/35204/diff/2/?file=981274#file981274line337>
> >
> > don't call new InetSocketAddress(0) from within the loop; do it once outside and use that.
I moved InetSocketAddress(0) to outside the loop.
- Yasuhito
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35204/#review87340
-----------------------------------------------------------
On 6月 11, 2015, 4:59 p.m., Yasuhito Fukuda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35204/
> -----------------------------------------------------------
>
> (Updated 6月 11, 2015, 4:59 p.m.)
>
>
> Review request for zookeeper.
>
>
> Bugs: ZOOKEEPER-2193
> https://issues.apache.org/jira/browse/ZOOKEEPER-2193
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> See ZOOKEEPER-2193
>
>
> Diffs
> -----
>
> src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 73fa4e67a09806c90a6037d9a170179cb806b896
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java f15f831701f9c8514db5003ebd550cd3880b48c7
>
> Diff: https://reviews.apache.org/r/35204/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Yasuhito Fukuda
>
>
Re: Review Request 35204: ZOOKEEPER-2193: reconfig command completes
even if parameter is wrong obviously
Posted by Raul Gutierrez Segales <rg...@itevenworks.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35204/#review87340
-----------------------------------------------------------
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
<https://reviews.apache.org/r/35204/#comment139674>
this'll override the default type which is Participant:
```java
public LearnerType type = LearnerType.PARTICIPANT;
```
so this should pass LearnerType.Participant instead of null.
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
<https://reviews.apache.org/r/35204/#comment139675>
ditto
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
<https://reviews.apache.org/r/35204/#comment139676>
ditto
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
<https://reviews.apache.org/r/35204/#comment139677>
to avoid duplicating this code you could probably call the constructor from below here and just move the setType call to after the constructor, i.e.:
```java
this(sid, addr, clientAddr, electionAddr, Participant);
if (serverParts.length == 4) {
setType(serverParts[3]);
}
```
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
<https://reviews.apache.org/r/35204/#comment139678>
nit: using String.format to build strings makes it easier to grep them afterwards
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
<https://reviews.apache.org/r/35204/#comment139672>
this method doesn't have to be public.
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
<https://reviews.apache.org/r/35204/#comment139673>
don't call new InetSocketAddress(0) from within the loop; do it once outside and use that.
- Raul Gutierrez Segales
On June 9, 2015, 6:45 a.m., Yasuhito Fukuda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35204/
> -----------------------------------------------------------
>
> (Updated June 9, 2015, 6:45 a.m.)
>
>
> Review request for zookeeper.
>
>
> Bugs: ZOOKEEPER-2193
> https://issues.apache.org/jira/browse/ZOOKEEPER-2193
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> See ZOOKEEPER-2193
>
>
> Diffs
> -----
>
> src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 73fa4e67a09806c90a6037d9a170179cb806b896
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java f15f831701f9c8514db5003ebd550cd3880b48c7
>
> Diff: https://reviews.apache.org/r/35204/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Yasuhito Fukuda
>
>
Re: Review Request 35204: ZOOKEEPER-2193: reconfig command completes
even if parameter is wrong obviously
Posted by Yasuhito Fukuda <fu...@po.ntts.co.jp>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35204/
-----------------------------------------------------------
(Updated 6月 9, 2015, 3:45 p.m.)
Review request for zookeeper.
Bugs: ZOOKEEPER-2193
https://issues.apache.org/jira/browse/ZOOKEEPER-2193
Repository: zookeeper-git
Description
-------
See ZOOKEEPER-2193
Diffs (updated)
-----
src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 73fa4e67a09806c90a6037d9a170179cb806b896
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java f15f831701f9c8514db5003ebd550cd3880b48c7
Diff: https://reviews.apache.org/r/35204/diff/
Testing
-------
Thanks,
Yasuhito Fukuda
Re: Review Request 35204: ZOOKEEPER-2193: reconfig command completes
even if parameter is wrong obviously
Posted by Yasuhito Fukuda <fu...@po.ntts.co.jp>.
> On 6月 9, 2015, 10:12 a.m., Raul Gutierrez Segales wrote:
> > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java, line 304
> > <https://reviews.apache.org/r/35204/diff/1/?file=980620#file980620line304>
> >
> > sounds like myAddrs could be constructed and saved from the constructor, to avoid building it on every call, no?
Yes indeed!
myAddr will construct in the constructor.
at the same time, I modified constructor overloading in QuorumPeer class.
- Yasuhito
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35204/#review87114
-----------------------------------------------------------
On 6月 9, 2015, 3:45 p.m., Yasuhito Fukuda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35204/
> -----------------------------------------------------------
>
> (Updated 6月 9, 2015, 3:45 p.m.)
>
>
> Review request for zookeeper.
>
>
> Bugs: ZOOKEEPER-2193
> https://issues.apache.org/jira/browse/ZOOKEEPER-2193
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> See ZOOKEEPER-2193
>
>
> Diffs
> -----
>
> src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 73fa4e67a09806c90a6037d9a170179cb806b896
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java f15f831701f9c8514db5003ebd550cd3880b48c7
>
> Diff: https://reviews.apache.org/r/35204/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Yasuhito Fukuda
>
>
Re: Review Request 35204: ZOOKEEPER-2193: reconfig command completes
even if parameter is wrong obviously
Posted by Raul Gutierrez Segales <rg...@itevenworks.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35204/#review87114
-----------------------------------------------------------
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
<https://reviews.apache.org/r/35204/#comment139438>
remove the extra whitespaces/tabs (i.e.: the red part in the RB).
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
<https://reviews.apache.org/r/35204/#comment139439>
sounds like myAddrs could be constructed and saved from the constructor, to avoid building it on every call, no?
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
<https://reviews.apache.org/r/35204/#comment139440>
typo, excludedSpecialAddress
src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
<https://reviews.apache.org/r/35204/#comment139441>
whitespace/tabs
- Raul Gutierrez Segales
On June 8, 2015, 9:44 a.m., Yasuhito Fukuda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35204/
> -----------------------------------------------------------
>
> (Updated June 8, 2015, 9:44 a.m.)
>
>
> Review request for zookeeper.
>
>
> Bugs: ZOOKEEPER-2193
> https://issues.apache.org/jira/browse/ZOOKEEPER-2193
>
>
> Repository: zookeeper-git
>
>
> Description
> -------
>
> See ZOOKEEPER-2193
>
>
> Diffs
> -----
>
> src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java 73fa4e67a09806c90a6037d9a170179cb806b896
> src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java f15f831701f9c8514db5003ebd550cd3880b48c7
>
> Diff: https://reviews.apache.org/r/35204/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Yasuhito Fukuda
>
>