From reviews-return-78414-apmail-mesos-reviews-archive=mesos.apache.org@mesos.apache.org Fri Jun 1 00:26:37 2018 Return-Path: X-Original-To: apmail-mesos-reviews-archive@minotaur.apache.org Delivered-To: apmail-mesos-reviews-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 2F40118474 for ; Fri, 1 Jun 2018 00:26:37 +0000 (UTC) Received: (qmail 51704 invoked by uid 500); 1 Jun 2018 00:26:37 -0000 Delivered-To: apmail-mesos-reviews-archive@mesos.apache.org Received: (qmail 51691 invoked by uid 500); 1 Jun 2018 00:26:37 -0000 Mailing-List: contact reviews-help@mesos.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: reviews@mesos.apache.org Delivered-To: mailing list reviews@mesos.apache.org Received: (qmail 51679 invoked by uid 99); 1 Jun 2018 00:26:36 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd1-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 01 Jun 2018 00:26:36 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd1-us-west.apache.org (ASF Mail Server at spamd1-us-west.apache.org) with ESMTP id 08191CA99C; Fri, 1 Jun 2018 00:26:36 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd1-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 0.949 X-Spam-Level: X-Spam-Status: No, score=0.949 tagged_above=-999 required=6.31 tests=[HEADER_FROM_DIFFERENT_DOMAINS=0.249, HTML_MESSAGE=2, KAM_LAZY_DOMAIN_SECURITY=1, RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled Received: from mx1-lw-eu.apache.org ([10.40.0.8]) by localhost (spamd1-us-west.apache.org [10.40.0.7]) (amavisd-new, port 10024) with ESMTP id 5-QL0myoTZUo; Fri, 1 Jun 2018 00:26:34 +0000 (UTC) Received: from mailrelay1-us-west.apache.org (mailrelay1-us-west.apache.org [209.188.14.139]) by mx1-lw-eu.apache.org (ASF Mail Server at mx1-lw-eu.apache.org) with ESMTP id F039A5FACE; Fri, 1 Jun 2018 00:26:33 +0000 (UTC) Received: from reviews.apache.org (unknown [10.41.0.12]) by mailrelay1-us-west.apache.org (ASF Mail Server at mailrelay1-us-west.apache.org) with ESMTP id 4024EE0016; Fri, 1 Jun 2018 00:26:33 +0000 (UTC) Received: from reviews-vm2.apache.org (localhost [IPv6:::1]) by reviews.apache.org (ASF Mail Server at reviews-vm2.apache.org) with ESMTP id 28E07C407A7; Fri, 1 Jun 2018 00:26:33 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============0143568392255770850==" MIME-Version: 1.0 Subject: Re: Review Request 67371: Introduced a random sorter as an alternative to the DRF sorter. From: Meng Zhu To: Meng Zhu , Gaston Kleiman , Greg Mann Cc: Benjamin Mahler , mesos Date: Fri, 01 Jun 2018 00:26:33 -0000 Message-ID: <20180601002633.38602.18928@reviews-vm2.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: Meng Zhu X-ReviewGroup: mesos X-Auto-Response-Suppress: DR, RN, OOF, AutoReply X-ReviewRequest-URL: https://reviews.apache.org/r/67371/ X-Sender: Meng Zhu X-ReviewBoard-ShipIt: 1 References: <20180530010044.51962.68658@reviews-vm2.apache.org> In-Reply-To: <20180530010044.51962.68658@reviews-vm2.apache.org> X-ReviewBoard-Diff-For: src/master/allocator/sorter/random/sorter.cpp X-ReviewBoard-Diff-For: src/master/allocator/sorter/random/sorter.hpp Reply-To: Meng Zhu X-ReviewRequest-Repository: mesos --===============0143568392255770850== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit ----------------------------------------------------------- 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) Document the complexity and determinism. src/master/allocator/sorter/random/sorter.hpp Lines 135 (patched) s/share/sampling probablity/ src/master/allocator/sorter/random/sorter.hpp Lines 233 (patched) Not needed. src/master/allocator/sorter/random/sorter.hpp Lines 295-296 (patched) invariant (2) no longer relevant. `s/It is up to....//` src/master/allocator/sorter/random/sorter.hpp Lines 397-404 (patched) Not needed. src/master/allocator/sorter/random/sorter.cpp Lines 466 (patched) 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 > > --===============0143568392255770850==--