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