----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66044/#review199288 -----------------------------------------------------------
src/master/allocator/mesos/hierarchical.cpp Lines 1577-1590 (patched) <https://reviews.apache.org/r/66044/#comment279594> This function seems rather specific to the usage in this code, which makes it not very easy to intuit its behavior. How about: ``` // Returns the result of shrinking the provided resources down to the // target scalar quantities. If a resource does not have a target // quantity provided, it will not be shrunk. // // Note that some resources are indivisible (e.g. MOUNT volume) and // may be excluded in entirety in order to acheive the target size. // // Note also that there may be more than 1 result that satisfies // the target sizes (e.g. need to exclude 1 of 2 disks); this function // will make a random choice in these cases. auto shrinkResources = []( const Resources& resources, hashmap<string, Value::Scalar> targetQuantities) { ... }; ``` Then the caller can omit resources if it wants to acheive a default target size of 0 (i.e. `filterNonExistScalar`): ``` Resources allocation = shrinkResources(allocation, targetQuantities); allocation = allocation.filter([](const Resource& r) { return targetQuantites.contains(r.name()); }); ``` src/master/allocator/mesos/hierarchical.cpp Lines 1595-1603 (patched) <https://reviews.apache.org/r/66044/#comment279591> This comment seems to belong in the interface now that it's a function, otherwise the user reading the documentation for shrinkResources does not know that the result can vary given the input. - Benjamin Mahler On March 13, 2018, 10:04 p.m., Meng Zhu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66044/ > ----------------------------------------------------------- > > (Updated March 13, 2018, 10:04 p.m.) > > > Review request for mesos, Benjamin Mahler, Kapil Arya, Joseph Wu, Michael > Park, and Till Toenshoff. > > > Repository: mesos > > > Description > ------- > > Introduced a `shrinkResources()` lambda in allocator > so that the same resource chopping logic can be re-used > in the future, in particular, when introducing the quota > limit. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.cpp > 0e8c2c4a52969448f99bd5f42252a84cc52b9271 > > > Diff: https://reviews.apache.org/r/66044/diff/1/ > > > Testing > ------- > > make check > > > Thanks, > > Meng Zhu > >
