----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37993/#review97478 -----------------------------------------------------------
First of all, thanks for adding comments! include/mesos/master/allocator.hpp (line 67) <https://reviews.apache.org/r/37993/#comment153366> s/start/starts/ include/mesos/master/allocator.hpp (line 68) <https://reviews.apache.org/r/37993/#comment153368> The second part of this sentence seem unnecessary. You could say: Initializes the allocator and allocator process. This is called when the master starts up. include/mesos/master/allocator.hpp (line 70) <https://reviews.apache.org/r/37993/#comment153385> Suggestion: Generally determines how often the allocator should update the allocation. Note that other calls to the allocator, like Allocator::reviveOffers, may also trigger an update. include/mesos/master/allocator.hpp (lines 71 - 72) <https://reviews.apache.org/r/37993/#comment153388> Suggestion: A callback the allocator should use to send offers to frameworks. See Master::offer for more details. --- You could also document the Master::offer function (currently undocumented). include/mesos/master/allocator.hpp (lines 91 - 92) <https://reviews.apache.org/r/37993/#comment153390> These two @param's don't add much, so you could exclude them. I added comments like this in the past, which led to this email thread: http://www.mail-archive.com/dev%40mesos.apache.org/msg32792.html include/mesos/master/allocator.hpp (line 94) <https://reviews.apache.org/r/37993/#comment153391> s/re-registered/re-registers/ include/mesos/master/allocator.hpp (lines 105 - 108) <https://reviews.apache.org/r/37993/#comment153392> Try not to document the implementation of the HierarchicalDRF. You can, of course, offer possible behaviors. Suggestion: Removes a framework from the Mesos cluster. The allocator can decide what to do with framework's resources. For example, the removed framework's resources may be released and added back to the shared pool of resources. include/mesos/master/allocator.hpp (line 110) <https://reviews.apache.org/r/37993/#comment153393> Again, this doesn't add much. It's up to you if you want to keep it or not. Same for all the similar @param's below (not marked). include/mesos/master/allocator.hpp (line 118) <https://reviews.apache.org/r/37993/#comment153394> s/resoure/resource/ include/mesos/master/allocator.hpp (line 129) <https://reviews.apache.org/r/37993/#comment153395> s/resoure/resource/ include/mesos/master/allocator.hpp (lines 140 - 144) <https://reviews.apache.org/r/37993/#comment153396> Run-on sentence. You could easily split this into 3-4 sentences. include/mesos/master/allocator.hpp (line 142) <https://reviews.apache.org/r/37993/#comment153397> Again, try not to document the HierarchicalDRF ("the built-in allocator"). s/revocable/revocable resources/ include/mesos/master/allocator.hpp (line 160) <https://reviews.apache.org/r/37993/#comment153398> Unnecessary @param. If you do keep it, s/add/added/ include/mesos/master/allocator.hpp (lines 216 - 217) <https://reviews.apache.org/r/37993/#comment153401> Suggestion: An agent may be deactivated if it is disconnnected from the master. include/mesos/master/allocator.hpp (line 227) <https://reviews.apache.org/r/37993/#comment153402> Missing a period. include/mesos/master/allocator.hpp (lines 229 - 231) <https://reviews.apache.org/r/37993/#comment153403> Run-on. Split this into 2-3 sentences. include/mesos/master/allocator.hpp (lines 241 - 244) <https://reviews.apache.org/r/37993/#comment153405> Run-on. Suggestion: A framework requests resources via this API. The allocator can choose how to respond. For example, requests may be ignored or the allocator may change its priorities accordingly. - Joseph Wu On Sept. 1, 2015, 8:06 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, 8:06 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 > >
