You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Jiang Yan Xu <ya...@jxu.me> on 2014/01/09 02:03:08 UTC
Review Request 16742: Made StartMaster() wait for leader election result
before returning.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16742/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
Bugs: MESOS-881
https://issues.apache.org/jira/browse/MESOS-881
Repository: mesos-git
Description
-------
See summary.
Diffs
-----
src/master/master.hpp 95b9cecd6f9ea0afb3f7f9753a3a06c0c8d8f9d4
src/tests/mesos.cpp 5359394f45475803e05d281710139e8cbe7c7364
Diff: https://reviews.apache.org/r/16742/diff/
Testing
-------
make check.
Thanks,
Jiang Yan Xu
Re: Review Request 16742: Made StartMaster() wait for leader election result
before returning.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16742/#review31938
-----------------------------------------------------------
Ship it!
Ship It!
- Vinod Kone
On Jan. 15, 2014, 9:21 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16742/
> -----------------------------------------------------------
>
> (Updated Jan. 15, 2014, 9:21 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Bugs: MESOS-881
> https://issues.apache.org/jira/browse/MESOS-881
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/master/master.hpp 6e89149728ab8d5c3760a5dbc4d1ceafe873f731
> src/tests/mesos.hpp b1239a653e515115a71e436c79b2d11db4a209f9
> src/tests/mesos.cpp 5359394f45475803e05d281710139e8cbe7c7364
>
> Diff: https://reviews.apache.org/r/16742/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 16742: Made StartMaster() wait for leader election result
before returning.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16742/#review31945
-----------------------------------------------------------
committed. please mark as submitted.
- Vinod Kone
On Jan. 15, 2014, 9:21 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16742/
> -----------------------------------------------------------
>
> (Updated Jan. 15, 2014, 9:21 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Bugs: MESOS-881
> https://issues.apache.org/jira/browse/MESOS-881
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/master/master.hpp 6e89149728ab8d5c3760a5dbc4d1ceafe873f731
> src/tests/mesos.hpp b1239a653e515115a71e436c79b2d11db4a209f9
> src/tests/mesos.cpp 5359394f45475803e05d281710139e8cbe7c7364
>
> Diff: https://reviews.apache.org/r/16742/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 16742: Made StartMaster() wait for leader election result
before returning.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16742/
-----------------------------------------------------------
(Updated Jan. 15, 2014, 1:21 p.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
Summary (updated)
-----------------
Made StartMaster() wait for leader election result before returning.
Bugs: MESOS-881
https://issues.apache.org/jira/browse/MESOS-881
Repository: mesos-git
Description
-------
See summary.
Diffs
-----
src/master/master.hpp 6e89149728ab8d5c3760a5dbc4d1ceafe873f731
src/tests/mesos.hpp b1239a653e515115a71e436c79b2d11db4a209f9
src/tests/mesos.cpp 5359394f45475803e05d281710139e8cbe7c7364
Diff: https://reviews.apache.org/r/16742/diff/
Testing
-------
make check.
Thanks,
Jiang Yan Xu
Re: Review Request 16742: Made StartMaster() wait for the master to realize
it is elected before returning it.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16742/
-----------------------------------------------------------
(Updated Jan. 15, 2014, 1:21 p.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
Changes
-------
Addressed comments.
Summary (updated)
-----------------
Made StartMaster() wait for the master to realize it is elected before returning it.
Bugs: MESOS-881
https://issues.apache.org/jira/browse/MESOS-881
Repository: mesos-git
Description
-------
See summary.
Diffs (updated)
-----
src/master/master.hpp 6e89149728ab8d5c3760a5dbc4d1ceafe873f731
src/tests/mesos.hpp b1239a653e515115a71e436c79b2d11db4a209f9
src/tests/mesos.cpp 5359394f45475803e05d281710139e8cbe7c7364
Diff: https://reviews.apache.org/r/16742/diff/
Testing
-------
make check.
Thanks,
Jiang Yan Xu
Re: Review Request 16742: Made StartMaster() wait for leader election result
before returning.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16742/#review31443
-----------------------------------------------------------
Ship it!
Looking forward to see the tests get sped up!
src/master/master.hpp
<https://reviews.apache.org/r/16742/#comment59935>
A comment about making these public for testing would be great.
src/tests/mesos.cpp
<https://reviews.apache.org/r/16742/#comment59938>
Let's add a TODO here (and below) to instead return a Future instead of doing an await and let the tests wait via ASSERT_READY instead of ASSERT_SOME. Sound good?
src/tests/mesos.cpp
<https://reviews.apache.org/r/16742/#comment59936>
s/Master/master/
src/tests/mesos.cpp
<https://reviews.apache.org/r/16742/#comment59937>
s/Master/master/
- Benjamin Hindman
On Jan. 9, 2014, 1:03 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16742/
> -----------------------------------------------------------
>
> (Updated Jan. 9, 2014, 1:03 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Bugs: MESOS-881
> https://issues.apache.org/jira/browse/MESOS-881
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/master/master.hpp 95b9cecd6f9ea0afb3f7f9753a3a06c0c8d8f9d4
> src/tests/mesos.cpp 5359394f45475803e05d281710139e8cbe7c7364
>
> Diff: https://reviews.apache.org/r/16742/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 16742: Made StartMaster() wait for leader election result
before returning.
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On Jan. 13, 2014, 9:48 a.m., Vinod Kone wrote:
> > src/tests/mesos.cpp, lines 85-88
> > <https://reviews.apache.org/r/16742/diff/1/?file=419177#file419177line85>
> >
> > Why did you have to do this? It doesn't seem to describe what this method is doing.
>
> Jiang Yan Xu wrote:
> It's described in https://issues.apache.org/jira/browse/MESOS-881, without it sometimes the method returns with the master not knowing (it is) the leader and rejects authentication requests and prolongs the test duration.
>
> Vinod Kone wrote:
> Ok, you should add a comment here saying so.
>
> Also, what if a future test does want to test the case where a master doesn't get detected message? How would they go about doing it? AFAICT, a Clock::advance(Seconds(10)) doesn't work here correct?
How about I make it an argument default to true?
virtual Try<process::PID<master::Master> > StartMaster(
const Option<master::Flags>& flags = None(), bool waitForLeader=true);
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16742/#review31621
-----------------------------------------------------------
On Jan. 8, 2014, 5:03 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16742/
> -----------------------------------------------------------
>
> (Updated Jan. 8, 2014, 5:03 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Bugs: MESOS-881
> https://issues.apache.org/jira/browse/MESOS-881
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/master/master.hpp 95b9cecd6f9ea0afb3f7f9753a3a06c0c8d8f9d4
> src/tests/mesos.cpp 5359394f45475803e05d281710139e8cbe7c7364
>
> Diff: https://reviews.apache.org/r/16742/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 16742: Made StartMaster() wait for leader election result
before returning.
Posted by Vinod Kone <vi...@gmail.com>.
> On Jan. 13, 2014, 5:48 p.m., Vinod Kone wrote:
> > src/master/master.hpp, lines 178-181
> > <https://reviews.apache.org/r/16742/diff/1/?file=419176#file419176line178>
> >
> > Doesn't look like these two needed to be pulled up?
>
> Jiang Yan Xu wrote:
> Right, but just thought these three were of similar nature and sort of a *group*.
We typically just pull up things on a as needed basis, especially for hacks like these.
> On Jan. 13, 2014, 5:48 p.m., Vinod Kone wrote:
> > src/tests/mesos.cpp, lines 85-88
> > <https://reviews.apache.org/r/16742/diff/1/?file=419177#file419177line85>
> >
> > Why did you have to do this? It doesn't seem to describe what this method is doing.
>
> Jiang Yan Xu wrote:
> It's described in https://issues.apache.org/jira/browse/MESOS-881, without it sometimes the method returns with the master not knowing (it is) the leader and rejects authentication requests and prolongs the test duration.
Ok, you should add a comment here saying so.
Also, what if a future test does want to test the case where a master doesn't get detected message? How would they go about doing it? AFAICT, a Clock::advance(Seconds(10)) doesn't work here correct?
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16742/#review31621
-----------------------------------------------------------
On Jan. 9, 2014, 1:03 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16742/
> -----------------------------------------------------------
>
> (Updated Jan. 9, 2014, 1:03 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Bugs: MESOS-881
> https://issues.apache.org/jira/browse/MESOS-881
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/master/master.hpp 95b9cecd6f9ea0afb3f7f9753a3a06c0c8d8f9d4
> src/tests/mesos.cpp 5359394f45475803e05d281710139e8cbe7c7364
>
> Diff: https://reviews.apache.org/r/16742/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 16742: Made StartMaster() wait for leader election result
before returning.
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On Jan. 13, 2014, 9:48 a.m., Vinod Kone wrote:
> > src/master/master.hpp, lines 178-181
> > <https://reviews.apache.org/r/16742/diff/1/?file=419176#file419176line178>
> >
> > Doesn't look like these two needed to be pulled up?
Right, but just thought these three were of similar nature and sort of a *group*.
> On Jan. 13, 2014, 9:48 a.m., Vinod Kone wrote:
> > src/tests/mesos.cpp, lines 85-88
> > <https://reviews.apache.org/r/16742/diff/1/?file=419177#file419177line85>
> >
> > Why did you have to do this? It doesn't seem to describe what this method is doing.
It's described in https://issues.apache.org/jira/browse/MESOS-881, without it sometimes the method returns with the master not knowing (it is) the leader and rejects authentication requests and prolongs the test duration.
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16742/#review31621
-----------------------------------------------------------
On Jan. 8, 2014, 5:03 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16742/
> -----------------------------------------------------------
>
> (Updated Jan. 8, 2014, 5:03 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Bugs: MESOS-881
> https://issues.apache.org/jira/browse/MESOS-881
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/master/master.hpp 95b9cecd6f9ea0afb3f7f9753a3a06c0c8d8f9d4
> src/tests/mesos.cpp 5359394f45475803e05d281710139e8cbe7c7364
>
> Diff: https://reviews.apache.org/r/16742/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 16742: Made StartMaster() wait for leader election result
before returning.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16742/#review31621
-----------------------------------------------------------
src/master/master.hpp
<https://reviews.apache.org/r/16742/#comment60245>
Doesn't look like these two needed to be pulled up?
src/tests/mesos.cpp
<https://reviews.apache.org/r/16742/#comment60248>
Why did you have to do this? It doesn't seem to describe what this method is doing.
- Vinod Kone
On Jan. 9, 2014, 1:03 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16742/
> -----------------------------------------------------------
>
> (Updated Jan. 9, 2014, 1:03 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone.
>
>
> Bugs: MESOS-881
> https://issues.apache.org/jira/browse/MESOS-881
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/master/master.hpp 95b9cecd6f9ea0afb3f7f9753a3a06c0c8d8f9d4
> src/tests/mesos.cpp 5359394f45475803e05d281710139e8cbe7c7364
>
> Diff: https://reviews.apache.org/r/16742/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>