You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by an...@apache.org on 2017/09/05 16:48:28 UTC
mesos git commit: Fixed Attributes comparison.
Repository: mesos
Updated Branches:
refs/heads/master b9aebbbca -> 21b26b99e
Fixed Attributes comparison.
Currently we don't enforce attribute name uniqueness. But the comparison
operator for `Attributes` expects that attribute name and type are
unique. This causes causes agent recovery failure.
This fixes the issue by allowing multiple attributes of same name and
type.
Review: https://reviews.apache.org/r/60508/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/21b26b99
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/21b26b99
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/21b26b99
Branch: refs/heads/master
Commit: 21b26b99e9e0275e2f014a48d070dfdcea374b88
Parents: b9aebbb
Author: Ilya Pronin <ip...@twopensource.com>
Authored: Tue Sep 5 09:47:44 2017 -0700
Committer: Anand Mazumdar <an...@apache.org>
Committed: Tue Sep 5 09:48:18 2017 -0700
----------------------------------------------------------------------
include/mesos/attributes.hpp | 6 ++++
include/mesos/v1/attributes.hpp | 6 ++++
src/common/attributes.cpp | 67 ++++++++++++++++++++++++------------
src/tests/attributes_tests.cpp | 3 ++
src/v1/attributes.cpp | 67 ++++++++++++++++++++++++------------
5 files changed, 105 insertions(+), 44 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/21b26b99/include/mesos/attributes.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/attributes.hpp b/include/mesos/attributes.hpp
index 641febb..c2727cc 100644
--- a/include/mesos/attributes.hpp
+++ b/include/mesos/attributes.hpp
@@ -86,11 +86,17 @@ public:
return attributes.Get(index);
}
+ // Gets an attribute with the same name and type as the given
+ // attribute.
const Option<Attribute> get(const Attribute& thatAttribute) const;
template <typename T>
T get(const std::string& name, const T& t) const;
+ // Checks if these Attributes contain an attribute with the same
+ // name, type and value as the given attribute.
+ bool contains(const Attribute& attribute) const;
+
typedef google::protobuf::RepeatedPtrField<Attribute>::iterator
iterator;
http://git-wip-us.apache.org/repos/asf/mesos/blob/21b26b99/include/mesos/v1/attributes.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/v1/attributes.hpp b/include/mesos/v1/attributes.hpp
index 022bfde..3155be3 100644
--- a/include/mesos/v1/attributes.hpp
+++ b/include/mesos/v1/attributes.hpp
@@ -87,11 +87,17 @@ public:
return attributes.Get(index);
}
+ // Gets an attribute with the same name and type as the given
+ // attribute.
const Option<Attribute> get(const Attribute& thatAttribute) const;
template <typename T>
T get(const std::string& name, const T& t) const;
+ // Checks if these Attributes contain an attribute with the same
+ // name, type and value as the given attribute.
+ bool contains(const Attribute& attribute) const;
+
typedef google::protobuf::RepeatedPtrField<Attribute>::iterator
iterator;
http://git-wip-us.apache.org/repos/asf/mesos/blob/21b26b99/src/common/attributes.cpp
----------------------------------------------------------------------
diff --git a/src/common/attributes.cpp b/src/common/attributes.cpp
index ac683c7..a0ad50b 100644
--- a/src/common/attributes.cpp
+++ b/src/common/attributes.cpp
@@ -55,30 +55,20 @@ bool Attributes::operator==(const Attributes& that) const
return false;
}
+ // We don't enforce that attributes (name, type, value tuples) are
+ // unique and don't check that both attribute sets contain equal
+ // number of duplicate attributes. This will be changed when we
+ // enforce attribute name uniqueness after adding SET type support
+ // for attributes (MESOS-1215).
foreach (const Attribute& attribute, attributes) {
- Option<Attribute> maybeAttribute = that.get(attribute);
- if (maybeAttribute.isNone()) {
- return false;
+ if (!that.contains(attribute)) {
+ return false;
}
- const Attribute& thatAttribute = maybeAttribute.get();
- switch (attribute.type()) {
- case Value::SCALAR:
- if (!(attribute.scalar() == thatAttribute.scalar())) {
- return false;
- }
- break;
- case Value::RANGES:
- if (!(attribute.ranges() == thatAttribute.ranges())) {
- return false;
- }
- break;
- case Value::TEXT:
- if (!(attribute.text() == thatAttribute.text())) {
- return false;
- }
- break;
- case Value::SET:
- LOG(FATAL) << "Sets not supported for attributes";
+ }
+
+ foreach (const Attribute& attribute, that) {
+ if (!contains(attribute)) {
+ return false;
}
}
@@ -99,6 +89,39 @@ const Option<Attribute> Attributes::get(const Attribute& thatAttribute) const
}
+bool Attributes::contains(const Attribute& attribute) const
+{
+ foreach (const Attribute& attr, attributes) {
+ if (attr.name() == attribute.name() && attr.type() == attribute.type()) {
+ switch (attr.type()) {
+ case Value::SCALAR:
+ if (attr.scalar() == attribute.scalar()) {
+ return true;
+ }
+ break;
+
+ case Value::RANGES:
+ if (attr.ranges() == attribute.ranges()) {
+ return true;
+ }
+ break;
+
+ case Value::TEXT:
+ if (attr.text() == attribute.text()) {
+ return true;
+ }
+ break;
+
+ case Value::SET:
+ LOG(FATAL) << "Sets not supported for attributes";
+ }
+ }
+ }
+
+ return false;
+}
+
+
Attribute Attributes::parse(const string& name, const string& text)
{
Attribute attribute;
http://git-wip-us.apache.org/repos/asf/mesos/blob/21b26b99/src/tests/attributes_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/attributes_tests.cpp b/src/tests/attributes_tests.cpp
index cb71be5..f053292 100644
--- a/src/tests/attributes_tests.cpp
+++ b/src/tests/attributes_tests.cpp
@@ -68,6 +68,9 @@ TEST(AttributesTest, Equality)
EXPECT_NE(a, Attributes::parse(""));
EXPECT_NE(a, Attributes::parse("cpus:45.55;ports:45.55;rack:45.55"));
EXPECT_NE(Attributes::parse(""), a);
+ EXPECT_EQ(
+ Attributes::parse("rack:rack1;dedicated:customer1;dedicated:customer2"),
+ Attributes::parse("dedicated:customer2;dedicated:customer1;rack:rack1"));
}
} // namespace tests {
http://git-wip-us.apache.org/repos/asf/mesos/blob/21b26b99/src/v1/attributes.cpp
----------------------------------------------------------------------
diff --git a/src/v1/attributes.cpp b/src/v1/attributes.cpp
index 409d664..4668e1b 100644
--- a/src/v1/attributes.cpp
+++ b/src/v1/attributes.cpp
@@ -56,30 +56,20 @@ bool Attributes::operator==(const Attributes& that) const
return false;
}
+ // We don't enforce that attributes (name, type, value tuples) are
+ // unique and don't check that both attribute sets contain equal
+ // number of duplicate attributes. This will be changed when we
+ // enforce attribute name uniqueness after adding SET type support
+ // for attributes (MESOS-1215).
foreach (const Attribute& attribute, attributes) {
- Option<Attribute> maybeAttribute = that.get(attribute);
- if (maybeAttribute.isNone()) {
- return false;
+ if (!that.contains(attribute)) {
+ return false;
}
- const Attribute& thatAttribute = maybeAttribute.get();
- switch (attribute.type()) {
- case Value::SCALAR:
- if (!(attribute.scalar() == thatAttribute.scalar())) {
- return false;
- }
- break;
- case Value::RANGES:
- if (!(attribute.ranges() == thatAttribute.ranges())) {
- return false;
- }
- break;
- case Value::TEXT:
- if (!(attribute.text() == thatAttribute.text())) {
- return false;
- }
- break;
- case Value::SET:
- LOG(FATAL) << "Sets not supported for attributes";
+ }
+
+ foreach (const Attribute& attribute, that) {
+ if (!contains(attribute)) {
+ return false;
}
}
@@ -100,6 +90,39 @@ const Option<Attribute> Attributes::get(const Attribute& thatAttribute) const
}
+bool Attributes::contains(const Attribute& attribute) const
+{
+ foreach (const Attribute& attr, attributes) {
+ if (attr.name() == attribute.name() && attr.type() == attribute.type()) {
+ switch (attr.type()) {
+ case Value::SCALAR:
+ if (attr.scalar() == attribute.scalar()) {
+ return true;
+ }
+ break;
+
+ case Value::RANGES:
+ if (attr.ranges() == attribute.ranges()) {
+ return true;
+ }
+ break;
+
+ case Value::TEXT:
+ if (attr.text() == attribute.text()) {
+ return true;
+ }
+ break;
+
+ case Value::SET:
+ LOG(FATAL) << "Sets not supported for attributes";
+ }
+ }
+ }
+
+ return false;
+}
+
+
Attribute Attributes::parse(const string& name, const string& text)
{
Attribute attribute;