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 2019/08/29 11:33:33 UTC
Review Request 71399: Renamed allocated -> offeredOrAllocated in
HierarchialAllocatorProcess.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71399/
-----------------------------------------------------------
Review request for mesos, Benjamin Mahler and Meng Zhu.
Bugs: MESOS-9949
https://issues.apache.org/jira/browse/MESOS-9949
Repository: mesos
Description
-------
Renamed allocated -> offeredOrAllocated in HierarchialAllocatorProcess.
Diffs
-----
src/master/allocator/mesos/hierarchical.hpp 65d103ea18b91adbdde3b0eb85113a1c0f4d990c
src/master/allocator/mesos/hierarchical.cpp dd73d5b46c95ac7827d39ed93e0da097f4e8937a
src/master/allocator/mesos/metrics.cpp f8465bef90815827c97ac2886d00c03a333f4df3
Diff: https://reviews.apache.org/r/71399/diff/1/
Testing
-------
Thanks,
Andrei Sekretenko
Re: Review Request 71399: Renamed `allocate` and its derivatives in
HierarchialAllocatorProcess.
Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71399/#review217552
-----------------------------------------------------------
Ship it!
Ship It!
- Meng Zhu
On Sept. 3, 2019, 6:23 a.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71399/
> -----------------------------------------------------------
>
> (Updated Sept. 3, 2019, 6:23 a.m.)
>
>
> Review request for mesos, Benjamin Mahler and Meng Zhu.
>
>
> Bugs: MESOS-9949
> https://issues.apache.org/jira/browse/MESOS-9949
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch makes the `HierarchialAllocatorProcess` code more consistent
> with naming of different subsets of resources in the master
> (where "resource belongs to `offered`" is equivalent to "there is an
> offer with this resource", "resource belongs to `allocated`"
> is equivalent to "there is a task/executor with this resource" and
> `offered` has no intersection with `allocated`).
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp 65d103ea18b91adbdde3b0eb85113a1c0f4d990c
> src/master/allocator/mesos/hierarchical.cpp dd73d5b46c95ac7827d39ed93e0da097f4e8937a
> src/master/allocator/mesos/metrics.cpp f8465bef90815827c97ac2886d00c03a333f4df3
>
>
> Diff: https://reviews.apache.org/r/71399/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Sekretenko
>
>
Re: Review Request 71399: Renamed `allocate` and its derivatives in
HierarchialAllocatorProcess.
Posted by Andrei Sekretenko <as...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71399/
-----------------------------------------------------------
(Updated Sept. 3, 2019, 1:23 p.m.)
Review request for mesos, Benjamin Mahler and Meng Zhu.
Changes
-------
Left (un)trackAllocatedResources as it is.
Summary (updated)
-----------------
Renamed `allocate` and its derivatives in HierarchialAllocatorProcess.
Bugs: MESOS-9949
https://issues.apache.org/jira/browse/MESOS-9949
Repository: mesos
Description (updated)
-------
This patch makes the `HierarchialAllocatorProcess` code more consistent
with naming of different subsets of resources in the master
(where "resource belongs to `offered`" is equivalent to "there is an
offer with this resource", "resource belongs to `allocated`"
is equivalent to "there is a task/executor with this resource" and
`offered` has no intersection with `allocated`).
Diffs (updated)
-----
src/master/allocator/mesos/hierarchical.hpp 65d103ea18b91adbdde3b0eb85113a1c0f4d990c
src/master/allocator/mesos/hierarchical.cpp dd73d5b46c95ac7827d39ed93e0da097f4e8937a
src/master/allocator/mesos/metrics.cpp f8465bef90815827c97ac2886d00c03a333f4df3
Diff: https://reviews.apache.org/r/71399/diff/3/
Changes: https://reviews.apache.org/r/71399/diff/2-3/
Testing
-------
Thanks,
Andrei Sekretenko
Re: Review Request 71399: Renamed allocated -> offeredOrAllocated in
HierarchialAllocatorProcess.
Posted by Andrei Sekretenko <as...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71399/
-----------------------------------------------------------
(Updated Aug. 30, 2019, 12:29 p.m.)
Review request for mesos, Benjamin Mahler and Meng Zhu.
Changes
-------
Fixed comment changes and formatting.
Bugs: MESOS-9949
https://issues.apache.org/jira/browse/MESOS-9949
Repository: mesos
Description
-------
Renamed allocated -> offeredOrAllocated in HierarchialAllocatorProcess.
Diffs (updated)
-----
src/master/allocator/mesos/hierarchical.hpp 65d103ea18b91adbdde3b0eb85113a1c0f4d990c
src/master/allocator/mesos/hierarchical.cpp dd73d5b46c95ac7827d39ed93e0da097f4e8937a
src/master/allocator/mesos/metrics.cpp f8465bef90815827c97ac2886d00c03a333f4df3
Diff: https://reviews.apache.org/r/71399/diff/2/
Changes: https://reviews.apache.org/r/71399/diff/1-2/
Testing
-------
Thanks,
Andrei Sekretenko
Re: Review Request 71399: Renamed allocated -> offeredOrAllocated in
HierarchialAllocatorProcess.
Posted by Andrei Sekretenko <as...@mesosphere.io>.
> On Aug. 30, 2019, 12:28 a.m., Meng Zhu wrote:
> > src/master/allocator/mesos/hierarchical.hpp
> > Lines 780-781 (original), 779-780 (patched)
> > <https://reviews.apache.org/r/71399/diff/1/?file=2163097#file2163097line781>
> >
> > Same as the previous patch, should this just be `trackOffered`? Ditto for untrack
No, definitely not.
Unlike similar `Slave`'s method from the previous patch, this one is also called in various resovery scenarios. In those cases some of the resources might be actually used by task/executor.
- Andrei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71399/#review217499
-----------------------------------------------------------
On Aug. 29, 2019, 11:33 a.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71399/
> -----------------------------------------------------------
>
> (Updated Aug. 29, 2019, 11:33 a.m.)
>
>
> Review request for mesos, Benjamin Mahler and Meng Zhu.
>
>
> Bugs: MESOS-9949
> https://issues.apache.org/jira/browse/MESOS-9949
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Renamed allocated -> offeredOrAllocated in HierarchialAllocatorProcess.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp 65d103ea18b91adbdde3b0eb85113a1c0f4d990c
> src/master/allocator/mesos/hierarchical.cpp dd73d5b46c95ac7827d39ed93e0da097f4e8937a
> src/master/allocator/mesos/metrics.cpp f8465bef90815827c97ac2886d00c03a333f4df3
>
>
> Diff: https://reviews.apache.org/r/71399/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Sekretenko
>
>
Re: Review Request 71399: Renamed `allocate` and its derivatives in
HierarchialAllocatorProcess.
Posted by Andrei Sekretenko <as...@mesosphere.io>.
> On Aug. 30, 2019, 12:28 a.m., Meng Zhu wrote:
> > src/master/allocator/mesos/hierarchical.hpp
> > Lines 780-781 (original), 779-780 (patched)
> > <https://reviews.apache.org/r/71399/diff/1/?file=2163097#file2163097line781>
> >
> > Same as the previous patch, should this just be `trackOffered`? Ditto for untrack
>
> Andrei Sekretenko wrote:
> No, definitely not.
>
> Unlike similar `Slave`'s method from the previous patch, this one is also called in various resovery scenarios. In those cases some of the resources might be actually used by task/executor.
Some cases in which both offered and alllocated can be tracked/untracked:
https://github.com/apache/mesos/blob/7e30597783f9501faab02b930f26a19eeb666989/src/master/allocator/mesos/hierarchical.cpp#L602
https://github.com/apache/mesos/blob/7e30597783f9501faab02b930f26a19eeb666989/src/master/allocator/mesos/hierarchical.cpp#L641
Probably I have to leave this pair as it is for now, as the suggested name might be misleading as well. Leaving this as TODO.
- Andrei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71399/#review217499
-----------------------------------------------------------
On Sept. 3, 2019, 1:23 p.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71399/
> -----------------------------------------------------------
>
> (Updated Sept. 3, 2019, 1:23 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Meng Zhu.
>
>
> Bugs: MESOS-9949
> https://issues.apache.org/jira/browse/MESOS-9949
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch makes the `HierarchialAllocatorProcess` code more consistent
> with naming of different subsets of resources in the master
> (where "resource belongs to `offered`" is equivalent to "there is an
> offer with this resource", "resource belongs to `allocated`"
> is equivalent to "there is a task/executor with this resource" and
> `offered` has no intersection with `allocated`).
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp 65d103ea18b91adbdde3b0eb85113a1c0f4d990c
> src/master/allocator/mesos/hierarchical.cpp dd73d5b46c95ac7827d39ed93e0da097f4e8937a
> src/master/allocator/mesos/metrics.cpp f8465bef90815827c97ac2886d00c03a333f4df3
>
>
> Diff: https://reviews.apache.org/r/71399/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Sekretenko
>
>
Re: Review Request 71399: Renamed allocated -> offeredOrAllocated in
HierarchialAllocatorProcess.
Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71399/#review217499
-----------------------------------------------------------
src/master/allocator/mesos/hierarchical.hpp
Line 558 (original), 558 (patched)
<https://reviews.apache.org/r/71399/#comment304753>
s/allocatable resources of//
src/master/allocator/mesos/hierarchical.hpp
Line 561 (original), 561 (patched)
<https://reviews.apache.org/r/71399/#comment304754>
s/resources of//
src/master/allocator/mesos/hierarchical.hpp
Line 564 (original), 564 (patched)
<https://reviews.apache.org/r/71399/#comment304755>
s/resources of//
src/master/allocator/mesos/hierarchical.hpp
Line 571 (original), 572 (patched)
<https://reviews.apache.org/r/71399/#comment304756>
I think we can go without the comment.
src/master/allocator/mesos/hierarchical.hpp
Line 573 (original), 574 (patched)
<https://reviews.apache.org/r/71399/#comment304757>
keep the blank line above?
src/master/allocator/mesos/hierarchical.hpp
Lines 780-781 (original), 779-780 (patched)
<https://reviews.apache.org/r/71399/#comment304758>
Same as the previous patch, should this just be `trackOffered`? Ditto for untrack
- Meng Zhu
On Aug. 29, 2019, 4:33 a.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71399/
> -----------------------------------------------------------
>
> (Updated Aug. 29, 2019, 4:33 a.m.)
>
>
> Review request for mesos, Benjamin Mahler and Meng Zhu.
>
>
> Bugs: MESOS-9949
> https://issues.apache.org/jira/browse/MESOS-9949
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Renamed allocated -> offeredOrAllocated in HierarchialAllocatorProcess.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp 65d103ea18b91adbdde3b0eb85113a1c0f4d990c
> src/master/allocator/mesos/hierarchical.cpp dd73d5b46c95ac7827d39ed93e0da097f4e8937a
> src/master/allocator/mesos/metrics.cpp f8465bef90815827c97ac2886d00c03a333f4df3
>
>
> Diff: https://reviews.apache.org/r/71399/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Sekretenko
>
>