You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Benjamin Hindman <be...@berkeley.edu> on 2014/04/15 19:56:22 UTC
Review Request 20374: Added timeouts to registrar.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20374/
-----------------------------------------------------------
Review request for mesos and Ben Mahler.
Repository: mesos-git
Description
-------
Getting this out for feedback then I'll likely add some tests (depending on what bmahler is adding).
Diffs
-----
src/master/flags.hpp 024f86d93824a20ce42c28b8264576f1cb715d0e
src/master/registrar.cpp 9c0537d86036dec3e6a0fac84e167d9131242e0c
Diff: https://reviews.apache.org/r/20374/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman
Re: Review Request 20374: Added timeouts to registrar.
Posted by Ben Mahler <be...@gmail.com>.
> On April 17, 2014, 10:31 p.m., Ben Mahler wrote:
> > src/master/flags.hpp, line 85
> > <https://reviews.apache.org/r/20374/diff/1/?file=558269#file558269line85>
> >
> > Failure to write will lead to a failover. Is 5 seconds enough for those placing their masters on shared disks? Maybe 10 seconds would be a better initial timeout since we're writing full snapshots?
How will this timeout interact with the timeout jie is introducing here:
https://reviews.apache.org/r/18600/
Probably we will want the log recovery timeout to be smaller than this one.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20374/#review40706
-----------------------------------------------------------
On April 15, 2014, 5:56 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20374/
> -----------------------------------------------------------
>
> (Updated April 15, 2014, 5:56 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Getting this out for feedback then I'll likely add some tests (depending on what bmahler is adding).
>
>
> Diffs
> -----
>
> src/master/flags.hpp 024f86d93824a20ce42c28b8264576f1cb715d0e
> src/master/registrar.cpp 9c0537d86036dec3e6a0fac84e167d9131242e0c
>
> Diff: https://reviews.apache.org/r/20374/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 20374: Added timeouts to registrar.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On April 17, 2014, 10:31 p.m., Ben Mahler wrote:
> > Adding integration tests here is difficult because the Master will LOG(FATAL) when a failure occurs in the Registrar.
> >
> > However, we can add tests in registrar_tests.cpp to ensure that failures are returned by manipulating the clock and perhaps implementing a blocking Storage abstraction for the test?
Done!
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20374/#review40706
-----------------------------------------------------------
On April 15, 2014, 5:56 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20374/
> -----------------------------------------------------------
>
> (Updated April 15, 2014, 5:56 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Getting this out for feedback then I'll likely add some tests (depending on what bmahler is adding).
>
>
> Diffs
> -----
>
> src/master/flags.hpp 024f86d93824a20ce42c28b8264576f1cb715d0e
> src/master/registrar.cpp 9c0537d86036dec3e6a0fac84e167d9131242e0c
>
> Diff: https://reviews.apache.org/r/20374/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 20374: Added timeouts to registrar.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20374/#review40706
-----------------------------------------------------------
Adding integration tests here is difficult because the Master will LOG(FATAL) when a failure occurs in the Registrar.
However, we can add tests in registrar_tests.cpp to ensure that failures are returned by manipulating the clock and perhaps implementing a blocking Storage abstraction for the test?
src/master/flags.hpp
<https://reviews.apache.org/r/20374/#comment73819>
Failure to write will lead to a failover. Is 5 seconds enough for those placing their masters on shared disks? Maybe 10 seconds would be a better initial timeout since we're writing full snapshots?
src/master/registrar.cpp
<https://reviews.apache.org/r/20374/#comment73817>
Why not on one line?
- Ben Mahler
On April 15, 2014, 5:56 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20374/
> -----------------------------------------------------------
>
> (Updated April 15, 2014, 5:56 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Getting this out for feedback then I'll likely add some tests (depending on what bmahler is adding).
>
>
> Diffs
> -----
>
> src/master/flags.hpp 024f86d93824a20ce42c28b8264576f1cb715d0e
> src/master/registrar.cpp 9c0537d86036dec3e6a0fac84e167d9131242e0c
>
> Diff: https://reviews.apache.org/r/20374/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 20374: Added timeouts to registrar.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20374/#review41167
-----------------------------------------------------------
Ship it!
Ship It!
- Ben Mahler
On April 22, 2014, 11:30 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20374/
> -----------------------------------------------------------
>
> (Updated April 22, 2014, 11:30 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/master/flags.hpp acf39636bca8b259763d2679d7cd7a946a8aa043
> src/master/registrar.cpp 38040bdb519c7f0562ddfeb58d2809cffa6f4c18
> src/tests/registrar_tests.cpp 7afa223349b0749cfa6a6ba4f64c41e7059b11a2
>
> Diff: https://reviews.apache.org/r/20374/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 20374: Added timeouts to registrar.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20374/
-----------------------------------------------------------
(Updated April 22, 2014, 11:30 p.m.)
Review request for mesos and Ben Mahler.
Repository: mesos-git
Description (updated)
-------
See summary.
Diffs
-----
src/master/flags.hpp acf39636bca8b259763d2679d7cd7a946a8aa043
src/master/registrar.cpp 38040bdb519c7f0562ddfeb58d2809cffa6f4c18
src/tests/registrar_tests.cpp 7afa223349b0749cfa6a6ba4f64c41e7059b11a2
Diff: https://reviews.apache.org/r/20374/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman
Re: Review Request 20374: Added timeouts to registrar.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20374/
-----------------------------------------------------------
(Updated April 22, 2014, 11:20 p.m.)
Review request for mesos and Ben Mahler.
Changes
-------
Added two tests and updates from review comments.
Repository: mesos-git
Description
-------
Getting this out for feedback then I'll likely add some tests (depending on what bmahler is adding).
Diffs (updated)
-----
src/master/flags.hpp acf39636bca8b259763d2679d7cd7a946a8aa043
src/master/registrar.cpp 38040bdb519c7f0562ddfeb58d2809cffa6f4c18
src/tests/registrar_tests.cpp 7afa223349b0749cfa6a6ba4f64c41e7059b11a2
Diff: https://reviews.apache.org/r/20374/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman
Re: Review Request 20374: Added timeouts to registrar.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20374/#review40746
-----------------------------------------------------------
Bad patch!
Reviews applied: [20374]
Failed command: git apply --index 20374.patch
Error:
error: patch failed: src/master/flags.hpp:72
error: src/master/flags.hpp: patch does not apply
error: patch failed: src/master/registrar.cpp:319
error: src/master/registrar.cpp: patch does not apply
- Mesos ReviewBot
On April 15, 2014, 5:56 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20374/
> -----------------------------------------------------------
>
> (Updated April 15, 2014, 5:56 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Getting this out for feedback then I'll likely add some tests (depending on what bmahler is adding).
>
>
> Diffs
> -----
>
> src/master/flags.hpp 024f86d93824a20ce42c28b8264576f1cb715d0e
> src/master/registrar.cpp 9c0537d86036dec3e6a0fac84e167d9131242e0c
>
> Diff: https://reviews.apache.org/r/20374/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>