mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 58510: Added two tests for hierarchical reservation.
Date Fri, 09 Jun 2017 19:22:14 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58510/#review177509
-----------------------------------------------------------



Thanks for adding tests!


src/tests/hierarchical_allocator_tests.cpp
Lines 4598 (patched)
<https://reviews.apache.org/r/58510/#comment251104>

    "descendant roles"? Ditto for the test name



src/tests/hierarchical_allocator_tests.cpp
Lines 4606-4607 (patched)
<https://reviews.apache.org/r/58510/#comment251105>

    I was a bit confused about why this test was using quota, then I figured out that this
is how you wanted to ensure that the allocation goes to the descendant.
    
    We should document this at the top of the test, or here, to describe that we leverage
quota to test this.
    
    Simpler to understand would be to not use quota and add two agents, expecting one agent
to have been allocated to the ancestor and descendant, each. Does that not work?



src/tests/reservation_tests.cpp
Lines 2399-2401 (patched)
<https://reviews.apache.org/r/58510/#comment251106>

    These two tests are very similar, the only difference seems to be that one tests that
the reservations are allocated using "fair sharing" between the roles (or at least if you
removed quota, that would be the case), and the other just tests that it is allocatable to
the role?
    
    That seems like a reasonable split to ensure that there is a ReservationTest covering
this, but it's only accurate if you remove the quota from the first test (otherwise these
tests are essentially the same?)


- Benjamin Mahler


On April 18, 2017, 3:45 p.m., Jay Guo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58510/
> -----------------------------------------------------------
> 
> (Updated April 18, 2017, 3:45 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-7149
>     https://issues.apache.org/jira/browse/MESOS-7149
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added two tests for hierarchical reservation.
> 
> 
> Diffs
> -----
> 
>   src/tests/hierarchical_allocator_tests.cpp 33e7b455f8664858eb4f03727b076a10c80cd6e0

>   src/tests/reservation_tests.cpp 4504831d77c1bfcf5f2ddf6d28cd45dea2c421ad 
> 
> 
> Diff: https://reviews.apache.org/r/58510/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jay Guo
> 
>


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