mesos-reviews mailing list archives

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


Fix it, then Ship it!





src/master/master.cpp
Line 4222 (original), 4222 (patched)
<https://reviews.apache.org/r/60283/#comment252914>

    remove "after validation"



src/master/master.cpp
Lines 4760 (patched)
<https://reviews.apache.org/r/60283/#comment252915>

    Per discussion, would be good to consider whether to validate/upgrade a task and its resources
in a single operation.



src/tests/master_tests.cpp
Lines 7130 (patched)
<https://reviews.apache.org/r/60283/#comment252916>

    `using` for `google::protobuf::RepeatedPtrField` ?



src/tests/master_tests.cpp
Lines 7132 (patched)
<https://reviews.apache.org/r/60283/#comment252917>

    Can we add a comment here?
    
    ```
    // If reservation refinement is enabled, inbound
    // resources should already be in the "post-refinement" format and should not need to
be upgraded.
    ```



src/tests/master_tests.cpp
Lines 7143 (patched)
<https://reviews.apache.org/r/60283/#comment252924>

    Comment would be helpful here also.



src/tests/master_tests.cpp
Lines 7201 (patched)
<https://reviews.apache.org/r/60283/#comment252918>

    Can we remove the `url` stuff?



src/tests/master_tests.cpp
Lines 7249 (patched)
<https://reviews.apache.org/r/60283/#comment252919>

    Update or remove this comment.



src/tests/master_tests.cpp
Lines 7403 (patched)
<https://reviews.apache.org/r/60283/#comment252921>

    "a RESERVE"



src/tests/master_tests.cpp
Lines 7406 (patched)
<https://reviews.apache.org/r/60283/#comment252920>

    "an UNRESERVE"



src/tests/master_tests.cpp
Lines 7407 (patched)
<https://reviews.apache.org/r/60283/#comment252922>

    "we test"


- Neil Conway


On June 21, 2017, 7:08 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60283/
> -----------------------------------------------------------
> 
> (Updated June 21, 2017, 7:08 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Initially, it seemed like calling `convertResourceFormat` after
> operation validation seemed safe since the operation validation
> themselves performed `Resources::validate` within them.
> 
> However, the rest of the operation validation code relies on
> the fact that the resources have been validated, and uses
> functions such as `isDynamicallyReserved`. Since functions such as
> `isDynamicallyReserved` now requires "post-reservation-refinement"
> format, we must perform this conversion earlier.
> 
> In this patch, we use `upgradeResources` to perform resources
> validation __and__ convert the resources before going into the
> operation and task validation.
> 
> We really need a better plan for this going forward. MESOS-7702.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp ec594a8f4fa95e77fc38103c5561d1797fe2b133 
>   src/tests/master_tests.cpp 652f37ab6f640ac87c48e419680501b705e11394 
>   src/tests/resource_offers_tests.cpp c2bbf834c1d46079af492887b9dd40e57f3f2ac7 
> 
> 
> Diff: https://reviews.apache.org/r/60283/diff/8/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Park
> 
>