mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ben Mahler" <benjamin.mah...@gmail.com>
Subject Re: Review Request 34910: Added task validation for task using revocable resources while its executor does not.
Date Wed, 03 Jun 2015 18:08:16 GMT

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


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