> On Feb. 4, 2019, 9:58 p.m., Benjamin Mahler wrote: > > Can you split this change apart so that I can review more easily and we can > > land it faster? > > > > (1) The plumbing and storage of the information: this is an easy change > > that doesn't require much thought, it looks good and can be shipped. > > > > (2) The update to the allocation / allocatable logic: this is a harder > > change that warrants careful thought about the existing allocation code and > > I'd like to review it in isolation from the other changes in the current > > patch. Left some comments in the second stage that also apply to the first > > stage. > > > > (3) Tests: also a rather easy change and would be good to ship in > > isolation. Haven't review this yet (would prefer to review in isolation > > since the review load is too high in this patch).
Done, created https://reviews.apache.org/r/69889/ and https://reviews.apache.org/r/69890/ which bracket this patch. I fixed small issues in https://reviews.apache.org/r/69889/, which I didn't see in the bigger patch, so I have to agree with you that the patches where hard to review :/ Since you left a couple of allocator-related issues here already, I am reusing this RR for the allocator changes. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69821/#review212523 ----------------------------------------------------------- On Feb. 4, 2019, 11:05 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69821/ > ----------------------------------------------------------- > > (Updated Feb. 4, 2019, 11:05 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/7/ > > > 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 > >
