----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71151/#review217143 -----------------------------------------------------------
Fix it, then Ship it! src/resource_provider/storage/provider.cpp Line 735 (original), 796 (patched) <https://reviews.apache.org/r/71151/#comment304408> Let's check that the `reconciled` future is pending at the very beginning of this function, similar to what we do in `reconcileStoragePools`, because `getExistingVolumes` and `getStoragePools` must be called after a reconciliation starts. src/resource_provider/storage/provider.cpp Line 752 (original), 811 (patched) <https://reviews.apache.org/r/71151/#comment304409> As commented in the previous patch, since we're having two functions, `reconcileStoragePools` and `reconcileResources`, it's alright to have a little bit of repeated code and don't introduce an extra function without a very focused purpose. In `reconcileResources`, we could have the logic that checks for `alwaysUpdate`. In `reconcileStoragePools`, since it's only called when profiles are updated or volumes of a stale profile are deleted, there's no need for the extra `alwaysUpdate` logic there. src/tests/storage_local_resource_provider_tests.cpp Lines 6743 (patched) <https://reviews.apache.org/r/71151/#comment304411> These are kinda ugly. Probably should find some time to refactor them in the future :) src/tests/storage_local_resource_provider_tests.cpp Lines 6871 (patched) <https://reviews.apache.org/r/71151/#comment304412> Let's do a `Clock::settle()` before `Clock::advance()` to ensure that the timer has been set up. src/tests/storage_local_resource_provider_tests.cpp Lines 6881 (patched) <https://reviews.apache.org/r/71151/#comment304413> Ditto. - Chun-Hung Hsiao On Aug. 6, 2019, 2:27 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71151/ > ----------------------------------------------------------- > > (Updated Aug. 6, 2019, 2:27 p.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/4/ > > > Testing > ------- > > `make check` > > > Thanks, > > Benjamin Bannier > >
