> On Aug. 15, 2018, 10:15 p.m., Chun-Hung Hsiao wrote: > > src/resource_provider/manager.cpp > > Lines 611-612 (patched) > > <https://reviews.apache.org/r/68144/diff/2/?file=2072997#file2072997line611> > > > > These should be done in the agent since it knows about the task > > resources.
I implemented this in https://reviews.apache.org/r/68147/. > On Aug. 15, 2018, 10:15 p.m., Chun-Hung Hsiao wrote: > > src/resource_provider/manager.cpp > > Lines 617 (patched) > > <https://reviews.apache.org/r/68144/diff/2/?file=2072997#file2072997line617> > > > > Since we log the removing attempt above, should we also log the result > > as well? Or do you prefer leaving the logging to the caller? We currently never log success scenarios. I'd leave this as is. We can always come back and add more debugging statements later, but I suspect that would not be _very_ useful as the code here usually does not involve long continuation chains. Dropping. > On Aug. 15, 2018, 10:15 p.m., Chun-Hung Hsiao wrote: > > src/resource_provider/manager.cpp > > Lines 625-626 (patched) > > <https://reviews.apache.org/r/68144/diff/2/?file=2072997#file2072997line625> > > > > Just for consistency. Anaig, no strong preference. > > ``` > > ResourceProviderMessage::Remove remove{resourceProviderId}; > > ``` Fixed. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68144/#review207343 ----------------------------------------------------------- On Aug. 16, 2018, 4:31 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68144/ > ----------------------------------------------------------- > > (Updated Aug. 16, 2018, 4:31 p.m.) > > > Review request for mesos, Chun-Hung Hsiao and Jan Schlicht. > > > Bugs: MESOS-8403 > https://issues.apache.org/jira/browse/MESOS-8403 > > > Repository: mesos > > > Description > ------- > > This patch adds a method to remove a resource provider from the > resource provider manager. The resource provider will be marked as > removed in the manager's registry and disconnected. We also expose a > new `REMOVE` event whenever a resource provider was removed. > > This patch does not add integration with e.g., the agent. > > > Diffs > ----- > > src/resource_provider/manager.hpp 6c57956f0ec51820bfbe78e7fa213af2352b5a7f > src/resource_provider/manager.cpp abd7e38e5517ea600f9fc9b8a96c7d0d26df0620 > src/resource_provider/message.hpp 9307f8859035dfafe0952e9026b078b44121537e > src/slave/slave.cpp 78e8666f402be58af5b6e20a715da4998af2615c > src/tests/resource_provider_manager_tests.cpp > 0b9e985cf3b4ded8590615b418e52a2a11f1c1aa > > > Diff: https://reviews.apache.org/r/68144/diff/3/ > > > Testing > ------- > > `make check` > > Additional testing with the test case added in > https://reviews.apache.org/r/68147/. > > > Thanks, > > Benjamin Bannier > >
