----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61183/#review185932 -----------------------------------------------------------
See comments below. I presume you won't need to update the oversubscription tests if you take my suggestion below? Thoughts? src/slave/slave.cpp Lines 1272-1284 (patched) <https://reviews.apache.org/r/61183/#comment262262> Can you add a comment here saying that we need to send the message here because some local resource provider might register with the resource provider manager while agent is registering with the master. Also, I realize another issue: what if the new agent send this message to an old master? will the old master remove all the oversubscribed resources (thus unnecssarily rescind offers? Maybe we do need to solve the `TODO` here? That means probably we need to make `UpdateSlaveMessage::Type` a mask, rather than an enum so that we can update both total and oversubscribed resources in one message? ``` message UpdateSlaveMessage { message ResourceCategories { optional boolean total = 1; optional boolean oversubscribed = 2; } // NOTE: If not set, means update oversubscribed resources. optional ResourceCategories resource_categories = 3; repeated Resource oversubscribed_resources = 2; repeated Resource total_resources = 4; } ``` src/slave/slave.cpp Lines 1366 (patched) <https://reviews.apache.org/r/61183/#comment262263> Ditto on adding some comments here. - Jie Yu On Sept. 21, 2017, 7:24 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61183/ > ----------------------------------------------------------- > > (Updated Sept. 21, 2017, 7:24 p.m.) > > > Review request for mesos, Jie Yu and Jan Schlicht. > > > Repository: mesos > > > Description > ------- > > The agent's resource provider manager sends a > 'ResourceProviderMessage' when its managed resources change. This > commit adds handling in the agent so that an 'UpdateSlaveMessage' is > sent to the master to update the total resource available on the > agent. > > In order to provide push-like handling of the resource provider > manager's message queue, we chain recursive calls to the handler for > continuous processing. Initially, processing is kicked off from > 'Slave::initialize'. In this simple implementation we e.g., provide no > direct way to stop processing of messages, yet, but it can be achieved > by e.g., replacing the manager with a new instance (this would also > require updating routes). > > Since the agent can only send an 'UpdateSlaveMessage' when it is > registered with a master, a simple back-off of 5 s is implemented which > will defer processing of a ready message should the agent not yet have > registered. > > To facilitate logging we add a stringification function for > 'ResourceProviderMessage's. > > This patch also adjusts a number of tests to no expect two > 'UpdateSlaveMessage's. > > > Diffs > ----- > > src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 > src/slave/slave.hpp aea1e948209c7c8945665915bc2f6d8eb47814ef > src/slave/slave.cpp 01020a624bdd67ee6371f607d897ad71a2cdcc82 > src/tests/oversubscription_tests.cpp > 02b10d6689dd6f01510cd2d5db2bb76b4b190eca > src/tests/slave_tests.cpp e9bcfefd52e1e99a7a5877b2e7c30af958ca1723 > > > Diff: https://reviews.apache.org/r/61183/diff/11/ > > > Testing > ------- > > Tested on a number of platforms on internal CI. > > > Thanks, > > Benjamin Bannier > >
