> On Jan. 4, 2019, 1:47 p.m., Benjamin Mahler wrote: > > src/Makefile.am > > Lines 1022-1023 (patched) > > <https://reviews.apache.org/r/69599/diff/3/?file=2117686#file2117686line1022> > > > > Tabs are not aligned?
Fixed. > On Jan. 4, 2019, 1:47 p.m., Benjamin Mahler wrote: > > src/common/resource_quantities.hpp > > Lines 38 (patched) > > <https://reviews.apache.org/r/69599/diff/3/?file=2117687#file2117687line38> > > > > The finite >=0 part isn't true anymore right? Removed. > On Jan. 4, 2019, 1:47 p.m., Benjamin Mahler wrote: > > src/common/resource_quantities.hpp > > Lines 43 (patched) > > <https://reviews.apache.org/r/69599/diff/3/?file=2117687#file2117687line43> > > > > "native" isn't clear here Clarified > On Jan. 4, 2019, 1:47 p.m., Benjamin Mahler wrote: > > src/common/resource_quantities.hpp > > Lines 49-54 (patched) > > <https://reviews.apache.org/r/69599/diff/3/?file=2117687#file2117687line49> > > > > "native" isn't clear here > > > > How about: > > > > ``` > > // An alternative design for this class was to provide addition and > > // subtraction functions as opposed to a map interface. However, this > > // leads to some un-obvious semantics around presence of zero values > > // (will zero entries be automatically removed?) and handling of > > negatives > > // (will negatives trigger a crash? will they be converted to zero? Note > > // that Value::Scalar should be non-negative but there is currently no > > // enforcement of this by Value::Scalar arithmetic operations; we > > probably > > // want all Value::Scalar operations to go through the same negative > > // handling). To avoid the confusion, we provided a map like interface > > // which produces clear semantics: insertion works like maps, and the > > // user is responsible for performing arithmetic operations on the > > values > > // (using the already provided Value::Scalar arithmetic overloads). > > ``` Done. Thanks. > On Jan. 4, 2019, 1:47 p.m., Benjamin Mahler wrote: > > src/common/resource_quantities.hpp > > Lines 83-88 (patched) > > <https://reviews.apache.org/r/69599/diff/3/?file=2117687#file2117687line83> > > > > Put this in the .cpp? Done. > On Jan. 4, 2019, 1:47 p.m., Benjamin Mahler wrote: > > src/common/resource_quantities.hpp > > Lines 103-117 (patched) > > <https://reviews.apache.org/r/69599/diff/3/?file=2117687#file2117687line103> > > > > These are a bit verbose for the header, put them in the .cpp? Done. > On Jan. 4, 2019, 1:47 p.m., Benjamin Mahler wrote: > > src/common/resource_quantities.cpp > > Lines 39-69 (patched) > > <https://reviews.apache.org/r/69599/diff/3/?file=2117688#file2117688line39> > > > > Let's make it more consistent with the Resources::fromSimpleString code > > and write down in a comment that it's what this is based on? Done. Commented in the header. > On Jan. 4, 2019, 1:47 p.m., Benjamin Mahler wrote: > > src/master/allocator/sorter/drf/sorter.cpp > > Lines 661-663 (original), 661-662 (patched) > > <https://reviews.apache.org/r/69599/diff/3/?file=2117690#file2117690line661> > > > > Why the change to this line? Let's keep the diff small and relevant to > > this change only Reverted. - Meng ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69599/#review211690 ----------------------------------------------------------- On Jan. 5, 2019, 3:28 p.m., Meng Zhu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69599/ > ----------------------------------------------------------- > > (Updated Jan. 5, 2019, 3:28 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Repository: mesos > > > Description > ------- > > There are many places that we need to express the concept of > resource quantities such as enforcing allocation quantities > in the allocator and set guaranteed resource quantities with quota. > However, the current code usually uses the complex Resources > class which is unnecessary and inefficient. > > This patch pulls class ScalarResourceQuantities in sorter.hpp > up, aiming to use it for all resource quantity related usages. > We mostly preserve the map interface and added other utilities such > as parsing. > > > Diffs > ----- > > src/CMakeLists.txt bde070445b644e15d46c390d1c983caabaa1fec8 > src/Makefile.am 7a4904a3d67479267087fd2313a263d8218843fa > src/common/resource_quantities.hpp PRE-CREATION > src/common/resource_quantities.cpp PRE-CREATION > src/master/allocator/sorter/drf/sorter.hpp > 084df82baef91eca5775b0bd17d943f3fb8df70b > src/master/allocator/sorter/drf/sorter.cpp > a648fac7e922ab0aefdf9363624d7242f1fc6588 > src/master/allocator/sorter/random/sorter.hpp > 800b22c67126c2b14c5259d9d122d2e196cc80d8 > src/master/allocator/sorter/sorter.hpp > 68cf6988ef1a156cf16951e3164261234e9abeeb > > > Diff: https://reviews.apache.org/r/69599/diff/4/ > > > Testing > ------- > > make check > Dedicated test in r/69600 > > > Thanks, > > Meng Zhu > >
