----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63751/#review190963 -----------------------------------------------------------
src/slave/slave.cpp Line 3720 (original), 3731 (patched) <https://reviews.apache.org/r/63751/#comment268560> Let's add a CHECK here to test the operation is old operation. ``` CHECK(protobuf::isSpeculativeOperation(message.operation_info())); ``` src/slave/slave.cpp Line 3726 (original), 3737 (patched) <https://reviews.apache.org/r/63751/#comment268556> insert a line above src/slave/slave.cpp Lines 3744-3745 (patched) <https://reviews.apache.org/r/63751/#comment268545> Use `protobuf::createOfferOperationStatus`? src/slave/slave.cpp Lines 3747-3754 (patched) <https://reviews.apache.org/r/63751/#comment268546> Can you introduce a `protobuf::createOfferOperationStatusUpdate` helper? src/tests/reservation_tests.cpp Lines 2300-2303 (patched) <https://reviews.apache.org/r/63751/#comment268559> I feel that a better way to test this is to parameterize the existing `ReservationTest` to handle agents w/ or w/o `RESOURCE_PROVIDER` capabilities. You might need to adjust some tests that explicitly test `CheckpointResourcesMessage`. I feel that `SendingCheckpointResourcesMessage` is not necessary, we can probably just remove that test. We probably should do the same for `PersistentVolumeTest` For those test that wait on `FUTURE_PROTOBUF(CheckpointResourcesMessage, ...)`, we can probably change those to use `FUTURE_DISPATCH` - Jie Yu On Nov. 13, 2017, 2:21 p.m., Jan Schlicht wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63751/ > ----------------------------------------------------------- > > (Updated Nov. 13, 2017, 2:21 p.m.) > > > Review request for mesos, Benjamin Bannier and Jie Yu. > > > Bugs: MESOS-8211 > https://issues.apache.org/jira/browse/MESOS-8211 > > > Repository: mesos > > > Description > ------- > > Agents the have the 'RESOURCE_PROVIDER' capability set, should get sent > 'ApplyOfferOperationMessage' instead of 'CheckpointResourcesMessage'. > The agent will then figure out how to apply the operation. For agent > local resources the agent will checkpoint resources. > > > Diffs > ----- > > src/common/protobuf_utils.hpp 0ca4c6d689bf7d2c477174b039432ed7b6d0b650 > src/common/protobuf_utils.cpp 5739a63f8d87923c034375b88c4f0b3b19f4b521 > src/master/master.cpp 49dbaa979d692061a7b479b1db4511e8357b8baf > src/slave/slave.cpp 7cb6661b55fb5437a1ffc447f974076aadd1eced > src/tests/reservation_tests.cpp 470f7341686e69d0a71fb234a26b277c45c29780 > > > Diff: https://reviews.apache.org/r/63751/diff/2/ > > > Testing > ------- > > make check > > > Thanks, > > Jan Schlicht > >
