mesos-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mp...@apache.org
Subject [3/5] mesos git commit: Modified `downgradeResources` to use protobuf reflection.
Date Wed, 03 Jan 2018 07:17:27 GMT
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 <mpark@apache.org>
Authored: Mon Nov 20 22:03:17 2017 -0800
Committer: Michael Park <mpark@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


Mime
View raw message