You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ben Mahler <be...@gmail.com> on 2013/10/30 18:43:38 UTC
Review Request 15099: Cleaned up the Registrar.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15099/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman and Vinod Kone.
Repository: mesos-git
Description
-------
This is addressing my comments on https://reviews.apache.org/r/14383/
Diffs
-----
src/master/registrar.hpp 5742c365bac0082efea53c35333d3c3503494066
src/master/registrar.cpp 42fe30ededd004e380abc835c224649e02999f11
src/master/registry.proto bd850997c57153cdfc0c39d0dae7d2b802034aa3
src/tests/registrar_tests.cpp 51975f5aa2725f64d2a83387a381e5078c8b9e1d
Diff: https://reviews.apache.org/r/15099/diff/
Testing
-------
make check
Thanks,
Ben Mahler
Re: Review Request 15099: Cleaned up the Registrar.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15099/
-----------------------------------------------------------
(Updated Feb. 20, 2014, 8:42 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Minor cleanups from reviews, NNFR.
Bugs: MESOS-764
https://issues.apache.org/jira/browse/MESOS-764
Repository: mesos-git
Description
-------
This is addressing my comments on https://reviews.apache.org/r/14383/
There was also a bug fix in the removal mutation.
Diffs (updated)
-----
src/master/registrar.hpp 5742c365bac0082efea53c35333d3c3503494066
src/master/registrar.cpp 915885a160f790399e8185c28c6e6555af1ee76e
src/master/registry.proto bd850997c57153cdfc0c39d0dae7d2b802034aa3
src/tests/registrar_tests.cpp 51975f5aa2725f64d2a83387a381e5078c8b9e1d
Diff: https://reviews.apache.org/r/15099/diff/
Testing
-------
make check
Thanks,
Ben Mahler
Re: Review Request 15099: Cleaned up the Registrar.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15099/#review34724
-----------------------------------------------------------
Ship it!
src/master/registrar.cpp
<https://reviews.apache.org/r/15099/#comment64958>
why lambda::_1 but "discarded" above?
- Vinod Kone
On Feb. 15, 2014, 12:57 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15099/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2014, 12:57 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Bugs: MESOS-764
> https://issues.apache.org/jira/browse/MESOS-764
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> This is addressing my comments on https://reviews.apache.org/r/14383/
>
> There was also a bug fix in the removal mutation.
>
>
> Diffs
> -----
>
> src/master/registrar.hpp 5742c365bac0082efea53c35333d3c3503494066
> src/master/registrar.cpp 915885a160f790399e8185c28c6e6555af1ee76e
> src/master/registry.proto bd850997c57153cdfc0c39d0dae7d2b802034aa3
> src/tests/registrar_tests.cpp 51975f5aa2725f64d2a83387a381e5078c8b9e1d
>
> Diff: https://reviews.apache.org/r/15099/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 15099: Cleaned up the Registrar.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15099/#review34844
-----------------------------------------------------------
Ship it!
src/master/registrar.cpp
<https://reviews.apache.org/r/15099/#comment65243>
Let's not expose this as a CHECK as it's making a contract with a caller that is rather hard to track down. How about RegistrarProcess::admit returns a Failure if 'info.has_id()' is false? And let's also add a comment on Registrar::admit that makes it clear that the SlaveInfo is expected to have 'id' set.
- Benjamin Hindman
On Feb. 15, 2014, 12:57 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15099/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2014, 12:57 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Bugs: MESOS-764
> https://issues.apache.org/jira/browse/MESOS-764
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> This is addressing my comments on https://reviews.apache.org/r/14383/
>
> There was also a bug fix in the removal mutation.
>
>
> Diffs
> -----
>
> src/master/registrar.hpp 5742c365bac0082efea53c35333d3c3503494066
> src/master/registrar.cpp 915885a160f790399e8185c28c6e6555af1ee76e
> src/master/registry.proto bd850997c57153cdfc0c39d0dae7d2b802034aa3
> src/tests/registrar_tests.cpp 51975f5aa2725f64d2a83387a381e5078c8b9e1d
>
> Diff: https://reviews.apache.org/r/15099/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 15099: Cleaned up the Registrar.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15099/
-----------------------------------------------------------
(Updated Feb. 15, 2014, 12:57 a.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Linking ticket.
Bugs: MESOS-764
https://issues.apache.org/jira/browse/MESOS-764
Repository: mesos-git
Description
-------
This is addressing my comments on https://reviews.apache.org/r/14383/
There was also a bug fix in the removal mutation.
Diffs
-----
src/master/registrar.hpp 5742c365bac0082efea53c35333d3c3503494066
src/master/registrar.cpp 915885a160f790399e8185c28c6e6555af1ee76e
src/master/registry.proto bd850997c57153cdfc0c39d0dae7d2b802034aa3
src/tests/registrar_tests.cpp 51975f5aa2725f64d2a83387a381e5078c8b9e1d
Diff: https://reviews.apache.org/r/15099/diff/
Testing
-------
make check
Thanks,
Ben Mahler
Re: Review Request 15099: Cleaned up the Registrar.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15099/
-----------------------------------------------------------
(Updated Feb. 14, 2014, 7:41 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Added Master cleanups as dependent review.
Repository: mesos-git
Description
-------
This is addressing my comments on https://reviews.apache.org/r/14383/
There was also a bug fix in the removal mutation.
Diffs
-----
src/master/registrar.hpp 5742c365bac0082efea53c35333d3c3503494066
src/master/registrar.cpp 915885a160f790399e8185c28c6e6555af1ee76e
src/master/registry.proto bd850997c57153cdfc0c39d0dae7d2b802034aa3
src/tests/registrar_tests.cpp 51975f5aa2725f64d2a83387a381e5078c8b9e1d
Diff: https://reviews.apache.org/r/15099/diff/
Testing
-------
make check
Thanks,
Ben Mahler
Re: Review Request 15099: Cleaned up the Registrar.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15099/
-----------------------------------------------------------
(Updated Feb. 14, 2014, 7:40 p.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Rebased.
Repository: mesos-git
Description (updated)
-------
This is addressing my comments on https://reviews.apache.org/r/14383/
There was also a bug fix in the removal mutation.
Diffs (updated)
-----
src/master/registrar.hpp 5742c365bac0082efea53c35333d3c3503494066
src/master/registrar.cpp 915885a160f790399e8185c28c6e6555af1ee76e
src/master/registry.proto bd850997c57153cdfc0c39d0dae7d2b802034aa3
src/tests/registrar_tests.cpp 51975f5aa2725f64d2a83387a381e5078c8b9e1d
Diff: https://reviews.apache.org/r/15099/diff/
Testing
-------
make check
Thanks,
Ben Mahler
Re: Review Request 15099: Cleaned up the Registrar.
Posted by Ben Mahler <be...@gmail.com>.
> On Nov. 7, 2013, 5:47 p.m., Vinod Kone wrote:
> > src/master/registrar.cpp, line 307
> > <https://reviews.apache.org/r/15099/diff/2/?file=377780#file377780line307>
> >
> > How is this possible? i.e., update() is called when there are no mutations.
It's not possible, but I believe benh was looking to just do the right thing if this was called with no mutations, in which case update() is a no-op. I've changed the logic a bit here to return immediately in this case.
We could use a CHECK instead for now, I think either are fine.
> On Nov. 7, 2013, 5:47 p.m., Vinod Kone wrote:
> > src/master/registrar.cpp, line 193
> > <https://reviews.apache.org/r/15099/diff/2/?file=377780#file377780line193>
> >
> > If there is a network partition with the zk are we going to wait forever? Are we assuming this is ok because the leading master would eventually commit suicide if there is a partition?
The storage operation will be stuck, the leading master will self-terminate at some point.
This TODO is also for pending operations for other reasons, we likely don't want to wait forever during normal operation if something gets stuck. However, using a fixed timeout is dangerous unless it determined to be much larger than any reasonable storage latency.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15099/#review28384
-----------------------------------------------------------
On Nov. 5, 2013, 2:34 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15099/
> -----------------------------------------------------------
>
> (Updated Nov. 5, 2013, 2:34 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> This is addressing my comments on https://reviews.apache.org/r/14383/
>
>
> Diffs
> -----
>
> src/master/registrar.hpp 5742c365bac0082efea53c35333d3c3503494066
> src/master/registrar.cpp 42fe30ededd004e380abc835c224649e02999f11
> src/master/registry.proto bd850997c57153cdfc0c39d0dae7d2b802034aa3
> src/tests/registrar_tests.cpp 51975f5aa2725f64d2a83387a381e5078c8b9e1d
>
> Diff: https://reviews.apache.org/r/15099/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 15099: Cleaned up the Registrar.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15099/#review28384
-----------------------------------------------------------
src/master/registrar.cpp
<https://reviews.apache.org/r/15099/#comment55205>
If there is a network partition with the zk are we going to wait forever? Are we assuming this is ok because the leading master would eventually commit suicide if there is a partition?
src/master/registrar.cpp
<https://reviews.apache.org/r/15099/#comment55208>
How is this possible? i.e., update() is called when there are no mutations.
- Vinod Kone
On Nov. 5, 2013, 2:34 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15099/
> -----------------------------------------------------------
>
> (Updated Nov. 5, 2013, 2:34 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> This is addressing my comments on https://reviews.apache.org/r/14383/
>
>
> Diffs
> -----
>
> src/master/registrar.hpp 5742c365bac0082efea53c35333d3c3503494066
> src/master/registrar.cpp 42fe30ededd004e380abc835c224649e02999f11
> src/master/registry.proto bd850997c57153cdfc0c39d0dae7d2b802034aa3
> src/tests/registrar_tests.cpp 51975f5aa2725f64d2a83387a381e5078c8b9e1d
>
> Diff: https://reviews.apache.org/r/15099/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 15099: Cleaned up the Registrar.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15099/
-----------------------------------------------------------
(Updated Nov. 5, 2013, 2:34 a.m.)
Review request for mesos, Benjamin Hindman and Vinod Kone.
Changes
-------
Fixed a bug with slave removal from https://reviews.apache.org/r/14383/ and added a test that exposes the bug.
Repository: mesos-git
Description
-------
This is addressing my comments on https://reviews.apache.org/r/14383/
Diffs (updated)
-----
src/master/registrar.hpp 5742c365bac0082efea53c35333d3c3503494066
src/master/registrar.cpp 42fe30ededd004e380abc835c224649e02999f11
src/master/registry.proto bd850997c57153cdfc0c39d0dae7d2b802034aa3
src/tests/registrar_tests.cpp 51975f5aa2725f64d2a83387a381e5078c8b9e1d
Diff: https://reviews.apache.org/r/15099/diff/
Testing
-------
make check
Thanks,
Ben Mahler