You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jo...@apache.org on 2016/01/15 23:58:05 UTC

[3/3] mesos git commit: Quota: Replaced `QuotaInfo` with `Quota` in allocator.

Quota: Replaced `QuotaInfo` with `Quota` in allocator.

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


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

Branch: refs/heads/master
Commit: b075f415aad92e87d1c25dd2a7b92b7096134238
Parents: a88e2a1
Author: Alexander Rukletsov <ru...@gmail.com>
Authored: Fri Jan 15 17:46:49 2016 -0500
Committer: Joris Van Remoortere <jo...@gmail.com>
Committed: Fri Jan 15 17:56:33 2016 -0500

----------------------------------------------------------------------
 include/mesos/master/allocator.hpp          |  2 +-
 src/master/allocator/mesos/allocator.hpp    |  6 ++---
 src/master/allocator/mesos/hierarchical.cpp | 16 +++++------
 src/master/allocator/mesos/hierarchical.hpp |  4 +--
 src/master/quota_handler.cpp                |  6 +++--
 src/tests/allocator.hpp                     |  2 +-
 src/tests/hierarchical_allocator_tests.cpp  | 32 +++++++++++-----------
 src/tests/master_quota_tests.cpp            | 34 +++++++++++-------------
 8 files changed, 49 insertions(+), 53 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/b075f415/include/mesos/master/allocator.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/master/allocator.hpp b/include/mesos/master/allocator.hpp
index fcebcab..e163669 100644
--- a/include/mesos/master/allocator.hpp
+++ b/include/mesos/master/allocator.hpp
@@ -361,7 +361,7 @@ public:
    */
   virtual void setQuota(
       const std::string& role,
-      const mesos::quota::QuotaInfo& quota) = 0;
+      const Quota& quota) = 0;
 
   /**
    * Informs the allocator to remove quota for the given role.

http://git-wip-us.apache.org/repos/asf/mesos/blob/b075f415/src/master/allocator/mesos/allocator.hpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/allocator.hpp b/src/master/allocator/mesos/allocator.hpp
index 50ef3b2..581eaad 100644
--- a/src/master/allocator/mesos/allocator.hpp
+++ b/src/master/allocator/mesos/allocator.hpp
@@ -144,7 +144,7 @@ public:
 
   void setQuota(
       const std::string& role,
-      const mesos::quota::QuotaInfo& quota);
+      const Quota& quota);
 
   void removeQuota(
       const std::string& role);
@@ -268,7 +268,7 @@ public:
 
   virtual void setQuota(
       const std::string& role,
-      const mesos::quota::QuotaInfo& quota) = 0;
+      const Quota& quota) = 0;
 
   virtual void removeQuota(
       const std::string& role) = 0;
@@ -600,7 +600,7 @@ inline void MesosAllocator<AllocatorProcess>::reviveOffers(
 template <typename AllocatorProcess>
 inline void MesosAllocator<AllocatorProcess>::setQuota(
     const std::string& role,
-    const mesos::quota::QuotaInfo& quota)
+    const Quota& quota)
 {
   process::dispatch(
       process,

http://git-wip-us.apache.org/repos/asf/mesos/blob/b075f415/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 512aaed..72e69a0 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -37,8 +37,6 @@ using std::vector;
 
 using mesos::master::InverseOfferStatus;
 
-using mesos::quota::QuotaInfo;
-
 using process::Failure;
 using process::Future;
 using process::Timeout;
@@ -196,7 +194,7 @@ void HierarchicalAllocatorProcess::recover(
 
   // NOTE: `quotaRoleSorter` is updated implicitly in `setQuota()`.
   foreachpair (const string& role, const Quota& quota, quotas) {
-    setQuota(role, quota.info);
+    setQuota(role, quota);
   }
 
   LOG(INFO) << "Triggered allocator recovery: waiting for "
@@ -978,7 +976,7 @@ void HierarchicalAllocatorProcess::reviveOffers(
 
 void HierarchicalAllocatorProcess::setQuota(
     const string& role,
-    const QuotaInfo& quota)
+    const Quota& quota)
 {
   CHECK(initialized);
 
@@ -1004,7 +1002,7 @@ void HierarchicalAllocatorProcess::setQuota(
   }
 
   // TODO(alexr): Print all quota info for the role.
-  LOG(INFO) << "Set quota " << quota.guarantee() << " for role '" << role
+  LOG(INFO) << "Set quota " << quota.info.guarantee() << " for role '" << role
             << "'";
 
   // Trigger the allocation explicitly in order to promptly react to the
@@ -1023,7 +1021,7 @@ void HierarchicalAllocatorProcess::removeQuota(
   CHECK(quotaRoleSorter->contains(role));
 
   // TODO(alexr): Print all quota info for the role.
-  LOG(INFO) << "Removed quota " << quotas[role].guarantee()
+  LOG(INFO) << "Removed quota " << quotas[role].info.guarantee()
             << " for role '" << role << "'";
 
   // Remove the role from the quota'ed allocation group.
@@ -1160,7 +1158,7 @@ void HierarchicalAllocatorProcess::allocate(
       // alternatives are:
       //   * A custom sorter that is aware of quotas and sorts accordingly.
       //   * Removing satisfied roles from the sorter.
-      if (roleConsumedResources.contains(quotas[role].guarantee())) {
+      if (roleConsumedResources.contains(quotas[role].info.guarantee())) {
         break;
       }
 
@@ -1226,11 +1224,11 @@ void HierarchicalAllocatorProcess::allocate(
   // Frameworks in a quota'ed role may temporarily reject resources by
   // filtering or suppressing offers. Hence quotas may not be fully allocated.
   Resources unallocatedQuotaResources;
-  foreachpair (const string& name, const QuotaInfo& quota, quotas) {
+  foreachpair (const string& name, const Quota& quota, quotas) {
     // Compute the amount of quota that the role does not have allocated.
     // NOTE: Reserved and revocable resources are excluded in `quotaRoleSorter`.
     Resources allocated = Resources::sum(quotaRoleSorter->allocation(name));
-    const Resources required = quota.guarantee();
+    const Resources required = quota.info.guarantee();
     unallocatedQuotaResources += (required - allocated);
   }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/b075f415/src/master/allocator/mesos/hierarchical.hpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.hpp b/src/master/allocator/mesos/hierarchical.hpp
index 5f08b6e..7fb9933 100644
--- a/src/master/allocator/mesos/hierarchical.hpp
+++ b/src/master/allocator/mesos/hierarchical.hpp
@@ -182,7 +182,7 @@ public:
 
   void setQuota(
       const std::string& role,
-      const mesos::quota::QuotaInfo& quota);
+      const Quota& quota);
 
   void removeQuota(
       const std::string& role);
@@ -378,7 +378,7 @@ protected:
   //
   // NOTE: We currently associate quota with roles, but this may
   // change in the future.
-  hashmap<std::string, mesos::quota::QuotaInfo> quotas;
+  hashmap<std::string, Quota> quotas;
 
   // Slaves to send offers for.
   Option<hashset<std::string>> whitelist;

http://git-wip-us.apache.org/repos/asf/mesos/blob/b075f415/src/master/quota_handler.cpp
----------------------------------------------------------------------
diff --git a/src/master/quota_handler.cpp b/src/master/quota_handler.cpp
index ace6537..f44736c 100644
--- a/src/master/quota_handler.cpp
+++ b/src/master/quota_handler.cpp
@@ -365,12 +365,14 @@ Future<http::Response> Master::QuotaHandler::_set(
     }
   }
 
+  Quota quota = Quota{quotaInfo};
+
   // Populate master's quota-related local state. We do this before updating
   // the registry in order to make sure that we are not already trying to
   // satisfy a request for this role (since this is a multi-phase event).
   // NOTE: We do not need to remove quota for the role if the registry update
   // fails because in this case the master fails as well.
-  master->quotas[quotaInfo.role()] = Quota{quotaInfo};
+  master->quotas[quotaInfo.role()] = quota;
 
   // Update the registry with the new quota and acknowledge the request.
   return master->registrar->apply(Owned<Operation>(
@@ -379,7 +381,7 @@ Future<http::Response> Master::QuotaHandler::_set(
       // See the top comment in "master/quota.hpp" for why this check is here.
       CHECK(result);
 
-      master->allocator->setQuota(quotaInfo.role(), quotaInfo);
+      master->allocator->setQuota(quotaInfo.role(), quota);
 
       // Rescind outstanding offers to facilitate satisfying the quota request.
       // NOTE: We set quota before we rescind to avoid a race. If we were to

http://git-wip-us.apache.org/repos/asf/mesos/blob/b075f415/src/tests/allocator.hpp
----------------------------------------------------------------------
diff --git a/src/tests/allocator.hpp b/src/tests/allocator.hpp
index 9bdfaec..206e9ac 100644
--- a/src/tests/allocator.hpp
+++ b/src/tests/allocator.hpp
@@ -444,7 +444,7 @@ public:
 
   MOCK_METHOD2(setQuota, void(
       const std::string&,
-      const mesos::quota::QuotaInfo&));
+      const Quota&));
 
   MOCK_METHOD1(removeQuota, void(
       const std::string&));

http://git-wip-us.apache.org/repos/asf/mesos/blob/b075f415/src/tests/hierarchical_allocator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/hierarchical_allocator_tests.cpp b/src/tests/hierarchical_allocator_tests.cpp
index 769b0e3..9362dd3 100644
--- a/src/tests/hierarchical_allocator_tests.cpp
+++ b/src/tests/hierarchical_allocator_tests.cpp
@@ -52,8 +52,6 @@ using mesos::internal::master::allocator::HierarchicalDRFAllocator;
 
 using mesos::master::allocator::Allocator;
 
-using mesos::quota::QuotaInfo;
-
 using process::Clock;
 using process::Future;
 
@@ -202,13 +200,13 @@ protected:
     return frameworkInfo;
   }
 
-  static QuotaInfo createQuotaInfo(const string& role, const string& resources)
+  static Quota createQuota(const string& role, const string& resources)
   {
-    QuotaInfo quota;
-    quota.set_role(role);
-    quota.mutable_guarantee()->CopyFrom(Resources::parse(resources).get());
+    mesos::quota::QuotaInfo quotaInfo;
+    quotaInfo.set_role(role);
+    quotaInfo.mutable_guarantee()->CopyFrom(Resources::parse(resources).get());
 
-    return quota;
+    return Quota{quotaInfo};
   }
 
   Resources createRevocableResources(
@@ -1204,7 +1202,7 @@ TEST_F(HierarchicalAllocatorTest, QuotaProvidesQuarantee)
   allocator->addFramework(
       framework1.id(), framework1, hashmap<SlaveID, Resources>());
 
-  const QuotaInfo quota1 = createQuotaInfo(QUOTA_ROLE, "cpus:2;mem:1024");
+  const Quota quota1 = createQuota(QUOTA_ROLE, "cpus:2;mem:1024");
   allocator->setQuota(QUOTA_ROLE, quota1);
 
   // Create `framework2` in a non-quota'ed role.
@@ -1330,7 +1328,7 @@ TEST_F(HierarchicalAllocatorTest, RemoveQuota)
   SlaveInfo agent1 = createSlaveInfo("cpus:1;mem:512;disk:0");
   SlaveInfo agent2 = createSlaveInfo("cpus:1;mem:512;disk:0");
 
-  const QuotaInfo quota1 = createQuotaInfo(QUOTA_ROLE, "cpus:2;mem:1024");
+  const Quota quota1 = createQuota(QUOTA_ROLE, "cpus:2;mem:1024");
 
   // Notify allocator of agents, frameworks, quota and current allocations.
   allocator->setQuota(QUOTA_ROLE, quota1);
@@ -1418,7 +1416,7 @@ TEST_F(HierarchicalAllocatorTest, MultipleFrameworksInRoleWithQuota)
   allocator->addFramework(
       framework1a.id(), framework1a, hashmap<SlaveID, Resources>());
 
-  const QuotaInfo quota1 = createQuotaInfo(QUOTA_ROLE, "cpus:4;mem:2048");
+  const Quota quota1 = createQuota(QUOTA_ROLE, "cpus:4;mem:2048");
   allocator->setQuota(QUOTA_ROLE, quota1);
 
   // Create `framework2` in a non-quota'ed role.
@@ -1539,7 +1537,7 @@ TEST_F(HierarchicalAllocatorTest, QuotaAllocationGranularity)
       framework1.id(), framework1, hashmap<SlaveID, Resources>());
 
   // Set quota to be less than the agent resources.
-  const QuotaInfo quota1 = createQuotaInfo(QUOTA_ROLE, "cpus:0.5;mem:200");
+  const Quota quota1 = createQuota(QUOTA_ROLE, "cpus:0.5;mem:200");
   allocator->setQuota(QUOTA_ROLE, quota1);
 
   // Create `framework2` in a non-quota'ed role.
@@ -1561,7 +1559,7 @@ TEST_F(HierarchicalAllocatorTest, QuotaAllocationGranularity)
   AWAIT_READY(allocation);
   EXPECT_EQ(framework1.id(), allocation.get().frameworkId);
   EXPECT_EQ(agent1.resources(), Resources::sum(allocation.get().resources));
-  EXPECT_TRUE(Resources(agent1.resources()).contains(quota1.guarantee()));
+  EXPECT_TRUE(Resources(agent1.resources()).contains(quota1.info.guarantee()));
 
   // Total cluster resources: cpus=1, mem=512.
   // QUOTA_ROLE share = 1 (cpus=1, mem=512) [quota: cpus=0.5, mem=200]
@@ -1598,7 +1596,7 @@ TEST_F(HierarchicalAllocatorTest, DRFWithQuota)
 
   SlaveInfo agent1 = createSlaveInfo("cpus:1;mem:512;disk:0");
 
-  const QuotaInfo quota1 = createQuotaInfo(QUOTA_ROLE, "cpus:0.25;mem:128");
+  const Quota quota1 = createQuota(QUOTA_ROLE, "cpus:0.25;mem:128");
 
   // Notify allocator of agents, frameworks, quota and current allocations.
   allocator->setQuota(QUOTA_ROLE, quota1);
@@ -1624,7 +1622,7 @@ TEST_F(HierarchicalAllocatorTest, DRFWithQuota)
       agent1,
       None(),
       agent1.resources(),
-      {std::make_pair(framework1.id(), Resources(quota1.guarantee()))});
+      {std::make_pair(framework1.id(), Resources(quota1.info.guarantee()))});
 
   // Some resources on `agent1` are now being used by `framework1` as part
   // of its role quota. All quotas are satisfied, all available resources
@@ -1635,7 +1633,7 @@ TEST_F(HierarchicalAllocatorTest, DRFWithQuota)
   Future<Allocation> allocation = allocations.get();
   AWAIT_READY(allocation);
   EXPECT_EQ(framework2.id(), allocation.get().frameworkId);
-  EXPECT_EQ(agent1.resources() - Resources(quota1.guarantee()),
+  EXPECT_EQ(agent1.resources() - Resources(quota1.info.guarantee()),
             Resources::sum(allocation.get().resources));
 
   // Total cluster resources (1 agent): cpus=1, mem=512.
@@ -1777,7 +1775,7 @@ TEST_F(HierarchicalAllocatorTest, QuotaAgainstStarvation)
       filter0s);
 
   // We set quota for the "starving" `QUOTA_ROLE` role.
-  QuotaInfo quota1 = createQuotaInfo(QUOTA_ROLE, "cpus:2;mem:1024");
+  Quota quota1 = createQuota(QUOTA_ROLE, "cpus:2;mem:1024");
   allocator->setQuota(QUOTA_ROLE, quota1);
 
   // Since `QUOTA_ROLE` is under quota, `agent2`'s resources will
@@ -1816,7 +1814,7 @@ TEST_F(HierarchicalAllocatorTest, QuotaAbsentFramework)
   allocator->addSlave(agent2.id(), agent2, None(), agent2.resources(), EMPTY);
 
   // Set quota for the quota'ed role.
-  const QuotaInfo quota1 = createQuotaInfo(QUOTA_ROLE, "cpus:2;mem:1024");
+  const Quota quota1 = createQuota(QUOTA_ROLE, "cpus:2;mem:1024");
   allocator->setQuota(QUOTA_ROLE, quota1);
 
   // Add `framework1` in the non-quota'ed role.

http://git-wip-us.apache.org/repos/asf/mesos/blob/b075f415/src/tests/master_quota_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_quota_tests.cpp b/src/tests/master_quota_tests.cpp
index 9d390d2..e8cb074 100644
--- a/src/tests/master_quota_tests.cpp
+++ b/src/tests/master_quota_tests.cpp
@@ -47,9 +47,7 @@ using mesos::internal::master::Master;
 
 using mesos::internal::slave::Slave;
 
-using mesos::quota::QuotaInfo;
 using mesos::quota::QuotaStatus;
-
 using process::Future;
 using process::PID;
 
@@ -741,7 +739,7 @@ TEST_F(MasterQuotaTest, AvailableResourcesSingleAgent)
   EXPECT_TRUE(agentTotalResources.get().contains(quotaResources));
 
   // Send a quota request for the specified role.
-  Future<QuotaInfo> receivedQuotaRequest;
+  Future<Quota> receivedQuotaRequest;
   EXPECT_CALL(allocator, setQuota(Eq(ROLE1), _))
     .WillOnce(DoAll(InvokeSetQuota(&allocator),
                     FutureArg<1>(&receivedQuotaRequest)));
@@ -758,8 +756,8 @@ TEST_F(MasterQuotaTest, AvailableResourcesSingleAgent)
   // got lost in-between.
   AWAIT_READY(receivedQuotaRequest);
 
-  EXPECT_EQ(ROLE1, receivedQuotaRequest.get().role());
-  EXPECT_EQ(quotaResources, receivedQuotaRequest.get().guarantee());
+  EXPECT_EQ(ROLE1, receivedQuotaRequest.get().info.role());
+  EXPECT_EQ(quotaResources, receivedQuotaRequest.get().info.guarantee());
 
   Shutdown();
 }
@@ -810,7 +808,7 @@ TEST_F(MasterQuotaTest, AvailableResourcesMultipleAgents)
     });
 
   // Send a quota request for the specified role.
-  Future<QuotaInfo> receivedQuotaRequest;
+  Future<Quota> receivedQuotaRequest;
   EXPECT_CALL(allocator, setQuota(Eq(ROLE1), _))
     .WillOnce(DoAll(InvokeSetQuota(&allocator),
                     FutureArg<1>(&receivedQuotaRequest)));
@@ -827,8 +825,8 @@ TEST_F(MasterQuotaTest, AvailableResourcesMultipleAgents)
   // got lost in-between.
   AWAIT_READY(receivedQuotaRequest);
 
-  EXPECT_EQ(ROLE1, receivedQuotaRequest.get().role());
-  EXPECT_EQ(quotaResources, receivedQuotaRequest.get().guarantee());
+  EXPECT_EQ(ROLE1, receivedQuotaRequest.get().info.role());
+  EXPECT_EQ(quotaResources, receivedQuotaRequest.get().info.guarantee());
 
   Shutdown();
 }
@@ -958,7 +956,7 @@ TEST_F(MasterQuotaTest, AvailableResourcesAfterRescinding)
     .Times(2);
 
   // Send a quota request for the specified role.
-  Future<QuotaInfo> receivedQuotaRequest;
+  Future<Quota> receivedQuotaRequest;
   EXPECT_CALL(allocator, setQuota(Eq(ROLE2), _))
     .WillOnce(DoAll(InvokeSetQuota(&allocator),
                     FutureArg<1>(&receivedQuotaRequest)));
@@ -985,8 +983,8 @@ TEST_F(MasterQuotaTest, AvailableResourcesAfterRescinding)
   // The quota request is granted and reached the allocator. Make sure nothing
   // got lost in-between.
   AWAIT_READY(receivedQuotaRequest);
-  EXPECT_EQ(ROLE2, receivedQuotaRequest.get().role());
-  EXPECT_EQ(quotaResources, receivedQuotaRequest.get().guarantee());
+  EXPECT_EQ(ROLE2, receivedQuotaRequest.get().info.role());
+  EXPECT_EQ(quotaResources, receivedQuotaRequest.get().info.guarantee());
 
   Shutdown();
 }
@@ -1052,7 +1050,7 @@ TEST_F(MasterQuotaTest, NoAuthenticationNoAuthorization)
   {
     Resources quotaResources = Resources::parse("cpus:1;mem:512").get();
 
-    Future<QuotaInfo> receivedSetRequest;
+    Future<Quota> receivedSetRequest;
     EXPECT_CALL(allocator, setQuota(Eq(ROLE1), _))
       .WillOnce(DoAll(InvokeSetQuota(&allocator),
                       FutureArg<1>(&receivedSetRequest)));
@@ -1202,10 +1200,10 @@ TEST_F(MasterQuotaTest, AuthorizeQuotaRequests)
     // request below to override the capacity heuristic check.
     Resources quotaResources = Resources::parse("cpus:1;mem:512").get();
 
-    Future<QuotaInfo> quotaInfo;
+  Future<Quota> quota;
     EXPECT_CALL(allocator, setQuota(Eq(ROLE1), _))
       .WillOnce(DoAll(InvokeSetQuota(&allocator),
-                      FutureArg<1>(&quotaInfo)));
+                    FutureArg<1>(&quota)));
 
     Future<Response> response = process::http::post(
         master.get(),
@@ -1216,16 +1214,16 @@ TEST_F(MasterQuotaTest, AuthorizeQuotaRequests)
     AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response)
       << response.get().body;
 
-    AWAIT_READY(quotaInfo);
+  AWAIT_READY(quota);
 
     // Extract the principal from `DEFAULT_CREDENTIAL` because `EXPECT_EQ`
     // does not compile if `DEFAULT_CREDENTIAL.principal()` is used as an
     // argument.
     const string principal = DEFAULT_CREDENTIAL.principal();
 
-    EXPECT_EQ(ROLE1, quotaInfo.get().role());
-    EXPECT_EQ(principal, quotaInfo.get().principal());
-    EXPECT_EQ(quotaResources, quotaInfo.get().guarantee());
+    EXPECT_EQ(ROLE1, quota.get().info.role());
+    EXPECT_EQ(principal, quota.get().info.principal());
+    EXPECT_EQ(quotaResources, quota.get().info.guarantee());
   }
 
   // Try to remove the previously requested quota using a principal that is