You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Chengwei Yang <ch...@gmail.com> on 2014/06/09 10:52:28 UTC
Review Request 22369: Use '--nr_of_masters' instead of '--quorum'
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22369/
-----------------------------------------------------------
Review request for mesos.
Bugs: https://issues.apache.org/jira/browse/MESOS-1465
https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1465
Repository: mesos-git
Description
-------
Use '--nr_of_masters' instead of '--quorum' because user may confused
with zookeeper quorum and may specify an incorrect quorum if he/she
doesn't know quorum well.
Use '--nr_of_masters' just simple and straight-forward, and quorum will
be nr_of_masters / 2 + 1, since specify a larger quorum doesn't make
sense in pratice.
Review: https://reviews.apache.org/r/22369
Diffs
-----
src/master/flags.hpp 486335970ef05b345c5584ac012dde63437ef149
src/master/main.cpp 8ceaae61a732a8d68d19c8aee97c1fd93a595893
Diff: https://reviews.apache.org/r/22369/diff/
Testing
-------
done.
Thanks,
Chengwei Yang
Re: Review Request 22369: Use '--nr_of_masters' instead of '--quorum'
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22369/#review45076
-----------------------------------------------------------
Bad patch!
Reviews applied: [22369]
Failed command: ./support/mesos-style.py
Error:
Checking 468 files using filter --filter=-,+build/class,+build/deprecated,+build/endif_comment,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/tab,+whitespace/todo
src/master/main.cpp:187: Lines should be <= 80 characters long [whitespace/line_length] [2]
src/master/main.cpp:188: Lines should be <= 80 characters long [whitespace/line_length] [2]
src/master/main.cpp:201: Lines should be <= 80 characters long [whitespace/line_length] [2]
src/master/flags.hpp:70: Tab found; better to use spaces [whitespace/tab] [1]
src/master/flags.hpp:71: Tab found; better to use spaces [whitespace/tab] [1]
Total errors found: 5
- Mesos ReviewBot
On June 9, 2014, 8:52 a.m., Chengwei Yang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22369/
> -----------------------------------------------------------
>
> (Updated June 9, 2014, 8:52 a.m.)
>
>
> Review request for mesos.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-1465
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1465
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Use '--nr_of_masters' instead of '--quorum' because user may confused
> with zookeeper quorum and may specify an incorrect quorum if he/she
> doesn't know quorum well.
>
> Use '--nr_of_masters' just simple and straight-forward, and quorum will
> be nr_of_masters / 2 + 1, since specify a larger quorum doesn't make
> sense in pratice.
>
> Review: https://reviews.apache.org/r/22369
>
>
> Diffs
> -----
>
> src/master/flags.hpp 486335970ef05b345c5584ac012dde63437ef149
> src/master/main.cpp 8ceaae61a732a8d68d19c8aee97c1fd93a595893
>
> Diff: https://reviews.apache.org/r/22369/diff/
>
>
> Testing
> -------
>
> done.
>
>
> Thanks,
>
> Chengwei Yang
>
>
Re: Review Request 22369: Use '--nr_of_masters' instead of '--quorum'
Posted by Ben Mahler <be...@gmail.com>.
> On June 10, 2014, 1:22 a.m., Ben Mahler wrote:
> > Please do a grep for 'quorum', I believe there are a few other places that need to be modified (mesos-local and tests).
> >
> > What if both flags are specified? We should either reject this or validate the values (e.g. quorum=1 masters=3 is not safe, quorum=4 masters=3 is invalid).
>
> Chengwei Yang wrote:
> I see, there is another bug in auto initialization code, which get clusterSize as quorum * 2 - 1, this is not correct, say below examples.
>
> num-masters quorum clusterSize
> 1 1 1
> 2 2 3 (wrong)
> 3 2 3
> 4 3 5 (wrong)
> 5 3 5
> ...
>
> So need more changes to code. I'd like to describe my idea before submit a new version of patch.
>
> 1. since 0.19.0 is freezing, this patch will go 0.20.x
>
> There are two options I'd like to hear from you.
>
> 1. --num_masters will replace --quorum, means overwrite --quorum for several releases and finally remove '--quorum', so no non-standard quorum will be used. quorum = num_master / 2 + 1
> 2. both --num_masters and --quorum are used, user can specify non-standard quorum if he like, specify a larger quorum may introduce performance regression while a smaller quorum may introduce inconsistency.
>
> Generally, I vote the first option, how do you think?
I think it would be nice to get to your suggestion (2):
--num_masters (required)
--quorum (optional, for those looking for stronger replication at the cost of performance)
If we pass in --num_masters to the log code as the clusterSize, is there anything you don't like about this option?
How about the following phased transition:
0.20.0: At least one of --quorum or --num_masters is required. This lets those using --quorum upgrade without effort and allows them to add --num_masters while running this version. We pass --num_masters to the relevant log initialization code.
0.21.0: --num_masters (required), --quorum (optional)
How does that sound to you?
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22369/#review45186
-----------------------------------------------------------
On June 10, 2014, 1:03 a.m., Chengwei Yang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22369/
> -----------------------------------------------------------
>
> (Updated June 10, 2014, 1:03 a.m.)
>
>
> Review request for mesos.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-1465
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1465
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Use '--nr_of_masters' instead of '--quorum' because user may confused
> with zookeeper quorum and may specify an incorrect quorum if he/she
> doesn't know quorum well.
>
> Use '--nr_of_masters' just simple and straight-forward, and quorum will
> be nr_of_masters / 2 + 1, since specify a larger quorum doesn't make
> sense in pratice.
>
> Review: https://reviews.apache.org/r/22369
>
>
> Diffs
> -----
>
> src/master/flags.hpp 4863359
> src/master/main.cpp 8ceaae6
>
> Diff: https://reviews.apache.org/r/22369/diff/
>
>
> Testing
> -------
>
> done.
>
>
> Thanks,
>
> Chengwei Yang
>
>
Re: Review Request 22369: Use '--nr_of_masters' instead of '--quorum'
Posted by Chengwei Yang <ch...@gmail.com>.
> On June 10, 2014, 9:22 a.m., Ben Mahler wrote:
> > Please do a grep for 'quorum', I believe there are a few other places that need to be modified (mesos-local and tests).
> >
> > What if both flags are specified? We should either reject this or validate the values (e.g. quorum=1 masters=3 is not safe, quorum=4 masters=3 is invalid).
I see, there is another bug in auto initialization code, which get clusterSize as quorum * 2 - 1, this is not correct, say below examples.
num-masters quorum clusterSize
1 1 1
2 2 3 (wrong)
3 2 3
4 3 5 (wrong)
5 3 5
...
So need more changes to code. I'd like to describe my idea before submit a new version of patch.
1. since 0.19.0 is freezing, this patch will go 0.20.x
There are two options I'd like to hear from you.
1. --num_masters will replace --quorum, means overwrite --quorum for several releases and finally remove '--quorum', so no non-standard quorum will be used. quorum = num_master / 2 + 1
2. both --num_masters and --quorum are used, user can specify non-standard quorum if he like, specify a larger quorum may introduce performance regression while a smaller quorum may introduce inconsistency.
Generally, I vote the first option, how do you think?
> On June 10, 2014, 9:22 a.m., Ben Mahler wrote:
> > src/master/flags.hpp, line 69
> > <https://reviews.apache.org/r/22369/diff/2/?file=606291#file606291line69>
> >
> > 'num_masters' here and elsewhere
- Chengwei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22369/#review45186
-----------------------------------------------------------
On June 10, 2014, 9:03 a.m., Chengwei Yang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22369/
> -----------------------------------------------------------
>
> (Updated June 10, 2014, 9:03 a.m.)
>
>
> Review request for mesos.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-1465
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1465
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Use '--nr_of_masters' instead of '--quorum' because user may confused
> with zookeeper quorum and may specify an incorrect quorum if he/she
> doesn't know quorum well.
>
> Use '--nr_of_masters' just simple and straight-forward, and quorum will
> be nr_of_masters / 2 + 1, since specify a larger quorum doesn't make
> sense in pratice.
>
> Review: https://reviews.apache.org/r/22369
>
>
> Diffs
> -----
>
> src/master/flags.hpp 4863359
> src/master/main.cpp 8ceaae6
>
> Diff: https://reviews.apache.org/r/22369/diff/
>
>
> Testing
> -------
>
> done.
>
>
> Thanks,
>
> Chengwei Yang
>
>
Re: Review Request 22369: Use '--nr_of_masters' instead of '--quorum'
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22369/#review45186
-----------------------------------------------------------
Please do a grep for 'quorum', I believe there are a few other places that need to be modified (mesos-local and tests).
What if both flags are specified? We should either reject this or validate the values (e.g. quorum=1 masters=3 is not safe, quorum=4 masters=3 is invalid).
src/master/flags.hpp
<https://reviews.apache.org/r/22369/#comment79911>
'num_masters' here and elsewhere
src/master/flags.hpp
<https://reviews.apache.org/r/22369/#comment79912>
Please add some comments to guide operators that this flag is used to infer log quorum size, I want to avoid people glossing over this flag.
src/master/main.cpp
<https://reviews.apache.org/r/22369/#comment79913>
Braces please :)
- Ben Mahler
On June 10, 2014, 1:03 a.m., Chengwei Yang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22369/
> -----------------------------------------------------------
>
> (Updated June 10, 2014, 1:03 a.m.)
>
>
> Review request for mesos.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-1465
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1465
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Use '--nr_of_masters' instead of '--quorum' because user may confused
> with zookeeper quorum and may specify an incorrect quorum if he/she
> doesn't know quorum well.
>
> Use '--nr_of_masters' just simple and straight-forward, and quorum will
> be nr_of_masters / 2 + 1, since specify a larger quorum doesn't make
> sense in pratice.
>
> Review: https://reviews.apache.org/r/22369
>
>
> Diffs
> -----
>
> src/master/flags.hpp 4863359
> src/master/main.cpp 8ceaae6
>
> Diff: https://reviews.apache.org/r/22369/diff/
>
>
> Testing
> -------
>
> done.
>
>
> Thanks,
>
> Chengwei Yang
>
>
Re: Review Request 22369: Use '--nr_of_masters' instead of '--quorum'
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22369/#review45270
-----------------------------------------------------------
Patch looks great!
Reviews applied: [22369]
All tests passed.
- Mesos ReviewBot
On June 10, 2014, 1:03 a.m., Chengwei Yang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22369/
> -----------------------------------------------------------
>
> (Updated June 10, 2014, 1:03 a.m.)
>
>
> Review request for mesos.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-1465
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1465
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Use '--nr_of_masters' instead of '--quorum' because user may confused
> with zookeeper quorum and may specify an incorrect quorum if he/she
> doesn't know quorum well.
>
> Use '--nr_of_masters' just simple and straight-forward, and quorum will
> be nr_of_masters / 2 + 1, since specify a larger quorum doesn't make
> sense in pratice.
>
> Review: https://reviews.apache.org/r/22369
>
>
> Diffs
> -----
>
> src/master/flags.hpp 4863359
> src/master/main.cpp 8ceaae6
>
> Diff: https://reviews.apache.org/r/22369/diff/
>
>
> Testing
> -------
>
> done.
>
>
> Thanks,
>
> Chengwei Yang
>
>
Re: Review Request 22369: Use '--nr_of_masters' instead of '--quorum'
Posted by Chengwei Yang <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22369/
-----------------------------------------------------------
(Updated June 10, 2014, 9:03 a.m.)
Review request for mesos.
Changes
-------
pass mesos-style.py check
Bugs: https://issues.apache.org/jira/browse/MESOS-1465
https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1465
Repository: mesos-git
Description
-------
Use '--nr_of_masters' instead of '--quorum' because user may confused
with zookeeper quorum and may specify an incorrect quorum if he/she
doesn't know quorum well.
Use '--nr_of_masters' just simple and straight-forward, and quorum will
be nr_of_masters / 2 + 1, since specify a larger quorum doesn't make
sense in pratice.
Review: https://reviews.apache.org/r/22369
Diffs (updated)
-----
src/master/flags.hpp 4863359
src/master/main.cpp 8ceaae6
Diff: https://reviews.apache.org/r/22369/diff/
Testing
-------
done.
Thanks,
Chengwei Yang