From reviews-return-30441-apmail-mesos-reviews-archive=mesos.apache.org@mesos.apache.org Fri Apr 8 23:03:27 2016 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 6F92E190BD for ; Fri, 8 Apr 2016 23:03:27 +0000 (UTC) Received: (qmail 62887 invoked by uid 500); 8 Apr 2016 23:03:27 -0000 Delivered-To: apmail-mesos-reviews-archive@mesos.apache.org Received: (qmail 62860 invoked by uid 500); 8 Apr 2016 23:03:27 -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 62844 invoked by uid 99); 8 Apr 2016 23:03:27 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 08 Apr 2016 23:03:27 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id 0C8462AEE5E; Fri, 8 Apr 2016 23:03:24 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============5023414619520145681==" MIME-Version: 1.0 Subject: Re: Review Request 45941: Add check in agent for incorrect oversubscribed resource. From: Ben Mahler To: Ben Mahler , Joseph Wu Cc: Zhitao Li , mesos Date: Fri, 08 Apr 2016 23:03:24 -0000 Message-ID: <20160408230324.20107.96941@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: Ben Mahler X-ReviewGroup: mesos X-Auto-Response-Suppress: DR, RN, OOF, AutoReply X-ReviewRequest-URL: https://reviews.apache.org/r/45941/ X-Sender: Ben Mahler References: <20160408200247.20106.21240@reviews.apache.org> In-Reply-To: <20160408200247.20106.21240@reviews.apache.org> Reply-To: Ben Mahler X-ReviewRequest-Repository: mesos --===============5023414619520145681== 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/45941/#review127914 ----------------------------------------------------------- src/slave/slave.cpp (lines 4989 - 4991) Any reason why this CHECK isn't below the VLOG(1) above? Having it above seems to make it more clear that we're validating the input from the estimator, no? Also, for the comment here, we should avoid talking about the hierarchical allocator since that just happens to be where the failure manifests. Perhaps something like: ``` // Oversubscrbable resources must be considered revocable. // // TODO(bmahler): Consider tagging input as revocable // rather than rejecting and crashing here. ``` - Ben Mahler On April 8, 2016, 8:02 p.m., Zhitao Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45941/ > ----------------------------------------------------------- > > (Updated April 8, 2016, 8:02 p.m.) > > > Review request for mesos, Ben Mahler and Joseph Wu. > > > Bugs: MESOS-5131 > https://issues.apache.org/jira/browse/MESOS-5131 > > > Repository: mesos > > > Description > ------- > > Add check in agent for incorrect oversubscribed resource. > > I've decided to let agent crash explicitly here instead of fail more silently. > > > Diffs > ----- > > src/slave/slave.cpp f090c853b8affc4be5eecb4f616ec881fc2b60c3 > > Diff: https://reviews.apache.org/r/45941/diff/ > > > Testing > ------- > > Running existing test, and verify manually that offending resource crashes the agent. > > (Any suggestion to test `CHECK` is welcomed). > > > Thanks, > > Zhitao Li > > --===============5023414619520145681==--