You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by mp...@apache.org on 2018/01/03 07:17:27 UTC
[3/5] mesos git commit: Modified `downgradeResources` to use protobuf
reflection.
Modified `downgradeResources` to use protobuf reflection.
Previously, our `downgradeResources` function only took a pointer to
`RepeatedPtrField<Resource>`. This wasn't great since we needed manually
invoke `downgradeResources` on every `Resource`s field of every message.
This patch makes it such that `downgradeResources` can take any
`protobuf::Message` or `protobuf::RepeatedPtrField<Resource>`.
Review: https://reviews.apache.org/r/63976
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/6c07f4d2
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/6c07f4d2
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/6c07f4d2
Branch: refs/heads/1.5.x
Commit: 6c07f4d241015fc54e677db2e15997703ed5affc
Parents: 7b1a10a
Author: Michael Park <mp...@apache.org>
Authored: Mon Nov 20 22:03:17 2017 -0800
Committer: Michael Park <mp...@apache.org>
Committed: Tue Jan 2 23:04:41 2018 -0800
----------------------------------------------------------------------
src/common/resources_utils.cpp | 132 +++++++++++++++++++++++++--
src/common/resources_utils.hpp | 24 ++++-
src/tests/resources_tests.cpp | 172 ++++++++++++++++++++++++++++++++++++
3 files changed, 315 insertions(+), 13 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/6c07f4d2/src/common/resources_utils.cpp
----------------------------------------------------------------------
diff --git a/src/common/resources_utils.cpp b/src/common/resources_utils.cpp
index 465bd54..bd9e025 100644
--- a/src/common/resources_utils.cpp
+++ b/src/common/resources_utils.cpp
@@ -21,6 +21,8 @@
using std::vector;
+using google::protobuf::Descriptor;
+using google::protobuf::Message;
using google::protobuf::RepeatedPtrField;
namespace mesos {
@@ -738,24 +740,136 @@ Option<Error> validateAndUpgradeResources(Offer::Operation* operation)
}
+Try<Nothing> downgradeResource(Resource* resource)
+{
+ CHECK(!resource->has_role());
+ CHECK(!resource->has_reservation());
+
+ if (Resources::hasRefinedReservations(*resource)) {
+ return Error("Cannot downgrade resources containing refined reservations");
+ }
+
+ convertResourceFormat(resource, PRE_RESERVATION_REFINEMENT);
+ return Nothing();
+}
+
+
Try<Nothing> downgradeResources(RepeatedPtrField<Resource>* resources)
{
- foreach (const Resource& resource, *resources) {
- CHECK(!resource.has_role());
- CHECK(!resource.has_reservation());
+ CHECK_NOTNULL(resources);
+
+ foreach (Resource& resource, *resources) {
+ Try<Nothing> result = downgradeResource(&resource);
+ if (result.isError()) {
+ return result;
+ }
}
- foreach (const Resource& resource, *resources) {
- if (Resources::hasRefinedReservations(resource)) {
- return Error(
- "Invalid resources downgrade: resource " + stringify(resource) +
- " with refined reservations cannot be downgraded");
+ return Nothing();
+}
+
+namespace internal {
+
+// Given a protobuf descriptor `descriptor`, returns `true` if `descriptor`
+// is a `mesos::Resource`, or contains a `mesos::Resource` somewhere within.
+//
+// The provided `result` is recursively populated, where the keys are the
+// message descriptors within `descriptor`'s schema (including itself), and
+// the corresponding value is `true` if the key contains a `mesos::Resource`.
+static void precomputeResourcesContainment(
+ const Descriptor* descriptor,
+ hashmap<const Descriptor*, bool>* result)
+{
+ CHECK_NOTNULL(descriptor);
+ CHECK_NOTNULL(result);
+
+ if (result->contains(descriptor)) {
+ return;
+ }
+
+ if (descriptor == mesos::Resource::descriptor()) {
+ result->insert({descriptor, true});
+ }
+
+ result->insert({descriptor, false});
+ for (int i = 0; i < descriptor->field_count(); ++i) {
+ // `message_type()` returns `nullptr` if the field is not a message type.
+ const Descriptor* messageDescriptor = descriptor->field(i)->message_type();
+ if (messageDescriptor == nullptr) {
+ continue;
}
+ precomputeResourcesContainment(messageDescriptor, result);
+ result->at(descriptor) |= result->at(messageDescriptor);
}
+}
+
- convertResourceFormat(resources, PRE_RESERVATION_REFINEMENT);
+static Try<Nothing> downgradeResourcesImpl(
+ Message* message,
+ const hashmap<const Descriptor*, bool>& resourcesContainment)
+{
+ CHECK_NOTNULL(message);
+
+ const Descriptor* descriptor = message->GetDescriptor();
+
+ if (descriptor == mesos::Resource::descriptor()) {
+ return downgradeResource(static_cast<mesos::Resource*>(message));
+ }
+
+ const google::protobuf::Reflection* reflection = message->GetReflection();
+
+ for (int i = 0; i < descriptor->field_count(); ++i) {
+ const google::protobuf::FieldDescriptor* field = descriptor->field(i);
+ const Descriptor* messageDescriptor = field->message_type();
+
+ if (messageDescriptor == nullptr ||
+ !resourcesContainment.at(messageDescriptor)) {
+ continue;
+ }
+
+ if (!field->is_repeated()) {
+ if (reflection->HasField(*message, field)) {
+ Try<Nothing> result = downgradeResourcesImpl(
+ reflection->MutableMessage(message, field), resourcesContainment);
+
+ if (result.isError()) {
+ return result;
+ }
+ }
+ } else {
+ const int size = reflection->FieldSize(*message, field);
+
+ for (int j = 0; j < size; ++j) {
+ Try<Nothing> result = downgradeResourcesImpl(
+ reflection->MutableRepeatedMessage(message, field, j),
+ resourcesContainment);
+
+ if (result.isError()) {
+ return result;
+ }
+ }
+ }
+ }
return Nothing();
}
+} // namespace internal {
+
+Try<Nothing> downgradeResources(Message* message)
+{
+ CHECK_NOTNULL(message);
+
+ const Descriptor* descriptor = message->GetDescriptor();
+
+ hashmap<const Descriptor*, bool> resourcesContainment;
+ internal::precomputeResourcesContainment(descriptor, &resourcesContainment);
+
+ if (!resourcesContainment.at(descriptor)) {
+ return Nothing();
+ }
+
+ return internal::downgradeResourcesImpl(message, resourcesContainment);
+}
+
} // namespace mesos {
http://git-wip-us.apache.org/repos/asf/mesos/blob/6c07f4d2/src/common/resources_utils.hpp
----------------------------------------------------------------------
diff --git a/src/common/resources_utils.hpp b/src/common/resources_utils.hpp
index a946f0a..115efc2 100644
--- a/src/common/resources_utils.hpp
+++ b/src/common/resources_utils.hpp
@@ -169,13 +169,29 @@ void convertResourceFormat(
Option<Error> validateAndUpgradeResources(Offer::Operation* operation);
-// Convert the given resources to the "pre-reservation-refinement" format
-// if none of the resources have refined reservations. Returns an `Error`
-// if there are any refined reservations present; in this case, the resources
-// are left in the "post-reservation-refinement" format.
+// Convert any resources contained in the given message(s)
+// to the "pre-reservation-refinement" format, if possible.
+//
+// These functions do not provide "all-or-nothing" semantics.
+// The resources are downgraded to "pre-" format until either
+// (1) there are no more resources, or
+// (2) a non-downgradable resource is encountered.
+//
+// For (1), `Nothing` is returned.
+// For (2), `Error` is returned, and the rest of the resources are untouched.
+//
+// This implies that components that have refined resources created cannot
+// be downgraded to a version that does not support reservation refinement.
+Try<Nothing> downgradeResource(Resource* resource);
+
+
Try<Nothing> downgradeResources(
google::protobuf::RepeatedPtrField<Resource>* resources);
+
+Try<Nothing> downgradeResources(google::protobuf::Message* message);
+
+
} // namespace mesos {
#endif // __RESOURCES_UTILS_HPP__
http://git-wip-us.apache.org/repos/asf/mesos/blob/6c07f4d2/src/tests/resources_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/resources_tests.cpp b/src/tests/resources_tests.cpp
index 64bde85..bd328b2 100644
--- a/src/tests/resources_tests.cpp
+++ b/src/tests/resources_tests.cpp
@@ -31,6 +31,8 @@
#include <mesos/v1/resources.hpp>
+#include "common/resources_utils.hpp"
+
#include "internal/evolve.hpp"
#include "master/master.hpp"
@@ -3014,6 +3016,176 @@ TEST(ResourceFormatTest, Endpoint)
}
+TEST(ResourceFormatTest, DowngradeWithoutResources)
+{
+ FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
+ EXPECT_SOME(downgradeResources(&frameworkInfo));
+ EXPECT_EQ(DEFAULT_FRAMEWORK_INFO, frameworkInfo);
+}
+
+
+TEST(ResourceFormatTest, DowngradeWithResourcesWithoutRefinedReservations)
+{
+ SlaveID slaveId;
+ slaveId.set_value("agent");
+
+ TaskInfo actual;
+ {
+ actual.set_name("task");
+ actual.mutable_task_id()->set_value("task_id");
+ actual.mutable_slave_id()->CopyFrom(slaveId);
+
+ Resource resource;
+ resource.set_name("cpus");
+ resource.set_type(Value::SCALAR);
+ resource.mutable_scalar()->set_value(555.5);
+
+ // Add "post-reservation-refinement" resources.
+
+ // Unreserved resource.
+ actual.add_resources()->CopyFrom(resource);
+
+ Resource::ReservationInfo* reservation = resource.add_reservations();
+
+ // Statically reserved resource.
+ reservation->set_type(Resource::ReservationInfo::STATIC);
+ reservation->set_role("foo");
+ actual.add_resources()->CopyFrom(resource);
+
+ // Dynamically reserved resource.
+ reservation->set_type(Resource::ReservationInfo::DYNAMIC);
+ reservation->set_role("bar");
+ reservation->set_principal("principal1");
+ actual.add_resources()->CopyFrom(resource);
+ }
+
+ TaskInfo expected;
+ {
+ expected.set_name("task");
+ expected.mutable_task_id()->set_value("task_id");
+ expected.mutable_slave_id()->CopyFrom(slaveId);
+
+ Resource resource;
+ resource.set_name("cpus");
+ resource.set_type(Value::SCALAR);
+ resource.mutable_scalar()->set_value(555.5);
+
+ // Add "pre-reservation-refinement" resources.
+
+ // Unreserved resource.
+ resource.set_role("*");
+ expected.add_resources()->CopyFrom(resource);
+
+ // Statically reserved resource.
+ resource.set_role("foo");
+ expected.add_resources()->CopyFrom(resource);
+
+ // Dynamically reserved resource.
+ resource.set_role("bar");
+ Resource::ReservationInfo* reservation = resource.mutable_reservation();
+ reservation->set_principal("principal1");
+ expected.add_resources()->CopyFrom(resource);
+ }
+
+ EXPECT_SOME(downgradeResources(&actual));
+ EXPECT_EQ(expected, actual);
+}
+
+
+TEST(ResourceFormatTest, DowngradeWithResourcesWithRefinedReservations)
+{
+ SlaveID slaveId;
+ slaveId.set_value("agent");
+
+ TaskInfo actual;
+ {
+ actual.set_name("task");
+ actual.mutable_task_id()->set_value("task_id");
+ actual.mutable_slave_id()->CopyFrom(slaveId);
+
+ Resource resource;
+ resource.set_name("cpus");
+ resource.set_type(Value::SCALAR);
+ resource.mutable_scalar()->set_value(555.5);
+
+ // Add "post-reservation-refinement" resources.
+
+ // Unreserved resource.
+ actual.add_resources()->CopyFrom(resource);
+
+ Resource::ReservationInfo* reservation = resource.add_reservations();
+
+ // Statically reserved resource.
+ reservation->set_type(Resource::ReservationInfo::STATIC);
+ reservation->set_role("foo");
+ actual.add_resources()->CopyFrom(resource);
+
+ // Dynamically reserved resource.
+ reservation->set_type(Resource::ReservationInfo::DYNAMIC);
+ reservation->set_role("bar");
+ reservation->set_principal("principal1");
+ actual.add_resources()->CopyFrom(resource);
+
+ // Dynamically refined reservation on top of dynamic reservation.
+ Resource::ReservationInfo* refinedReservation = resource.add_reservations();
+ refinedReservation->set_type(Resource::ReservationInfo::DYNAMIC);
+ refinedReservation->set_role("bar/baz");
+ refinedReservation->set_principal("principal2");
+ actual.add_resources()->CopyFrom(resource);
+ }
+
+ TaskInfo expected;
+ {
+ expected.set_name("task");
+ expected.mutable_task_id()->set_value("task_id");
+ expected.mutable_slave_id()->CopyFrom(slaveId);
+
+ Resource resource;
+ resource.set_name("cpus");
+ resource.set_type(Value::SCALAR);
+ resource.mutable_scalar()->set_value(555.5);
+
+ // Add "pre-reservation-refinement" resources.
+
+ // Unreserved resource.
+ resource.set_role("*");
+ expected.add_resources()->CopyFrom(resource);
+
+ // Statically reserved resource.
+ resource.set_role("foo");
+ expected.add_resources()->CopyFrom(resource);
+
+ // Dynamically reserved resource.
+ resource.set_role("bar");
+ Resource::ReservationInfo* reservation = resource.mutable_reservation();
+ reservation->set_principal("principal1");
+ expected.add_resources()->CopyFrom(resource);
+
+ // Add non-downgradable resources. Note that the non-downgradable
+ // resources remain in "post-reservation-refinement" format.
+
+ // Dynamically refined reservation on top of dynamic reservation.
+ resource.clear_role();
+ resource.clear_reservation();
+
+ Resource::ReservationInfo* dynamicReservation = resource.add_reservations();
+ dynamicReservation->set_type(Resource::ReservationInfo::DYNAMIC);
+ dynamicReservation->set_role("bar");
+ dynamicReservation->set_principal("principal1");
+
+ Resource::ReservationInfo* refinedReservation = resource.add_reservations();
+ refinedReservation->set_type(Resource::ReservationInfo::DYNAMIC);
+ refinedReservation->set_role("bar/baz");
+ refinedReservation->set_principal("principal2");
+
+ expected.add_resources()->CopyFrom(resource);
+ }
+
+ EXPECT_ERROR(downgradeResources(&actual));
+ EXPECT_EQ(expected, actual);
+}
+
+
TEST(ResourcesTest, Count)
{
// The summation of identical shared resources is valid and