----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45959/#review127964 -----------------------------------------------------------
include/mesos/resources.hpp (line 64) <https://reviews.apache.org/r/45959/#comment191358> s/regular/non-shareable We often user `regular` resources as `non revocable` resources, so here it is better use `non-shareable` to make it more clear include/mesos/v1/resources.hpp (lines 466 - 467) <https://reviews.apache.org/r/45959/#comment191359> adjust the comment to make it less jagged. // The add and subtract operators on Resource_ is only allowed // from within Resources class, so we hide these operators. src/common/resources.cpp (lines 296 - 304) <https://reviews.apache.org/r/45959/#comment191360> Why not return false if the resource type is not `SCALAR` because only `SCALAR` was supportted for now src/common/resources.cpp (line 892) <https://reviews.apache.org/r/45959/#comment191361> So for such case, do you mean that a shareable resource with 3 cpus can contain a shareable resource with 5 cpus? src/common/resources.cpp (lines 1282 - 1283) <https://reviews.apache.org/r/45959/#comment191373> blank line here src/common/resources.cpp (lines 1430 - 1431) <https://reviews.apache.org/r/45959/#comment191362> void Resources::_addConsumers( const Resource& resource, Option<int> num_consumers) src/common/resources.cpp (lines 1474 - 1476) <https://reviews.apache.org/r/45959/#comment191363> void Resources::_replaceConsumers( const Resource& from, const Resource& to, Option<int> consumers) src/tests/resources_tests.cpp (line 2409) <https://reviews.apache.org/r/45959/#comment191374> Can you please add more test cases for two resources with consumer greater than 1? Ditto for the subtracble case. src/tests/resources_tests.cpp (lines 2423 - 2424) <https://reviews.apache.org/r/45959/#comment191364> EXPECT_SOME_EQ(0, r1.getConsumerCount(disk1)); This can cover Line 2423 and Line 2424 Ditto for the following in this test: Combine `EXPECT_SOME` and `EXPECT_EQ` to `EXPECT_SOME_EQ` src/tests/resources_tests.cpp (lines 2437 - 2438) <https://reviews.apache.org/r/45959/#comment191365> EXPECT_SOME_EQ(0, r2.getConsumerCount(disk2)); src/tests/resources_tests.cpp (lines 2449 - 2450) <https://reviews.apache.org/r/45959/#comment191366> EXPECT_SOME_EQ(1, sum.getConsumerCount(disk1)); src/tests/resources_tests.cpp (lines 2453 - 2454) <https://reviews.apache.org/r/45959/#comment191367> EXPECT_SOME_EQ(1, sum.getConsumerCount(disk2)); src/tests/resources_tests.cpp (lines 2466 - 2467) <https://reviews.apache.org/r/45959/#comment191368> EXPECT_SOME_EQ(1, r.getConsumerCount(disk1)); src/tests/resources_tests.cpp (lines 2470 - 2471) <https://reviews.apache.org/r/45959/#comment191369> EXPECT_EQ(1, r.getConsumerCount(disk2)); src/tests/resources_tests.cpp (lines 2501 - 2502) <https://reviews.apache.org/r/45959/#comment191370> EXPECT_SOME_EQ(1, r1.getConsumerCount(disk1)); - Guangya Liu On 四月 8, 2016, 11:16 p.m., Anindya Sinha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45959/ > ----------------------------------------------------------- > > (Updated 四月 8, 2016, 11:16 p.m.) > > > Review request for mesos and Jiang Yan Xu. > > > Bugs: MESOS-4892 > https://issues.apache.org/jira/browse/MESOS-4892 > > > Repository: mesos > > > Description > ------- > > A new struct Resoure_ is added to keep track of Resource and its > consumer count. As a result, Resources maintains a single container > for all resources. Private operators for addition and subtraction > to/from Resource_ have been added. > > All resources have consumer counts that is tracked within Resources. For > resource addition and subtraction, the consumer counts are adjusted for > shared resources as follows: > a) Addition: If shared resource is absent from original, then the > resource is added with a consumer count of 0. Otherwise, the > consumer count for the shared resource is incremented by 1. > b) Subtraction: If shared resource's consumer count is already 0, then > the shared resource is removed from the original. Otherwise, its > consumer count is decremented by 1. > > > Diffs > ----- > > include/mesos/resources.hpp a557e97c65194d4aad879fb88d8edefd1c95b8d8 > include/mesos/v1/resources.hpp a5ba8fec4c9c3643646308f75a4b28cefe0b3df3 > src/cli/execute.cpp df93e92252addaa794898ba95604eb2f91284b87 > src/common/resources.cpp f6ff92b591c15bc8e93fd85e1896349c3a7bb968 > src/examples/no_executor_framework.cpp > f578edfd99f3b7adf19cf06eab20696532c7b67d > src/examples/persistent_volume_framework.cpp > b4faa0ee25dc3a72c17ef2b0640a3695423ef79a > src/examples/test_framework.cpp 79113fbe47fda0912f0b01dc10429495a96ba8b8 > src/examples/test_http_framework.cpp > cba520e326ff8b0b4ed36a0f4cea6879b57f400c > src/hook/manager.cpp 17a42f8362f58f0857acabeb2c3113354589fa1b > src/master/http.cpp f781fd0102c247b2e77a71f7be82b872b0831681 > src/master/master.cpp 781402c04fded159183e1ca28894e48355200f0c > src/master/validation.cpp 504cd9b8bd5d40bb591b7aa5a23bd74cc210c2fc > src/slave/containerizer/containerizer.cpp > d0cae79834e451594d7675f00c5f7d2d2cd3a264 > src/slave/containerizer/external_containerizer.cpp > cf4384cce44172a028c890f52f71ceb8ae109383 > src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3 > src/slave/state.hpp 0de2a4ee4fabaad612c4526166157b001c380bdb > src/tests/containerizer/docker_containerizer_tests.cpp > 7accd32fba5eed196a82b1a171cb16d37b9e0539 > src/tests/containerizer/isolator_tests.cpp > 7b4d47bd9e99b71269093d7c11559f3b74a3e22b > src/tests/containerizer/port_mapping_tests.cpp > 21ad1e1c53316a3bb6d914aa228ccf3658acdfbf > src/tests/containerizer/runtime_isolator_tests.cpp > a11a3ffb1df1c5bb760041125c83b7b66d44300b > src/tests/fetcher_cache_tests.cpp 9ffcd2375f1203bd3d7c5d0cc898e955d5cb124e > src/tests/hierarchical_allocator_tests.cpp > a5dd57a4e0c244fb099433eb7b5777982698ebfd > src/tests/master_quota_tests.cpp e4a65bf09c8fdd2d6b6161042c3702a8cc4cd454 > src/tests/master_tests.cpp a5b21d3d60f944fd52ceacb4bbbad2613f384db7 > src/tests/master_validation_tests.cpp > 8a5bf9477596f13b2fb3a1348337ad2fe53a034d > src/tests/mesos.hpp 20370a277d55efeea8daae7ea5e2f6575b5a2d62 > src/tests/oversubscription_tests.cpp > 23671746da2ac505d75bc2bd59114697d9161d52 > src/tests/persistent_volume_endpoints_tests.cpp > 9b8ad34469c0c9a986aa60f3a52584a3a9eabb2b > src/tests/persistent_volume_tests.cpp > d246f35046fff469b847c908de2b305ae629212f > src/tests/registrar_tests.cpp f18e8030f69d8ebf8de81ff03632106e08823df1 > src/tests/reservation_endpoints_tests.cpp > 2e0f6c1aba95d918b8c42219ee97f79f1070d56e > src/tests/resources_tests.cpp dc12bd8f1e2da6972bc8aed598811c55d664036e > src/tests/role_tests.cpp 24959d6e0f83ef7b62b0586be18661aa3cac91dd > src/tests/slave_recovery_tests.cpp 79132344be3bcd2bda54357cd5e7e0c59a766fd8 > src/tests/slave_tests.cpp 4a576b98d1cc58072626ac2c41c599bd3c8385c5 > src/v1/resources.cpp 8c3f2d1c1529915a59d47fe37bb3fc7a3267079a > > 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 > >
