> On 九月 1, 2015, 4:56 p.m., Alexander Rukletsov wrote: > > include/mesos/master/allocator.hpp, line 90 > > <https://reviews.apache.org/r/37993/diff/2/?file=1061146#file1061146line90> > > > > I think "register" is misleading. Allocator is notified that a new > > framework joins the cluster and is entitled to participate in resource > > sharing.
I was now using adds and re-adds instead in the new patch, comments? > On 九月 1, 2015, 4:56 p.m., Alexander Rukletsov wrote: > > include/mesos/master/allocator.hpp, line 109 > > <https://reviews.apache.org/r/37993/diff/2/?file=1061146#file1061146line109> > > > > Let's capiralize `Id` for clarity. Here and everywhere. Done > On 九月 1, 2015, 4:56 p.m., Alexander Rukletsov wrote: > > include/mesos/master/allocator.hpp, lines 139-142 > > <https://reviews.apache.org/r/37993/diff/2/?file=1061146#file1061146line139> > > > > 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? Since the updateFramework is a new API and I can only think up of cases for revocable resources related with allocator, can we update this comments in future if there are more cases? Thaks. > On 九月 1, 2015, 4:56 p.m., Alexander Rukletsov wrote: > > include/mesos/master/allocator.hpp, line 171 > > <https://reviews.apache.org/r/37993/diff/2/?file=1061146#file1061146line171> > > > > 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". I see that we already switched to AgentID in v1 API, but the V1 API was not finished. Is it possible that we keep slave here and fix this in another RR? Do you think that we need to file a new bug or epic to address this? Thanks. > On 九月 1, 2015, 4:56 p.m., Alexander Rukletsov wrote: > > include/mesos/master/allocator.hpp, line 213 > > <https://reviews.apache.org/r/37993/diff/2/?file=1061146#file1061146line213> > > > > I think we remove only outstanding offers, right? Alex, can you please show mode detail for what is outstanding offers? Thanks. > On 九月 1, 2015, 4:56 p.m., Alexander Rukletsov wrote: > > include/mesos/master/allocator.hpp, line 222 > > <https://reviews.apache.org/r/37993/diff/2/?file=1061146#file1061146line222> > > > > 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. Here I was updating to "host" to "slave" instead and want to address the issue of "slave"->"agent" in another patch, make sense? > On 九月 1, 2015, 4:56 p.m., Alexander Rukletsov wrote: > > include/mesos/master/allocator.hpp, line 226 > > <https://reviews.apache.org/r/37993/diff/2/?file=1061146#file1061146line226> > > > > I don't think this is correct. AFAIK whitelist contains slaves that > > should not participate in the allocation, basically, they are deactivated. I cite some code from allocator here, it seems that only hosts in whitelist can offer resources? foreach (const SlaveID& slaveId, slaveIds) { // Don't send offers for non-whitelisted and deactivated slaves. if (!isWhitelisted(slaveId) || !slaves[slaveId].activated) { continue; } - Guangya ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37993/#review97294 ----------------------------------------------------------- On 九月 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 九月 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 > >
