mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ben Mahler" <benjamin.mah...@gmail.com>
Subject Re: Review Request 32509: Documented the scheduler Event/Call protobufs.
Date Wed, 22 Apr 2015 23:26:33 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32509/#review81234
-----------------------------------------------------------

Ship it!


Looks great, only 1 question which we can tackle separately from this review:

Is it possible to describe when to acknowledge an update in a more explicit way (currently
source == SLAVE of source == EXECUTOR seems implicit). Can we make the 'uuid' optional? Also
we should think about how the master can drop acknowledgements for its own updates.


include/mesos/scheduler/scheduler.proto
<https://reviews.apache.org/r/32509/#comment131495>

    Hm, what do you mean by "can be offered"? Do you want to just s/that can be// ?



include/mesos/scheduler/scheduler.proto
<https://reviews.apache.org/r/32509/#comment131496>

    Hm, this last sentence sort of implies no optimistic offers? We'll need to be sure that
we update the documentation later I suppose.



include/mesos/scheduler/scheduler.proto
<https://reviews.apache.org/r/32509/#comment131499>

    "no longer valid" seems a little specific, we could rescind in cases where the offer would
have remained valid but we choose to change the resource allocations dynamically. How about
just s/is no longer valid/is rescinded/ and removing the "hence" part?
    
    Also, do we need the example or can we trim this down a bit?



include/mesos/scheduler/scheduler.proto
<https://reviews.apache.org/r/32509/#comment131502>

    "or mesos"? Slave and master might be a bit too implementation specific in the future,
plus we'd like to rename "Slave".
    
    On that note, before we finalize the http api, can we get rid of "slave" in the api?



include/mesos/scheduler/scheduler.proto
<https://reviews.apache.org/r/32509/#comment131510>

    Specifically, we have to think about how to do reconciliation between mesos and the scheduler,
yes?



include/mesos/scheduler/scheduler.proto
<https://reviews.apache.org/r/32509/#comment131511>

    Isn't this more general than these two specific cases? Maybe we should describe it more
generally instead of documenting examples like this? For example, what should the scheduler
do when Error is received? Will the connection be closed? Will they need to re-subscribe?



include/mesos/scheduler/scheduler.proto
<https://reviews.apache.org/r/32509/#comment131512>

    Do you want to add something about how the "removes" the framework too?



include/mesos/scheduler/scheduler.proto
<https://reviews.apache.org/r/32509/#comment131518>

    Hm.. are we expecting schedulers to use the source to decide whether to acknowledge? That
seems a bit implicit, should we make 'uuid' optional to make it more clear?
    
    Also, that way, the master can tell when one of its updates is being acknowledged (empty
uuid), which should also make it possible for the master to drop the acknowledgments, yes?



include/mesos/scheduler/scheduler.proto
<https://reviews.apache.org/r/32509/#comment131519>

    Hm.. schedulers can't use Update as a replacement for this Message, since Message is scheduler
-> executor but Update is executor -> scheduler..


- Ben Mahler


On April 22, 2015, 9:44 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32509/
> -----------------------------------------------------------
> 
> (Updated April 22, 2015, 9:44 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-1127
>     https://issues.apache.org/jira/browse/MESOS-1127
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   include/mesos/scheduler/scheduler.proto 783a63ad1cc0edd86605d638046fb959cb6e97e8 
> 
> Diff: https://reviews.apache.org/r/32509/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message