mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jiang Yan Xu <...@jxu.me>
Subject Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.
Date Mon, 01 Aug 2016 16:38:53 GMT

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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message