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



src/master/master.cpp (line 3048)
<https://reviews.apache.org/r/42086/#comment174976>

    This comment seems not consistent with the code below. In the code, if 
there are some errors when validating regular offers, we will still go ahead to 
handle inverse offers, but the comment here says we will reject the entire set 
of offers.



src/master/master.cpp (line 3067)
<https://reviews.apache.org/r/42086/#comment174978>

    So we handle inverse offers even some of them are invalid?



src/master/master.cpp (line 3093)
<https://reviews.apache.org/r/42086/#comment174975>

    Do we really care about ```!inverseOfferIds.empty()``` here? I think only 
checking ```offerIds.empty()``` should be enough.



src/master/master.cpp (line 3132)
<https://reviews.apache.org/r/42086/#comment174984>

    Should be offerError.get().message?


- Qian Zhang


On Jan. 13, 2016, 6:20 a.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42086/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2016, 6:20 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4301
>     https://issues.apache.org/jira/browse/MESOS-4301
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Adds `validation::offer::inverse::validate` to validate inverse offers along 
> the same lines as `validation::offer::validate`, except `SlaveId` is not 
> validated for inverse offers.
> 
> Fixes and refactors `Master::accept` to allow `ACCEPT` calls that contain 
> both offers and inverse offers.
> Also tweaks `Master::accept` to not print a misleading log line "ACCEPT call 
> used invalid offers ..." when the call only includes inverse offers.
> 
> 
> Diffs
> -----
> 
>   include/mesos/type_utils.hpp efe2b1de0c277db62d7f7cc5ff1cd9143b9f632a 
>   src/common/type_utils.cpp 76f48f6a1f5467db032ded8acd296d03353b4172 
>   src/master/master.hpp f02d165874fa8023675e545793de699aeecae29b 
>   src/master/master.cpp 5268408fc63a28afabc27cba96d3ecb360608a65 
>   src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
> 
> Diff: https://reviews.apache.org/r/42086/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> # Ran the following and checked for blank output:
> bin/mesos-tests.sh --gtest_filter="*Inverse*Offer*" --verbose 2>&1 /dev/null 
> |  grep "ACCEPT call used invalid offers"
> 
> # Check new test for flakiness:
> bin/mesos-tests.sh --gtest_filter="*OffersAndInverseOffers" 
> --gtest_repeat=1500 --gtest_break_on_failure
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Reply via email to