You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Andrei Sekretenko <as...@mesosphere.io> on 2020/08/06 16:33:20 UTC
Review Request 72738: Added protobuf messages for constraints-based
offer filtering.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72738/
-----------------------------------------------------------
Review request for mesos and Benjamin Mahler.
Bugs: MESOS-10171
https://issues.apache.org/jira/browse/MESOS-10171
Repository: mesos
Description
-------
This patch adds framework's offer constraints into `Subscribe` and
`UpdateFramework` calls, which is a prerequisite to implementing
constraints-based offer filtering (see MESOS-10161).
Diffs
-----
include/mesos/scheduler/scheduler.proto 6e1639a9baf017fa87b274daeedc821389216ddc
include/mesos/v1/scheduler/scheduler.proto eb5fdeb984b28403bd8281742bd0a5f2053863e3
src/internal/devolve.cpp 4527c522473b74622055e1765740e3706b95afdb
Diff: https://reviews.apache.org/r/72738/diff/1/
Testing
-------
Thanks,
Andrei Sekretenko
Re: Review Request 72738: Added protobuf messages for
constraints-based offer filtering.
Posted by Andrei Sekretenko <as...@mesosphere.io>.
> On Aug. 11, 2020, 7:16 p.m., Benjamin Mahler wrote:
> > src/internal/devolve.cpp
> > Lines 248-249 (original), 248-249 (patched)
> > <https://reviews.apache.org/r/72738/diff/1/?file=2237298#file2237298line248>
> >
> > Just to double check, I assume you found this by searching for suppressed_roles or some other piece of the message?
No, actually it was a failure of the new test of UpdateFramework with constraints.
Double-checked by serached once again and realized that we also have unused `evolve` with no test coverage and a `TODO` for considering removal.
Removing that in the preceding patch.
- Andrei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72738/#review221547
-----------------------------------------------------------
On Aug. 13, 2020, 6:54 p.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72738/
> -----------------------------------------------------------
>
> (Updated Aug. 13, 2020, 6:54 p.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-10171
> https://issues.apache.org/jira/browse/MESOS-10171
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds framework's offer constraints into `Subscribe` and
> `UpdateFramework` calls, which is a prerequisite to implementing
> constraints-based offer filtering (see MESOS-10161).
>
>
> Diffs
> -----
>
> include/mesos/scheduler/scheduler.proto 6e1639a9baf017fa87b274daeedc821389216ddc
> include/mesos/v1/scheduler/scheduler.proto eb5fdeb984b28403bd8281742bd0a5f2053863e3
> src/internal/devolve.cpp 4527c522473b74622055e1765740e3706b95afdb
>
>
> Diff: https://reviews.apache.org/r/72738/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Sekretenko
>
>
Re: Review Request 72738: Added protobuf messages for
constraints-based offer filtering.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72738/#review221547
-----------------------------------------------------------
Ship it!
Overall structure looks good! I just left a documentation suggestion, but giving a ship it anyway and happy to take another look once updated if you like.
include/mesos/scheduler/scheduler.proto
Lines 250 (patched)
<https://reviews.apache.org/r/72738/#comment310611>
Hm.. seems fine to me, but I do wonder if this is the best name vs say.. FirstClassAttribute
include/mesos/scheduler/scheduler.proto
Lines 264-270 (patched)
<https://reviews.apache.org/r/72738/#comment310612>
I guess we can put the braces on the same line for these? Seems a bit cleaner
include/mesos/scheduler/scheduler.proto
Lines 273-274 (patched)
<https://reviews.apache.org/r/72738/#comment310613>
Is there a TODO here to show intent of more being added? (like you have for resources below)
include/mesos/scheduler/scheduler.proto
Lines 282 (patched)
<https://reviews.apache.org/r/72738/#comment310605>
Here's a suggestion:
```
Frameworks can express offer constraints for their roles.
Constraints restrict which offers will be sent to the framework:
if an offer does not match the provided constraints, it will not
be sent to the framework.
Constraints are expressed on a per role basis. If you consider a
scheduler that has multiple apps to launch within a single role,
the structure of the constraints for that role looks as follows:
app 1 app 2 app n
constraints OR constraints OR ... OR constraints
/\
/ \
constraint 1 AND constraint 2 AND ... AND constraint n
That is, one of the constraint groups must match for an offer to
be generated, and within a group all the constraints must match.
As a concrete example, consider a scheduler with two applications
with multiple instances it wants to launch within a role:
application 1: hostname == "foo"
application 2: unique "rack" attribute
Assuming there are already some instances of application 2 launched,
the constraints might look like the following:
app 1 app 2
constraints OR constraints
/\ /\
/ \ / \
hostname == "foo" rack != "X" AND
rack != "Y" AND
rack != "Z"
==
(hostname == "foo")
OR
(rack != "X" AND rack != "Y" AND rack != "Z")
The benefits of expressing constraints are:
(1) reduced amount of unusable offers received, and hence:
(2) reduced traffic and processing overhead due to unusable
offer / DECLINE back and forth churn
(3) most importantly, reduced latency to receive the desired
offer for a particular task
```
Hope that eliminates the need for the lower level comments on RoleConstraints and Group. Would be good to get some another pair of eyes on this documentation to make sure people can understand it clearly.
This would also translate pretty cleanly to our website docs?
src/internal/devolve.cpp
Lines 248-249 (original), 248-249 (patched)
<https://reviews.apache.org/r/72738/#comment310610>
Just to double check, I assume you found this by searching for suppressed_roles or some other piece of the message?
- Benjamin Mahler
On Aug. 6, 2020, 4:33 p.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72738/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2020, 4:33 p.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-10171
> https://issues.apache.org/jira/browse/MESOS-10171
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds framework's offer constraints into `Subscribe` and
> `UpdateFramework` calls, which is a prerequisite to implementing
> constraints-based offer filtering (see MESOS-10161).
>
>
> Diffs
> -----
>
> include/mesos/scheduler/scheduler.proto 6e1639a9baf017fa87b274daeedc821389216ddc
> include/mesos/v1/scheduler/scheduler.proto eb5fdeb984b28403bd8281742bd0a5f2053863e3
> src/internal/devolve.cpp 4527c522473b74622055e1765740e3706b95afdb
>
>
> Diff: https://reviews.apache.org/r/72738/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Sekretenko
>
>
Re: Review Request 72738: Added protobuf messages for
constraints-based offer filtering.
Posted by Andrei Sekretenko <as...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72738/
-----------------------------------------------------------
(Updated Aug. 18, 2020, 5:59 p.m.)
Review request for mesos and Benjamin Mahler.
Changes
-------
Multiple attributes: added a comment in `Selector` and a warining into the docs.
Bugs: MESOS-10171
https://issues.apache.org/jira/browse/MESOS-10171
Repository: mesos
Description
-------
This patch adds framework's offer constraints into `Subscribe` and
`UpdateFramework` calls, which is a prerequisite to implementing
constraints-based offer filtering (see MESOS-10161).
Diffs (updated)
-----
docs/attributes-resources.md 2caa44927d45c0ab13f8160794b50f4fde3711aa
include/mesos/scheduler/scheduler.proto 6e1639a9baf017fa87b274daeedc821389216ddc
include/mesos/v1/scheduler/scheduler.proto eb5fdeb984b28403bd8281742bd0a5f2053863e3
src/internal/devolve.cpp 4527c522473b74622055e1765740e3706b95afdb
Diff: https://reviews.apache.org/r/72738/diff/4/
Changes: https://reviews.apache.org/r/72738/diff/3-4/
Testing
-------
Thanks,
Andrei Sekretenko
Re: Review Request 72738: Added protobuf messages for
constraints-based offer filtering.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72738/#review221606
-----------------------------------------------------------
include/mesos/scheduler/scheduler.proto
Lines 261-264 (patched)
<https://reviews.apache.org/r/72738/#comment310668>
Thanks for documenting the subtlety here! Perhaps say that we do not advise using multiple attributes with the same name. (maybe also a TODO that if it becomes a problem we could add a `lookup_mode`, but oy.. I would try really hard to avoid that).
- Benjamin Mahler
On Aug. 14, 2020, 4:36 p.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72738/
> -----------------------------------------------------------
>
> (Updated Aug. 14, 2020, 4:36 p.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-10171
> https://issues.apache.org/jira/browse/MESOS-10171
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds framework's offer constraints into `Subscribe` and
> `UpdateFramework` calls, which is a prerequisite to implementing
> constraints-based offer filtering (see MESOS-10161).
>
>
> Diffs
> -----
>
> include/mesos/scheduler/scheduler.proto 6e1639a9baf017fa87b274daeedc821389216ddc
> include/mesos/v1/scheduler/scheduler.proto eb5fdeb984b28403bd8281742bd0a5f2053863e3
> src/internal/devolve.cpp 4527c522473b74622055e1765740e3706b95afdb
>
>
> Diff: https://reviews.apache.org/r/72738/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Sekretenko
>
>
Re: Review Request 72738: Added protobuf messages for
constraints-based offer filtering.
Posted by Andrei Sekretenko <as...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72738/
-----------------------------------------------------------
(Updated Aug. 14, 2020, 4:36 p.m.)
Review request for mesos and Benjamin Mahler.
Changes
-------
Added a comment describing behaviour for the case of multiple attributes with the same name.
Bugs: MESOS-10171
https://issues.apache.org/jira/browse/MESOS-10171
Repository: mesos
Description
-------
This patch adds framework's offer constraints into `Subscribe` and
`UpdateFramework` calls, which is a prerequisite to implementing
constraints-based offer filtering (see MESOS-10161).
Diffs (updated)
-----
include/mesos/scheduler/scheduler.proto 6e1639a9baf017fa87b274daeedc821389216ddc
include/mesos/v1/scheduler/scheduler.proto eb5fdeb984b28403bd8281742bd0a5f2053863e3
src/internal/devolve.cpp 4527c522473b74622055e1765740e3706b95afdb
Diff: https://reviews.apache.org/r/72738/diff/3/
Changes: https://reviews.apache.org/r/72738/diff/2-3/
Testing
-------
Thanks,
Andrei Sekretenko
Re: Review Request 72738: Added protobuf messages for
constraints-based offer filtering.
Posted by Andrei Sekretenko <as...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72738/
-----------------------------------------------------------
(Updated Aug. 13, 2020, 6:54 p.m.)
Review request for mesos and Benjamin Mahler.
Changes
-------
Accommodated the suggested explanation if the constraints structure (with slight adjustments).
Checked for suppressed_roles once again and found `evolve()`; inserted a patch before this one to remove that.
Bugs: MESOS-10171
https://issues.apache.org/jira/browse/MESOS-10171
Repository: mesos
Description
-------
This patch adds framework's offer constraints into `Subscribe` and
`UpdateFramework` calls, which is a prerequisite to implementing
constraints-based offer filtering (see MESOS-10161).
Diffs (updated)
-----
include/mesos/scheduler/scheduler.proto 6e1639a9baf017fa87b274daeedc821389216ddc
include/mesos/v1/scheduler/scheduler.proto eb5fdeb984b28403bd8281742bd0a5f2053863e3
src/internal/devolve.cpp 4527c522473b74622055e1765740e3706b95afdb
Diff: https://reviews.apache.org/r/72738/diff/2/
Changes: https://reviews.apache.org/r/72738/diff/1-2/
Testing
-------
Thanks,
Andrei Sekretenko