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



src/tests/hierarchical_allocator_tests.cpp (lines 1918 - 1919)
<https://reviews.apache.org/r/40632/#comment176206>

    Note: This separation comment is probably unnecessary.  As long as the new 
tests are named properly (i.e. AllocationSlack...) we can tell them apart quite 
easily.



src/tests/hierarchical_allocator_tests.cpp (line 1922)
<https://reviews.apache.org/r/40632/#comment176207>

    s/resources on a slave that are statically reserved/statically reserved 
resources/



src/tests/hierarchical_allocator_tests.cpp (lines 1924 - 1925)
<https://reviews.apache.org/r/40632/#comment176211>

    s/to any role and/to any role, and/
    
    s/for a role are only offered to frameworks in that role/are still only 
allocated to the reserved role/



src/tests/hierarchical_allocator_tests.cpp (lines 1952 - 1960)
<https://reviews.apache.org/r/40632/#comment176212>

    It would be great to have a comment above this :)



src/tests/hierarchical_allocator_tests.cpp (lines 1995 - 1997)
<https://reviews.apache.org/r/40632/#comment176198>

    Consider moving this test to after you've implemented optimistic offers for 
`Allocator::recoverResources`.



src/tests/hierarchical_allocator_tests.cpp (lines 2045 - 2047)
<https://reviews.apache.org/r/40632/#comment176199>

    Consider moving this test to after you've implemented optimistic offers for 
`Allocator::recoverResources`.



src/tests/hierarchical_allocator_tests.cpp (line 2089)
<https://reviews.apache.org/r/40632/#comment176213>

    s/was not count in Quota/can be allocated regardless of Quota/



src/tests/hierarchical_allocator_tests.cpp (line 2107)
<https://reviews.apache.org/r/40632/#comment176215>

    It may improve readability to split up that big comment.  Suggestion:
    
    Set quota for the quota'd role.  In the first allocation cycle, this 
effectively prevents `agent1` from being offered to any role except the quota'd 
role (as doing so would put the quota'd role under quota).



src/tests/hierarchical_allocator_tests.cpp (line 2111)
<https://reviews.apache.org/r/40632/#comment176218>

    Continued suggestion:
    
    Add a framework that can only be offered non-revocable resources from 
`agent2`.



src/tests/hierarchical_allocator_tests.cpp (lines 2116 - 2123)
<https://reviews.apache.org/r/40632/#comment176221>

    Continued suggestion:
    
    Check that `framework1` gets offered resources from `agent2` as well as 
allocation slack from `agent1`.


- Joseph Wu


On Jan. 19, 2016, 12:03 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40632/
> -----------------------------------------------------------
> 
> (Updated Jan. 19, 2016, 12:03 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
> 
> 
> Bugs: MESOS-4145
>     https://issues.apache.org/jira/browse/MESOS-4145
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Enabled oversubscribed resources for reservations in allocator.
> The allocator part including 5 patches:
> 1) https://reviews.apache.org/r/40632 Enabled oversubscribed resources for 
> reservations in allocator
> 2) https://reviews.apache.org/r/41847 Updated allocation slack when slave was 
> updated.
> 3) https://reviews.apache.org/r/41791 Updated allocation slack for dynamic 
> reserve (1/3).
> 4) https://reviews.apache.org/r/42113 Handle unreserve logic for dynamic 
> reservation (2/3).
> 5) https://reviews.apache.org/r/42194 Handle unreserve logic for dynamic 
> reservation (3/3).
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 48acde69b1a2f305b568a7e322a58708063dd30a 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9362dd306497ba01e0f387c3862456cdcac6f863 
> 
> Diff: https://reviews.apache.org/r/40632/diff/
> 
> 
> Testing
> -------
> 
> make
> make check
> GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="HierarchicalAllocatorTest.*" 
> --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Reply via email to