You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Meng Zhu <mz...@mesosphere.io> on 2019/08/17 00:00:55 UTC

Review Request 71303: Tracked frameworks in the role sorter.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71303/
-----------------------------------------------------------

Review request for mesos, Andrei Sekretenko and Benjamin Mahler.


Bugs: MESOS-9917
    https://issues.apache.org/jira/browse/MESOS-9917


Repository: mesos


Description
-------

This paves the way for removing the framework sorters.


Diffs
-----

  src/master/allocator/mesos/hierarchical.cpp 580d35a3b71c1f7e851fa0504c6b9f037c05c378 
  src/master/allocator/mesos/sorter/drf/sorter.hpp 537086039fd804453ea8c682cda775d8fdff038f 
  src/master/allocator/mesos/sorter/drf/sorter.cpp 09889cdf3dc8c0e773b8e2e24154fb0edd2cc254 
  src/master/allocator/mesos/sorter/random/sorter.hpp f18b014ed15ff8906fbdbd3248becefde896651c 
  src/master/allocator/mesos/sorter/random/sorter.cpp 60a5797472460a8d3d9be938af9f6711ea51d484 
  src/master/allocator/mesos/sorter/sorter.hpp 52b8a7b57bf17759311b32aa56c26e614119b773 


Diff: https://reviews.apache.org/r/71303/diff/1/


Testing
-------

make check


Thanks,

Meng Zhu


Re: Review Request 71303: Tracked frameworks in the role sorter.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71303/#review217328
-----------------------------------------------------------



Bad review!

Reviews applied: [71303, 71301, 71269, 71258, 71257, 71255, 71254]

Error:
2019-08-20 11:33:32 URL:https://reviews.apache.org/r/71254/diff/raw/ [5967/5967] -> "71254.patch" [1]
error: patch failed: src/master/allocator/mesos/sorter/drf/sorter.cpp:17
error: src/master/allocator/mesos/sorter/drf/sorter.cpp: patch does not apply
error: patch failed: src/master/allocator/mesos/sorter/random/sorter.cpp:17
error: src/master/allocator/mesos/sorter/random/sorter.cpp: patch does not apply

- Mesos Reviewbot


On Aug. 17, 2019, midnight, Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71303/
> -----------------------------------------------------------
> 
> (Updated Aug. 17, 2019, midnight)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9917
>     https://issues.apache.org/jira/browse/MESOS-9917
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This paves the way for removing the framework sorters.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.cpp 580d35a3b71c1f7e851fa0504c6b9f037c05c378 
>   src/master/allocator/mesos/sorter/drf/sorter.hpp 537086039fd804453ea8c682cda775d8fdff038f 
>   src/master/allocator/mesos/sorter/drf/sorter.cpp 09889cdf3dc8c0e773b8e2e24154fb0edd2cc254 
>   src/master/allocator/mesos/sorter/random/sorter.hpp f18b014ed15ff8906fbdbd3248becefde896651c 
>   src/master/allocator/mesos/sorter/random/sorter.cpp 60a5797472460a8d3d9be938af9f6711ea51d484 
>   src/master/allocator/mesos/sorter/sorter.hpp 52b8a7b57bf17759311b32aa56c26e614119b773 
> 
> 
> Diff: https://reviews.apache.org/r/71303/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 71303: Tracked frameworks in the role sorter.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71303/#review217263
-----------------------------------------------------------



Bad review!

Reviews applied: [71303, 71301, 71269, 71258, 71257, 71255, 71254]

Error:
2019-08-18 19:35:36 URL:https://reviews.apache.org/r/71254/diff/raw/ [5967/5967] -> "71254.patch" [1]
error: patch failed: src/master/allocator/mesos/sorter/drf/sorter.cpp:17
error: src/master/allocator/mesos/sorter/drf/sorter.cpp: patch does not apply
error: patch failed: src/master/allocator/mesos/sorter/random/sorter.cpp:17
error: src/master/allocator/mesos/sorter/random/sorter.cpp: patch does not apply

- Mesos Reviewbot


On Aug. 16, 2019, 5 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71303/
> -----------------------------------------------------------
> 
> (Updated Aug. 16, 2019, 5 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9917
>     https://issues.apache.org/jira/browse/MESOS-9917
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This paves the way for removing the framework sorters.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.cpp 580d35a3b71c1f7e851fa0504c6b9f037c05c378 
>   src/master/allocator/mesos/sorter/drf/sorter.hpp 537086039fd804453ea8c682cda775d8fdff038f 
>   src/master/allocator/mesos/sorter/drf/sorter.cpp 09889cdf3dc8c0e773b8e2e24154fb0edd2cc254 
>   src/master/allocator/mesos/sorter/random/sorter.hpp f18b014ed15ff8906fbdbd3248becefde896651c 
>   src/master/allocator/mesos/sorter/random/sorter.cpp 60a5797472460a8d3d9be938af9f6711ea51d484 
>   src/master/allocator/mesos/sorter/sorter.hpp 52b8a7b57bf17759311b32aa56c26e614119b773 
> 
> 
> Diff: https://reviews.apache.org/r/71303/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 71303: Tracked frameworks in the role sorter.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71303/#review217370
-----------------------------------------------------------



Won't we need the allocated amount for each framework (leaf node) in the DRF case?

- Benjamin Mahler


On Aug. 17, 2019, midnight, Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71303/
> -----------------------------------------------------------
> 
> (Updated Aug. 17, 2019, midnight)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9917
>     https://issues.apache.org/jira/browse/MESOS-9917
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This paves the way for removing the framework sorters.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.cpp 580d35a3b71c1f7e851fa0504c6b9f037c05c378 
>   src/master/allocator/mesos/sorter/drf/sorter.hpp 537086039fd804453ea8c682cda775d8fdff038f 
>   src/master/allocator/mesos/sorter/drf/sorter.cpp 09889cdf3dc8c0e773b8e2e24154fb0edd2cc254 
>   src/master/allocator/mesos/sorter/random/sorter.hpp f18b014ed15ff8906fbdbd3248becefde896651c 
>   src/master/allocator/mesos/sorter/random/sorter.cpp 60a5797472460a8d3d9be938af9f6711ea51d484 
>   src/master/allocator/mesos/sorter/sorter.hpp 52b8a7b57bf17759311b32aa56c26e614119b773 
> 
> 
> Diff: https://reviews.apache.org/r/71303/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 71303: Tracked frameworks in the role sorter.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71303/#review217256
-----------------------------------------------------------



Bad review!

Reviews applied: [71303, 71301, 71269, 71258, 71257, 71255, 71254]

Error:
2019-08-17 03:08:34 URL:https://reviews.apache.org/r/71254/diff/raw/ [5967/5967] -> "71254.patch" [1]
error: patch failed: src/master/allocator/mesos/sorter/drf/sorter.cpp:17
error: src/master/allocator/mesos/sorter/drf/sorter.cpp: patch does not apply
error: patch failed: src/master/allocator/mesos/sorter/random/sorter.cpp:17
error: src/master/allocator/mesos/sorter/random/sorter.cpp: patch does not apply

- Mesos Reviewbot


On Aug. 17, 2019, 2 a.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71303/
> -----------------------------------------------------------
> 
> (Updated Aug. 17, 2019, 2 a.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9917
>     https://issues.apache.org/jira/browse/MESOS-9917
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This paves the way for removing the framework sorters.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.cpp 580d35a3b71c1f7e851fa0504c6b9f037c05c378 
>   src/master/allocator/mesos/sorter/drf/sorter.hpp 537086039fd804453ea8c682cda775d8fdff038f 
>   src/master/allocator/mesos/sorter/drf/sorter.cpp 09889cdf3dc8c0e773b8e2e24154fb0edd2cc254 
>   src/master/allocator/mesos/sorter/random/sorter.hpp f18b014ed15ff8906fbdbd3248becefde896651c 
>   src/master/allocator/mesos/sorter/random/sorter.cpp 60a5797472460a8d3d9be938af9f6711ea51d484 
>   src/master/allocator/mesos/sorter/sorter.hpp 52b8a7b57bf17759311b32aa56c26e614119b773 
> 
> 
> Diff: https://reviews.apache.org/r/71303/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 71303: Tracked frameworks in the role sorter.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71303/#review217301
-----------------------------------------------------------



Bad review!

Reviews applied: [71303, 71301, 71269, 71258, 71257, 71255, 71254]

Error:
2019-08-19 21:44:50 URL:https://reviews.apache.org/r/71254/diff/raw/ [5967/5967] -> "71254.patch" [1]
error: patch failed: src/master/allocator/mesos/sorter/drf/sorter.cpp:17
error: src/master/allocator/mesos/sorter/drf/sorter.cpp: patch does not apply
error: patch failed: src/master/allocator/mesos/sorter/random/sorter.cpp:17
error: src/master/allocator/mesos/sorter/random/sorter.cpp: patch does not apply

- Mesos Reviewbot


On Aug. 17, 2019, midnight, Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71303/
> -----------------------------------------------------------
> 
> (Updated Aug. 17, 2019, midnight)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9917
>     https://issues.apache.org/jira/browse/MESOS-9917
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This paves the way for removing the framework sorters.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.cpp 580d35a3b71c1f7e851fa0504c6b9f037c05c378 
>   src/master/allocator/mesos/sorter/drf/sorter.hpp 537086039fd804453ea8c682cda775d8fdff038f 
>   src/master/allocator/mesos/sorter/drf/sorter.cpp 09889cdf3dc8c0e773b8e2e24154fb0edd2cc254 
>   src/master/allocator/mesos/sorter/random/sorter.hpp f18b014ed15ff8906fbdbd3248becefde896651c 
>   src/master/allocator/mesos/sorter/random/sorter.cpp 60a5797472460a8d3d9be938af9f6711ea51d484 
>   src/master/allocator/mesos/sorter/sorter.hpp 52b8a7b57bf17759311b32aa56c26e614119b773 
> 
> 
> Diff: https://reviews.apache.org/r/71303/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 71303: Tracked frameworks in the role sorter.

Posted by Meng Zhu <mz...@mesosphere.io>.

> On Aug. 19, 2019, 9:44 a.m., Benjamin Mahler wrote:
> > What's the thinking on taking the allocator's framework struct? Why not just take the information we need so that we can keep the interface self-contained?

One of the goals of this refactoring is to eliminate state duplication between allocator and the sorter. My plan is to let sorter have back pointers pointing to both the framework (and the role in subsequent patches) and the role struct in the allocator.

Strictly speaking, there is not much information we need about the framework in the sorter:
- For random sorter, we only need frameworkId
- For DRF sorter, we only need to know allocations (which can be built in the sorter->allocated callbacks, however, note the duplication with the allocator).

Also, ideally, for any potential other sorter implementation that requires any other framework info (just a random example: sorting based on capability), carry a pointer makes more sense.

I am not sure what's the benefit of keeping the interface self-contained. If for unit testing, we can probably still make it unit-testable even if we carry the back pointer.


- Meng


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71303/#review217293
-----------------------------------------------------------


On Aug. 16, 2019, 5 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71303/
> -----------------------------------------------------------
> 
> (Updated Aug. 16, 2019, 5 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9917
>     https://issues.apache.org/jira/browse/MESOS-9917
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This paves the way for removing the framework sorters.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.cpp 580d35a3b71c1f7e851fa0504c6b9f037c05c378 
>   src/master/allocator/mesos/sorter/drf/sorter.hpp 537086039fd804453ea8c682cda775d8fdff038f 
>   src/master/allocator/mesos/sorter/drf/sorter.cpp 09889cdf3dc8c0e773b8e2e24154fb0edd2cc254 
>   src/master/allocator/mesos/sorter/random/sorter.hpp f18b014ed15ff8906fbdbd3248becefde896651c 
>   src/master/allocator/mesos/sorter/random/sorter.cpp 60a5797472460a8d3d9be938af9f6711ea51d484 
>   src/master/allocator/mesos/sorter/sorter.hpp 52b8a7b57bf17759311b32aa56c26e614119b773 
> 
> 
> Diff: https://reviews.apache.org/r/71303/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 71303: Tracked frameworks in the role sorter.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71303/#review217293
-----------------------------------------------------------



What's the thinking on taking the allocator's framework struct? Why not just take the information we need so that we can keep the interface self-contained?

- Benjamin Mahler


On Aug. 17, 2019, midnight, Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71303/
> -----------------------------------------------------------
> 
> (Updated Aug. 17, 2019, midnight)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9917
>     https://issues.apache.org/jira/browse/MESOS-9917
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This paves the way for removing the framework sorters.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.cpp 580d35a3b71c1f7e851fa0504c6b9f037c05c378 
>   src/master/allocator/mesos/sorter/drf/sorter.hpp 537086039fd804453ea8c682cda775d8fdff038f 
>   src/master/allocator/mesos/sorter/drf/sorter.cpp 09889cdf3dc8c0e773b8e2e24154fb0edd2cc254 
>   src/master/allocator/mesos/sorter/random/sorter.hpp f18b014ed15ff8906fbdbd3248becefde896651c 
>   src/master/allocator/mesos/sorter/random/sorter.cpp 60a5797472460a8d3d9be938af9f6711ea51d484 
>   src/master/allocator/mesos/sorter/sorter.hpp 52b8a7b57bf17759311b32aa56c26e614119b773 
> 
> 
> Diff: https://reviews.apache.org/r/71303/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 71303: Enabled role sorters to track frameworks.

Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71303/
-----------------------------------------------------------

(Updated Aug. 27, 2019, 10 p.m.)


Review request for mesos, Andrei Sekretenko and Benjamin Mahler.


Summary (updated)
-----------------

Enabled role sorters to track frameworks.


Bugs: MESOS-9917
    https://issues.apache.org/jira/browse/MESOS-9917


Repository: mesos


Description (updated)
-------

This paves the way for deprecating the framework sorters.

Four new public sorter methods are added:

```
addFramework(role, framework)
removeFramework(role, framework)
contains(framework)
sortFrameworks(role)
```


Diffs (updated)
-----

  src/master/allocator/mesos/sorter/drf/metrics.cpp 82cb5300c5a498d2042562c660a9bd5108158538 
  src/master/allocator/mesos/sorter/drf/sorter.hpp f157ec6153325b0457ab1bc76f9eb018712d753a 
  src/master/allocator/mesos/sorter/drf/sorter.cpp 09889cdf3dc8c0e773b8e2e24154fb0edd2cc254 
  src/master/allocator/mesos/sorter/random/sorter.hpp 8663ccd4ccdd36c8ea7513b492d3f46a0b15ff5c 
  src/master/allocator/mesos/sorter/random/sorter.cpp 60a5797472460a8d3d9be938af9f6711ea51d484 
  src/master/allocator/mesos/sorter/sorter.hpp 52b8a7b57bf17759311b32aa56c26e614119b773 


Diff: https://reviews.apache.org/r/71303/diff/2/

Changes: https://reviews.apache.org/r/71303/diff/1-2/


Testing
-------

make check


Thanks,

Meng Zhu


Re: Review Request 71303: Tracked frameworks in the role sorter.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71303/#review217341
-----------------------------------------------------------



Bad review!

Reviews applied: [71303, 71301, 71269, 71258, 71257, 71255, 71254]

Error:
2019-08-20 23:14:47 URL:https://reviews.apache.org/r/71254/diff/raw/ [5967/5967] -> "71254.patch" [1]
error: patch failed: src/master/allocator/mesos/sorter/drf/sorter.cpp:17
error: src/master/allocator/mesos/sorter/drf/sorter.cpp: patch does not apply
error: patch failed: src/master/allocator/mesos/sorter/random/sorter.cpp:17
error: src/master/allocator/mesos/sorter/random/sorter.cpp: patch does not apply

- Mesos Reviewbot


On Aug. 16, 2019, 5 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71303/
> -----------------------------------------------------------
> 
> (Updated Aug. 16, 2019, 5 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9917
>     https://issues.apache.org/jira/browse/MESOS-9917
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This paves the way for removing the framework sorters.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.cpp 580d35a3b71c1f7e851fa0504c6b9f037c05c378 
>   src/master/allocator/mesos/sorter/drf/sorter.hpp 537086039fd804453ea8c682cda775d8fdff038f 
>   src/master/allocator/mesos/sorter/drf/sorter.cpp 09889cdf3dc8c0e773b8e2e24154fb0edd2cc254 
>   src/master/allocator/mesos/sorter/random/sorter.hpp f18b014ed15ff8906fbdbd3248becefde896651c 
>   src/master/allocator/mesos/sorter/random/sorter.cpp 60a5797472460a8d3d9be938af9f6711ea51d484 
>   src/master/allocator/mesos/sorter/sorter.hpp 52b8a7b57bf17759311b32aa56c26e614119b773 
> 
> 
> Diff: https://reviews.apache.org/r/71303/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>