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 4703b23143ee806ed5e68d9ff6eabe9600ffc9c9 Author: Meng Zhu <[email protected]> AuthorDate: Wed Jun 5 16:44:00 2019 -0700 Added `updateQuota` method to the allocator. This call updates a role's quota guarantees and limits. All roles have a default quota defined as `DEFAULT_QUOTA`. Currently, it is no guarantees and limits. Thus to "remove" a quota, one should simply update the quota to be `DEFAULT_QUOTA`. Master `setQuota` and `removeQuota` calls into the allocator are replaced with the `updateQuota`. `setQuota` and `removeQuota` calls are now only used in the tests. They will be removed once those tests are refactored. Also fixed affected tests. Review: https://reviews.apache.org/r/70803 --- include/mesos/allocator/allocator.hpp | 18 ++++ src/master/allocator/mesos/allocator.hpp | 21 +++++ src/master/allocator/mesos/hierarchical.cpp | 16 ++++ src/master/allocator/mesos/hierarchical.hpp | 4 + src/master/quota_handler.cpp | 4 +- src/tests/allocator.hpp | 15 ++++ src/tests/master_quota_tests.cpp | 122 ++++++++++++++-------------- 7 files changed, 136 insertions(+), 64 deletions(-) diff --git a/include/mesos/allocator/allocator.hpp b/include/mesos/allocator/allocator.hpp index eafcdfd..a01775b 100644 --- a/include/mesos/allocator/allocator.hpp +++ b/include/mesos/allocator/allocator.hpp @@ -430,6 +430,9 @@ public: * * TODO(alexr): Consider adding an `updateQuota()` method which allows * updating existing quota. + * + * TODO(mzhu): Remove this call in favor of `updateQuota`. + * */ virtual void setQuota( const std::string& role, @@ -448,11 +451,26 @@ public: * * TODO(alexr): Consider returning a future which an allocator can fail in * order to report failure. + * + * TODO(mzhu): Remove this call in favor of `updateQuota`. */ virtual void removeQuota( const std::string& role) = 0; /** + * Informs the allocator to update quota for the given role. + * + * It is up to the allocator implementation how to satisfy quota. An + * implementation may employ different strategies for roles with or + * without quota. All roles have a default quota defined as `DEFAULT_QUOTA`. + * Currently, it is no guarantees and no limits. Thus to "remove" a quota, + * one should simply update the quota to be `DEFAULT_QUOTA`. + */ + virtual void updateQuota( + const std::string& role, + const Quota2& quota) = 0; + + /** * Updates the weight associated with one or more roles. If a role * was previously configured to have a weight and that role is * omitted from this list, it keeps its old weight. diff --git a/src/master/allocator/mesos/allocator.hpp b/src/master/allocator/mesos/allocator.hpp index 2d83f38..3c04b4f 100644 --- a/src/master/allocator/mesos/allocator.hpp +++ b/src/master/allocator/mesos/allocator.hpp @@ -165,6 +165,10 @@ public: void removeQuota( const std::string& role) override; + void updateQuota( + const std::string& role, + const Quota2& quota) override; + void updateWeights( const std::vector<WeightInfo>& weightInfos) override; @@ -312,6 +316,10 @@ public: virtual void removeQuota( const std::string& role) = 0; + virtual void updateQuota( + const std::string& role, + const Quota2& quota) = 0; + virtual void updateWeights( const std::vector<WeightInfo>& weightInfos) = 0; @@ -701,6 +709,19 @@ inline void MesosAllocator<AllocatorProcess>::removeQuota( template <typename AllocatorProcess> +inline void MesosAllocator<AllocatorProcess>::updateQuota( + const std::string& role, + const Quota2& quota) +{ + process::dispatch( + process, + &MesosAllocatorProcess::updateQuota, + role, + quota); +} + + +template <typename AllocatorProcess> inline void MesosAllocator<AllocatorProcess>::updateWeights( const std::vector<WeightInfo>& weightInfos) { diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp index a7c237e..50a4982 100644 --- a/src/master/allocator/mesos/hierarchical.cpp +++ b/src/master/allocator/mesos/hierarchical.cpp @@ -1463,6 +1463,22 @@ void HierarchicalAllocatorProcess::removeQuota( } +void HierarchicalAllocatorProcess::updateQuota( + const string& role, + const Quota2& quota) +{ + CHECK(initialized); + + roles[role].quota = quota; + + metrics.updateQuota(role, quota); + + LOG(INFO) << "Updated quota for role '" << role << "', " + << " guarantees: " << quota.guarantees + << " limits: " << quota.limits; +} + + void HierarchicalAllocatorProcess::updateWeights( const vector<WeightInfo>& weightInfos) { diff --git a/src/master/allocator/mesos/hierarchical.hpp b/src/master/allocator/mesos/hierarchical.hpp index 0a6482a..93555f6 100644 --- a/src/master/allocator/mesos/hierarchical.hpp +++ b/src/master/allocator/mesos/hierarchical.hpp @@ -429,6 +429,10 @@ public: void removeQuota( const std::string& role) override; + void updateQuota( + const std::string& role, + const Quota2& quota) override; + void updateWeights( const std::vector<WeightInfo>& weightInfos) override; diff --git a/src/master/quota_handler.cpp b/src/master/quota_handler.cpp index b51e663..854189c 100644 --- a/src/master/quota_handler.cpp +++ b/src/master/quota_handler.cpp @@ -642,7 +642,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(), quota); + master->allocator->updateQuota(quotaInfo.role(), Quota2{quota.info}); // Rescind outstanding offers to facilitate satisfying the quota request. // NOTE: We set quota before we rescind to avoid a race. If we were to @@ -764,7 +764,7 @@ Future<http::Response> Master::QuotaHandler::__remove(const string& role) const // See the top comment in "master/quota.hpp" for why this check is here. CHECK(result); - master->allocator->removeQuota(role); + master->allocator->updateQuota(role, DEFAULT_QUOTA); return OK(); })); diff --git a/src/tests/allocator.hpp b/src/tests/allocator.hpp index 0718bef..bc6e3a9 100644 --- a/src/tests/allocator.hpp +++ b/src/tests/allocator.hpp @@ -212,6 +212,12 @@ ACTION_P(InvokeRemoveQuota, allocator) } +ACTION_P(InvokeUpdateQuota, allocator) +{ + allocator->real->updateQuota(arg0, arg1); +} + + ACTION_P(InvokeUpdateWeights, allocator) { allocator->real->updateWeights(arg0); @@ -382,6 +388,11 @@ public: EXPECT_CALL(*this, removeQuota(_)) .WillRepeatedly(DoDefault()); + ON_CALL(*this, updateQuota(_, _)) + .WillByDefault(InvokeUpdateQuota(this)); + EXPECT_CALL(*this, updateQuota(_, _)) + .WillRepeatedly(DoDefault()); + ON_CALL(*this, updateWeights(_)) .WillByDefault(InvokeUpdateWeights(this)); EXPECT_CALL(*this, updateWeights(_)) @@ -516,6 +527,10 @@ public: MOCK_METHOD1(removeQuota, void( const std::string&)); + MOCK_METHOD2(updateQuota, void( + const std::string&, + const Quota2&)); + MOCK_METHOD1(updateWeights, void( const std::vector<WeightInfo>&)); diff --git a/src/tests/master_quota_tests.cpp b/src/tests/master_quota_tests.cpp index 146b977..9b4cde1 100644 --- a/src/tests/master_quota_tests.cpp +++ b/src/tests/master_quota_tests.cpp @@ -39,6 +39,7 @@ #include "common/resources_utils.hpp" +#include "master/constants.hpp" #include "master/flags.hpp" #include "master/master.hpp" @@ -487,17 +488,15 @@ TEST_F(MasterQuotaTest, RemoveSingleQuota) EXPECT_EQ(1, metrics.values[metricKey]); // Remove the previously requested quota. - Future<Nothing> receivedRemoveRequest; - EXPECT_CALL(allocator, removeQuota(Eq(ROLE1))) - .WillOnce(DoAll(InvokeRemoveQuota(&allocator), - FutureSatisfy(&receivedRemoveRequest))); + Future<Nothing> receivedQuotaRequest; + EXPECT_CALL(allocator, updateQuota(Eq(ROLE1), Eq(master::DEFAULT_QUOTA))) + .WillOnce(DoAll( + InvokeUpdateQuota(&allocator), FutureSatisfy(&receivedQuotaRequest))); response = removeQuota("quota/" + ROLE1); AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response); - - // Ensure that the quota remove request has reached the allocator. - AWAIT_READY(receivedRemoveRequest); + AWAIT_READY(receivedQuotaRequest); // Ensure metrics update is finished. Clock::settle(); @@ -840,9 +839,9 @@ TEST_F(MasterQuotaTest, AvailableResourcesSingleAgent) EXPECT_TRUE(agentTotalResources->contains(quotaResources)); // Send a quota request for the specified role. - Future<Quota> receivedQuotaRequest; - EXPECT_CALL(allocator, setQuota(Eq(ROLE1), _)) - .WillOnce(DoAll(InvokeSetQuota(&allocator), + Future<Quota2> receivedQuotaRequest; + EXPECT_CALL(allocator, updateQuota(Eq(ROLE1), _)) + .WillOnce(DoAll(InvokeUpdateQuota(&allocator), FutureArg<1>(&receivedQuotaRequest))); Future<Response> response = process::http::post( @@ -857,8 +856,9 @@ TEST_F(MasterQuotaTest, AvailableResourcesSingleAgent) // got lost in-between. AWAIT_READY(receivedQuotaRequest); - EXPECT_EQ(ROLE1, receivedQuotaRequest->info.role()); - EXPECT_EQ(quotaResources, receivedQuotaRequest->info.guarantee()); + EXPECT_EQ( + ResourceQuantities::fromScalarResources(quotaResources), + receivedQuotaRequest->guarantees); } @@ -894,9 +894,9 @@ TEST_F(MasterQuotaTest, AvailableResourcesSingleReservedAgent) Resources quotaResources = Resources::parse("cpus:1;mem:512").get(); // Send a quota request for the specified role. - Future<Quota> receivedQuotaRequest; - EXPECT_CALL(allocator, setQuota(Eq(ROLE1), _)) - .WillOnce(DoAll(InvokeSetQuota(&allocator), + Future<Quota2> receivedQuotaRequest; + EXPECT_CALL(allocator, updateQuota(Eq(ROLE1), _)) + .WillOnce(DoAll(InvokeUpdateQuota(&allocator), FutureArg<1>(&receivedQuotaRequest))); Future<Response> response = process::http::post( @@ -911,8 +911,9 @@ TEST_F(MasterQuotaTest, AvailableResourcesSingleReservedAgent) // got lost in-between. AWAIT_READY(receivedQuotaRequest); - EXPECT_EQ(ROLE1, receivedQuotaRequest->info.role()); - EXPECT_EQ(quotaResources, receivedQuotaRequest->info.guarantee()); + EXPECT_EQ( + ResourceQuantities::fromScalarResources(quotaResources), + receivedQuotaRequest->guarantees); } @@ -960,9 +961,9 @@ TEST_F(MasterQuotaTest, AvailableResourcesSingleDisconnectedAgent) EXPECT_TRUE(agentTotalResources->contains(quotaResources)); // Send a quota request for the specified role. - Future<Quota> receivedQuotaRequest; - EXPECT_CALL(allocator, setQuota(Eq(ROLE1), _)) - .WillOnce(DoAll(InvokeSetQuota(&allocator), + Future<Quota2> receivedQuotaRequest; + EXPECT_CALL(allocator, updateQuota(Eq(ROLE1), _)) + .WillOnce(DoAll(InvokeUpdateQuota(&allocator), FutureArg<1>(&receivedQuotaRequest))); Future<Response> response = process::http::post( @@ -977,8 +978,9 @@ TEST_F(MasterQuotaTest, AvailableResourcesSingleDisconnectedAgent) // got lost in-between. AWAIT_READY(receivedQuotaRequest); - EXPECT_EQ(ROLE1, receivedQuotaRequest->info.role()); - EXPECT_EQ(quotaResources, receivedQuotaRequest->info.guarantee()); + EXPECT_EQ( + ResourceQuantities::fromScalarResources(quotaResources), + receivedQuotaRequest->guarantees); } @@ -1028,9 +1030,9 @@ TEST_F(MasterQuotaTest, AvailableResourcesMultipleAgents) }); // Send a quota request for the specified role. - Future<Quota> receivedQuotaRequest; - EXPECT_CALL(allocator, setQuota(Eq(ROLE1), _)) - .WillOnce(DoAll(InvokeSetQuota(&allocator), + Future<Quota2> receivedQuotaRequest; + EXPECT_CALL(allocator, updateQuota(Eq(ROLE1), _)) + .WillOnce(DoAll(InvokeUpdateQuota(&allocator), FutureArg<1>(&receivedQuotaRequest))); Future<Response> response = process::http::post( @@ -1044,9 +1046,9 @@ TEST_F(MasterQuotaTest, AvailableResourcesMultipleAgents) // Quota request is granted and reached the allocator. Make sure nothing // got lost in-between. AWAIT_READY(receivedQuotaRequest); - - EXPECT_EQ(ROLE1, receivedQuotaRequest->info.role()); - EXPECT_EQ(quotaResources, receivedQuotaRequest->info.guarantee()); + EXPECT_EQ( + ResourceQuantities::fromScalarResources(quotaResources), + receivedQuotaRequest->guarantees); } @@ -1188,9 +1190,9 @@ TEST_F(MasterQuotaTest, AvailableResourcesAfterRescinding) .WillRepeatedly(Return()); // Send a quota request for the specified role. - Future<Quota> receivedQuotaRequest; - EXPECT_CALL(allocator, setQuota(Eq(ROLE2), _)) - .WillOnce(DoAll(InvokeSetQuota(&allocator), + Future<Quota2> receivedQuotaRequest; + EXPECT_CALL(allocator, updateQuota(Eq(ROLE2), _)) + .WillOnce(DoAll(InvokeUpdateQuota(&allocator), FutureArg<1>(&receivedQuotaRequest))); Future<Response> response = process::http::post( @@ -1215,8 +1217,9 @@ 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->info.role()); - EXPECT_EQ(quotaResources, receivedQuotaRequest->info.guarantee()); + EXPECT_EQ( + ResourceQuantities::fromScalarResources(quotaResources), + receivedQuotaRequest->guarantees); // Ensure `RescindResourceOfferMessage`s are processed by `sched1`. AWAIT_READY(offerRescinded1); @@ -1300,10 +1303,10 @@ TEST_F(MasterQuotaTest, RecoverQuotaEmptyCluster) // Delete quota. { - Future<Nothing> receivedRemoveRequest; - EXPECT_CALL(allocator, removeQuota(Eq(ROLE1))) - .WillOnce(DoAll(InvokeRemoveQuota(&allocator), - FutureSatisfy(&receivedRemoveRequest))); + Future<Nothing> receivedQuotaRequest; + EXPECT_CALL(allocator, updateQuota(Eq(ROLE1), Eq(master::DEFAULT_QUOTA))) + .WillOnce(DoAll( + InvokeUpdateQuota(&allocator), FutureSatisfy(&receivedQuotaRequest))); Future<Response> response = process::http::requestDelete( master.get()->pid, @@ -1311,7 +1314,7 @@ TEST_F(MasterQuotaTest, RecoverQuotaEmptyCluster) createBasicAuthHeaders(DEFAULT_CREDENTIAL)); // Quota request succeeds and reaches the allocator. - AWAIT_READY(receivedRemoveRequest); + AWAIT_READY(receivedQuotaRequest); AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response); } } @@ -1345,9 +1348,9 @@ TEST_F(MasterQuotaTest, NoAuthenticationNoAuthorization) { Resources quotaResources = Resources::parse("cpus:1;mem:512").get(); - Future<Quota> receivedSetRequest; - EXPECT_CALL(allocator, setQuota(Eq(ROLE1), _)) - .WillOnce(DoAll(InvokeSetQuota(&allocator), + Future<Quota2> receivedSetRequest; + EXPECT_CALL(allocator, updateQuota(Eq(ROLE1), _)) + .WillOnce(DoAll(InvokeUpdateQuota(&allocator), FutureArg<1>(&receivedSetRequest))); // Send a set quota request with absent credentials. @@ -1364,10 +1367,10 @@ TEST_F(MasterQuotaTest, NoAuthenticationNoAuthorization) // Check whether quota can be removed. { - Future<Nothing> receivedRemoveRequest; - EXPECT_CALL(allocator, removeQuota(Eq(ROLE1))) - .WillOnce(DoAll(InvokeRemoveQuota(&allocator), - FutureSatisfy(&receivedRemoveRequest))); + Future<Nothing> receivedQuotaRequest; + EXPECT_CALL(allocator, updateQuota(Eq(ROLE1), Eq(master::DEFAULT_QUOTA))) + .WillOnce(DoAll( + InvokeUpdateQuota(&allocator), FutureSatisfy(&receivedQuotaRequest))); // Send a remove quota request with absent credentials. Future<Response> response = process::http::requestDelete( @@ -1376,7 +1379,7 @@ TEST_F(MasterQuotaTest, NoAuthenticationNoAuthorization) None()); // Quota request succeeds and reaches the allocator. - AWAIT_READY(receivedRemoveRequest); + AWAIT_READY(receivedQuotaRequest); AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response); } } @@ -1487,9 +1490,9 @@ TEST_F(MasterQuotaTest, AuthorizeGetUpdateQuotaRequests) // request below to override the capacity heuristic check. Resources quotaResources = Resources::parse("cpus:1;mem:512").get(); - Future<Quota> quota; - EXPECT_CALL(allocator, setQuota(Eq(ROLE1), _)) - .WillOnce(DoAll(InvokeSetQuota(&allocator), + Future<Quota2> quota; + EXPECT_CALL(allocator, updateQuota(Eq(ROLE1), _)) + .WillOnce(DoAll(InvokeUpdateQuota(&allocator), FutureArg<1>("a))); Future<Response> response = process::http::post( @@ -1502,14 +1505,9 @@ TEST_F(MasterQuotaTest, AuthorizeGetUpdateQuotaRequests) 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, quota->info.role()); - EXPECT_EQ(principal, quota->info.principal()); - EXPECT_EQ(quotaResources, quota->info.guarantee()); + EXPECT_EQ( + ResourceQuantities::fromScalarResources(quotaResources), + quota->guarantees); } // Try to get the previously requested quota using a principal that is @@ -1582,10 +1580,10 @@ TEST_F(MasterQuotaTest, AuthorizeGetUpdateQuotaRequests) // Remove the previously requested quota using the default principal. { - Future<Nothing> receivedRemoveRequest; - EXPECT_CALL(allocator, removeQuota(Eq(ROLE1))) - .WillOnce(DoAll(InvokeRemoveQuota(&allocator), - FutureSatisfy(&receivedRemoveRequest))); + Future<Nothing> receivedQuotaRequest; + EXPECT_CALL(allocator, updateQuota(Eq(ROLE1), Eq(master::DEFAULT_QUOTA))) + .WillOnce(DoAll( + InvokeUpdateQuota(&allocator), FutureSatisfy(&receivedQuotaRequest))); Future<Response> response = process::http::requestDelete( master.get()->pid, @@ -1594,7 +1592,7 @@ TEST_F(MasterQuotaTest, AuthorizeGetUpdateQuotaRequests) AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response); - AWAIT_READY(receivedRemoveRequest); + AWAIT_READY(receivedQuotaRequest); } }
