----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69811/#review212233 -----------------------------------------------------------
This looks in general like a great simplification, but it does at least superficually look like this is more that a (non-functional) refactoring; we now do pull some internal state before making some calls, but the actual call to the plugin is deferred there. I.e., before we had ```` handle = getService; THEN request = ..internal state.. send(handle, request) ```` while now we could have ```` request = .. internal state.. handle = call; THEN send(handle, request) ```` where the state we depend on for the request could change (e.g., from concurrently executing calls) between us being able to obtain a handle with `call` and actually sending the request with `_call`. Is there anything guaranteeing that this is safe (e.g., some general idempotency guarantee or sim)? src/resource_provider/storage/provider.cpp Lines 395 (patched) <https://reviews.apache.org/r/69811/#comment297955> We don't seem to need to disable this for `PROBE`. If we need it let's at least add comment for context. src/resource_provider/storage/provider.cpp Lines 396 (patched) <https://reviews.apache.org/r/69811/#comment297953> I don't think we _need_ this to be `inline`. src/resource_provider/storage/provider.cpp Lines 2315 (patched) <https://reviews.apache.org/r/69811/#comment297966> We could move this to the block just before it is used. src/resource_provider/storage/provider.cpp Lines 2371 (patched) <https://reviews.apache.org/r/69811/#comment297962> Can move this down. src/resource_provider/storage/provider.cpp Lines 2496 (patched) <https://reviews.apache.org/r/69811/#comment297963> Move down. - Benjamin Bannier On Jan. 23, 2019, 8:07 a.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69811/ > ----------------------------------------------------------- > > (Updated Jan. 23, 2019, 8:07 a.m.) > > > Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht. > > > Bugs: MESOS-9517 > https://issues.apache.org/jira/browse/MESOS-9517 > > > Repository: mesos > > > Description > ------- > > This patch refactors the `StorageLocalResourceProvider::call` function > to obtain the latest service future through `getService` before making > the actual RPC call. The subsequent patch would utilize this to support > RPC retry across plugin restarts. > > > Diffs > ----- > > src/resource_provider/storage/provider.cpp > d6e20a549ede189c757ae3ae922ab7cb86d2be2c > > > Diff: https://reviews.apache.org/r/69811/diff/1/ > > > Testing > ------- > > make check > > > Thanks, > > Chun-Hung Hsiao > >
