----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68763/#review208846 -----------------------------------------------------------
src/resource_provider/daemon.cpp Lines 426-427 (original), 478-481 (patched) <https://reviews.apache.org/r/68763/#comment293083> Wouldn't this be a situation where we'd like to respond with a `Failure`? src/resource_provider/daemon.cpp Lines 431-433 (original), 483-485 (patched) <https://reviews.apache.org/r/68763/#comment293080> How does this hold? It seems we should e.g., insert a `remove` call before continuing with `_launch`. src/resource_provider/daemon.cpp Lines 505-508 (patched) <https://reviews.apache.org/r/68763/#comment293082> Wouldn't this be a situation where we'd like to respond with a `Failure`? src/resource_provider/daemon.cpp Lines 566 (patched) <https://reviews.apache.org/r/68763/#comment293078> Let's assert here instead of letting the exception propagate. src/resource_provider/daemon.cpp Lines 626 (patched) <https://reviews.apache.org/r/68763/#comment293079> Nit: we could use an early `continue` here to reduce nesting, ``` if (!strings::startsWith(containerId.value(), cidPrefix)) { continue; } ``` - Benjamin Bannier On Sept. 21, 2018, 1:21 a.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, 1:21 a.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/2/ > > > Testing > ------- > > make check > > > Thanks, > > Chun-Hung Hsiao > >
