You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Mahler <bm...@apache.org> on 2017/11/30 01:45:22 UTC
Review Request 64194: Stripped AllocationInfo from offers to
non-MULTI_ROLE schedulers.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64194/
-----------------------------------------------------------
Review request for mesos, James DeFelice and Michael Park.
Bugs: MESOS-8237
https://issues.apache.org/jira/browse/MESOS-8237
Repository: mesos
Description
-------
Per MESOS-8237, it is problematic to show `Resource.allocation_info`
for "old" schedulers. The example that was presented was the
"mesos-go" 3rd party library that provides resource comparison logic.
The library has both MULTI-ROLE and non-MULTI_ROLE schedulers as
clients, these clients use the comparison logic supplied by the
library to see if the offer contains the resources they need.
The library author does not want to have to do any stripping of the
allocation info to preserve the equality.
Diffs
-----
src/master/master.cpp 700e12433b0b66efc3f5dd296711c0f203a13144
Diff: https://reviews.apache.org/r/64194/diff/1/
Testing
-------
Updated the tests in the subsequent review.
Thanks,
Benjamin Mahler
Re: Review Request 64194: Stripped AllocationInfo from offers to
non-MULTI_ROLE schedulers.
Posted by Benjamin Mahler <bm...@apache.org>.
> On Dec. 1, 2017, 12:54 a.m., Michael Park wrote:
> > src/master/master.cpp
> > Lines 8129-8136 (patched)
> > <https://reviews.apache.org/r/64194/diff/1/?file=1904323#file1904323line8129>
> >
> > I found this example a bit difficult to follow...
> > Maybe it's enough to link the JIRA and leave a simple
> > comment saying:
> >
> > > "pre-MULTI_ROLE" schedulers are not `AllocationInfo`
> > > aware, and since they may be performing operations
> > > that implicitly uses all of `Resource`'s state
> > > (e.g., equality comparison), we strip
> > > the `AllocationInfo` from `Resource`, as well as
> > > `Offer`.
Sounds good, thanks for the suggestion!
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64194/#review192417
-----------------------------------------------------------
On Nov. 30, 2017, 1:45 a.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64194/
> -----------------------------------------------------------
>
> (Updated Nov. 30, 2017, 1:45 a.m.)
>
>
> Review request for mesos, James DeFelice and Michael Park.
>
>
> Bugs: MESOS-8237
> https://issues.apache.org/jira/browse/MESOS-8237
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Per MESOS-8237, it is problematic to show `Resource.allocation_info`
> for "old" schedulers. The example that was presented was the
> "mesos-go" 3rd party library that provides resource comparison logic.
> The library has both MULTI-ROLE and non-MULTI_ROLE schedulers as
> clients, these clients use the comparison logic supplied by the
> library to see if the offer contains the resources they need.
> The library author does not want to have to do any stripping of the
> allocation info to preserve the equality.
>
>
> Diffs
> -----
>
> src/master/master.cpp 700e12433b0b66efc3f5dd296711c0f203a13144
>
>
> Diff: https://reviews.apache.org/r/64194/diff/1/
>
>
> Testing
> -------
>
> Updated the tests in the subsequent review.
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 64194: Stripped AllocationInfo from offers to
non-MULTI_ROLE schedulers.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64194/#review192417
-----------------------------------------------------------
Ship it!
src/master/master.cpp
Lines 8128 (patched)
<https://reviews.apache.org/r/64194/#comment270541>
`s/"old"/pre-MULTI_ROLE/`?
src/master/master.cpp
Lines 8129-8136 (patched)
<https://reviews.apache.org/r/64194/#comment270547>
I found this example a bit difficult to follow...
Maybe it's enough to link the JIRA and leave a simple
comment saying:
> "pre-MULTI_ROLE" schedulers are not `AllocationInfo`
> aware, and since they may be performing operations
> that implicitly uses all of `Resource`'s state
> (e.g., equality comparison), we strip
> the `AllocationInfo` from `Resource`, as well as
> `Offer`.
src/master/master.cpp
Lines 8129-8139 (patched)
<https://reviews.apache.org/r/64194/#comment270548>
I found this example a bit difficult to follow...
Maybe it's enough to link the JIRA and leave a simple
comment saying:
> "pre-MULTI_ROLE" schedulers are not `AllocationInfo`
> aware, and since they may be performing operations
> that implicitly uses all of `Resource`'s state
> (e.g., equality comparison), we strip
> the `AllocationInfo` from `Resource`, as well as
> `Offer`.
src/master/master.cpp
Lines 8143 (patched)
<https://reviews.apache.org/r/64194/#comment270542>
`s/r/resource/`?
- Michael Park
On Nov. 29, 2017, 5:45 p.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64194/
> -----------------------------------------------------------
>
> (Updated Nov. 29, 2017, 5:45 p.m.)
>
>
> Review request for mesos, James DeFelice and Michael Park.
>
>
> Bugs: MESOS-8237
> https://issues.apache.org/jira/browse/MESOS-8237
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Per MESOS-8237, it is problematic to show `Resource.allocation_info`
> for "old" schedulers. The example that was presented was the
> "mesos-go" 3rd party library that provides resource comparison logic.
> The library has both MULTI-ROLE and non-MULTI_ROLE schedulers as
> clients, these clients use the comparison logic supplied by the
> library to see if the offer contains the resources they need.
> The library author does not want to have to do any stripping of the
> allocation info to preserve the equality.
>
>
> Diffs
> -----
>
> src/master/master.cpp 700e12433b0b66efc3f5dd296711c0f203a13144
>
>
> Diff: https://reviews.apache.org/r/64194/diff/1/
>
>
> Testing
> -------
>
> Updated the tests in the subsequent review.
>
>
> Thanks,
>
> Benjamin Mahler
>
>