----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/#review144059 -----------------------------------------------------------
Fix it, then Ship it! I'll commit with the following minor adjustments. include/mesos/resources.hpp (lines 460 - 461) <https://reviews.apache.org/r/45959/#comment210321> include/mesos/resources.hpp (lines 481 - 484) <https://reviews.apache.org/r/45959/#comment210322> Moved them to below the comments below as it applies to them as well. I don't think we actually need ``` void add(const Resource& r); void subtract(const Resource& r); ``` to be public to let's address them separately. src/tests/resources_tests.cpp (line 2564) <https://reviews.apache.org/r/45959/#comment210286> Moved the tests for shared resources to above the benchmarks so the unit tests and the benchmarks are clearly delimited. src/tests/resources_tests.cpp (line 2566) <https://reviews.apache.org/r/45959/#comment210316> Added a comment here and in similar places to explain the `true` argument. ``` // Shared persistent volume. ``` src/tests/resources_tests.cpp (line 2590) <https://reviews.apache.org/r/45959/#comment210253> s/same/the same/ src/tests/resources_tests.cpp (line 2616) <https://reviews.apache.org/r/45959/#comment210254> s/same/the same/ src/tests/resources_tests.cpp (line 2685) <https://reviews.apache.org/r/45959/#comment210289> s/differs/differ/ src/tests/resources_tests.cpp (lines 2689 - 2691) <https://reviews.apache.org/r/45959/#comment210317> Renamed disk1 and disk2 as `sharedDisk` and `nonSharedDisk` so their sharedness is more self-explanatory. src/tests/resources_tests.cpp (line 2753) <https://reviews.apache.org/r/45959/#comment210315> Pulled this test to the top of the shared resources tests as this is a basic test that establishes the validity of basic operations on shared resources. src/tests/resources_tests.cpp (line 2755) <https://reviews.apache.org/r/45959/#comment210314> Added ``` // The summation of identical shared resources is valid and // the result is reflected in the count. ``` - Jiang Yan Xu On July 29, 2016, 11:59 p.m., Anindya Sinha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45959/ > ----------------------------------------------------------- > > (Updated July 29, 2016, 11:59 p.m.) > > > Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and Jiang > Yan Xu. > > > Bugs: MESOS-4892 > https://issues.apache.org/jira/browse/MESOS-4892 > > > Repository: mesos > > > Description > ------- > > A new class Resource_ is added that allows 'Resources' to group > identical shared resource objects together into a single 'Resource_' > object and tracked by its shared count. Non-shared resource objects > are not grouped. > > For resource addition and subtraction, the shared count is adjusted for > shared resources as follows: > a) Addition: If shared resource is absent from original, then the > resource is added initialized with a consumer count of 1. Otherwise, > the share count for the shared resource is incremented. > b) Subtraction: If shared resource's share count is already 1, then > the shared resource is removed from the original. Otherwise, its > consumer count is decremented. > > Note that v1 changes for shared resources are in the next commit. > > > Diffs > ----- > > include/mesos/resources.hpp 6638c8f79e45c0ff3efa4342e30478d2e1e4740b > src/common/resources.cpp 468581da550bcabf44fbaba8897d5fbbc330c2cb > src/master/validation.cpp 52002beac29c371411348cb026a216e99ac96ab2 > src/tests/mesos.hpp 51c66f175c80ebacd5af230222ea7e4c81dfc1e8 > src/tests/resources_tests.cpp 4111e080b84079e100b731c9a56861b204f17388 > > Diff: https://reviews.apache.org/r/45959/diff/ > > > Testing > ------- > > New tests added to demonstrate arithmetic operations for shared resources > with consumer counts. > Tests successful. > > > Thanks, > > Anindya Sinha > >
