-----------------------------------------------------------
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
> 
>

Reply via email to