----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69821/#review212573 -----------------------------------------------------------
Much appreciated for having split out the changes! This is nice and small now :) I had a hard time figuring out what the logic should be if MESOS-9554 isn't addressed by this patch, so I ended up pulling out a fix: https://reviews.apache.org/r/69902/ (note the 1 small dependency patch in front of it) We can base this patch on this fixed version of the code and it should be pretty clear. If we can land the fix quickly then we could just commit it and rebase this patch. src/master/allocator/mesos/hierarchical.cpp Lines 1985-1996 (original), 1985-1988 (patched) <https://reviews.apache.org/r/69821/#comment298401> It's hard to review this logic without fixing MESOS-9554, so I've put up a patch: https://reviews.apache.org/r/69902/ Note that there's a small dependency pulled out in front of it to make it a smaller change. We can make this review depend on it, or probably easier, land that patch and then rebase this against master. - Benjamin Mahler On Feb. 5, 2019, 4:38 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69821/ > ----------------------------------------------------------- > > (Updated Feb. 5, 2019, 4:38 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/8/ > > > 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 > >
