----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37993/#review97294 -----------------------------------------------------------
include/mesos/master/allocator.hpp (lines 62 - 64) <https://reviews.apache.org/r/37993/#comment153117> I think we can kill this for brevity. include/mesos/master/allocator.hpp (line 68) <https://reviews.apache.org/r/37993/#comment153118> Period in the end, please! include/mesos/master/allocator.hpp (line 70) <https://reviews.apache.org/r/37993/#comment153119> 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) <https://reviews.apache.org/r/37993/#comment153122> Please a period at the end! include/mesos/master/allocator.hpp (line 87) <https://reviews.apache.org/r/37993/#comment153120> I think we prefer 's' at the end of the verb in such cases: `Add*s* a framework`. include/mesos/master/allocator.hpp (line 89) <https://reviews.apache.org/r/37993/#comment153121> 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) <https://reviews.apache.org/r/37993/#comment153123> 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) <https://reviews.apache.org/r/37993/#comment153124> Let's capiralize `Id` for clarity. Here and everywhere. include/mesos/master/allocator.hpp (line 117) <https://reviews.apache.org/r/37993/#comment153125> s/activated/active include/mesos/master/allocator.hpp (lines 138 - 141) <https://reviews.apache.org/r/37993/#comment153128> 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) <https://reviews.apache.org/r/37993/#comment153133> 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) <https://reviews.apache.org/r/37993/#comment153136> 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) <https://reviews.apache.org/r/37993/#comment153138> I think we remove only outstanding offers, right? include/mesos/master/allocator.hpp (line 213) <https://reviews.apache.org/r/37993/#comment153140> ... and resources recovered in a separate call. include/mesos/master/allocator.hpp (line 221) <https://reviews.apache.org/r/37993/#comment153141> 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) <https://reviews.apache.org/r/37993/#comment153142> 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) <https://reviews.apache.org/r/37993/#comment153143> 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) <https://reviews.apache.org/r/37993/#comment153144> 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 > >
