----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68763/#review208891 -----------------------------------------------------------
Fix it, then Ship it! src/resource_provider/daemon.cpp Lines 129 (patched) <https://reviews.apache.org/r/68763/#comment293164> s/removed/removing/ ? src/resource_provider/daemon.cpp Lines 556 (patched) <https://reviews.apache.org/r/68763/#comment293183> It's very unfortunate that we have this SLRP specific code in the general LRP daemon. I'd add a very big TODO here about the proposed solution for refactoring. I think it's possible that we can intorduce a virtual method for LRP interface (e.g., `shutdown`). If the RP instance is not yet available, the `remove` will simply skip the `shutdown` step. Since the launch of an RP is async (due to `generateAuthToken(data.info)`), as a result, we need to be careful there. We can either fail the launch by checking the state in `_launch`, or chain remove under launch. Sounds like the former is easier. Anyway, let's record a big TODO here. src/resource_provider/daemon.cpp Lines 686-688 (patched) <https://reviews.apache.org/r/68763/#comment293184> Can we LOG all errors in the `futures` list, not just the first one? You need to combine the error messages. - Jie Yu On Sept. 21, 2018, 7:47 p.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68763/ > ----------------------------------------------------------- > > (Updated Sept. 21, 2018, 7:47 p.m.) > > > Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht. > > > Bugs: MESOS-9228 > https://issues.apache.org/jira/browse/MESOS-9228 > > > Repository: mesos > > > Description > ------- > > When processing `REMOVE_RESOURCE_PROVIDER_CONFIG`, the local resource > provider daemon now performs a best-effort cleanup by killing all > standalone containers prefixed by the 'cid_prefix' of the resource > provider. During the cleanup, no resource provider config with the same > type and name can be added. > > > Diffs > ----- > > src/resource_provider/daemon.cpp 0a76cc6d1d34413674d1af1aa514679a4d2b7b55 > > > Diff: https://reviews.apache.org/r/68763/diff/3/ > > > Testing > ------- > > make check > > > Thanks, > > Chun-Hung Hsiao > >
