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...@apache.org> on 2020/09/24 15:07:31 UTC
Re: Review Request 72897: Made offer constraints filter and protobuf
non-optional inside the code.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72897/
-----------------------------------------------------------
(Updated Sept. 24, 2020, 5:07 p.m.)
Review request for mesos and Benjamin Mahler.
Changes
-------
Removed filter/constraints optionality instead of the previous approach.
Summary (updated)
-----------------
Made offer constraints filter and protobuf non-optional inside the code.
Bugs: MESOS-10189
https://issues.apache.org/jira/browse/MESOS-10189
Repository: mesos
Description (updated)
-------
Given that Mesos now provides a guarantee that specifying no offer
constraint in UPDATE_FRAMEWORK/SUBSCRIBE call is equivalent to
specifying default-constructed `OfferConstraints`, and that we are
intending to make the V0 scheduler driver always require offer
constraints as an argument to the `updateFramework()`, it no longer
makes sense to keep `OfferConstraints`/`OfferConstraintsFilter`
optional inside the Mesos code base.
This patch replaces a non-set `Option<OfferConstraints>` with
default-constructed `OfferConstraints`, and a non-set
`Option<OfferConstraintsFilter>` with a default-constructed filter.
Diffs (updated)
-----
include/mesos/allocator/allocator.hpp b0a5d6aaa08a05cb1ece318999a8760df932ba64
src/master/allocator/mesos/offer_constraints_filter.cpp 441ebc10219bf3bd623fac2bb08945ea9deb7ea3
src/master/framework.cpp 980828e04cae96f091a6b30e553ff667fa58a917
src/master/http.cpp 3d8e7095ee58c2505042095b2c593c2317f3232f
src/master/master.hpp 95aca586f17a206b6ac7f0c48ea99c8525c336ad
src/master/master.cpp 576ae107a6a3c99b51516fdde4df9bff86c90a99
src/master/readonly_handler.cpp df499d1a5e9622222ee2fe0c47bc77fb1ef9d8bc
Diff: https://reviews.apache.org/r/72897/diff/2/
Changes: https://reviews.apache.org/r/72897/diff/1-2/
Testing
-------
Thanks,
Andrei Sekretenko
Re: Review Request 72897: Made offer constraints filter and protobuf
non-optional inside the code.
Posted by Andrei Sekretenko <as...@apache.org>.
> On Sept. 24, 2020, 7:58 p.m., Benjamin Mahler wrote:
> > include/mesos/allocator/allocator.hpp
> > Line 113 (original), 113-118 (patched)
> > <https://reviews.apache.org/r/72897/diff/2/?file=2239966#file2239966line113>
> >
> > Hm.. is this used? I would think we're always passing in the object to create, even if not set?
There are at least two cases where we would otherwise need to explicitly perform something like
`CHECK_NOTERROR(OfferConstraintsFilter::create({Bytes(0), 0}, {})`:
- recovering a non-subscribed framework from agent's FrameworkInfo
- various locations in tests, especially the hierarchical allocator tests
Given the sheer number of call sites of `Allocator::addFramework` in the testing codebase, we'll have to add some kind of wrapper for that.
I don't think this is better than just keeping framewok's allocator options default-constructable.
- Andrei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72897/#review221952
-----------------------------------------------------------
On Sept. 24, 2020, 5:07 p.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72897/
> -----------------------------------------------------------
>
> (Updated Sept. 24, 2020, 5:07 p.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-10189
> https://issues.apache.org/jira/browse/MESOS-10189
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Given that Mesos now provides a guarantee that specifying no offer
> constraint in UPDATE_FRAMEWORK/SUBSCRIBE call is equivalent to
> specifying default-constructed `OfferConstraints`, and that we are
> intending to make the V0 scheduler driver always require offer
> constraints as an argument to the `updateFramework()`, it no longer
> makes sense to keep `OfferConstraints`/`OfferConstraintsFilter`
> optional inside the Mesos code base.
>
> This patch replaces a non-set `Option<OfferConstraints>` with
> default-constructed `OfferConstraints`, and a non-set
> `Option<OfferConstraintsFilter>` with a default-constructed filter.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp b0a5d6aaa08a05cb1ece318999a8760df932ba64
> src/master/allocator/mesos/offer_constraints_filter.cpp 441ebc10219bf3bd623fac2bb08945ea9deb7ea3
> src/master/framework.cpp 980828e04cae96f091a6b30e553ff667fa58a917
> src/master/http.cpp 3d8e7095ee58c2505042095b2c593c2317f3232f
> src/master/master.hpp 95aca586f17a206b6ac7f0c48ea99c8525c336ad
> src/master/master.cpp 576ae107a6a3c99b51516fdde4df9bff86c90a99
> src/master/readonly_handler.cpp df499d1a5e9622222ee2fe0c47bc77fb1ef9d8bc
>
>
> Diff: https://reviews.apache.org/r/72897/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Sekretenko
>
>
Re: Review Request 72897: Made offer constraints filter and protobuf
non-optional inside the code.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72897/#review221952
-----------------------------------------------------------
Ship it!
include/mesos/allocator/allocator.hpp
Line 113 (original), 113-118 (patched)
<https://reviews.apache.org/r/72897/#comment311019>
Hm.. is this used? I would think we're always passing in the object to create, even if not set?
- Benjamin Mahler
On Sept. 24, 2020, 3:07 p.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72897/
> -----------------------------------------------------------
>
> (Updated Sept. 24, 2020, 3:07 p.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-10189
> https://issues.apache.org/jira/browse/MESOS-10189
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Given that Mesos now provides a guarantee that specifying no offer
> constraint in UPDATE_FRAMEWORK/SUBSCRIBE call is equivalent to
> specifying default-constructed `OfferConstraints`, and that we are
> intending to make the V0 scheduler driver always require offer
> constraints as an argument to the `updateFramework()`, it no longer
> makes sense to keep `OfferConstraints`/`OfferConstraintsFilter`
> optional inside the Mesos code base.
>
> This patch replaces a non-set `Option<OfferConstraints>` with
> default-constructed `OfferConstraints`, and a non-set
> `Option<OfferConstraintsFilter>` with a default-constructed filter.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp b0a5d6aaa08a05cb1ece318999a8760df932ba64
> src/master/allocator/mesos/offer_constraints_filter.cpp 441ebc10219bf3bd623fac2bb08945ea9deb7ea3
> src/master/framework.cpp 980828e04cae96f091a6b30e553ff667fa58a917
> src/master/http.cpp 3d8e7095ee58c2505042095b2c593c2317f3232f
> src/master/master.hpp 95aca586f17a206b6ac7f0c48ea99c8525c336ad
> src/master/master.cpp 576ae107a6a3c99b51516fdde4df9bff86c90a99
> src/master/readonly_handler.cpp df499d1a5e9622222ee2fe0c47bc77fb1ef9d8bc
>
>
> Diff: https://reviews.apache.org/r/72897/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Sekretenko
>
>