-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72745/#review221663
-----------------------------------------------------------
include/mesos/scheduler/scheduler.proto
Lines 279-285 (original), 279-317 (patched)
<https://reviews.apache.org/r/72745/#comment310729>
Ok, after the discussion on slack / reviewboard / and your comment here in the diff, I
have a good grasp on the approach and it makes sense.
I think we can get away with a simplified naming scheme and logical explanation centered
around: we only support text, non text is not supported and will always be matched to let
the scheduler do its own filtering.
I took a stab at the simplified API naming:
```
// Predicates for (pseudo)attribute value equality.
//
// We currently only support TEXT (string for pseudoattributes)
// equality, so these predicates will always match (yield `true`)
// for SCALAR or RANGES based attributes. This way, schedulers
// will still receive offers for SCALAR and RANGES attributes where
// a string equality constraint was specified to mesos, and the
// scheduler's own constraint filtering will determine how to
// filter these other types of attributes.
//
// For example: if a scheduler sets a constraint
//
// { "selector": {"attribute_name": "foo"},
// "predicate": {"equals":"2.0"} }
//
// Agents with a SCALAR attribute {"name": "foo", "scalar": {"value": 1.0}}
// will match (since it's SCALAR, not TEXT!) and the scheduler side
// filtering will need to filter it according to its desired semantics.
//
// Therefore, it is strongly recommended to only use TEXT attributes
// on agents. For users with existing attributes, this can be done
// by adding a variant of the existing attribute that uses TEXT instead:
//
// --attributes=foo2:v1.0
// which gets parsed into
// {"name": "foo2", "text": {"value": "v1.0"}}, etc.
// Yields `true` if the (pseudo)attribute exists and has a value
// equal to this value.
//
// Non-TEXT (string for pseudoattributes) attributes will always
// yield `true` for the reason explained above.
message Equals {
required string value = 1;
}
// Yields `true` if the (pseudo)attribute does not exist or has
// a value not equal to this value.
//
// Non-TEXT (string for pseudoattributes) attributes will always
// yield `true` for the reason explained above.
message NotEquals {
required string value = 1;
}
oneof predicate {
Exists exists = 1;
NotExists not_exists = 2;
Equals equals = 3;
NotEquals not_equals = 4;
}
```
Let me know your thoughts!
- Benjamin Mahler
On Aug. 20, 2020, 9:09 a.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72745/
> -----------------------------------------------------------
>
> (Updated Aug. 20, 2020, 9:09 a.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-10172
> https://issues.apache.org/jira/browse/MESOS-10172
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds protobuf messages for setting offer constraints
> on equality/non-equality of agent's (pseudo)attribute to a specified
> string.
>
> Both added contsraint predicates will evaluate to `true` when the
> attribute is not TEXT. This way, schedulers will still perform all
> filtration based on non-TEXT attributes on their own, which helps to
> avoid subtle integration bugs caused by discrepancies in Scalar/Ranges
> comparison between Mesos and the scheduler.
>
> Given that schedulers seem to rarely put constraints on Scalar/Ranges
> attributes in the real world, this should not prevent them from
> obtaining performance benefits by setting offer constraints.
>
>
> Diffs
> -----
>
> include/mesos/scheduler/scheduler.proto 6e1639a9baf017fa87b274daeedc821389216ddc
> include/mesos/v1/scheduler/scheduler.proto eb5fdeb984b28403bd8281742bd0a5f2053863e3
>
>
> Diff: https://reviews.apache.org/r/72745/diff/4/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Sekretenko
>
>
|