> On May 28, 2019, 10:35 a.m., Benjamin Mahler wrote: > > src/common/resource_quantities.cpp > > Lines 558-560 (patched) > > <https://reviews.apache.org/r/70735/diff/1/?file=2146892#file2146892line558> > > > > Why not max? > > > > ``` > > limit.second = std::max(limit.second - quantity.second, > > Value::Scalar()); > > ``` > > > > Do we generally try to avoid producing an negative?
Used `max`. Not sure if I understand the question. But I think the non-negative logic would better be encoded in the Value::Scalar itself. > On May 28, 2019, 10:35 a.m., Benjamin Mahler wrote: > > src/tests/resource_quantities_tests.cpp > > Lines 565-568 (patched) > > <https://reviews.apache.org/r/70735/diff/1/?file=2146893#file2146893line565> > > > > Seems a little cleaner like this: > > > > ``` > > EXPECT_EQ(limits("cpus:0"), > > limits("cpus:1") - quantities("cpus:1")); > > > > EXPECT_EQ(limits("cpus:0"), > > limits("cpus:1") - quantities("cpus:2")); > > ``` > > > > etc, i.e. a `limits()` and `quantities()` labmdas, and no need for the > > variables and the same parsing / CHECKing that's also in the `subtract()` > > lambda. Thanks! Much cleaner. - Meng ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70735/#review215544 ----------------------------------------------------------- On May 29, 2019, 9:37 a.m., Meng Zhu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70735/ > ----------------------------------------------------------- > > (Updated May 29, 2019, 9:37 a.m.) > > > Review request for mesos, Andrei Sekretenko and Benjamin Mahler. > > > Bugs: MESOS-8456 > https://issues.apache.org/jira/browse/MESOS-8456 > > > Repository: mesos > > > Description > ------- > > This patch also makes `ResourceLimits` a friend class of > `ResourceQuantities` to achieve one-pass operation complexities. > > Also added unit test. > > > Diffs > ----- > > src/common/resource_quantities.hpp 21f08c6114de4f8459c0a5f7e25d7b6bba0f060b > src/common/resource_quantities.cpp eb741e36669da4ebd1936c1d9caf7266b6105dbb > src/tests/resource_quantities_tests.cpp > ebe53be4129cceb4429c45d098a718987f557d3a > > > Diff: https://reviews.apache.org/r/70735/diff/2/ > > > Testing > ------- > > make check > > > Thanks, > > Meng Zhu > >
