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 35AAD182FE for ; Wed, 2 Dec 2015 22:24:24 +0000 (UTC) Received: (qmail 32307 invoked by uid 500); 2 Dec 2015 22:24:11 -0000 Delivered-To: apmail-mesos-reviews-archive@mesos.apache.org Received: (qmail 32295 invoked by uid 500); 2 Dec 2015 22:24:11 -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 31810 invoked by uid 99); 2 Dec 2015 22:24:11 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 02 Dec 2015 22:24:11 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id EB299291D52; Wed, 2 Dec 2015 22:24:10 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============3243889921831941050==" MIME-Version: 1.0 Subject: Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors From: "Vinod Kone" To: "Isabel Jimenez" , "Ben Mahler" , "Vinod Kone" Cc: "Anand Mazumdar" , "mesos" Date: Wed, 02 Dec 2015 22:24:10 -0000 Message-ID: <20151202222410.1719.1091@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: "Vinod Kone" X-ReviewGroup: mesos X-Auto-Response-Suppress: DR, RN, OOF, AutoReply X-ReviewRequest-URL: https://reviews.apache.org/r/38877/ X-Sender: "Vinod Kone" References: <20151130035603.28068.68584@reviews.apache.org> In-Reply-To: <20151130035603.28068.68584@reviews.apache.org> Reply-To: "Vinod Kone" X-ReviewRequest-Repository: mesos --===============3243889921831941050== 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/38877/#review108391 ----------------------------------------------------------- I see a very basic test in the next review. Are you planning to write more comprehensive tests? Is that plan to templatize slave recovery tests for both pid based and http executors? src/slave/slave.cpp (lines 2414 - 2415) I see that 'RECOVERING' is a possible state in `registerExecutor()`. Is that not possible for http executors? src/slave/slave.cpp (line 2450) s/retry/retried/ src/slave/slave.cpp (line 2454) i don't think 'in lieu of' is correct here. just remove the second part starting from "in lieu of...". src/slave/slave.cpp (line 2475) s/http/HTTP/ - Vinod Kone On Nov. 30, 2015, 3:56 a.m., Anand Mazumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38877/ > ----------------------------------------------------------- > > (Updated Nov. 30, 2015, 3:56 a.m.) > > > Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone. > > > Bugs: MESOS-3515 > https://issues.apache.org/jira/browse/MESOS-3515 > > > Repository: mesos > > > Description > ------- > > This change adds the functionality for executors to `Subscribe` via the `api/v1/executor` endpoint. It also stores a marker file as part of the `Subscribe` call if framework `checkpointing` is enabled. This can then be used by the agent when recovering to wait for reconnecting back with the executor. > > > Diffs > ----- > > src/slave/http.cpp c3247f17e9faed32a46d3ab9ee83c399cd2c8d5e > src/slave/slave.hpp 5ee133ae52998d05c8afabb2ade7095e363c75c5 > src/slave/slave.cpp 9055f2a789cb19f3579c15a379ea505dfef0578c > > Diff: https://reviews.apache.org/r/38877/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Anand Mazumdar > > --===============3243889921831941050==--