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 2018/06/01 00:26:33 UTC
Re: Review Request 67371: Introduced a random sorter as an
alternative to the DRF sorter.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67371/#review204159
-----------------------------------------------------------
Fix it, then Ship it!
src/master/allocator/sorter/random/sorter.hpp
Lines 101 (patched)
<https://reviews.apache.org/r/67371/#comment286556>
Document the complexity and determinism.
src/master/allocator/sorter/random/sorter.hpp
Lines 135 (patched)
<https://reviews.apache.org/r/67371/#comment286554>
s/share/sampling probablity/
src/master/allocator/sorter/random/sorter.hpp
Lines 233 (patched)
<https://reviews.apache.org/r/67371/#comment286553>
Not needed.
src/master/allocator/sorter/random/sorter.hpp
Lines 295-296 (patched)
<https://reviews.apache.org/r/67371/#comment286551>
invariant (2) no longer relevant.
`s/It is up to....//`
src/master/allocator/sorter/random/sorter.hpp
Lines 397-404 (patched)
<https://reviews.apache.org/r/67371/#comment286555>
Not needed.
src/master/allocator/sorter/random/sorter.cpp
Lines 466 (patched)
<https://reviews.apache.org/r/67371/#comment286552>
The allocator currently calls `sort()` as it cycles through the roles/frameworks, this is unnecessarily expensive and would lead to nonideal random distribution. Ideally, we only need to sort once for each allocation cycle (once for each complete triple loop) and cycle through the randomized list. This would require changes to the sorter interface. We can do it in subsequent patches. Can you add a TODO note here?
- Meng Zhu
On May 29, 2018, 6 p.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67371/
> -----------------------------------------------------------
>
> (Updated May 29, 2018, 6 p.m.)
>
>
> Review request for mesos, Gaston Kleiman, Greg Mann, and Meng Zhu.
>
>
> Bugs: MESOS-8936
> https://issues.apache.org/jira/browse/MESOS-8936
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This sorter returns a weighted random shuffling of its clients
> upon each `sort()` request.
>
> This implementation is a copy of the `DRFSorter` with share
> calculation logic (including the `dirty` bit) removed and an
> adjusted `sort()` implementation. Work needs to be done to
> reduce the amount of duplicated logic needed across sorter
> implementations, but it is left unaddressed here.
>
>
> Diffs
> -----
>
> src/Makefile.am b7184ceccef5f2e985d905c155156f95c7a7c7b4
> src/master/allocator/sorter/random/sorter.hpp PRE-CREATION
> src/master/allocator/sorter/random/sorter.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/67371/diff/1/
>
>
> Testing
> -------
>
> Unit tests added in subsequent patch.
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 67371: Introduced a random sorter as an
alternative to the DRF sorter.
Posted by Benjamin Mahler <bm...@apache.org>.
> On June 1, 2018, 12:26 a.m., Meng Zhu wrote:
> >
Great review, thanks for catching all of these!
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67371/#review204159
-----------------------------------------------------------
On May 30, 2018, 1 a.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67371/
> -----------------------------------------------------------
>
> (Updated May 30, 2018, 1 a.m.)
>
>
> Review request for mesos, Gaston Kleiman, Greg Mann, and Meng Zhu.
>
>
> Bugs: MESOS-8936
> https://issues.apache.org/jira/browse/MESOS-8936
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This sorter returns a weighted random shuffling of its clients
> upon each `sort()` request.
>
> This implementation is a copy of the `DRFSorter` with share
> calculation logic (including the `dirty` bit) removed and an
> adjusted `sort()` implementation. Work needs to be done to
> reduce the amount of duplicated logic needed across sorter
> implementations, but it is left unaddressed here.
>
>
> Diffs
> -----
>
> src/Makefile.am b7184ceccef5f2e985d905c155156f95c7a7c7b4
> src/master/allocator/sorter/random/sorter.hpp PRE-CREATION
> src/master/allocator/sorter/random/sorter.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/67371/diff/1/
>
>
> Testing
> -------
>
> Unit tests added in subsequent patch.
>
>
> Thanks,
>
> Benjamin Mahler
>
>