This is an automated email from the ASF dual-hosted git repository.
asekretenko pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 470665e1575b9a91dffca974485940c7a0a1fe37
Author: Andrei Sekretenko <asekrete...@apache.org>
AuthorDate: Mon Aug 3 18:35:06 2020 +0200
Implemented regex-based offer constraints.
Review: https://reviews.apache.org/r/72786
---
include/mesos/allocator/allocator.hpp | 16 +++
src/Makefile.am | 1 +
.../allocator/mesos/offer_constraints_filter.cpp | 107 +++++++++++++++++++--
src/master/constants.hpp | 10 ++
src/master/flags.cpp | 18 ++++
src/master/flags.hpp | 3 +
src/master/master.cpp | 8 +-
src/master/master.hpp | 3 +
.../master/offer_constraints_filter_tests.cpp | 33 ++++---
9 files changed, 174 insertions(+), 25 deletions(-)
diff --git a/include/mesos/allocator/allocator.hpp
b/include/mesos/allocator/allocator.hpp
index 05d0e9c..c6fca65 100644
--- a/include/mesos/allocator/allocator.hpp
+++ b/include/mesos/allocator/allocator.hpp
@@ -78,7 +78,23 @@ class OfferConstraintsFilterImpl;
class OfferConstraintsFilter
{
public:
+ // TODO(asekretenko): Given that most dependants of the public allocator
+ // interface do not care about filter creation, we should consider decoupling
+ // the filter construction interface (which at this point consists of the
+ // `struct Options` and the `create()` method) from the allocator interface.
+ struct Options
+ {
+ struct RE2Limits
+ {
+ Bytes maxMem;
+ int maxProgramSize;
+ };
+
+ RE2Limits re2Limits;
+ };
+
static Try<OfferConstraintsFilter> create(
+ const Options& options,
scheduler::OfferConstraints&& constraints);
OfferConstraintsFilter() = delete;
diff --git a/src/Makefile.am b/src/Makefile.am
index 1043c7b..943ed77 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1133,6 +1133,7 @@ libmesos_no_3rdparty_la_SOURCES +=
\
master/allocator/mesos/hierarchical.hpp \
master/allocator/mesos/metrics.cpp \
master/allocator/mesos/metrics.hpp \
+ master/allocator/mesos/offer_constraints_filter.hpp \
master/allocator/mesos/offer_constraints_filter.cpp \
master/allocator/mesos/sorter/drf/metrics.cpp
\
master/allocator/mesos/sorter/drf/metrics.hpp
\
diff --git a/src/master/allocator/mesos/offer_constraints_filter.cpp
b/src/master/allocator/mesos/offer_constraints_filter.cpp
index d724fd0..30b671b 100644
--- a/src/master/allocator/mesos/offer_constraints_filter.cpp
+++ b/src/master/allocator/mesos/offer_constraints_filter.cpp
@@ -23,6 +23,8 @@
#include <stout/try.hpp>
#include <stout/variant.hpp>
+#include <re2/re2.h>
+
#include <mesos/allocator/allocator.hpp>
#include <mesos/attributes.hpp>
@@ -34,11 +36,42 @@ using std::unordered_map;
using ::mesos::scheduler::AttributeConstraint;
using ::mesos::scheduler::OfferConstraints;
+using RE2Limits =
+ ::mesos::allocator::OfferConstraintsFilter::Options::RE2Limits;
+
namespace mesos {
namespace allocator {
namespace internal {
+// TODO(asekretenko): Cache returned RE2s to make sure that:
+// - a RE2 corresponding to a pattern is not re-created needlessly
+// - multiple RE2s corresponding to the same pattern don't coexist
+static Try<unique_ptr<const RE2>> createRE2(
+ const RE2Limits& limits,
+ const string& regex)
+{
+ RE2::Options options{RE2::CannedOptions::Quiet};
+ options.set_max_mem(limits.maxMem.bytes());
+ unique_ptr<const RE2> re2{new RE2(regex, options)};
+
+ if (!re2->ok()) {
+ return Error(
+ "Failed to construct regex from pattern"
+ " '" + regex + "': " + re2->error());
+ }
+
+ if (re2->ProgramSize() > limits.maxProgramSize) {
+ return Error(
+ "Regex '" + regex + "' is too complex: program size of " +
+ stringify(re2->ProgramSize()) + " is larger than maximum of " +
+ stringify(limits.maxProgramSize) + " allowed");
+ }
+
+ return re2;
+}
+
+
using Selector = AttributeConstraint::Selector;
@@ -78,6 +111,7 @@ public:
bool apply(const Attribute& attr) const { return apply_(attr); }
static Try<AttributeConstraintPredicate> create(
+ const RE2Limits& re2Limits,
AttributeConstraint::Predicate&& predicate)
{
using Self = AttributeConstraintPredicate;
@@ -96,6 +130,28 @@ public:
return Self(TextNotEquals{
std::move(*predicate.mutable_text_not_equals()->mutable_value())});
+ case AttributeConstraint::Predicate::kTextMatches: {
+ Try<unique_ptr<const RE2>> re2 =
+ createRE2(re2Limits, predicate.text_matches().regex());
+
+ if (re2.isError()) {
+ return Error(re2.error());
+ }
+
+ return Self(TextMatches{std::move(*re2)});
+ }
+
+ case AttributeConstraint::Predicate::kTextNotMatches: {
+ Try<unique_ptr<const RE2>> re2 =
+ createRE2(re2Limits, predicate.text_not_matches().regex());
+
+ if (re2.isError()) {
+ return Error(re2.error());
+ }
+
+ return Self(TextNotMatches{std::move(*re2)});
+ }
+
case AttributeConstraint::Predicate::PREDICATE_NOT_SET:
return Error("Unknown predicate type");
}
@@ -146,15 +202,43 @@ private:
}
};
- // TODO(asekretenko): Introduce offer constraints for regex match
- // (MESOS-10173).
+ struct TextMatches
+ {
+ unique_ptr<const RE2> re2;
+
+ bool apply(const Nothing&) const { return false; }
+ bool apply(const string& str) const { return RE2::FullMatch(str, *re2); }
+
+ bool apply(const Attribute& attr) const
+ {
+ return attr.type() != Value::TEXT ||
+ RE2::FullMatch(attr.text().value(), *re2);
+ }
+ };
+
+ struct TextNotMatches
+ {
+ unique_ptr<const RE2> re2;
+
+ bool apply(const Nothing&) const { return true; }
+ bool apply(const string& str) const { return !RE2::FullMatch(str, *re2); }
+
+ bool apply(const Attribute& attr) const
+ {
+ return attr.type() != Value::TEXT ||
+ !RE2::FullMatch(attr.text().value(), *re2);
+ }
+ };
+
using Predicate = Variant<
Nothing,
Exists,
NotExists,
TextEquals,
- TextNotEquals>;
+ TextNotEquals,
+ TextMatches,
+ TextNotMatches>;
Predicate predicate;
@@ -171,7 +255,9 @@ private:
[&](const Exists& p) { return p.apply(attribute); },
[&](const NotExists& p) { return p.apply(attribute); },
[&](const TextEquals& p) { return p.apply(attribute); },
- [&](const TextNotEquals& p) { return p.apply(attribute); });
+ [&](const TextNotEquals& p) { return p.apply(attribute); },
+ [&](const TextMatches& p) { return p.apply(attribute); },
+ [&](const TextNotMatches& p) { return p.apply(attribute); });
}
};
@@ -223,6 +309,7 @@ public:
}
static Try<AttributeConstraintEvaluator> create(
+ const RE2Limits& re2Limits,
AttributeConstraint&& constraint)
{
Option<Error> error = validate(constraint.selector());
@@ -232,7 +319,7 @@ public:
Try<AttributeConstraintPredicate> predicate =
AttributeConstraintPredicate::create(
- std::move(*constraint.mutable_predicate()));
+ re2Limits, std::move(*constraint.mutable_predicate()));
if (predicate.isError()) {
return Error(predicate.error());
@@ -293,7 +380,9 @@ public:
});
}
- static Try<OfferConstraintsFilterImpl> create(OfferConstraints&& constraints)
+ static Try<OfferConstraintsFilterImpl> create(
+ const OfferConstraintsFilter::Options& options,
+ OfferConstraints&& constraints)
{
// TODO(asekretenko): This method performs a dumb 1:1 translation of
// `AttributeConstraint`s without any reordering; this leaves room for
@@ -333,7 +422,8 @@ public:
for (AttributeConstraint& constraint :
*group_.mutable_attribute_constraints()) {
Try<AttributeConstraintEvaluator> evaluator =
- AttributeConstraintEvaluator::create(std::move(constraint));
+ AttributeConstraintEvaluator::create(
+ options.re2Limits, std::move(constraint));
if (evaluator.isError()) {
return Error(
@@ -360,10 +450,11 @@ using internal::OfferConstraintsFilterImpl;
Try<OfferConstraintsFilter> OfferConstraintsFilter::create(
+ const Options& options,
OfferConstraints&& constraints)
{
Try<OfferConstraintsFilterImpl> impl =
- OfferConstraintsFilterImpl::create(std::move(constraints));
+ OfferConstraintsFilterImpl::create(options, std::move(constraints));
if (impl.isError()) {
return Error(impl.error());
diff --git a/src/master/constants.hpp b/src/master/constants.hpp
index c384b68..9aa075d 100644
--- a/src/master/constants.hpp
+++ b/src/master/constants.hpp
@@ -179,6 +179,16 @@ const Quota DEFAULT_QUOTA;
// Default weight for a role.
constexpr double DEFAULT_WEIGHT = 1.0;
+// Default values for the `max_mem` option and the limit on
`RE2::ProgramSize()`
+// of RE2 regualr expressions used in offer constraints.
+//
+// As an example, for a regexp
+// "192.168.(1[0-9]|3[4-7]|[1-9]|4[2-9]|[1-4][0-9]|5[3-8]|20[4-7]|53[0-5]).1"
+// re2-2020-07-06 produces a RE2 object with a ProgramSize() of 54,
+// and that can be successfully constructed only with `max_mem` >= 1499.
+constexpr Bytes DEFAULT_OFFER_CONSTRAINTS_RE2_MAX_MEM = Bytes(4096);
+constexpr int DEFAULT_OFFER_CONSTRAINTS_RE2_MAX_PROGRAM_SIZE = 100;
+
} // namespace master {
} // namespace internal {
} // namespace mesos {
diff --git a/src/master/flags.cpp b/src/master/flags.cpp
index 74f4daa..31a8da1 100644
--- a/src/master/flags.cpp
+++ b/src/master/flags.cpp
@@ -743,4 +743,22 @@ mesos::internal::master::Flags::Flags()
}
return None();
});
+
+ add(&Flags::offer_constraints_re2_max_mem,
+ "offer_constraints_re2_max_mem",
+ "Limit on the memory usage of each RE2 regular expression in\n"
+ "framework's offer constraints. If `OfferConstraints` contain a regex\n"
+ "from which a RE2 object cannot be constructed without exceeding this\n"
+ "limit, then framework's attempt to subscribe or update subscription\n"
+ "with these `OfferConstraints` will fail.",
+ DEFAULT_OFFER_CONSTRAINTS_RE2_MAX_MEM);
+
+ add(&Flags::offer_constraints_re2_max_program_size,
+ "offer_constraints_re2_max_program_size",
+ "Limit on the RE2 program size of each regular expression in\n"
+ "framework's offer constraints. If `OfferConstraints` contain a regex\n"
+ "which results in a RE2 object exceeding this limit,\n"
+ "then framework's attempt to subscribe or update subscription\n"
+ "with these `OfferConstraints` will fail.",
+ DEFAULT_OFFER_CONSTRAINTS_RE2_MAX_PROGRAM_SIZE);
}
diff --git a/src/master/flags.hpp b/src/master/flags.hpp
index 78623d6..9500a0a 100644
--- a/src/master/flags.hpp
+++ b/src/master/flags.hpp
@@ -121,6 +121,9 @@ public:
#ifdef ENABLE_PORT_MAPPING_ISOLATOR
Option<size_t> max_executors_per_agent;
#endif // ENABLE_PORT_MAPPING_ISOLATOR
+
+ Bytes offer_constraints_re2_max_mem;
+ int offer_constraints_re2_max_program_size;
};
} // namespace master {
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 4428985..e3858b9 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -333,7 +333,10 @@ Master::Master(
subscribers(this, flags.max_operator_event_stream_subscribers),
authenticator(None()),
metrics(new Metrics(*this)),
- electedTime(None())
+ electedTime(None()),
+ offerConstraintsFilterOptions(
+ {flags.offer_constraints_re2_max_mem,
+ flags.offer_constraints_re2_max_program_size})
{
slaves.limiter = _slaveRemovalLimiter;
@@ -2671,6 +2674,7 @@ void Master::subscribe(
// TODO(asekretenko): Validate roles in offer constraints (see MESOS-10176).
if (validationError.isNone() && subscribe.has_offer_constraints()) {
Try<OfferConstraintsFilter> filter = OfferConstraintsFilter::create(
+ offerConstraintsFilterOptions,
std::move(*subscribe.mutable_offer_constraints()));
if (filter.isError()) {
@@ -2913,6 +2917,7 @@ void Master::subscribe(
// TODO(asekretenko): Validate roles in offer constraints (see MESOS-10176).
if (validationError.isNone() && subscribe.has_offer_constraints()) {
Try<OfferConstraintsFilter> filter = OfferConstraintsFilter::create(
+ offerConstraintsFilterOptions,
std::move(*subscribe.mutable_offer_constraints()));
if (filter.isError()) {
@@ -3253,6 +3258,7 @@ Future<process::http::Response> Master::updateFramework(
if (call.has_offer_constraints()) {
// TODO(asekretenko): Validate roles in offer constraints (see
MESOS-10176).
Try<OfferConstraintsFilter> filter = OfferConstraintsFilter::create(
+ offerConstraintsFilterOptions,
std::move(*call.mutable_offer_constraints()));
if (filter.isError()) {
diff --git a/src/master/master.hpp b/src/master/master.hpp
index e478f80..3d1d472 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -2379,6 +2379,9 @@ private:
process::Time startTime; // Start time used to calculate uptime.
Option<process::Time> electedTime; // Time when this master is elected.
+
+ ::mesos::allocator::OfferConstraintsFilter::Options
+ offerConstraintsFilterOptions;
};
diff --git a/src/tests/master/offer_constraints_filter_tests.cpp
b/src/tests/master/offer_constraints_filter_tests.cpp
index 84a1e91..db1976c 100644
--- a/src/tests/master/offer_constraints_filter_tests.cpp
+++ b/src/tests/master/offer_constraints_filter_tests.cpp
@@ -38,6 +38,15 @@ using ::mesos::allocator::OfferConstraintsFilter;
using ::mesos::scheduler::OfferConstraints;
+static Try<OfferConstraintsFilter> createFilter(OfferConstraints constraints)
+{
+ OfferConstraintsFilter::Options options;
+ options.re2Limits.maxMem = Kilobytes(4);
+ options.re2Limits.maxProgramSize = 100;
+ return OfferConstraintsFilter::create(options, std::move(constraints));
+}
+
+
static Try<OfferConstraints> OfferConstraintsFromJSON(const string& json)
{
Try<JSON::Object> jsonObject = JSON::parse<JSON::Object>(json);
@@ -77,8 +86,7 @@ TEST(OfferConstraintsFilter, NamedAttributeExists)
ASSERT_SOME(constraints);
- const Try<OfferConstraintsFilter> filter =
- OfferConstraintsFilter::create(std::move(*constraints));
+ const Try<OfferConstraintsFilter> filter = createFilter(*constraints);
ASSERT_SOME(filter);
@@ -112,8 +120,7 @@ TEST(OfferConstraintsFilter, NamedAttributeNotExists)
ASSERT_SOME(constraints);
- const Try<OfferConstraintsFilter> filter =
- OfferConstraintsFilter::create(std::move(*constraints));
+ const Try<OfferConstraintsFilter> filter = createFilter(*constraints);
ASSERT_SOME(filter);
@@ -147,8 +154,7 @@ TEST(OfferConstraintsFilter, NamedAttributeTextEquals)
ASSERT_SOME(constraints);
- const Try<OfferConstraintsFilter> filter =
- OfferConstraintsFilter::create(std::move(*constraints));
+ const Try<OfferConstraintsFilter> filter = createFilter(*constraints);
ASSERT_SOME(filter);
@@ -192,8 +198,7 @@ TEST(OfferConstraintsFilter, NamedAttributeTextNotEquals)
ASSERT_SOME(constraints);
- const Try<OfferConstraintsFilter> filter =
- OfferConstraintsFilter::create(std::move(*constraints));
+ const Try<OfferConstraintsFilter> filter = createFilter(*constraints);
ASSERT_SOME(filter);
@@ -238,8 +243,7 @@ TEST(OfferConstraintsFilter, TwoAttributesWithTheSameName)
ASSERT_SOME(constraints);
- const Try<OfferConstraintsFilter> filter =
- OfferConstraintsFilter::create(std::move(*constraints));
+ const Try<OfferConstraintsFilter> filter = createFilter(*constraints);
ASSERT_SOME(filter);
@@ -284,8 +288,7 @@ TEST(OfferConstraintsFilter, TwoConstraintsInGroup)
ASSERT_SOME(constraints);
- const Try<OfferConstraintsFilter> filter =
- OfferConstraintsFilter::create(std::move(*constraints));
+ const Try<OfferConstraintsFilter> filter = createFilter(*constraints);
ASSERT_SOME(filter);
@@ -332,8 +335,7 @@ TEST(OfferConstraintsFilter, TwoGroups)
ASSERT_SOME(constraints);
- const Try<OfferConstraintsFilter> filter =
- OfferConstraintsFilter::create(std::move(*constraints));
+ const Try<OfferConstraintsFilter> filter = createFilter(*constraints);
ASSERT_SOME(filter);
@@ -377,8 +379,7 @@ TEST(OfferConstraintsFilter, TwoRoles)
ASSERT_SOME(constraints);
- const Try<OfferConstraintsFilter> filter =
- OfferConstraintsFilter::create(std::move(*constraints));
+ const Try<OfferConstraintsFilter> filter = createFilter(*constraints);
ASSERT_SOME(filter);
