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 8eba78cbddc8b70f78c07a501ee0dc1d6204f280 Author: Meng Zhu <[email protected]> AuthorDate: Thu Jun 20 17:29:28 2019 -0700 Replaced `Quota` with `Quota2` in the master state. This paves way to remove `struct Quota`. Review: https://reviews.apache.org/r/70916 --- src/master/master.cpp | 9 ++---- src/master/master.hpp | 4 +-- src/master/quota_handler.cpp | 68 ++++++++++++++++++++++++++++++++--------- src/master/readonly_handler.cpp | 5 +-- 4 files changed, 59 insertions(+), 27 deletions(-) diff --git a/src/master/master.cpp b/src/master/master.cpp index d15b2d9..163d774 100644 --- a/src/master/master.cpp +++ b/src/master/master.cpp @@ -1730,7 +1730,7 @@ Future<Nothing> Master::_recover(const Registry& registry) // Save the quotas for each role. foreach (const Registry::Quota& quota, registry.quotas()) { - quotas[quota.info().role()] = Quota{quota.info()}; + quotas[quota.info().role()] = Quota2{quota.info()}; } // We notify the allocator via the `recover()` call. This has to be @@ -1741,12 +1741,7 @@ Future<Nothing> Master::_recover(const Registry& registry) // until after it restores a view of the cluster state. int expectedAgentCount = registry.slaves().slaves().size(); - // TODO(mzhu): Remove this conversion once we replace `Quota` with `Quota2`. - hashmap<string, Quota2> quota2s; - foreachpair (const string& role, const Quota& quota, quotas) { - quota2s[role] = Quota2{quota.info}; - } - allocator->recover(expectedAgentCount, quota2s); + allocator->recover(expectedAgentCount, quotas); // TODO(alexr): Consider adding a sanity check: whether quotas are // satisfiable given all recovering agents reregister. We may want diff --git a/src/master/master.hpp b/src/master/master.hpp index 7acaa82..c966a93 100644 --- a/src/master/master.hpp +++ b/src/master/master.hpp @@ -1243,7 +1243,7 @@ private: // as well as reservations (both static and dynamic ones). static Option<Error> overcommitCheck( const std::vector<Resources>& agents, - const hashmap<std::string, Quota>& quotas, + const hashmap<std::string, Quota2>& quotas, const mesos::quota::QuotaInfo& request); // We always want to rescind offers after the capacity heuristic. The @@ -2239,7 +2239,7 @@ private: // Configured quota for each role, if any. We store quotas by role // because we set them at the role level. - hashmap<std::string, Quota> quotas; + hashmap<std::string, Quota2> quotas; // Authenticator names as supplied via flags. std::vector<std::string> authenticatorNames; diff --git a/src/master/quota_handler.cpp b/src/master/quota_handler.cpp index 854189c..0180b27 100644 --- a/src/master/quota_handler.cpp +++ b/src/master/quota_handler.cpp @@ -91,11 +91,11 @@ namespace master { class QuotaTree { public: - QuotaTree(const hashmap<string, Quota>& quotas) + QuotaTree(const hashmap<string, Quota2>& quotas) : root(new Node("")) { - foreachpair (const string& role, const Quota& quota, quotas) { - insert(role, Quota2(quota.info)); + foreachpair (const string& role, const Quota2& quota, quotas) { + insert(role, quota); } } @@ -197,15 +197,15 @@ private: Option<Error> Master::QuotaHandler::overcommitCheck( const vector<Resources>& agents, - const hashmap<string, Quota>& quotas, + const hashmap<string, Quota2>& quotas, const QuotaInfo& request) { ResourceQuantities totalGuarantees = [&]() { QuotaTree quotaTree({}); - foreachpair (const string& role, const Quota& quota, quotas) { + foreachpair (const string& role, const Quota2& quota, quotas) { if (role != request.role()) { - quotaTree.insert(role, Quota2(quota.info)); + quotaTree.insert(role, quota); } } @@ -370,8 +370,30 @@ Future<QuotaStatus> Master::QuotaHandler::_status( vector<QuotaInfo> quotaInfos; quotaInfos.reserve(master->quotas.size()); - foreachvalue (const Quota& quota, master->quotas) { - quotaInfos.push_back(quota.info); + foreachpair (const string& role, const Quota2& quota, master->quotas) { + quotaInfos.push_back([&role, "a]() { + // Construct the legacy `QuotaInfo`. + // + // This is needed for backwards compatibility reasons. + // Authorizable action `GET_QUOTA` expects an object + // with `QuotaInfo` set. + // + // TODO(mzhu): we plan to deprecate the use of `QuotaInfo` + // in the `GET_QUOTA` action. And instead, just set the value + // field in the object using the role. This legacy construction + // will be removed. + QuotaInfo info; + info.set_role(role); + foreach (auto& quantity, quota.guarantees) { + Resource resource; + resource.set_type(Value::SCALAR); + *resource.mutable_name() = quantity.first; + *resource.mutable_scalar() = quantity.second; + + *info.add_guarantee() = std::move(resource); + } + return info; + }()); } // Create a list of authorization actions for each role we may return. @@ -542,9 +564,9 @@ Future<http::Response> Master::QuotaHandler::_set( { QuotaTree quotaTree({}); - foreachpair (const string& role, const Quota& quota, master->quotas) { + foreachpair (const string& role, const Quota2& quota, master->quotas) { if (role != quotaInfo.role()) { - quotaTree.insert(role, Quota2(quota.info)); + quotaTree.insert(role, quota); } } @@ -626,7 +648,7 @@ Future<http::Response> Master::QuotaHandler::__set( } } - Quota quota = Quota{quotaInfo}; + Quota2 quota = Quota2{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 @@ -642,7 +664,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->updateQuota(quotaInfo.role(), Quota2{quota.info}); + master->allocator->updateQuota(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 @@ -710,7 +732,7 @@ Future<http::Response> Master::QuotaHandler::remove( "': Role '" + role + "' has no quota set"); } - hashmap<string, Quota> quotaMap = master->quotas; + hashmap<string, Quota2> quotaMap = master->quotas; // Validate that removing the quota for `role` does not violate the // hierarchical relationship between quotas. @@ -733,7 +755,25 @@ Future<http::Response> Master::QuotaHandler::_remove( const string& role, const Option<Principal>& principal) const { - return authorizeUpdateQuota(principal, master->quotas.at(role).info) + // Construct the legacy `QuotaInfo`. This is needed for backwards + // compatibility reasons. Authorizable action `UPDATE_QUOTA` which is + // used for `SET_QUOTA` and `REMOVE_QUOTA` expects an object with + // `QuotaInfo` set. The new API `UPDATE_QUOTA` uses a different + // action `UPDATE_QUOTA_WITH_CONFIG`. The old authorizable action + // and this legacy construction should be removed in Mesos 2.0 + // when we remove the old APIs. + QuotaInfo info; + info.set_role(role); + foreach (const auto& quantity, master->quotas.at(role).guarantees) { + Resource resource; + resource.set_type(Value::SCALAR); + *resource.mutable_name() = quantity.first; + *resource.mutable_scalar() = quantity.second; + + *info.add_guarantee() = std::move(resource); + } + + return authorizeUpdateQuota(principal, info) .then(defer(master->self(), [=](bool authorized) -> Future<http::Response> { return !authorized ? Forbidden() : __remove(role); })); diff --git a/src/master/readonly_handler.cpp b/src/master/readonly_handler.cpp index 694f3af..aa5be2c 100644 --- a/src/master/readonly_handler.cpp +++ b/src/master/readonly_handler.cpp @@ -724,10 +724,7 @@ process::http::Response Master::ReadOnlyHandler::roles( // // - We name the field using singular `guarantee` and `limit` // which is different from the plural used in `QuotaConfig`. - // - // TODO(mzhu): This conversion will not be needed if we - // store `Quota2` in master. - Quota2 quota{master->quotas.at(name).info}; + const Quota2& quota = master->quotas.at(name); writer->field("quota", [&](JSON::ObjectWriter* writer) { writer->field("role", name); writer->field("guarantee", quota.guarantees);
