> On March 27, 2019, 2:41 p.m., Benjamin Mahler wrote: > > src/master/allocator/mesos/hierarchical.cpp > > Lines 1928 (patched) > > <https://reviews.apache.org/r/70320/diff/1/?file=2134448#file2134448line1938> > > > > Rather than adding this additional function, can't `shrinkResoures` > > take a `ResourceQuantities`? > > Meng Zhu wrote: > We need to create a map-like a copy somewhere. `shrinkResoures` needs a > copy of its own to do bookkeeping: > > https://github.com/apache/mesos/blob/c10def96653fb99e309459476f5bc09c1da686ee/src/master/allocator/mesos/hierarchical.cpp#L1666 > > Or do you mean to create the map in the `shrinkResoures` function? My > concern with that is currently `shrinkResources` has this semantic: > >If a resource does not have a target quantity provided, it will not be > shrunk. > > This is not compatible with `ResourceQuantities`. Given the semantic of > `ResourceQuantities`, one would expect `shrinkResources` should drop (shrink > to zero) for resources that are absent in the input `ResourceQuantities`. > > Of course, since we are the only consumer of the function here, we could > change the function semantic. But I do not see much benefits since, as > mentioned above, we need to pass a map anyway. > > Another thing to consider is we might need to get a copy of the scalar > map somewhere else where direct mutations are needed.
> This is not compatible with ResourceQuantities. Given the semantic of > ResourceQuantities, one would expect shrinkResources should drop (shrink to > zero) for resources that are absent in the input ResourceQuantities. Seems like we want to pass in ResourceLimits then? (that makes sense given the argument name too :)). It seems weird to have the map function in the class, can we just do it manually here? - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70320/#review214106 ----------------------------------------------------------- On March 27, 2019, 5:31 a.m., Meng Zhu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70320/ > ----------------------------------------------------------- > > (Updated March 27, 2019, 5:31 a.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-9504 > https://issues.apache.org/jira/browse/MESOS-9504 > > > Repository: mesos > > > Description > ------- > > Replaced `Resources` with `ResourceQuantities` when > appropriate in `__allocate()`. This simplifies the > code and improves performance. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.hpp > 36bf90baf413e99c1580d516dfac0f074335d322 > src/master/allocator/mesos/hierarchical.cpp > 8bc749903b8ceb09a02e260919377483479302b5 > > > Diff: https://reviews.apache.org/r/70320/diff/2/ > > > Testing > ------- > > make check > Benchmark results in r/70330 > > > Thanks, > > Meng Zhu > >
