----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71151/#review217315 -----------------------------------------------------------
Fix it, then Ship it! src/resource_provider/storage/provider.cpp Lines 777 (patched) <https://reviews.apache.org/r/71151/#comment304613> Shouldn't `alwaysUpdate` be false here? src/resource_provider/storage/provider.cpp Line 753 (original), 820 (patched) <https://reviews.apache.org/r/71151/#comment304616> `resourceVersion` needs to be updated every time `UPDATE_STATE` is sent to avoid races. In the original implementation, since it is initialized to a random UUID in the constructor, we avoided assigning a new random value for the first `UPDATE_STATE` call. In this new codepath, it's possible that we update the random UUID for the first `UPDATE_STATE` call, which is okay, just that the initial random UUID is wasted. If we don't care about this small overhead, I'd suggest that we move this line into the `sendResourceProviderStateUpdate` function. src/resource_provider/storage/provider.cpp Lines 760-775 (original), 827-842 (patched) <https://reviews.apache.org/r/71151/#comment304614> These should be be removed. L741-753 already handled the transition. src/resource_provider/storage/provider.cpp Line 779 (original), 846 (patched) <https://reviews.apache.org/r/71151/#comment304615> This should be removed. L739 resumed the SUM. - Chun-Hung Hsiao On Aug. 19, 2019, 11:58 a.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71151/ > ----------------------------------------------------------- > > (Updated Aug. 19, 2019, 11:58 a.m.) > > > Review request for mesos and Chun-Hung Hsiao. > > > Bugs: MESOS-9254 > https://issues.apache.org/jira/browse/MESOS-9254 > > > Repository: mesos > > > Description > ------- > > Performed periodic storage local provider reconciliations. > > > Diffs > ----- > > src/resource_provider/storage/provider.cpp > 6d632606f411d3ca99d3573a57c9f68b02ba8072 > src/tests/storage_local_resource_provider_tests.cpp > 69b59d48ceefebbb7accefe411c54ac5cecff1c3 > > > Diff: https://reviews.apache.org/r/71151/diff/6/ > > > Testing > ------- > > `make check` > > > Thanks, > > Benjamin Bannier > >
