mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Meng Zhu <m...@mesosphere.io>
Subject Re: Review Request 67371: Introduced a random sorter as an alternative to the DRF sorter.
Date Fri, 01 Jun 2018 00:26:33 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message