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/06/03 05:57:32 UTC
Re: Review Request 20309: Introduced a low-level scheduler API.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20309/
-----------------------------------------------------------
(Updated June 3, 2014, 3:57 a.m.)
Review request for mesos.
Changes
-------
Updates from review comments.
Repository: mesos-git
Description
-------
See summary. Note that in an incremental review I plan to implement Scheduler and SchedulerDriver on this API so that we can run all existing tests using this API. For now I've included at least one test as a demonstration.
Diffs (updated)
-----
include/mesos/mesos.proto 82388e1ed5ee36aaf7fc31d7a59aa0923ee9ab46
include/mesos/scheduler.hpp 85db1117122441b5d9fb013380ea37aa0acc84ea
src/Makefile.am ffde59be8683dd40cc5bc7cb88cd88c5bc91cf96
src/sched/sched.cpp be23e012fba99363b72b749b110f12ce3c902014
src/scheduler/scheduler.cpp PRE-CREATION
src/tests/master_tests.cpp 7183cb7d567664a2f07b8d11980c8fbbec1711af
src/tests/scheduler_tests.cpp bd3c22d5a500c6f9046afa5ff5185a4d86796c57
Diff: https://reviews.apache.org/r/20309/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman
Re: Review Request 20309: Introduced a low-level scheduler API.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On June 3, 2014, 9:55 p.m., Ben Mahler wrote:
> > Modulo the stuff we noticed when going over it together that are not in my comments below.
> >
> > Can you create some tickets that show the plan to getting event/call integrated fully?
> > The explicit plan with fix versions would reduce the mental overhead. :)
> >
> > I think the key insight for me with this review was understanding that this now supports nice layering:
> >
> > Event/Call Protocol
> > Event/Call C++ API (networking + master detection + authentication)
> > ? Abstract C++ Scheduler (state + reconciliation + ...) ?
> > Application C++ Scheduler
Yes, will create the tickets. Thanks Ben.
> On June 3, 2014, 9:55 p.m., Ben Mahler wrote:
> > include/mesos/scheduler/scheduler.proto, lines 163-165
> > <https://reviews.apache.org/r/20309/diff/3/?file=603117#file603117line163>
> >
> > Per discussions around reconciliation, it sounds like we'll want to only be sending task ids and be receiving updates for each task.
> >
> > Could we perhaps leave a TODO for this one since we'll probably only want to send the ids?
> >
> > I created MESOS-1453 for this, would be nice to reference that here!
Added a comment, thanks for creating the ticket!
> On June 3, 2014, 9:55 p.m., Ben Mahler wrote:
> > src/scheduler/scheduler.cpp, lines 541-544
> > <https://reviews.apache.org/r/20309/diff/3/?file=603120#file603120line541>
> >
> > As discussed, can you add a note somewhere about how the 'connected' and 'disconnected' events may be sent out-of-order with the events that occur in the interim? Maybe a TODO to fix?
Added a comment and a TODO.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20309/#review44673
-----------------------------------------------------------
On June 3, 2014, 7:52 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20309/
> -----------------------------------------------------------
>
> (Updated June 3, 2014, 7:52 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary. Note that in an incremental review I plan to implement Scheduler and SchedulerDriver on this API so that we can run all existing tests using this API. For now I've included at least one test as a demonstration.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 82388e1ed5ee36aaf7fc31d7a59aa0923ee9ab46
> include/mesos/scheduler.hpp 85db1117122441b5d9fb013380ea37aa0acc84ea
> include/mesos/scheduler/scheduler.hpp PRE-CREATION
> include/mesos/scheduler/scheduler.proto PRE-CREATION
> src/Makefile.am ffde59be8683dd40cc5bc7cb88cd88c5bc91cf96
> src/sched/sched.cpp be23e012fba99363b72b749b110f12ce3c902014
> src/scheduler/scheduler.cpp PRE-CREATION
> src/tests/master_tests.cpp 7183cb7d567664a2f07b8d11980c8fbbec1711af
> src/tests/scheduler_tests.cpp bd3c22d5a500c6f9046afa5ff5185a4d86796c57
>
> Diff: https://reviews.apache.org/r/20309/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 20309: Introduced a low-level scheduler API.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20309/#review44673
-----------------------------------------------------------
Ship it!
Modulo the stuff we noticed when going over it together that are not in my comments below.
Can you create some tickets that show the plan to getting event/call integrated fully?
The explicit plan with fix versions would reduce the mental overhead. :)
I think the key insight for me with this review was understanding that this now supports nice layering:
Event/Call Protocol
Event/Call C++ API (networking + master detection + authentication)
? Abstract C++ Scheduler (state + reconciliation + ...) ?
Application C++ Scheduler
include/mesos/scheduler/scheduler.proto
<https://reviews.apache.org/r/20309/#comment79125>
Per discussions around reconciliation, it sounds like we'll want to only be sending task ids and be receiving updates for each task.
Could we perhaps leave a TODO for this one since we'll probably only want to send the ids?
I created MESOS-1453 for this, would be nice to reference that here!
src/scheduler/scheduler.cpp
<https://reviews.apache.org/r/20309/#comment79126>
As discussed, can you add a note somewhere about how the 'connected' and 'disconnected' events may be sent out-of-order with the events that occur in the interim? Maybe a TODO to fix?
- Ben Mahler
On June 3, 2014, 7:52 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20309/
> -----------------------------------------------------------
>
> (Updated June 3, 2014, 7:52 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary. Note that in an incremental review I plan to implement Scheduler and SchedulerDriver on this API so that we can run all existing tests using this API. For now I've included at least one test as a demonstration.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 82388e1ed5ee36aaf7fc31d7a59aa0923ee9ab46
> include/mesos/scheduler.hpp 85db1117122441b5d9fb013380ea37aa0acc84ea
> include/mesos/scheduler/scheduler.hpp PRE-CREATION
> include/mesos/scheduler/scheduler.proto PRE-CREATION
> src/Makefile.am ffde59be8683dd40cc5bc7cb88cd88c5bc91cf96
> src/sched/sched.cpp be23e012fba99363b72b749b110f12ce3c902014
> src/scheduler/scheduler.cpp PRE-CREATION
> src/tests/master_tests.cpp 7183cb7d567664a2f07b8d11980c8fbbec1711af
> src/tests/scheduler_tests.cpp bd3c22d5a500c6f9046afa5ff5185a4d86796c57
>
> Diff: https://reviews.apache.org/r/20309/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 20309: Introduced a low-level scheduler API.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20309/
-----------------------------------------------------------
(Updated June 3, 2014, 7:52 p.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Added reviewers!
Repository: mesos-git
Description
-------
See summary. Note that in an incremental review I plan to implement Scheduler and SchedulerDriver on this API so that we can run all existing tests using this API. For now I've included at least one test as a demonstration.
Diffs
-----
include/mesos/mesos.proto 82388e1ed5ee36aaf7fc31d7a59aa0923ee9ab46
include/mesos/scheduler.hpp 85db1117122441b5d9fb013380ea37aa0acc84ea
include/mesos/scheduler/scheduler.hpp PRE-CREATION
include/mesos/scheduler/scheduler.proto PRE-CREATION
src/Makefile.am ffde59be8683dd40cc5bc7cb88cd88c5bc91cf96
src/sched/sched.cpp be23e012fba99363b72b749b110f12ce3c902014
src/scheduler/scheduler.cpp PRE-CREATION
src/tests/master_tests.cpp 7183cb7d567664a2f07b8d11980c8fbbec1711af
src/tests/scheduler_tests.cpp bd3c22d5a500c6f9046afa5ff5185a4d86796c57
Diff: https://reviews.apache.org/r/20309/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman
Re: Review Request 20309: Introduced a low-level scheduler API.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20309/
-----------------------------------------------------------
(Updated June 3, 2014, 7:52 p.m.)
Review request for mesos.
Changes
-------
Added missing files.
Repository: mesos-git
Description
-------
See summary. Note that in an incremental review I plan to implement Scheduler and SchedulerDriver on this API so that we can run all existing tests using this API. For now I've included at least one test as a demonstration.
Diffs (updated)
-----
include/mesos/mesos.proto 82388e1ed5ee36aaf7fc31d7a59aa0923ee9ab46
include/mesos/scheduler.hpp 85db1117122441b5d9fb013380ea37aa0acc84ea
include/mesos/scheduler/scheduler.hpp PRE-CREATION
include/mesos/scheduler/scheduler.proto PRE-CREATION
src/Makefile.am ffde59be8683dd40cc5bc7cb88cd88c5bc91cf96
src/sched/sched.cpp be23e012fba99363b72b749b110f12ce3c902014
src/scheduler/scheduler.cpp PRE-CREATION
src/tests/master_tests.cpp 7183cb7d567664a2f07b8d11980c8fbbec1711af
src/tests/scheduler_tests.cpp bd3c22d5a500c6f9046afa5ff5185a4d86796c57
Diff: https://reviews.apache.org/r/20309/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman