----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69338/#review211106 -----------------------------------------------------------
src/slave/slave.cpp Line 7595 (original), 7598 (patched) <https://reviews.apache.org/r/69338/#comment296035> It seems to me that the compiler should be able to hoist `message.mutable_resource_providers()` outside the loop so there is no need to do it manually. But I'm fine with this change. src/slave/slave.cpp Lines 7873-7874 (original), 7875-7876 (patched) <https://reviews.apache.org/r/69338/#comment296036> These comments need to be updated. I was wondering now that, since we already have this empty resource logic here, why do we still see stale resource in the master? Is it because that in the case of agent restart, no `DISCONNECT` message is triggered, and thus we never clean up the resources? It seems to me that we have the original logic in place for a reason. Is it because we want the master to cache the resource provider infos without any resource for a reason? If yes, we may want to revisit this and think about if we want to erase it, or keep the original behavior but just clean up the resources/operations in the master. - Chun-Hung Hsiao On Nov. 14, 2018, 2:36 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69338/ > ----------------------------------------------------------- > > (Updated Nov. 14, 2018, 2:36 p.m.) > > > Review request for mesos, Chun-Hung Hsiao and Jan Schlicht. > > > Bugs: MESOS-9384 > https://issues.apache.org/jira/browse/MESOS-9384 > > > Repository: mesos > > > Description > ------- > > This patch changes the agent to not report disconnected resource > providers in the `UpdateSlaveMessage` it emits. We also fix how it > generates resource provider-related updates when no resource providers > are connected to allow the master to react properly when a resource > provider disappears. > > > Diffs > ----- > > src/slave/slave.cpp 74f6fb9036a9ac4f587f53ec2df04eeb4c167bfb > src/tests/resource_provider_manager_tests.cpp > 5bb740edc7c3cc8698aede4f2ed57c21232fe378 > > > Diff: https://reviews.apache.org/r/69338/diff/1/ > > > Testing > ------- > > `make check` > > > Thanks, > > Benjamin Bannier > >
