From reviews-return-82807-apmail-mesos-reviews-archive=mesos.apache.org@mesos.apache.org Wed Oct 17 14:07:17 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 AD82919DCA for ; Wed, 17 Oct 2018 14:07:17 +0000 (UTC) Received: (qmail 57620 invoked by uid 500); 17 Oct 2018 14:07:17 -0000 Delivered-To: apmail-mesos-reviews-archive@mesos.apache.org Received: (qmail 57588 invoked by uid 500); 17 Oct 2018 14:07:17 -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 57576 invoked by uid 99); 17 Oct 2018 14:07:16 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd3-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 17 Oct 2018 14:07:16 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd3-us-west.apache.org (ASF Mail Server at spamd3-us-west.apache.org) with ESMTP id 8F1B2182426; Wed, 17 Oct 2018 14:07:16 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd3-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 0.951 X-Spam-Level: X-Spam-Status: No, score=0.951 tagged_above=-999 required=6.31 tests=[HEADER_FROM_DIFFERENT_DOMAINS=0.001, HTML_MESSAGE=2, KAM_LAZY_DOMAIN_SECURITY=1, KAM_LOTSOFHASH=0.25, RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled Received: from mx1-lw-us.apache.org ([10.40.0.8]) by localhost (spamd3-us-west.apache.org [10.40.0.10]) (amavisd-new, port 10024) with ESMTP id ZEQ0VxcgDJLS; Wed, 17 Oct 2018 14:07:15 +0000 (UTC) Received: from mailrelay1-us-west.apache.org (mailrelay1-us-west.apache.org [209.188.14.139]) by mx1-lw-us.apache.org (ASF Mail Server at mx1-lw-us.apache.org) with ESMTP id 07F655F577; Wed, 17 Oct 2018 14:07:15 +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 BC417E00D4; Wed, 17 Oct 2018 14:07:14 +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 8E59DC40221; Wed, 17 Oct 2018 14:07:14 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============8105184273495023067==" MIME-Version: 1.0 Subject: Re: Review Request 68147: Added agent support to remove local resource providers. From: Benjamin Bannier To: Chun-Hung Hsiao , =?utf-8?q?Gast=C3=B3n_Kleiman?= , Jan Schlicht , Greg Mann Cc: Benjamin Bannier , Mesos Reviewbot Windows , Mesos Reviewbot , mesos Date: Wed, 17 Oct 2018 14:07:14 -0000 Message-ID: <20181017140714.55547.5666@reviews-vm2.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: Benjamin Bannier X-ReviewGroup: mesos X-Auto-Response-Suppress: DR, RN, OOF, AutoReply X-ReviewRequest-URL: https://reviews.apache.org/r/68147/ X-Sender: Benjamin Bannier References: <20180817222924.27878.59376@reviews-vm2.apache.org> In-Reply-To: <20180817222924.27878.59376@reviews-vm2.apache.org> Reply-To: Benjamin Bannier X-ReviewRequest-Repository: mesos --===============8105184273495023067== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit > On Aug. 18, 2018, 12:29 a.m., Chun-Hung Hsiao wrote: > > src/slave/slave.cpp > > Lines 8195 (patched) > > > > > > Let's validate that there is no task using the resources provided by this RP before doing the removal, or fail the call. > > Benjamin Bannier wrote: > I believe this should be treated as orthogonal issue. It might e.g., happen that the user triggering the RP removal cannot kill tasks or prevent them from being rescheduled. I'd suggest to leaves as is. > > WDYT? > > Chun-Hung Hsiao wrote: > The scenario I am worried about is that when the RP is marked as gone, the RP may fail to tear itself down if there is any task using its resources. We don't seem to have a good story on incompatible resource changes ATM. If we e.g., update a RP config in an incompatible way, we make not attempt to detect or rectify this in any way, either. At least in the agent we were in general we were operating under the assumption that tasks would die on incompatible changes to their resources. I'd suggest to drop this issue for now and potentially file a JIRA to track draining and situations where it might be needed. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/#review207559 ----------------------------------------------------------- On Aug. 20, 2018, 8:47 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68147/ > ----------------------------------------------------------- > > (Updated Aug. 20, 2018, 8:47 p.m.) > > > Review request for mesos, Chun-Hung Hsiao, Gastón Kleiman, Greg Mann, and Jan Schlicht. > > > Bugs: MESOS-8403 > https://issues.apache.org/jira/browse/MESOS-8403 > > > Repository: mesos > > > Description > ------- > > This patch adds support for triggering permanent removal of local > resource providers. We also add authorization and tests as part of this > patch. > > > Diffs > ----- > > include/mesos/agent/agent.proto 74488e873cbf99ca487403b70691912cf3788288 > include/mesos/v1/agent/agent.proto 5d1ab6fd7f6f21593444467c66ed35ee90aea07f > src/slave/http.hpp 78200874cb03a3d5af4ee0443c03a1d94cd494cc > src/slave/http.cpp 1b6d266a1a7821c9de6871cbca43317b3c392a32 > src/slave/slave.hpp b1d695b59a63a5af34b19faff16bf6c82b7e7d88 > src/slave/slave.cpp e6c7e686f287fb4448a0074d4e99298665fc866d > src/slave/validation.cpp df5e1373dbe497bc859455dcaf4e064e923bd72e > src/tests/api_tests.cpp ee82350f7a6c6d44ba0590608e7d9a223c0be169 > > > Diff: https://reviews.apache.org/r/68147/diff/8/ > > > Testing > ------- > > `make check` > > > Thanks, > > Benjamin Bannier > > --===============8105184273495023067==--