-----------------------------------------------------------
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
> 
>

Reply via email to