----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34910/#review87465 -----------------------------------------------------------
Ship it! src/master/validation.hpp <https://reviews.apache.org/r/34910/#comment139777> Can you instead use namespace task::internal? See my comments below. src/master/validation.cpp <https://reviews.apache.org/r/34910/#comment139781> This looks wiered. Can we instead put all the validateXXX function under task::internal namespace? You can keep validateResources close to validateResourceUsage as it is right now. And this code block will look more consistent: ``` lambda::bind(internal::validateTaskID, ...), lambda::bind(internal::validateUnique...), ... ``` src/master/validation.cpp <https://reviews.apache.org/r/34910/#comment139782> See my above comments. You may want to move this close to where all the task validation functions are located. src/master/validation.cpp <https://reviews.apache.org/r/34910/#comment139786> Is this check redundent? Can you calculate the total first and do one check instead? src/master/validation.cpp <https://reviews.apache.org/r/34910/#comment139790> I would rather swap the order of these two checks: ``` if (!resources.revocable.empty() && resources.revocable() != resources) ``` I found the above more easy to read:) - Jie Yu On June 10, 2015, 7:10 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34910/ > ----------------------------------------------------------- > > (Updated June 10, 2015, 7:10 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.hpp a74e844b39595deb9d907cc8ab1b8aee622a8765 > src/master/validation.cpp 1793b0e16fc36ead147880ee06cda40b23fa0962 > src/slave/slave.cpp 98036b2d5f2c765aef4a416c3cbc082df77ab3ac > src/tests/master_validation_tests.cpp > dc9e91e120c2af9e72013557730f6a2fbb5b00fe > > Diff: https://reviews.apache.org/r/34910/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
