> On Feb. 8, 2019, 10:05 p.m., Benjamin Mahler wrote: > > src/master/allocator/mesos/hierarchical.cpp > > Lines 1886-1888 (patched) > > <https://reviews.apache.org/r/69821/diff/9/?file=2124578#file2124578line1886> > > > > Ditto the comment left below for the equivalent check in the second loop
Removed for now. > On Feb. 8, 2019, 10:05 p.m., Benjamin Mahler wrote: > > src/master/allocator/mesos/hierarchical.cpp > > Lines 2110-2112 (patched) > > <https://reviews.apache.org/r/69821/diff/9/?file=2124578#file2124578line2110> > > > > Hm.. why bother with this? Seems correct without it and this doesn't > > seem to save much in terms of cost? It also seems like an orthogonal change? > > > > It seems ok to add this one and the one above, but seems cleaner to do > > that in a follow up change that measures the performance impact? The quota > > one does seem to potentially save some expensive work Removed for now. > On Feb. 8, 2019, 10:05 p.m., Benjamin Mahler wrote: > > src/master/allocator/mesos/hierarchical.cpp > > Line 2125 (original), 2138-2141 (patched) > > <https://reviews.apache.org/r/69821/diff/9/?file=2124578#file2124578line2138> > > > > Let's not bother with this additional check? If `toAllocate` is empty, > > we'll continue from the check below (allocatable always is false for no > > resources), followed by the top of this loop breaking because there's > > nothing left for the role. So this only saves having to execute a few lines > > at the top of the loop? Doesn't seem worth it? Removed. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69821/#review212679 ----------------------------------------------------------- On Feb. 11, 2019, 4:46 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69821/ > ----------------------------------------------------------- > > (Updated Feb. 11, 2019, 4:46 p.m.) > > > Review request for mesos, Benjamin Mahler and Meng Zhu. > > > Bugs: MESOS-9523 > https://issues.apache.org/jira/browse/MESOS-9523 > > > Repository: mesos > > > Description > ------- > > This patch modifies the hierarchical allocator to take > framework-specified minimal allocatable resources into account. > > While previously the allocator was inspecting the minimal allocatable > resources specified in its global options, it can now also inspects > framework-specific resource requirements. With that frameworks can e.g., > configure resource requirements above the default minimal allocatable > resource, or opt into receiving resources considered too small to be > allocatable by the allocator in its default behavior. > > For that we change the hierarchical allocator's `allocatable` function > to be framework and role-specific. As that does in some places not allow > us to abort iterating over candidate resource consumers like before an > additional check on whether any resources are left in an allocation > cycle is added as a break condition. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.hpp > 1420c2638786d85f7b04379e5d79e59990c3e6cf > src/master/allocator/mesos/hierarchical.cpp > bb9a9c95979f36c0564af5b3babb1c43077a363b > > > Diff: https://reviews.apache.org/r/69821/diff/11/ > > > Testing > ------- > > `make check` > > > File Attachments > ---------------- > > Ratio new/old timings > > https://reviews.apache.org/media/uploaded/files/2019/01/31/d76189de-8882-4aff-956b-090dab729358__new_over_old.png > > > Thanks, > > Benjamin Bannier > >
