> On April 10, 2018, 3 p.m., Benjamin Mahler wrote: > > Looks good! Just thinking we probably should pull the shrink function up > > and unit test it? Feel free to do that in a second patch.
Got it. Will put it in the resource headers in a followup patch. > On April 10, 2018, 3 p.m., Benjamin Mahler wrote: > > src/master/allocator/mesos/hierarchical.cpp > > Lines 1600 (patched) > > <https://reviews.apache.org/r/66044/diff/2/?file=1982719#file1982719line1600> > > > > To enable move performance improvements later, we can do: > > > > ``` > > result += std::move(resource); > > ``` Done. > On April 10, 2018, 3 p.m., Benjamin Mahler wrote: > > src/master/allocator/mesos/hierarchical.cpp > > Lines 1607-1610 (patched) > > <https://reviews.apache.org/r/66044/diff/2/?file=1982719#file1982719line1607> > > > > I assume this continues to work as the value scalar goes negative? We > > should probably pull this up into the header and unit test it? By "continues to work", I guess you expect the function to ignore negative shrink? Then no, current `Resources::shrink()` will return a `Resource` object with negative values with the same meta-data (if shrinkable). https://github.com/apache/mesos/blob/88f5629e510d71a32bd7e0ff7ee09e150f944e72/src/v1/resources.cpp#L1296-L1315 I can fix that in a separate patch (if negative return directly). Though I think in most use cases, the scalar values will most likely come from another resource(s) object, I do not think a negative resource object can be created easily (unless intensionally). - Meng ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66044/#review200846 ----------------------------------------------------------- On April 10, 2018, 3:54 p.m., Meng Zhu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66044/ > ----------------------------------------------------------- > > (Updated April 10, 2018, 3:54 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 > 32e88952101d8dbbae9728478b1f5663bf46c3bb > > > Diff: https://reviews.apache.org/r/66044/diff/3/ > > > Testing > ------- > > make check > > > Thanks, > > Meng Zhu > >
