This is an automatically generated e-mail. To reply, visit:

include/mesos/master/allocator.hpp (lines 62 - 64)

    I think we can kill this for brevity.

include/mesos/master/allocator.hpp (line 68)

    Period in the end, please!

include/mesos/master/allocator.hpp (line 70)

    I think we tend to merge detailed description with the summary in case both 
are very short. Here and below.
    Also, let's mention that calling `initialize()` supposes to start 
allocation process.

include/mesos/master/allocator.hpp (line 72)

    Please a period at the end!

include/mesos/master/allocator.hpp (line 87)

    I think we prefer 's' at the end of the verb in such cases: `Add*s* a 

include/mesos/master/allocator.hpp (line 89)

    I think "register" is misleading. Allocator is notified that a new 
framework joins the cluster and is entitled to participate in resource sharing.

include/mesos/master/allocator.hpp (lines 105 - 106)

    It's an interface, it cannot guarantee that resoruces will be released. We 
should document an expectation or a contract here. It's up to an allocator what 
to do in this case, the built-in just removes the framework from the fair share 
pool AFAIK. Let's reword.

include/mesos/master/allocator.hpp (line 108)

    Let's capiralize `Id` for clarity. Here and everywhere.

include/mesos/master/allocator.hpp (line 117)


include/mesos/master/allocator.hpp (lines 138 - 141)

    This it true for the built-in allocator, but not necessarily for *any* 
allocator. Could you please reword putting an accent on *under what 
cercumstances* the method is called and maybe what an expected behaviour may be?

include/mesos/master/allocator.hpp (lines 154 - 158)

    The comment about static reservations is important, let's keep it in the 
description. Again, let's add some information on when it's called.

include/mesos/master/allocator.hpp (line 170)

    We try to switch to "agent" instead of "slave". Let's do it in the comments 
(here and everywhere). Also, let's have a note in the beginning of the doc 
saying "agent" is the new "slave".

include/mesos/master/allocator.hpp (line 212)

    I think we remove only outstanding offers, right?

include/mesos/master/allocator.hpp (line 213)

    ... and resources recovered in a separate call.

include/mesos/master/allocator.hpp (line 221)

    I think "agent" is more clear than "host" here. You maybe refered to the 
fact that the whitelist consists of hostnames, but that's slightly different 
and should be documented in the whitelist class.

include/mesos/master/allocator.hpp (line 225)

    I don't think this is correct. AFAIK whitelist contains slaves that should 
not participate in the allocation, basically, they are deactivated.

include/mesos/master/allocator.hpp (lines 235 - 237)

    Let's say here that a framework may request resources, but it is up to 
allocator whether and how to satisfy this request.

include/mesos/master/allocator.hpp (lines 280 - 283)

    Also, when framework is deactivated.

- Alexander Rukletsov

On Sept. 1, 2015, 2:59 p.m., Guangya Liu wrote:
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37993/
> -----------------------------------------------------------
> (Updated Sept. 1, 2015, 2:59 p.m.)
> Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.
> Bugs: MESOS-2224
>     https://issues.apache.org/jira/browse/MESOS-2224
> Repository: mesos
> Description
> -------
> Add explanatory comments for Allocator interface
> Diffs
> -----
>   include/mesos/master/allocator.hpp 659f37b3f9d9fa02da9bdb6c85cd3c180a24b73a 
> Diff: https://reviews.apache.org/r/37993/diff/
> Testing
> -------
> Thanks,
> Guangya Liu

Reply via email to