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/12/13 22:13:56 UTC
[2/3] mesos git commit: Removed resource categories in
UpdateSlaveMessage.
Removed resource categories in UpdateSlaveMessage.
Given that now we use `UpdateSlaveMessage` to send resource provider
information directly, having resource categories in the message is
unnecessary and misleading.
Instead, this patch introduced a single optional boolean to indicate if
oversubscribed resources need to be updated or not.
Review: https://reviews.apache.org/r/64561
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/da71f588
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/da71f588
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/da71f588
Branch: refs/heads/master
Commit: da71f5881a6b7e06dbc4a64837583f559fb78754
Parents: 00a96a3
Author: Jie Yu <yu...@gmail.com>
Authored: Tue Dec 12 16:53:52 2017 -0800
Committer: Jie Yu <yu...@gmail.com>
Committed: Wed Dec 13 14:13:36 2017 -0800
----------------------------------------------------------------------
src/master/master.cpp | 11 ++++----
src/messages/messages.proto | 19 +++++---------
src/slave/slave.cpp | 43 +++++++++++--------------------
src/slave/slave.hpp | 8 +++---
src/tests/oversubscription_tests.cpp | 20 ++++++--------
5 files changed, 39 insertions(+), 62 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/da71f588/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 806fbc2..34ae82d 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -7246,13 +7246,12 @@ void Master::updateSlave(const UpdateSlaveMessage& message)
// updating the agent in the allocator. This would lead us to
// re-send out the stale oversubscribed resources!
- // If the caller did not specify a resource category we assume we should set
- // `oversubscribedResources` to be backwards-compatibility with older clients.
+ // If agent does not specify the `update_oversubscribed_resources`
+ // field, we assume we should set `oversubscribedResources` to be
+ // backwards-compatibility with older agents (version < 1.5).
const bool hasOversubscribed =
- (message.has_resource_categories() &&
- message.resource_categories().has_oversubscribed() &&
- message.resource_categories().oversubscribed()) ||
- !message.has_resource_categories();
+ !message.has_update_oversubscribed_resources() ||
+ message.update_oversubscribed_resources();
Option<Resources> newOversubscribed;
http://git-wip-us.apache.org/repos/asf/mesos/blob/da71f588/src/messages/messages.proto
----------------------------------------------------------------------
diff --git a/src/messages/messages.proto b/src/messages/messages.proto
index e680cd5..b8eb8fa 100644
--- a/src/messages/messages.proto
+++ b/src/messages/messages.proto
@@ -725,18 +725,13 @@ message UpdateSlaveMessage {
// TODO(bbannier): Consider passing agent information inside a
// `ResourceProvider` value as well where applicable.
- // This message can contain `oversubscribed_resources` or resource
- // providers. Callers are expected to set the `oversubscribed`
- // category in `resource_categories` to denote whether the
- // `oversubscribed_resources` field should be examined. For
- // backwards compatibility we interpret an unset `category` field as
- // if only oversubscribed was set.
- message ResourceCategories {
- optional bool oversubscribed = 2;
- }
-
- optional ResourceCategories resource_categories = 5;
-
+ // Whether to update oversubscribed resources or not. If we just use
+ // `oversubscribed_resources`, we don't have a way to tell if the
+ // intention is to update the oversubscribed resoruces to empty, or
+ // leave it unchanged. For backwards compatibility, if this field is
+ // unset (version < 1.5), we treat that as if only
+ // `oversubscribed_resources` was set.
+ optional bool update_oversubscribed_resources = 5;
repeated Resource oversubscribed_resources = 2;
message OfferOperations {
http://git-wip-us.apache.org/repos/asf/mesos/blob/da71f588/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 15bdd8f..e8f7591 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -6976,17 +6976,10 @@ void Slave::_forwardOversubscribed(const Future<Resources>& oversubscribable)
// Add oversubscribable resources to the total.
oversubscribed += oversubscribable.get();
- // Remember the previous amount of oversubscribed resources.
- const Option<Resources> previousOversubscribedResources =
- oversubscribedResources;
-
- // Update the estimate.
- oversubscribedResources = oversubscribed;
-
// Only forward the estimate if it's different from the previous
// estimate. We also send this whenever we get (re-)registered
// (i.e. whenever we transition into the RUNNING state).
- if (state == RUNNING && previousOversubscribedResources != oversubscribed) {
+ if (state == RUNNING && oversubscribedResources != oversubscribed) {
LOG(INFO) << "Forwarding total oversubscribed resources "
<< oversubscribed;
@@ -6999,11 +6992,17 @@ void Slave::_forwardOversubscribed(const Future<Resources>& oversubscribable)
// TODO(bbannier): Revisit this if we modify the operations
// possible on oversubscribed resources.
- UpdateSlaveMessage message = generateOversubscribedUpdate();
+ UpdateSlaveMessage message;
+ message.mutable_slave_id()->CopyFrom(info.id());
+ message.set_update_oversubscribed_resources(true);
+ message.mutable_oversubscribed_resources()->CopyFrom(oversubscribed);
CHECK_SOME(master);
send(master.get(), message);
}
+
+ // Update the estimate.
+ oversubscribedResources = oversubscribed;
}
delay(flags.oversubscribed_resources_interval,
@@ -7012,22 +7011,6 @@ void Slave::_forwardOversubscribed(const Future<Resources>& oversubscribable)
}
-UpdateSlaveMessage Slave::generateOversubscribedUpdate() const
-{
- UpdateSlaveMessage message;
-
- message.mutable_slave_id()->CopyFrom(info.id());
- message.mutable_resource_categories()->set_oversubscribed(true);
-
- if (oversubscribedResources.isSome()) {
- message.mutable_oversubscribed_resources()->CopyFrom(
- oversubscribedResources.get());
- }
-
- return message;
-}
-
-
UpdateSlaveMessage Slave::generateResourceProviderUpdate() const
{
// Agent information (total resources, offer operations, resource
@@ -7037,6 +7020,7 @@ UpdateSlaveMessage Slave::generateResourceProviderUpdate() const
// TODO(bbannier): Pass agent information as a resource provider.
UpdateSlaveMessage message;
message.mutable_slave_id()->CopyFrom(info.id());
+ message.set_update_oversubscribed_resources(false);
message.set_resource_version_uuid(resourceVersion.toBytes());
message.mutable_offer_operations();
@@ -7076,10 +7060,13 @@ UpdateSlaveMessage Slave::generateResourceProviderUpdate() const
UpdateSlaveMessage Slave::generateUpdateSlaveMessage() const
{
- UpdateSlaveMessage message;
+ UpdateSlaveMessage message = generateResourceProviderUpdate();
- message.MergeFrom(generateResourceProviderUpdate());
- message.MergeFrom(generateOversubscribedUpdate());
+ if (oversubscribedResources.isSome()) {
+ message.set_update_oversubscribed_resources(true);
+ message.mutable_oversubscribed_resources()->CopyFrom(
+ oversubscribedResources.get());
+ }
return message;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/da71f588/src/slave/slave.hpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.hpp b/src/slave/slave.hpp
index 7c40fc7..de2b2e2 100644
--- a/src/slave/slave.hpp
+++ b/src/slave/slave.hpp
@@ -556,10 +556,10 @@ private:
void _forwardOversubscribed(
const process::Future<Resources>& oversubscribable);
- // Helper functions to generate `UpdateSlaveMessage` for either just
- // updates to oversubscribed resources, resource provider-related
- // information, or both.
- UpdateSlaveMessage generateOversubscribedUpdate() const;
+ // Helper functions to generate `UpdateSlaveMessage` for either
+ // just updates to resource provider-related information, or both
+ // resource provider-related information and oversubscribed
+ // resources.
UpdateSlaveMessage generateResourceProviderUpdate() const;
UpdateSlaveMessage generateUpdateSlaveMessage() const;
http://git-wip-us.apache.org/repos/asf/mesos/blob/da71f588/src/tests/oversubscription_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/oversubscription_tests.cpp b/src/tests/oversubscription_tests.cpp
index 3f57ce1..e6139b0 100644
--- a/src/tests/oversubscription_tests.cpp
+++ b/src/tests/oversubscription_tests.cpp
@@ -323,9 +323,8 @@ TEST_F(OversubscriptionTest, ForwardUpdateSlaveMessage)
AWAIT_READY(update);
- EXPECT_TRUE(update->has_resource_categories());
- EXPECT_TRUE(update->resource_categories().has_oversubscribed());
- EXPECT_TRUE(update->resource_categories().oversubscribed());
+ EXPECT_TRUE(update->has_update_oversubscribed_resources());
+ EXPECT_TRUE(update->update_oversubscribed_resources());
EXPECT_EQ(update->oversubscribed_resources(), resources);
// Ensure the metric is updated.
@@ -698,9 +697,8 @@ TEST_F(OversubscriptionTest, FixedResourceEstimator)
Clock::settle();
AWAIT_READY(update);
- ASSERT_TRUE(update->has_resource_categories());
- ASSERT_TRUE(update->resource_categories().has_oversubscribed());
- ASSERT_TRUE(update->resource_categories().oversubscribed());
+ ASSERT_TRUE(update->has_update_oversubscribed_resources());
+ ASSERT_TRUE(update->update_oversubscribed_resources());
Resources resources = update->oversubscribed_resources();
EXPECT_SOME_EQ(2.0, resources.cpus());
@@ -902,9 +900,8 @@ TEST_F(OversubscriptionTest, Reregistration)
Clock::advance(agentFlags.registration_backoff_factor);
AWAIT_READY(slaveRegistered);
AWAIT_READY(update);
- ASSERT_TRUE(update->has_resource_categories());
- ASSERT_TRUE(update->resource_categories().has_oversubscribed());
- ASSERT_TRUE(update->resource_categories().oversubscribed());
+ ASSERT_TRUE(update->has_update_oversubscribed_resources());
+ ASSERT_TRUE(update->update_oversubscribed_resources());
Resources resources = update->oversubscribed_resources();
EXPECT_SOME_EQ(2.0, resources.cpus());
@@ -921,9 +918,8 @@ TEST_F(OversubscriptionTest, Reregistration)
Clock::advance(agentFlags.registration_backoff_factor);
AWAIT_READY(slaveReregistered);
AWAIT_READY(update);
- EXPECT_TRUE(update->has_resource_categories());
- EXPECT_TRUE(update->resource_categories().has_oversubscribed());
- EXPECT_TRUE(update->resource_categories().oversubscribed());
+ EXPECT_TRUE(update->has_update_oversubscribed_resources());
+ EXPECT_TRUE(update->update_oversubscribed_resources());
}