You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2017/07/09 04:11:59 UTC

[1/3] mesos git commit: Allowed to pass total resources in 'UpdateSlaveMessage'.

Repository: mesos
Updated Branches:
  refs/heads/master 786713756 -> d345ebccf


Allowed to pass total resources in 'UpdateSlaveMessage'.

This commit both extends the existing 'UpdateSlaveMessage' proto
message, and adjusts its handling on the agent and master side.

To distinguish updates to 'oversubscribed_resources' from updates to
'total_resources' the message now contains a 'type' field to allow
disambiguation among an empty list of resources and an unset list of
resources. For backwards-compatibility we assume whenever the type
field was not set that caller intended to use the
'oversubscribed_resources' field as opposed to the 'total_resources'
field.

Currently, passing 'total_resources' is handled neither in the master
nor the default allocator; we will implement this in a subsequent
patch.

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


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

Branch: refs/heads/master
Commit: 13b1fcf55a929cb8addc9a1fdf366b91999518bf
Parents: 7867137
Author: Benjamin Bannier <be...@mesosphere.io>
Authored: Sat Jul 8 20:50:11 2017 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Sat Jul 8 20:50:11 2017 -0700

----------------------------------------------------------------------
 src/master/master.cpp                | 64 +++++++++++++++++++++----------
 src/master/master.hpp                |  4 +-
 src/messages/messages.proto          | 21 +++++++++-
 src/slave/slave.cpp                  |  3 ++
 src/tests/oversubscription_tests.cpp |  3 ++
 5 files changed, 71 insertions(+), 24 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/13b1fcf5/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 56b170e..d01cb97 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -66,6 +66,7 @@
 #include <stout/option.hpp>
 #include <stout/path.hpp>
 #include <stout/stringify.hpp>
+#include <stout/unimplemented.hpp>
 #include <stout/unreachable.hpp>
 #include <stout/utils.hpp>
 #include <stout/uuid.hpp>
@@ -894,10 +895,7 @@ void Master::initialize()
       &ExitedExecutorMessage::executor_id,
       &ExitedExecutorMessage::status);
 
-  install<UpdateSlaveMessage>(
-      &Master::updateSlave,
-      &UpdateSlaveMessage::slave_id,
-      &UpdateSlaveMessage::oversubscribed_resources);
+  install<UpdateSlaveMessage>(&Master::updateSlave);
 
   install<AuthenticateMessage>(
       &Master::authenticate,
@@ -6523,46 +6521,72 @@ void Master::updateFramework(
 }
 
 
-void Master::updateSlave(
-    const SlaveID& slaveId,
-    const Resources& oversubscribedResources)
+void Master::updateSlave(const UpdateSlaveMessage& message)
 {
   ++metrics->messages_update_slave;
 
+  const SlaveID& slaveId = message.slave_id();
+
   if (slaves.removed.get(slaveId).isSome()) {
     // If the slave has been removed, drop the status update. The
     // master is no longer trying to health check this slave; when the
     // slave realizes it hasn't received any pings from the master, it
     // will eventually try to reregister.
-    LOG(WARNING)
-      << "Ignoring update of agent with total oversubscribed resources "
-      << oversubscribedResources << " on removed agent " << slaveId;
+    LOG(WARNING) << "Ignoring update on removed agent " << slaveId;
     return;
   }
 
   Slave* slave = slaves.registered.get(slaveId);
 
   if (slave == nullptr) {
-    LOG(WARNING)
-      << "Ignoring update of agent with total oversubscribed resources "
-      << oversubscribedResources << " on unknown agent " << slaveId;
+    LOG(WARNING) << "Ignoring update on removed agent " << slaveId;
     return;
   }
 
-  LOG(INFO) << "Received update of agent " << *slave << " with total"
-            << " oversubscribed resources " << oversubscribedResources;
-
   // NOTE: We must *first* update the agent's resources before we
   // recover the resources. If we recovered the resources first,
   // an allocation could trigger between recovering resources and
   // updating the agent in the allocator. This would lead us to
   // re-send out the stale oversubscribed resources!
 
-  slave->totalResources =
-    slave->totalResources.nonRevocable() + oversubscribedResources.revocable();
+  // If the caller did not specify a type we assume we should set
+  // `oversubscribedResources` to be backwards-compatibility with
+  // older clients.
+  const UpdateSlaveMessage::Type type =
+    message.has_type() ? message.type() : UpdateSlaveMessage::OVERSUBSCRIBED;
+
+  switch (type) {
+    case UpdateSlaveMessage::OVERSUBSCRIBED: {
+      const Resources oversubscribedResources =
+        message.oversubscribed_resources();
+
+      LOG(INFO) << "Received update of agent " << *slave << " with total"
+                << " oversubscribed resources " << oversubscribedResources;
+
+      slave->totalResources =
+        slave->totalResources.nonRevocable() +
+        oversubscribedResources.revocable();
+
+      // Now update the agent's resources in the allocator.
+      allocator->updateSlave(slaveId, message.oversubscribed_resources());
 
-  // First update the agent's resources in the allocator.
-  allocator->updateSlave(slaveId, oversubscribedResources);
+      break;
+    }
+    case UpdateSlaveMessage::TOTAL: {
+      const Resources totalResources =
+        message.total_resources();
+
+      LOG(INFO) << "Received update of agent " << *slave << " with total"
+                << " resources " << totalResources;
+
+      UNIMPLEMENTED;
+    }
+    case UpdateSlaveMessage::UNKNOWN: {
+      LOG(WARNING) << "Ignoring update on agent " << slaveId
+                   << " since the update type is not understood";
+      return;
+    }
+  }
 
   // Then rescind any outstanding offers with revocable resources.
   // NOTE: Need a copy of offers because the offers are removed inside the loop.

http://git-wip-us.apache.org/repos/asf/mesos/blob/13b1fcf5/src/master/master.hpp
----------------------------------------------------------------------
diff --git a/src/master/master.hpp b/src/master/master.hpp
index 95c2d0f..b9b3ea4 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -424,9 +424,7 @@ public:
       const ExecutorID& executorId,
       int32_t status);
 
-  void updateSlave(
-      const SlaveID& slaveId,
-      const Resources& oversubscribedResources);
+  void updateSlave(const UpdateSlaveMessage& message);
 
   void updateUnavailability(
       const MachineID& machineId,

http://git-wip-us.apache.org/repos/asf/mesos/blob/13b1fcf5/src/messages/messages.proto
----------------------------------------------------------------------
diff --git a/src/messages/messages.proto b/src/messages/messages.proto
index 67cee31..dc4e19c 100644
--- a/src/messages/messages.proto
+++ b/src/messages/messages.proto
@@ -605,11 +605,30 @@ message CheckpointResourcesMessage {
 /**
  * This message is sent by the agent to the master to inform the
  * master about the total amount of oversubscribed (allocated and
- * allocatable) resources.
+ * allocatable), or total resources.
  */
 message UpdateSlaveMessage {
   required SlaveID slave_id = 1;
+
+  // This message can contain one of `oversubscribed_resources` or
+  // `total_resources`. Callers are expected to set the `type` field
+  // to denote which field should be examined. For backwards
+  // compatibility we interpret an unset `type` field as if it was
+  // `OVERSUBSCRIBED`.
+  //
+  // It is suggested that callers use the `total_resources` field
+  // exclusively as the `oversubscribed_resources` field might be
+  // removed in the future.
+  enum Type {
+    UNKNOWN = 0;
+    OVERSUBSCRIBED = 1;
+    TOTAL = 2;
+  }
+
+  optional Type type = 3;
+
   repeated Resource oversubscribed_resources = 2;
+  repeated Resource total_resources = 4;
 }
 
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/13b1fcf5/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 52f6736..a1a6b64 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -1230,6 +1230,7 @@ void Slave::registered(
 
     UpdateSlaveMessage message;
     message.mutable_slave_id()->CopyFrom(info.id());
+    message.set_type(UpdateSlaveMessage::OVERSUBSCRIBED);
     message.mutable_oversubscribed_resources()->CopyFrom(
         oversubscribedResources.get());
 
@@ -1309,6 +1310,7 @@ void Slave::reregistered(
 
     UpdateSlaveMessage message;
     message.mutable_slave_id()->CopyFrom(info.id());
+    message.set_type(UpdateSlaveMessage::OVERSUBSCRIBED);
     message.mutable_oversubscribed_resources()->CopyFrom(
         oversubscribedResources.get());
 
@@ -6434,6 +6436,7 @@ void Slave::_forwardOversubscribed(const Future<Resources>& oversubscribable)
 
       UpdateSlaveMessage message;
       message.mutable_slave_id()->CopyFrom(info.id());
+      message.set_type(UpdateSlaveMessage::OVERSUBSCRIBED);
       message.mutable_oversubscribed_resources()->CopyFrom(oversubscribed);
 
       CHECK_SOME(master);

http://git-wip-us.apache.org/repos/asf/mesos/blob/13b1fcf5/src/tests/oversubscription_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/oversubscription_tests.cpp b/src/tests/oversubscription_tests.cpp
index 2266510..54535a3 100644
--- a/src/tests/oversubscription_tests.cpp
+++ b/src/tests/oversubscription_tests.cpp
@@ -323,6 +323,7 @@ TEST_F(OversubscriptionTest, ForwardUpdateSlaveMessage)
 
   AWAIT_READY(update);
 
+  EXPECT_EQ(update->type(), UpdateSlaveMessage::OVERSUBSCRIBED);
   EXPECT_EQ(update->oversubscribed_resources(), resources);
 
   // Ensure the metric is updated.
@@ -695,6 +696,7 @@ TEST_F(OversubscriptionTest, FixedResourceEstimator)
   Clock::settle();
 
   AWAIT_READY(update);
+  ASSERT_EQ(UpdateSlaveMessage::OVERSUBSCRIBED, update->type());
 
   Resources resources = update->oversubscribed_resources();
   EXPECT_SOME_EQ(2.0, resources.cpus());
@@ -896,6 +898,7 @@ TEST_F(OversubscriptionTest, Reregistration)
   Clock::advance(agentFlags.registration_backoff_factor);
   AWAIT_READY(slaveRegistered);
   AWAIT_READY(update);
+  ASSERT_EQ(UpdateSlaveMessage::OVERSUBSCRIBED, update->type());
 
   Resources resources = update->oversubscribed_resources();
   EXPECT_SOME_EQ(2.0, resources.cpus());


[3/3] mesos git commit: Added MESOS-7755 to the upgrades guide and the CHANGELOG.

Posted by ji...@apache.org.
Added MESOS-7755 to the upgrades guide and the CHANGELOG.

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


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

Branch: refs/heads/master
Commit: d345ebccf19387d75d67c99f8f6e72a301f55bc3
Parents: 9e8e013
Author: Benjamin Bannier <be...@mesosphere.io>
Authored: Sat Jul 8 20:50:26 2017 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Sat Jul 8 20:50:26 2017 -0700

----------------------------------------------------------------------
 CHANGELOG        |  9 +++++++++
 docs/upgrades.md | 12 +++++++++---
 2 files changed, 18 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/d345ebcc/CHANGELOG
----------------------------------------------------------------------
diff --git a/CHANGELOG b/CHANGELOG
index e59c3e7..05b149c 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -19,6 +19,15 @@ Deprecations/Removals:
   * [MESOS-7477] - The agent `--allowed_capabilities` flag is
     deprecated in favor of `--effective_capabilities`
 
+Additional API Changes:
+
+  * [MESOS-7755] The interpretation of the optional resource argument
+    passed in `Allocator::updateSlave` was changed from the total
+    amount of oversubscribed resources on the agent to the new total
+    resources (both revocable and non-revocable) on the agent. Custom
+    allocator implementation should be changed to interpretation of the
+    passed value as a total before updating.
+
 
 Release Notes - Mesos - Version 1.3.1 (WIP)
 -------------------------------------------

http://git-wip-us.apache.org/repos/asf/mesos/blob/d345ebcc/docs/upgrades.md
----------------------------------------------------------------------
diff --git a/docs/upgrades.md b/docs/upgrades.md
index 5511880..dda55f9 100644
--- a/docs/upgrades.md
+++ b/docs/upgrades.md
@@ -70,6 +70,7 @@ We categorize the changes as follows:
 
   <td style="word-wrap: break-word; overflow-wrap: break-word;"><!--Module API-->
     <ul style="padding-left:10px;">
+      <li>C <a href="1-4-x-allocator-update-slave">Changed semantics of Allocator::updateSlave</a></li>
     </ul>
   </td>
 
@@ -329,9 +330,14 @@ We categorize the changes as follows:
 
 <a name="1-4-x-agent-capabilities-flags"></a>
 
-* The agent `--effective_capabilities` flag has been added to specify the default effective capability set for tasks.
-* The agent `--bounding_capabilities` flag has been added to specify the default bounding capability set for tasks.
-* The agent `--allowed-capabilities` flag has been deprecated in favor of `--effective_capabilities`.
+* Changes to capability-related agent flags:
+  * The agent `--effective_capabilities` flag has been added to specify the default effective capability set for tasks.
+  * The agent `--bounding_capabilities` flag has been added to specify the default bounding capability set for tasks.
+  * The agent `--allowed-capabilities` flag has been deprecated in favor of `--effective_capabilities`.
+
+<a name="1-4-x-allocator-update-slave"></a>
+
+* The semantics of the optional resource argument passed in `Allocator::updateSlave` was change. While previously the passed value denoted a new amount of oversubscribed (revocable) resources on the agent, it now denotes the new amount of total resources on the agent. This requires custom allocator implementations to update their interpretation of the passed value.
 
 ## Upgrading from 1.2.x to 1.3.x ##
 


[2/3] mesos git commit: Changed semantics of allocator 'updateSlave' method.

Posted by ji...@apache.org.
Changed semantics of allocator 'updateSlave' method.

We change the semantics of the 'updateSlave' method present in the
allocator interface. While previously the passed optional resource
argument was interpreted as the amount of (new) oversubscribed
resources, it now represents the new amount of total resources on the
given agent.

We addtionally add an optimization of
'HierarchicalAllocatorProcess::updateSlaveTotal' for cases where the
passed total is identical to the current total. This operation is a
no-op now and we prevent updating the sorters.

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


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

Branch: refs/heads/master
Commit: 9e8e013340be57fb85d4fa64e43d2d090e114422
Parents: 13b1fcf
Author: Benjamin Bannier <be...@mesosphere.io>
Authored: Sat Jul 8 20:50:20 2017 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Sat Jul 8 20:50:20 2017 -0700

----------------------------------------------------------------------
 include/mesos/allocator/allocator.hpp       |  11 +--
 src/master/allocator/mesos/allocator.hpp    |   8 +-
 src/master/allocator/mesos/hierarchical.cpp |  50 +++---------
 src/master/allocator/mesos/hierarchical.hpp |   6 +-
 src/master/master.cpp                       |  11 ++-
 src/tests/hierarchical_allocator_tests.cpp  | 100 +++++++++++++++++++++--
 6 files changed, 121 insertions(+), 65 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/9e8e0133/include/mesos/allocator/allocator.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/allocator/allocator.hpp b/include/mesos/allocator/allocator.hpp
index bec9e0b..9d116c6 100644
--- a/include/mesos/allocator/allocator.hpp
+++ b/include/mesos/allocator/allocator.hpp
@@ -204,19 +204,12 @@ public:
   /**
    * Updates an agent.
    *
-   * Updates the latest oversubscribed resources or capabilities for an agent.
-   * TODO(vinod): Instead of just oversubscribed resources have this
-   * method take total resources. We can then reuse this method to
-   * update Agent's total resources in the future.
-   *
-   * @param oversubscribed The new oversubscribed resources estimate from
-   *     the agent. The oversubscribed resources include the total amount
-   *     of oversubscribed resources that are allocated and available.
+   * @param total The new total resources on the agent.
    * @param capabilities The new capabilities of the agent.
    */
   virtual void updateSlave(
       const SlaveID& slave,
-      const Option<Resources>& oversubscribed = None(),
+      const Option<Resources>& total = None(),
       const Option<std::vector<SlaveInfo::Capability>>&
           capabilities = None()) = 0;
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/9e8e0133/src/master/allocator/mesos/allocator.hpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/allocator.hpp b/src/master/allocator/mesos/allocator.hpp
index 2e780c9..725ec7c 100644
--- a/src/master/allocator/mesos/allocator.hpp
+++ b/src/master/allocator/mesos/allocator.hpp
@@ -98,7 +98,7 @@ public:
 
   void updateSlave(
       const SlaveID& slave,
-      const Option<Resources>& oversubscribed = None(),
+      const Option<Resources>& total = None(),
       const Option<std::vector<SlaveInfo::Capability>>& capabilities = None());
 
   void activateSlave(
@@ -237,7 +237,7 @@ public:
 
   virtual void updateSlave(
       const SlaveID& slave,
-      const Option<Resources>& oversubscribed = None(),
+      const Option<Resources>& total = None(),
       const Option<std::vector<SlaveInfo::Capability>>&
           capabilities = None()) = 0;
 
@@ -473,14 +473,14 @@ inline void MesosAllocator<AllocatorProcess>::removeSlave(
 template <typename AllocatorProcess>
 inline void MesosAllocator<AllocatorProcess>::updateSlave(
     const SlaveID& slaveId,
-    const Option<Resources>& oversubscribed,
+    const Option<Resources>& total,
     const Option<std::vector<SlaveInfo::Capability>>& capabilities)
 {
   process::dispatch(
       process,
       &MesosAllocatorProcess::updateSlave,
       slaveId,
-      oversubscribed,
+      total,
       capabilities);
 }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/9e8e0133/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index eb01d8e..fad9330 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -623,7 +623,7 @@ void HierarchicalAllocatorProcess::removeSlave(
 
 void HierarchicalAllocatorProcess::updateSlave(
     const SlaveID& slaveId,
-    const Option<Resources>& oversubscribed,
+    const Option<Resources>& total,
     const Option<vector<SlaveInfo::Capability>>& capabilities)
 {
   CHECK(initialized);
@@ -648,42 +648,11 @@ void HierarchicalAllocatorProcess::updateSlave(
     }
   }
 
-  if (oversubscribed.isSome()) {
-    // Check that all the oversubscribed resources are revocable.
-    CHECK_EQ(oversubscribed.get(), oversubscribed->revocable());
+  if (total.isSome()) {
+    updated = updateSlaveTotal(slaveId, total.get());
 
-    const Resources oldRevocable = slave.total.revocable();
-
-    if (oldRevocable != oversubscribed.get()) {
-      // Update the total resources.
-      //
-      // Reset the total resources to include the non-revocable resources,
-      // plus the new estimate of oversubscribed resources.
-      //
-      // NOTE: All modifications to revocable resources in the allocator for
-      // `slaveId` are lost.
-      //
-      // TODO(alexr): Update this math once the source of revocable resources
-      // is extended beyond oversubscription.
-      slave.total = slave.total.nonRevocable() + oversubscribed.get();
-
-      // Update the total resources in the `roleSorter` by removing the
-      // previous oversubscribed resources and adding the new
-      // oversubscription estimate.
-      roleSorter->remove(slaveId, oldRevocable);
-      roleSorter->add(slaveId, oversubscribed.get());
-
-      updated = true;
-
-      // NOTE: We do not need to update `quotaRoleSorter` because this
-      // function only changes the revocable resources on the slave, but
-      // the quota role sorter only manages non-revocable resources.
-
-      LOG(INFO) << "Agent " << slaveId << " (" << slave.hostname << ")"
-                << " updated with oversubscribed resources "
-                << oversubscribed.get() << " (total: " << slave.total
-                << ", allocated: " << slave.allocated << ")";
-    }
+    LOG(INFO) << "Agent " << slaveId << " (" << slave.hostname << ")"
+              << " updated with total resources " << total.get();
   }
 
   if (updated) {
@@ -2365,7 +2334,7 @@ void HierarchicalAllocatorProcess::untrackFrameworkUnderRole(
 }
 
 
-void HierarchicalAllocatorProcess::updateSlaveTotal(
+bool HierarchicalAllocatorProcess::updateSlaveTotal(
     const SlaveID& slaveId,
     const Resources& total)
 {
@@ -2374,6 +2343,11 @@ void HierarchicalAllocatorProcess::updateSlaveTotal(
   Slave& slave = slaves.at(slaveId);
 
   const Resources oldTotal = slave.total;
+
+  if (oldTotal == total) {
+    return false;
+  }
+
   slave.total = total;
 
   // Currently `roleSorter` and `quotaRoleSorter`, being the root-level
@@ -2387,6 +2361,8 @@ void HierarchicalAllocatorProcess::updateSlaveTotal(
   // See comment at `quotaRoleSorter` declaration regarding non-revocable.
   quotaRoleSorter->remove(slaveId, oldTotal.nonRevocable());
   quotaRoleSorter->add(slaveId, total.nonRevocable());
+
+  return true;
 }
 
 } // namespace internal {

http://git-wip-us.apache.org/repos/asf/mesos/blob/9e8e0133/src/master/allocator/mesos/hierarchical.hpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.hpp b/src/master/allocator/mesos/hierarchical.hpp
index 5c58cf4..81d1b96 100644
--- a/src/master/allocator/mesos/hierarchical.hpp
+++ b/src/master/allocator/mesos/hierarchical.hpp
@@ -143,7 +143,7 @@ public:
 
   void updateSlave(
       const SlaveID& slave,
-      const Option<Resources>& oversubscribed = None(),
+      const Option<Resources>& total = None(),
       const Option<std::vector<SlaveInfo::Capability>>& capabilities = None());
 
   void deactivateSlave(
@@ -525,8 +525,8 @@ private:
 
   // Helper to update the agent's total resources maintained in the allocator
   // and the role and quota sorters (whose total resources match the agent's
-  // total resources).
-  void updateSlaveTotal(const SlaveID& slaveId, const Resources& total);
+  // total resources). Returns true iff the stored agent total was changed.
+  bool updateSlaveTotal(const SlaveID& slaveId, const Resources& total);
 };
 
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/9e8e0133/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index d01cb97..7668749 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -66,7 +66,6 @@
 #include <stout/option.hpp>
 #include <stout/path.hpp>
 #include <stout/stringify.hpp>
-#include <stout/unimplemented.hpp>
 #include <stout/unreachable.hpp>
 #include <stout/utils.hpp>
 #include <stout/uuid.hpp>
@@ -6566,10 +6565,6 @@ void Master::updateSlave(const UpdateSlaveMessage& message)
       slave->totalResources =
         slave->totalResources.nonRevocable() +
         oversubscribedResources.revocable();
-
-      // Now update the agent's resources in the allocator.
-      allocator->updateSlave(slaveId, message.oversubscribed_resources());
-
       break;
     }
     case UpdateSlaveMessage::TOTAL: {
@@ -6579,7 +6574,8 @@ void Master::updateSlave(const UpdateSlaveMessage& message)
       LOG(INFO) << "Received update of agent " << *slave << " with total"
                 << " resources " << totalResources;
 
-      UNIMPLEMENTED;
+      slave->totalResources = totalResources;
+      break;
     }
     case UpdateSlaveMessage::UNKNOWN: {
       LOG(WARNING) << "Ignoring update on agent " << slaveId
@@ -6588,6 +6584,9 @@ void Master::updateSlave(const UpdateSlaveMessage& message)
     }
   }
 
+  // Now update the agent's resources in the allocator.
+  allocator->updateSlave(slaveId, slave->totalResources);
+
   // Then rescind any outstanding offers with revocable resources.
   // NOTE: Need a copy of offers because the offers are removed inside the loop.
   foreach (Offer* offer, utils::copy(slave->offers)) {

http://git-wip-us.apache.org/repos/asf/mesos/blob/9e8e0133/src/tests/hierarchical_allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index 2a312a9..e68e39a 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -1872,7 +1872,7 @@ TEST_F(HierarchicalAllocatorTest, UpdateSlaveOversubscribedResources)
 
   // Update the slave with 10 oversubscribed cpus.
   Resources oversubscribed = createRevocableResources("cpus", "10");
-  allocator->updateSlave(slave.id(), oversubscribed);
+  allocator->updateSlave(slave.id(), slave.resources() + oversubscribed);
 
   // The next allocation should be for 10 oversubscribed resources.
   expected = Allocation(
@@ -1883,7 +1883,7 @@ TEST_F(HierarchicalAllocatorTest, UpdateSlaveOversubscribedResources)
 
   // Update the slave again with 12 oversubscribed cpus.
   Resources oversubscribed2 = createRevocableResources("cpus", "12");
-  allocator->updateSlave(slave.id(), oversubscribed2);
+  allocator->updateSlave(slave.id(), slave.resources() + oversubscribed2);
 
   // The next allocation should be for 2 oversubscribed cpus.
   expected = Allocation(
@@ -1894,7 +1894,7 @@ TEST_F(HierarchicalAllocatorTest, UpdateSlaveOversubscribedResources)
 
   // Update the slave again with 5 oversubscribed cpus.
   Resources oversubscribed3 = createRevocableResources("cpus", "5");
-  allocator->updateSlave(slave.id(), oversubscribed3);
+  allocator->updateSlave(slave.id(), slave.resources() + oversubscribed3);
 
   // Since there are no more available oversubscribed resources there
   // shouldn't be an allocation.
@@ -1905,6 +1905,93 @@ TEST_F(HierarchicalAllocatorTest, UpdateSlaveOversubscribedResources)
 }
 
 
+// This test ensures that we can update the total of an agent. We
+// check that we can expand and shrink the resources available on an
+// agent. Agents can be overallocated, meaning the amount of allocated
+// resources can exceed the total available resources.
+TEST_F(HierarchicalAllocatorTest, UpdateSlaveTotalResources)
+{
+  // Pause clock to disable batch allocation.
+  Clock::pause();
+
+  initialize();
+
+  // Create an agent and a framework. This triggers allocation
+  // of the agent's resources to the framework.
+  const SlaveInfo agent = createSlaveInfo("cpus:100;mem:100;disk:100");
+
+  allocator->addSlave(
+      agent.id(),
+      agent,
+      AGENT_CAPABILITIES(),
+      None(),
+      agent.resources(),
+      {});
+
+  const FrameworkInfo framework = createFrameworkInfo({"role1"});
+  allocator->addFramework(framework.id(), framework, {}, true, {});
+
+  const Allocation expected1 = Allocation(
+      framework.id(),
+      {{"role1", {{agent.id(), agent.resources()}}}});
+
+  AWAIT_EXPECT_EQ(expected1, allocations.get());
+
+  // Increase the agent's total. The additional
+  // resources will be offered to the framework.
+  const Resources addedResources = Resources::parse("cpus:12").get();
+
+  allocator->updateSlave(
+      agent.id(),
+      agent.resources() + addedResources);
+
+  const Allocation expected2 = Allocation(
+      framework.id(),
+      {{"role1", {{agent.id(), addedResources}}}});
+
+  Future<Allocation> allocation = allocations.get();
+  AWAIT_EXPECT_EQ(expected2, allocation);
+
+  // Decrease the agent's total to half its original value. The allocated now
+  // exceeds to total; nothing will be offered due to this operation.
+  const Resources agentResources2 =
+    Resources::parse("cpus:50;mem:50;disk:50").get();
+
+  allocator->updateSlave(agent.id(), agentResources2);
+
+  // Recover all agent resources allocated to the framework in the last two
+  // allocations. We will subsequently be offered the complete agent which has
+  // `agentResources2` resources.
+  allocator->recoverResources(
+      framework.id(),
+      agent.id(),
+      expected1.resources.at("role1").at(agent.id()) +
+        expected2.resources.at("role1").at(agent.id()),
+      None());
+
+  // Advance the clock to trigger allocation of
+  // the available `agentResources2` resources.
+  Clock::advance(flags.allocation_interval);
+
+  const Allocation expected3 = Allocation(
+      framework.id(),
+      {{"role1", {{agent.id(), agentResources2}}}});
+
+  AWAIT_EXPECT_EQ(expected3, allocations.get());
+
+  // Set the agent's total resources to its original value. This will trigger
+  // allocation of the newly added `agentResources2` resources now available on
+  // the agent.
+  allocator->updateSlave(agent.id(), agent.resources());
+
+  const Allocation expected4 = Allocation(
+      framework.id(),
+      {{"role1", {{agent.id(), agentResources2}}}});
+
+  AWAIT_EXPECT_EQ(expected4, allocations.get());
+}
+
+
 // This test ensures that when agent capabilities are updated
 // subsequent allocations properly account for that.
 TEST_F(HierarchicalAllocatorTest, UpdateSlaveCapabilities)
@@ -1939,6 +2026,7 @@ TEST_F(HierarchicalAllocatorTest, UpdateSlaveCapabilities)
   ASSERT_TRUE(allocation.isPending());
 
   // Update the agent to be MULTI_ROLE capable.
+
   allocator->updateSlave(agent.id(), None(), AGENT_CAPABILITIES());
 
   Clock::settle();
@@ -1983,7 +2071,7 @@ TEST_F(HierarchicalAllocatorTest, OversubscribedNotAllocated)
 
   // Update the slave with 10 oversubscribed cpus.
   Resources oversubscribed = createRevocableResources("cpus", "10");
-  allocator->updateSlave(slave.id(), oversubscribed);
+  allocator->updateSlave(slave.id(), slave.resources() + oversubscribed);
 
   // No allocation should be made for oversubscribed resources because
   // the framework has not opted in for them.
@@ -2027,7 +2115,7 @@ TEST_F(HierarchicalAllocatorTest, RecoverOversubscribedResources)
 
   // Update the slave with 10 oversubscribed cpus.
   Resources oversubscribed = createRevocableResources("cpus", "10");
-  allocator->updateSlave(slave.id(), oversubscribed);
+  allocator->updateSlave(slave.id(), slave.resources() + oversubscribed);
 
   // The next allocation should be for 10 oversubscribed cpus.
   expected = Allocation(
@@ -5068,7 +5156,7 @@ TEST_P(HierarchicalAllocator_BENCHMARK_Test, AddAndUpdateSlave)
   watch.start(); // Reset.
 
   foreach (const SlaveInfo& slave, slaves) {
-    allocator->updateSlave(slave.id(), oversubscribed);
+    allocator->updateSlave(slave.id(), slave.resources() + oversubscribed);
   }
 
   // Wait for all the `updateSlave` operations to be processed.