[5/5] mesos git commit: Rescinded offers possibly affected by updates to agent total resources.

[bb...@apache.org]

Rescinded offers possibly affected by updates to agent total resources.

When an agent changes its resources, the master should rescind any
offers affected by the change. We already performed the rescind for
updates to the agent's oversubscribed resources; this patch adds offer
rescinding when an update an agent's total resources is processed.

While for updates to an agent's oversubscribed resources we currently
only rescind offers containing revocable resources to e.g., reduce
offer churn, for updates to the total we here currently rescind all
offers for resources on the agent.

As an optimization, this patch adds logic to ignore redundant updates
to agent resources.

Review: https://reviews.apache.org/r/62158


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/d38fe9d5
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/d38fe9d5
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/d38fe9d5

Branch: refs/heads/master
Commit: d38fe9d5a4db0a37b876c55c99b547d4c8fbd8dd
Parents: a88469b
Author: Benjamin Bannier <bb...@gmail.com>
Authored: Thu Sep 7 16:09:10 2017 +0200
Committer: Benjamin Bannier <bb...@apache.org>
Committed: Thu Oct 5 16:50:43 2017 +0200

----------------------------------------------------------------------
 src/master/master.cpp | 49 ++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 41 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/d38fe9d5/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 169fee4..3603878 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -6820,27 +6820,60 @@ void Master::updateSlave(const UpdateSlaveMessage& message)
     newOversubscribed = oversubscribedResources;
   }
 
-  slave->totalResources =
+  const Resources newSlaveResources =
     newTotal.getOrElse(slave->totalResources.nonRevocable()) +
     newOversubscribed.getOrElse(slave->totalResources.revocable());
 
+  if (newSlaveResources == slave->totalResources) {
+    LOG(INFO) << "Ignoring update on agent " << *slave
+              << " as it reports no changes";
+    return;
+  }
+
+  slave->totalResources = newSlaveResources;
+
   // Now update the agent's resources in the allocator.
   allocator->updateSlave(slaveId, slave->totalResources);
 
-  // Then rescind any outstanding offers with revocable resources.
+  // Then rescind outstanding offers affected by the update.
   // NOTE: Need a copy of offers because the offers are removed inside the loop.
   foreach (Offer* offer, utils::copy(slave->offers)) {
+    bool rescind = false;
+
     const Resources& offered = offer->resources();
-    if (!offered.revocable().empty()) {
+    // Since updates of the agent's oversubscribed resources are sent at regular
+    // intervals, we only rescind offers containing revocable resources to
+    // reduce churn.
+    if (hasOversubscribed && !offered.revocable().empty()) {
       LOG(INFO) << "Removing offer " << offer->id()
-                << " with revocable resources " << offered
-                << " on agent " << *slave;
+                << " with revocable resources " << offered << " on agent "
+                << *slave;
 
-      allocator->recoverResources(
-          offer->framework_id(), offer->slave_id(), offered, None());
+      rescind = true;
+    }
+
+    // For updates to the agent's total resources all offers are rescinded.
+    //
+    // TODO(bbannier): Only rescind offers possibly containing removed
+    // resources.
+    if (hasTotal) {
+      LOG(INFO) << "Removing offer " << offer->id() << " with resources "
+                << offered << " on agent " << *slave;
 
-      removeOffer(offer, true); // Rescind.
+      rescind = true;
     }
+
+    if (!rescind) {
+      continue;
+    }
+
+    allocator->recoverResources(
+        offer->framework_id(),
+        offer->slave_id(),
+        offered,
+        None());
+
+    removeOffer(offer, true); // Rescind.
   }
 
   // NOTE: We don't need to rescind inverse offers here as they are unrelated to