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


Can we make the test a unit test? Looks like we could pull up 
'`validateResources`' to make this unit-testable? Chatting with Jie, we should 
probably place it in an `'internal::task'` namespace towards the bottom of the 
header, since it's only for testing purposes and we want to make it clear which 
validation functions are expected to be used by the master.


src/master/validation.cpp
<https://reviews.apache.org/r/34910/#comment138437>

    Are we otherwise allowing mixing within a resource? Looking at the existing 
cpushare.cpp logic, it seems that we have a binary decision:
    
      (1) If any of the cpus are revocable, *all* cpus are treated revocable 
for isolation purposes.
      (2) Otherwise, non-revocable.
    
    Is the idea here that the mixing that (1) allows enables a framework to 
reduce the "risk" of revocation?
    
    Maybe a TODO here and/or on the cpushare code since it's not that difficult 
to change the cpushare implementation to allow transitioning between revocable 
and non-revocable after the container is launched?



src/tests/master_validation_tests.cpp
<https://reviews.apache.org/r/34910/#comment138453>

    From this comment, I expected this test to be testing that a task with 
revocable resources is allowed when the executor has revocable resources, but 
it's only testing the opposite condition.
    
    The unit test would make it easier to test both cases, yes?



src/tests/master_validation_tests.cpp
<https://reviews.apache.org/r/34910/#comment138443>

    its :)



src/tests/master_validation_tests.cpp
<https://reviews.apache.org/r/34910/#comment138439>

    I find it nice to reduce "jaggedness" on comments, e.g.:
    
    ```
      // Create a task that uses revocable resources
      // while it's executor does not.
    ```


- Ben Mahler


On June 1, 2015, 11:11 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34910/
> -----------------------------------------------------------
> 
> (Updated June 1, 2015, 11:11 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, Jie Yu, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-2753
>     https://issues.apache.org/jira/browse/MESOS-2753
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Enforces the invariant that a task cannot use revocable resources unless its 
> executor does.
> 
> 
> Diffs
> -----
> 
>   src/master/validation.cpp 1793b0e16fc36ead147880ee06cda40b23fa0962 
>   src/tests/master_validation_tests.cpp 
> dc9e91e120c2af9e72013557730f6a2fbb5b00fe 
> 
> Diff: https://reviews.apache.org/r/34910/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Reply via email to