You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ya...@apache.org on 2016/08/31 23:51:59 UTC
mesos git commit: Improved shared resources handling in Resources.
Repository: mesos
Updated Branches:
refs/heads/master ab58f9085 -> d4970b783
Improved shared resources handling in Resources.
- Added `shared()` and `nonShared()` filter methods.
- Improved `Resources::validate()` to allow only persistent volumes
to be shared.
Review: https://reviews.apache.org/r/45960/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/d4970b78
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/d4970b78
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/d4970b78
Branch: refs/heads/master
Commit: d4970b78313943aecff7d0366621fbab43563c84
Parents: ab58f90
Author: Anindya Sinha <an...@apple.com>
Authored: Mon Aug 29 16:57:41 2016 -0700
Committer: Jiang Yan Xu <xu...@apple.com>
Committed: Wed Aug 31 16:51:01 2016 -0700
----------------------------------------------------------------------
include/mesos/resources.hpp | 9 ++++++++
include/mesos/v1/resources.hpp | 9 ++++++++
src/common/resources.cpp | 34 +++++++++++++++++++++++++++++-
src/tests/master_validation_tests.cpp | 19 +++++++++++++++++
src/tests/resources_tests.cpp | 24 +++++++++++++++++++++
src/v1/resources.cpp | 34 +++++++++++++++++++++++++++++-
6 files changed, 127 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/d4970b78/include/mesos/resources.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/resources.hpp b/include/mesos/resources.hpp
index b132933..87828aa 100644
--- a/include/mesos/resources.hpp
+++ b/include/mesos/resources.hpp
@@ -245,6 +245,9 @@ public:
// Tests if the given Resource object is revocable.
static bool isRevocable(const Resource& resource);
+ // Tests if the given Resource object is shared.
+ static bool isShared(const Resource& resource);
+
// Returns the summed up Resources given a hashmap<Key, Resources>.
//
// NOTE: While scalar resources such as "cpus" sum correctly,
@@ -332,6 +335,12 @@ public:
// Returns the non-revocable resources, effectively !revocable().
Resources nonRevocable() const;
+ // Returns the shared resources.
+ Resources shared() const;
+
+ // Returns the non-shared resources.
+ Resources nonShared() const;
+
// Returns a Resources object with the same amount of each resource
// type as these Resources, but with all Resource objects marked as
// the specified (role, reservation) pair. This is used to cross
http://git-wip-us.apache.org/repos/asf/mesos/blob/d4970b78/include/mesos/v1/resources.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/v1/resources.hpp b/include/mesos/v1/resources.hpp
index e7f5329..27bdd27 100644
--- a/include/mesos/v1/resources.hpp
+++ b/include/mesos/v1/resources.hpp
@@ -245,6 +245,9 @@ public:
// Tests if the given Resource object is revocable.
static bool isRevocable(const Resource& resource);
+ // Tests if the given Resource object is shared.
+ static bool isShared(const Resource& resource);
+
// Returns the summed up Resources given a hashmap<Key, Resources>.
//
// NOTE: While scalar resources such as "cpus" sum correctly,
@@ -332,6 +335,12 @@ public:
// Returns the non-revocable resources, effectively !revocable().
Resources nonRevocable() const;
+ // Returns the shared resources.
+ Resources shared() const;
+
+ // Returns the non-shared resources.
+ Resources nonShared() const;
+
// Returns a Resources object with the same amount of each resource
// type as these Resources, but with all Resource objects marked as
// the specified (role, reservation) pair. This is used to cross
http://git-wip-us.apache.org/repos/asf/mesos/blob/d4970b78/src/common/resources.cpp
----------------------------------------------------------------------
diff --git a/src/common/resources.cpp b/src/common/resources.cpp
index a5f5902..bf7e7e7 100644
--- a/src/common/resources.cpp
+++ b/src/common/resources.cpp
@@ -367,7 +367,7 @@ static bool contains(const Resource& left, const Resource& right)
{
// NOTE: This is a necessary condition for 'contains'.
// 'subtractable' will verify name, role, type, ReservationInfo,
- // DiskInfo and RevocableInfo compatibility.
+ // DiskInfo, SharedInfo and RevocableInfo compatibility.
if (!subtractable(left, right)) {
return false;
}
@@ -744,6 +744,19 @@ Option<Error> Resources::validate(const Resource& resource)
return error;
}
+ // Check that shareability is enabled for supported resource types.
+ // For now, it is for persistent volumes only.
+ // NOTE: We need to modify this once we extend shareability to other
+ // resource types.
+ if (resource.has_shared()) {
+ if (resource.name() != "disk") {
+ return Error("Resource " + resource.name() + " cannot be shared");
+ }
+
+ if (!resource.has_disk() || !resource.disk().has_persistence()) {
+ return Error("Only persistent volumes can be shared");
+ }
+ }
return None();
}
@@ -813,6 +826,12 @@ bool Resources::isRevocable(const Resource& resource)
}
+bool Resources::isShared(const Resource& resource)
+{
+ return resource.has_shared();
+}
+
+
/////////////////////////////////////////////////
// Public member functions.
/////////////////////////////////////////////////
@@ -1044,6 +1063,19 @@ Resources Resources::nonRevocable() const
}
+Resources Resources::shared() const
+{
+ return filter(isShared);
+}
+
+
+Resources Resources::nonShared() const
+{
+ return filter(
+ [](const Resource& resource) { return !isShared(resource); });
+}
+
+
Resources Resources::flatten(
const string& role,
const Option<Resource::ReservationInfo>& reservation) const
http://git-wip-us.apache.org/repos/asf/mesos/blob/d4970b78/src/tests/master_validation_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_validation_tests.cpp b/src/tests/master_validation_tests.cpp
index e440ff3..d40b392 100644
--- a/src/tests/master_validation_tests.cpp
+++ b/src/tests/master_validation_tests.cpp
@@ -191,6 +191,25 @@ TEST_F(ResourceValidationTest, RevocablePersistentVolume)
}
+TEST_F(ResourceValidationTest, UnshareableResource)
+{
+ Resource volume = Resources::parse("disk", "128", "role1").get();
+ volume.mutable_shared();
+
+ EXPECT_SOME(resource::validate(CreateResources(volume)));
+}
+
+
+TEST_F(ResourceValidationTest, SharedPersistentVolume)
+{
+ Resource volume = Resources::parse("disk", "128", "role1").get();
+ volume.mutable_disk()->CopyFrom(createDiskInfo("id1", "path1"));
+ volume.mutable_shared();
+
+ EXPECT_NONE(resource::validate(CreateResources(volume)));
+}
+
+
class ReserveOperationValidationTest : public MesosTest {};
http://git-wip-us.apache.org/repos/asf/mesos/blob/d4970b78/src/tests/resources_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/resources_tests.cpp b/src/tests/resources_tests.cpp
index 4932d95..0cecac8 100644
--- a/src/tests/resources_tests.cpp
+++ b/src/tests/resources_tests.cpp
@@ -2610,6 +2610,30 @@ TEST(SharedResourcesTest, ScalarSharedAndNonSharedOperations)
}
+// This test verifies that shared resources can be filtered.
+TEST(SharedResourcesTest, Filter)
+{
+ Resources r1 = createDiskResource("10", "role1", "1", "path", None(), true);
+ EXPECT_EQ(r1, r1.shared());
+ EXPECT_TRUE(r1.nonShared().empty());
+
+ Resources r2 = createDiskResource(
+ "20", "role2", None(), None(), None(), false);
+
+ EXPECT_TRUE(r2.shared().empty());
+ EXPECT_EQ(r2, r2.nonShared());
+
+ EXPECT_EQ(r1, (r1 + r2).shared());
+ EXPECT_EQ(r2, (r1 + r2).nonShared());
+
+ Resources resources = Resources::parse("cpus:1;mem:512;disk:1000").get();
+ Resources sum = resources + r1 + r2;
+
+ EXPECT_EQ(r1, sum.shared());
+ EXPECT_EQ(resources + r2, sum.nonShared());
+}
+
+
struct Parameter
{
Resources resources;
http://git-wip-us.apache.org/repos/asf/mesos/blob/d4970b78/src/v1/resources.cpp
----------------------------------------------------------------------
diff --git a/src/v1/resources.cpp b/src/v1/resources.cpp
index 1722175..b098b10 100644
--- a/src/v1/resources.cpp
+++ b/src/v1/resources.cpp
@@ -369,7 +369,7 @@ static bool contains(const Resource& left, const Resource& right)
{
// NOTE: This is a necessary condition for 'contains'.
// 'subtractable' will verify name, role, type, ReservationInfo,
- // DiskInfo and RevocableInfo compatibility.
+ // DiskInfo, SharedInfo and RevocableInfo compatibility.
if (!subtractable(left, right)) {
return false;
}
@@ -747,6 +747,19 @@ Option<Error> Resources::validate(const Resource& resource)
return error;
}
+ // Check that shareability is enabled for supported resource types.
+ // For now, it is for persistent volumes only.
+ // NOTE: We need to modify this once we extend shareability to other
+ // resource types.
+ if (resource.has_shared()) {
+ if (resource.name() != "disk") {
+ return Error("Resource " + resource.name() + " cannot be shared");
+ }
+
+ if (!resource.has_disk() || !resource.disk().has_persistence()) {
+ return Error("Only persistent volumes can be shared");
+ }
+ }
return None();
}
@@ -816,6 +829,12 @@ bool Resources::isRevocable(const Resource& resource)
}
+bool Resources::isShared(const Resource& resource)
+{
+ return resource.has_shared();
+}
+
+
/////////////////////////////////////////////////
// Public member functions.
/////////////////////////////////////////////////
@@ -1047,6 +1066,19 @@ Resources Resources::nonRevocable() const
}
+Resources Resources::shared() const
+{
+ return filter(isShared);
+}
+
+
+Resources Resources::nonShared() const
+{
+ return filter(
+ [](const Resource& resource) { return !isShared(resource); });
+}
+
+
Resources Resources::flatten(
const string& role,
const Option<Resource::ReservationInfo>& reservation) const