> On Feb. 4, 2019, 9:58 p.m., Benjamin Mahler wrote: > > src/master/allocator/mesos/hierarchical.cpp > > Lines 2058-2060 (original), 2092-2094 (patched) > > <https://reviews.apache.org/r/69821/diff/5/?file=2123824#file2123824line2101> > > > > We lost the comment here about why it's safe to break? It still seems > > relevant > > Benjamin Bannier wrote: > Hmm, not sure what you are after. My patch `continue`s instead of > `break`s. The way the code is structured now we cannot `break` here, but > instead must iterate over all frameworks. We could `break` if we'd e.g., made > `allocatable` independent of framework settings like before (e.g., by > computing a minimal allocatable resources given all framework information), > but we'd likely reject many allocation decision last minute that way in the > same spot where we currently check filters. The code as proposed here looks > simpler to me. > > Can we drop this?
Dropping. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69821/#review212523 ----------------------------------------------------------- On Feb. 8, 2019, 12:32 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69821/ > ----------------------------------------------------------- > > (Updated Feb. 8, 2019, 12:32 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/9/ > > > 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 > >
