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 31ac45be0a55fc33982641516bcc5eb3226ef406 Author: Meng Zhu <[email protected]> AuthorDate: Tue May 28 16:28:28 2019 +0200 Added a function to shrink `Resources` to target `ResourceLimits`. Also added unit tests. Review: https://reviews.apache.org/r/70737 --- src/common/resources_utils.cpp | 35 ++++++++++++++++++++++ src/common/resources_utils.hpp | 20 +++++++++---- src/tests/resources_tests.cpp | 67 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 5 deletions(-) diff --git a/src/common/resources_utils.cpp b/src/common/resources_utils.cpp index 1555373..58fdb31 100644 --- a/src/common/resources_utils.cpp +++ b/src/common/resources_utils.cpp @@ -25,6 +25,7 @@ using google::protobuf::Descriptor; using google::protobuf::Message; using google::protobuf::RepeatedPtrField; +using mesos::internal::ResourceLimits; using mesos::internal::ResourceQuantities; namespace mesos { @@ -940,4 +941,38 @@ Resources shrinkResources(const Resources& resources, ResourceQuantities target) } +Resources shrinkResources(const Resources& resources, ResourceLimits target) +{ + if (target.empty()) { + return resources; + } + + // TODO(mzhu): Add a `shuffle()` method in `Resources` to avoid this copy. + google::protobuf::RepeatedPtrField<Resource> resourceVector = resources; + + random_shuffle(resourceVector.begin(), resourceVector.end()); + + Resources result; + foreach (Resource resource, resourceVector) { + Option<Value::Scalar> limit = target.get(resource.name()); + + if (limit.isNone()) { + // Resource that has infinite limit is kept as is. + result += std::move(resource); + continue; + } + + // Target can only be explicitly specified for scalar resources. + CHECK_EQ(Value::SCALAR, resource.type()) << " Resources: " << resources; + + if (Resources::shrink(&resource, *limit)) { + target -= ResourceQuantities::fromScalarResources(resource); + result += std::move(resource); + } + } + + return result; +} + + } // namespace mesos { diff --git a/src/common/resources_utils.hpp b/src/common/resources_utils.hpp index 6bfe24c..9b78ca2 100644 --- a/src/common/resources_utils.hpp +++ b/src/common/resources_utils.hpp @@ -218,11 +218,19 @@ Try<Nothing> downgradeResources(std::vector<Resource>* resources); Try<Nothing> downgradeResources(google::protobuf::Message* message); -// Returns the result of shrinking resources down to the target -// scalar resource quantities. Target quantities can only be explicitly -// specified for scalar resources. Otherwise a `CHECK` error will occur. -// Note `ResourceQuantities` has absent-means-zero semantics. This means -// all non-scalar resources (if any) will be dropped post shrinking. +// These two functions shrink resources down to the target scalar resource +// quantities or limits respectively. Target can only be specified for +// scalar resources. Otherwise a `CHECK` error will occur. +// +// The primary difference between these two shrink functions is about resources +// that have no quantity/limit specified in the `target`: +// +// - If the target is `ResourceQuantities`, due to its absent-means-zero +// semantic, such resources will the dropped i.e. shrink down to zero. +// +// - If the target is `ResourceLimits`, due to its absent-means-infinite +// semantic, such resources will be kept as-is in the result since it +// is already below the (infinite) limit. // // Note that some resources are indivisible (e.g. MOUNT volume) and // may be excluded in entirety in order to achieve the target size @@ -233,6 +241,8 @@ Try<Nothing> downgradeResources(google::protobuf::Message* message); // will make a random choice in these cases. Resources shrinkResources( const Resources& resources, mesos::internal::ResourceQuantities target); +Resources shrinkResources( + const Resources& resources, mesos::internal::ResourceLimits target); } // namespace mesos { diff --git a/src/tests/resources_tests.cpp b/src/tests/resources_tests.cpp index 32d103f..6063c01 100644 --- a/src/tests/resources_tests.cpp +++ b/src/tests/resources_tests.cpp @@ -59,6 +59,7 @@ using mesos::internal::evolve; using mesos::internal::protobuf::createLabel; +using mesos::internal::ResourceLimits; using mesos::internal::ResourceQuantities; namespace mesos { @@ -1939,6 +1940,72 @@ TEST(ResourcesTest, ShrinkToQuantities) } +// This test verifies the correctness of shrinking resources to the target +// resource limits. +TEST(ResourcesTest, ShrinkToLimits) +{ + auto shrink = [](const Resources& resources, const string& limitsString) { + ResourceLimits limits = + CHECK_NOTERROR(ResourceLimits::fromString(limitsString)); + + return shrinkResources(resources, limits); + }; + + // Emptiness. + Resources empty; + EXPECT_EQ(empty, shrink(empty, "")); + EXPECT_EQ(empty, shrink(empty, "cpus:1")); + + // Simple shrink. + Resources cpu1 = CHECK_NOTERROR(Resources::parse("cpus:1")); + EXPECT_EQ(cpu1, shrink(cpu1, "")); + + Resources cpu2 = CHECK_NOTERROR(Resources::parse("cpus:2")); + EXPECT_EQ(cpu1, shrink(cpu2, "cpus:1")); + + // Multiple resources. + Resources cpu2mem20 = CHECK_NOTERROR(Resources::parse("cpus:2;mem:20")); + Resources cpu1mem10 = CHECK_NOTERROR(Resources::parse("cpus:1;mem:10")); + EXPECT_EQ(cpu1mem10, shrink(cpu2mem20, "cpus:1;mem:10")); + EXPECT_EQ(cpu1mem10, shrink(cpu2mem20, "cpus:1;mem:10;disk:10")); + + Resources cpu1mem20 = CHECK_NOTERROR(Resources::parse("cpus:1;mem:20")); + EXPECT_EQ(cpu1mem20, shrink(cpu2mem20, "cpus:1")); + + // Non-choppable resources. + Resources mountDisk20 = createDiskResource( + "20", "role", None(), None(), createDiskSourceMount("mnt")); + EXPECT_EQ(Resources(), shrink(mountDisk20, "disk:10")); + + // Random chopping. + // + // We construct 20 disk resources consisted of 10 regular disk and + // 10 mount disk. A chopping of 10 disk should make a random choice + // of picking either the regular disk or the mount disk. + Resources regularDisk10 = CHECK_NOTERROR(Resources::parse("disk:10")); + Resources mountDisk10 = createDiskResource( + "10", "role", None(), None(), createDiskSourceMount("mnt")); + Resources combinedDisk20 = regularDisk10 + mountDisk10; + + // A chopping of 10 disk could return either 10 regular disk or + // 10 mount disk. + Resources shrunk10 = shrink(combinedDisk20, "disk:10"); + + // Repeat the same chopping for 1000 times, expecting the chopping + // result to be different from the first time at least once. + const size_t count = 1000u; + bool atLeastDifferentOnce; + + for (size_t i = 0; i < count; i++) { + if (shrink(combinedDisk20, "disk:10") != shrunk10) { + atLeastDifferentOnce = true; + break; + } + } + CHECK(atLeastDifferentOnce); +} + + // This test verifies that we can filter resources of a given name // from Resources. TEST(ResourcesTest, Get)
