You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by mz...@apache.org on 2019/07/10 05:01:22 UTC

[mesos] branch master updated (121dcb5 -> 0026ea4)

This is an automated email from the ASF dual-hosted git repository.

mzhu pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git.


    from 121dcb5  Added a test for /roles quota.consumed field.
     new 231200c  Refactored `QuotaTree` insertion logic.
     new 16f0b0c  Added overcommit and hierarchical inclusion check for `UPDATE_QUOTA`.
     new 0026ea4  Implemented `UPDATE_QUOTA` operator call.

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 src/master/constants.cpp         |   1 +
 src/master/http.cpp              |   2 +-
 src/master/master.hpp            |   4 ++
 src/master/quota_handler.cpp     | 127 +++++++++++++++++++++++++++++++++------
 src/tests/master_quota_tests.cpp |   4 +-
 src/tests/master_tests.cpp       |   2 +-
 6 files changed, 118 insertions(+), 22 deletions(-)


[mesos] 01/03: Refactored `QuotaTree` insertion logic.

Posted by mz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

mzhu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 231200cf395cfbe651b9d995f4f9a6ed0d99fa58
Author: Meng Zhu <mz...@mesosphere.io>
AuthorDate: Fri Jul 5 14:55:35 2019 -0700

    Refactored `QuotaTree` insertion logic.
    
    This patch replaces the `insert` method with `update` to make it
    easier to update individual role's quota information in the tree.
    
    Review: https://reviews.apache.org/r/71019
---
 src/master/quota_handler.cpp | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/src/master/quota_handler.cpp b/src/master/quota_handler.cpp
index f9c7431..9a43110 100644
--- a/src/master/quota_handler.cpp
+++ b/src/master/quota_handler.cpp
@@ -97,11 +97,11 @@ public:
     : root(new Node(""))
   {
     foreachpair (const string& role, const Quota& quota, quotas) {
-      insert(role, quota);
+      update(role, quota);
     }
   }
 
-  void insert(const string& role, const Quota& quota)
+  void update(const string& role, const Quota& quota)
   {
     // Create the path from root->leaf in the tree. Any missing nodes
     // are created implicitly.
@@ -117,10 +117,6 @@ public:
       current = current->children.at(component).get();
     }
 
-    // Update `current` with the configured quota for this role.
-    // A path in the tree should be associated with at most one
-    // quota, so the current quota should be unset (default).
-    CHECK(current->quota == DEFAULT_QUOTA);
     current->quota = quota;
   }
 
@@ -206,12 +202,10 @@ Option<Error> Master::QuotaHandler::overcommitCheck(
     QuotaTree quotaTree({});
 
     foreachpair (const string& role, const Quota& quota, quotas) {
-      if (role != request.role()) {
-        quotaTree.insert(role, quota);
-      }
+        quotaTree.update(role, quota);
     }
 
-    quotaTree.insert(request.role(), Quota{request});
+    quotaTree.update(request.role(), Quota{request});
 
     // Hard CHECK since this is already validated earlier
     // during request validation.
@@ -564,15 +558,15 @@ Future<http::Response> Master::QuotaHandler::_set(
   // Validate that adding this quota does not violate the hierarchical
   // relationship between quotas.
   {
+     // TODO(mzhu): Keep an update-to-date `QuotaTree` in the memory
+     // to avoid construction from scratch every time.
     QuotaTree quotaTree({});
 
     foreachpair (const string& role, const Quota& quota, master->quotas) {
-      if (role != quotaInfo.role()) {
-        quotaTree.insert(role, quota);
-      }
+      quotaTree.update(role, quota);
     }
 
-    quotaTree.insert(quotaInfo.role(), Quota{quotaInfo});
+    quotaTree.update(quotaInfo.role(), Quota{quotaInfo});
 
     Option<Error> error = quotaTree.validate();
     if (error.isSome()) {


[mesos] 02/03: Added overcommit and hierarchical inclusion check for `UPDATE_QUOTA`.

Posted by mz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

mzhu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 16f0b0c295960e397e56f6d504b8075cb62e6e4f
Author: Meng Zhu <mz...@mesosphere.io>
AuthorDate: Fri Jul 5 15:41:01 2019 -0700

    Added overcommit and hierarchical inclusion check for `UPDATE_QUOTA`.
    
    The overcommit check validates that the total quota guarantees in
    the cluster is contained by the cluster capacity.
    
    The hierarchical inclusion check validates that the sum of
    children's  guarantees is contained by the parent guarantee.
    
    Further validation is needed for:
    
    - Check a role's limit is less than its current consumption.
    - Check a role's limit is less than its parent's limit.
    
    Review: https://reviews.apache.org/r/71020
---
 src/master/quota_handler.cpp | 60 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 59 insertions(+), 1 deletion(-)

diff --git a/src/master/quota_handler.cpp b/src/master/quota_handler.cpp
index 9a43110..9afab28 100644
--- a/src/master/quota_handler.cpp
+++ b/src/master/quota_handler.cpp
@@ -89,7 +89,8 @@ namespace master {
 //       parent role and its children.
 //
 // TODO(mzhu): The above check is only about guarantees. We should extend
-// the check to also cover limits.
+// the check to also cover limits: a role's limit is less than its
+// parent's limit.
 class QuotaTree
 {
 public:
@@ -462,6 +463,63 @@ Future<http::Response> Master::QuotaHandler::update(
     }
   }
 
+  // TODO(mzhu): Validate a role's limit is below its current consumption
+  // (otherwise a `force` flag is needed).
+  //
+  // TODO(mzhu): Pull out these validation in a function that can be shared
+  // between this and the old handlers.
+
+  // Validate hierarchical quota.
+
+  // TODO(mzhu): Keep an up-to-date `QuotaTree` in memory.
+  QuotaTree quotaTree{{}};
+
+  foreachpair (const string& role, const Quota& quota, master->quotas) {
+    quotaTree.update(role, quota);
+  }
+
+  foreach (auto&& config, call.update_quota().quota_configs()) {
+    quotaTree.update(config.role(), Quota{config});
+  }
+
+  Option<Error> error = quotaTree.validate();
+  if (error.isSome()) {
+    return BadRequest("Invalid QuotaConfig: " + error->message);
+  }
+
+  // Overcommitment check.
+
+  // Check for quota overcommit. We include resources from all
+  // registered agents, even if they are disconnected.
+  //
+  // Disconnection tends to be a transient state (e.g. agent
+  // might be getting restarted as part of an upgrade, there
+  // might be a transient networking issue, etc), so excluding
+  // disconnected agents could produce an unstable capacity
+  // calculation.
+  //
+  // TODO(bmahler): In the same vein, include agents that
+  // are recovered from the registry but not yet registered.
+  // Because we currently exclude them, the calculated capacity
+  // is 0 immediately after a failover and slowly works its way
+  // up to the pre-failover capacity as the agents re-register.
+  ResourceQuantities clusterCapacity;
+  foreachvalue (const Slave* agent, master->slaves.registered) {
+    clusterCapacity += ResourceQuantities::fromScalarResources(
+        agent->totalResources.nonRevocable().scalars());
+  }
+
+  if (!clusterCapacity.contains(quotaTree.totalGuarantees())) {
+    if (call.update_quota().force()) {
+      LOG(INFO) << "Using force flag to override quota overcommit check";
+    } else {
+      return BadRequest("Invalid QuotaConfig: total quota guarantees '" +
+              stringify(quotaTree.totalGuarantees()) + "'"
+              " exceed cluster capacity '" + stringify(clusterCapacity) + "'"
+              " (use 'force' flag to bypass this check)");
+    }
+  }
+
   return NotImplemented();
 }
 


[mesos] 03/03: Implemented `UPDATE_QUOTA` operator call.

Posted by mz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

mzhu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 0026ea46dc35cbba1f442b8e425c6cbaf81ee8f8
Author: Meng Zhu <mz...@mesosphere.io>
AuthorDate: Fri Jul 5 18:05:59 2019 -0700

    Implemented `UPDATE_QUOTA` operator call.
    
    This patch wires up the master, auth, registar and allocator
    pieces for `UPDATE_QUOTA` call.
    
    This enables the master capability `QUOTA_V2`. The capability
    implies the quota v2 API is capable of writes (`UPDATE_QUOTA`)
    and the master is capable of recovering from V2 quota
    (`QuotaConfig`) in registry.
    
    This patch lacks the rescind offer logic. When quota limits
    and guarantees are configured, it might be necessary to
    rescind offers on the fly to satisfy new guarantees or be
    constrained by the new limits. A todo is left and will be
    tackled in subsequent patches.
    
    Also enabled test `MasterQuotaTest.RecoverQuotaEmptyCluster`.
    
    Review: https://reviews.apache.org/r/71021
---
 src/master/constants.cpp         |  1 +
 src/master/http.cpp              |  2 +-
 src/master/master.hpp            |  4 ++++
 src/master/quota_handler.cpp     | 47 +++++++++++++++++++++++++++++++++++++---
 src/tests/master_quota_tests.cpp |  4 +---
 src/tests/master_tests.cpp       |  2 +-
 6 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/src/master/constants.cpp b/src/master/constants.cpp
index 13b3467..6bd89c6 100644
--- a/src/master/constants.cpp
+++ b/src/master/constants.cpp
@@ -25,6 +25,7 @@ std::vector<MasterInfo::Capability> MASTER_CAPABILITIES()
   MasterInfo::Capability::Type types[] = {
     MasterInfo::Capability::AGENT_UPDATE,
     MasterInfo::Capability::AGENT_DRAINING,
+    MasterInfo::Capability::QUOTA_V2,
   };
 
   std::vector<MasterInfo::Capability> result;
diff --git a/src/master/http.cpp b/src/master/http.cpp
index 1b3d48d..cd0f40c 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -380,7 +380,7 @@ Future<Response> Master::Http::api(
       return quotaHandler.status(call, principal, acceptType);
 
     case mesos::master::Call::UPDATE_QUOTA:
-      return NotImplemented();
+      return quotaHandler.update(call, principal);
 
     // TODO(bmahler): Add this to a deprecated call section
     // at the bottom once deprecated by `UPDATE_QUOTA`.
diff --git a/src/master/master.hpp b/src/master/master.hpp
index b57483c..9b86155 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -1288,6 +1288,10 @@ private:
         const Option<process::http::authentication::Principal>&
             principal) const;
 
+    process::Future<process::http::Response> _update(
+        const google::protobuf::RepeatedPtrField<mesos::quota::QuotaConfig>&
+          quotaConfigs) const;
+
     process::Future<process::http::Response> _set(
         const mesos::quota::QuotaRequest& quotaRequest,
         const Option<process::http::authentication::Principal>&
diff --git a/src/master/quota_handler.cpp b/src/master/quota_handler.cpp
index 9afab28..13789d8 100644
--- a/src/master/quota_handler.cpp
+++ b/src/master/quota_handler.cpp
@@ -436,8 +436,11 @@ Future<http::Response> Master::QuotaHandler::update(
   CHECK_EQ(mesos::master::Call::UPDATE_QUOTA, call.type());
   CHECK(call.has_update_quota());
 
+  const RepeatedPtrField<QuotaConfig>& configs =
+    call.update_quota().quota_configs();
+
   // Validate `QuotaConfig`.
-  foreach (auto&& config, call.update_quota().quota_configs()) {
+  foreach (const auto& config, configs) {
     // Check that the role is on the role whitelist, if it exists.
     if (!master->isWhitelistedRole(config.role())) {
       return BadRequest(
@@ -478,7 +481,7 @@ Future<http::Response> Master::QuotaHandler::update(
     quotaTree.update(role, quota);
   }
 
-  foreach (auto&& config, call.update_quota().quota_configs()) {
+  foreach (const auto& config, configs) {
     quotaTree.update(config.role(), Quota{config});
   }
 
@@ -520,7 +523,45 @@ Future<http::Response> Master::QuotaHandler::update(
     }
   }
 
-  return NotImplemented();
+  // Create a list of authorization actions
+  // for each quota configuration update.
+  vector<Future<bool>> authorizedUpdates;
+  authorizedUpdates.reserve(configs.size());
+  foreach (const QuotaConfig& config, configs) {
+    authorizedUpdates.push_back(authorizeUpdateQuotaConfig(principal, config));
+  }
+
+  return process::collect(authorizedUpdates)
+    .then(defer(
+        master->self(),
+        [=](const vector<bool>& authorizations) -> Future<http::Response> {
+          return std::all_of(
+                     authorizations.begin(),
+                     authorizations.end(),
+                     [](bool authorized) { return authorized; })
+                   ? _update(configs)
+                   : Forbidden();
+        }));
+}
+
+
+Future<http::Response> Master::QuotaHandler::_update(
+    const RepeatedPtrField<QuotaConfig>& configs) const
+{
+  return master->registrar
+    ->apply(Owned<RegistryOperation>(new quota::UpdateQuota(configs)))
+    .then(defer(master->self(), [=](bool result) -> Future<http::Response> {
+      // Currently, quota registry entry mutation never fails.
+      CHECK(result);
+
+      foreach (const QuotaConfig& config, configs) {
+        master->allocator->updateQuota(config.role(), Quota{config});
+      }
+
+      // TODO(mzhu): Rescind offers.
+
+      return OK();
+    }));
 }
 
 
diff --git a/src/tests/master_quota_tests.cpp b/src/tests/master_quota_tests.cpp
index 34a6520..4b805e9 100644
--- a/src/tests/master_quota_tests.cpp
+++ b/src/tests/master_quota_tests.cpp
@@ -1268,9 +1268,7 @@ TEST_F(MasterQuotaTest, AvailableResourcesAfterRescinding)
 
 // Checks that quota is recovered correctly after master failover if
 // the expected size of the cluster is zero.
-//
-// TODO(mzhu): Enable this once master is QUOTA_V2 capable.
-TEST_F(MasterQuotaTest, DISABLED_RecoverQuotaEmptyCluster)
+TEST_F(MasterQuotaTest, RecoverQuotaEmptyCluster)
 {
   master::Flags masterFlags = CreateMasterFlags();
   masterFlags.registry = "replicated_log";
diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp
index f817450..b9ef13c 100644
--- a/src/tests/master_tests.cpp
+++ b/src/tests/master_tests.cpp
@@ -5036,7 +5036,7 @@ TEST_F(MasterTest, StateEndpoint)
 
   // Master should always have these default capabilities.
   Try<JSON::Value> expectedCapabilities =
-    JSON::parse("[\"AGENT_UPDATE\", \"AGENT_DRAINING\"]");
+    JSON::parse("[\"AGENT_UPDATE\", \"AGENT_DRAINING\", \"QUOTA_V2\"]");
 
   ASSERT_SOME(expectedCapabilities);
   EXPECT_TRUE(masterCapabilities.contains(expectedCapabilities.get()));