mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Vinod Kone" <vinodk...@gmail.com>
Subject Re: Review Request 32509: Documented the scheduler Event/Call protobufs.
Date Thu, 23 Apr 2015 02:54:50 GMT


> On April 22, 2015, 11:26 p.m., Ben Mahler wrote:
> > include/mesos/scheduler/scheduler.proto, line 52
> > <https://reviews.apache.org/r/32509/diff/6/?file=939979#file939979line52>
> >
> >     Hm, what do you mean by "can be offered"? Do you want to just s/that can be//
?

done.


> On April 22, 2015, 11:26 p.m., Ben Mahler wrote:
> > include/mesos/scheduler/scheduler.proto, lines 54-55
> > <https://reviews.apache.org/r/32509/diff/6/?file=939979#file939979line54>
> >
> >     Hm, this last sentence sort of implies no optimistic offers? We'll need to be
sure that we update the documentation later I suppose.

yup.


> On April 22, 2015, 11:26 p.m., Ben Mahler wrote:
> > include/mesos/scheduler/scheduler.proto, lines 60-63
> > <https://reviews.apache.org/r/32509/diff/6/?file=939979#file939979line60>
> >
> >     "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?

"A Rescind is received when an offer is rescinded" doesn't seem to offer much value to framework
writers? I used "invalid" because from framework's perspective the offer is no longer valid,
as in, it cannot use it. Does that make sense?


> On April 22, 2015, 11:26 p.m., Ben Mahler wrote:
> > include/mesos/scheduler/scheduler.proto, line 69
> > <https://reviews.apache.org/r/32509/diff/6/?file=939979#file939979line69>
> >
> >     "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?

"mesos" might be weird because frameworks might not consider executors sending updates as
getting from mesos? Regarding Slave renaming, we should have a separate discussion around
it because the api uses protobufs from mesos.proto which uses it.


> On April 22, 2015, 11:26 p.m., Ben Mahler wrote:
> > include/mesos/scheduler/scheduler.proto, lines 101-102
> > <https://reviews.apache.org/r/32509/diff/6/?file=939979#file939979line101>
> >
> >     Specifically, we have to think about how to do reconciliation between mesos
and the scheduler, yes?

yup. expanded the TODO.


> On April 22, 2015, 11:26 p.m., Ben Mahler wrote:
> > include/mesos/scheduler/scheduler.proto, line 151
> > <https://reviews.apache.org/r/32509/diff/6/?file=939979#file939979line151>
> >
> >     Do you want to add something about how the "removes" the framework too?

done.


> On April 22, 2015, 11:26 p.m., Ben Mahler wrote:
> > include/mesos/scheduler/scheduler.proto, lines 235-238
> > <https://reviews.apache.org/r/32509/diff/6/?file=939979#file939979line235>
> >
> >     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?

SGTM. I'll remove uuid from Update, now that it is part of TaskStatus. Will do that in a dependent
review.


> On April 22, 2015, 11:26 p.m., Ben Mahler wrote:
> > include/mesos/scheduler/scheduler.proto, lines 268-270
> > <https://reviews.apache.org/r/32509/diff/6/?file=939979#file939979line268>
> >
> >     Hm.. schedulers can't use Update as a replacement for this Message, since Message
is scheduler -> executor but Update is executor -> scheduler..

Alex asked the same thing earlier. My response was that schedulers could use tasks and Updates
to do 2 way communication. I'll just remove this sentence to avoid confusion.


> On April 22, 2015, 11:26 p.m., Ben Mahler wrote:
> > include/mesos/scheduler/scheduler.proto, lines 113-116
> > <https://reviews.apache.org/r/32509/diff/6/?file=939979#file939979line113>
> >
> >     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?

This is there for backwards compatibility with the old driver and hence talks about the conditions
that generate Error with the old driver. I plan to remove this in the new API (as the TODO
says) because the errors will be delivered as HTTP responses instead of on the SUBSCRIBE stream.
That makes sense?


- Vinod


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


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