Return-Path: 
 <dev-return-21101-apmail-mesos-dev-archive=mesos.apache.org@mesos.apache.org>
X-Original-To: apmail-mesos-dev-archive@www.apache.org
Delivered-To: apmail-mesos-dev-archive@www.apache.org
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by minotaur.apache.org (Postfix) with SMTP id 5AED01035B
	for <apmail-mesos-dev-archive@www.apache.org>;
 Wed, 19 Mar 2014 19:37:11 +0000 (UTC)
Received: (qmail 80313 invoked by uid 500); 19 Mar 2014 19:37:10 -0000
Delivered-To: apmail-mesos-dev-archive@mesos.apache.org
Received: (qmail 80261 invoked by uid 500); 19 Mar 2014 19:37:09 -0000
Mailing-List: contact dev-help@mesos.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:dev-help@mesos.apache.org>
List-Unsubscribe: <mailto:dev-unsubscribe@mesos.apache.org>
List-Post: <mailto:dev@mesos.apache.org>
List-Id: <dev.mesos.apache.org>
Reply-To: dev@mesos.apache.org
Delivered-To: mailing list dev@mesos.apache.org
Received: (qmail 80246 invoked by uid 99); 19 Mar 2014 19:37:09 -0000
Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40)
    by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 19 Mar 2014 19:37:09 +0000
Received: from reviews.apache.org (localhost [127.0.0.1])
	by reviews.apache.org (Postfix) with ESMTP id D2C421D54DC;
	Wed, 19 Mar 2014 19:37:04 +0000 (UTC)
Content-Type: multipart/alternative;
 boundary="===============5942866125775383078=="
MIME-Version: 1.0
Subject: Re: Review Request 18403: Added support for launching tasks by
 TaskInfo.
From: "Niklas Nielsen" <nik@qni.dk>
To: "Adam B" <adam@mesosphere.io>, "Benjamin Hindman" <benh@berkeley.edu>,
 "Vinod Kone" <vinodkone@gmail.com>, "Till Toenshoff" <toenshoff@me.com>,
 "Ben Mahler" <benjamin.mahler@gmail.com>, "Ian Downes" <ian.downes@gmail.com>
Cc: "mesos" <dev@mesos.apache.org>, "Niklas Nielsen" <nik@qni.dk>
Date: Wed, 19 Mar 2014 19:37:04 -0000
Message-ID: <20140319193704.6335.29947@reviews.apache.org>
X-ReviewBoard-URL: https://reviews.apache.org
Auto-Submitted: auto-generated
Sender: "Niklas Nielsen" <noreply@reviews.apache.org>
X-ReviewGroup: mesos
X-ReviewRequest-URL: https://reviews.apache.org/r/18403/
X-Sender: "Niklas Nielsen" <noreply@reviews.apache.org>
References: <20140318162357.27430.1663@reviews.apache.org>
In-Reply-To: <20140318162357.27430.1663@reviews.apache.org>
Reply-To: "Niklas Nielsen" <nik@qni.dk>
X-ReviewRequest-Repository: mesos-git

--===============5942866125775383078==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit



> On March 18, 2014, 9:23 a.m., Ian Downes wrote:
> > Some minor points for consideration.

Thanks for the review Ian! The executor->info change became a bit more substantial change. Mind taking a second look? 


> On March 18, 2014, 9:23 a.m., Ian Downes wrote:
> > src/slave/slave.hpp, line 460
> > <https://reviews.apache.org/r/18403/diff/5/?file=522998#file522998line460>
> >
> >     Could this not be Future<ExecutorInfo> and then remove the Future<bool> future below? This better reflects this variable - it's not available until launch is complete, rather than being optional.

I like it but this became a bigger change; try to take a look at the latest review :)


> On March 18, 2014, 9:23 a.m., Ian Downes wrote:
> > src/slave/slave.hpp, line 417
> > <https://reviews.apache.org/r/18403/diff/5/?file=522998#file522998line417>
> >
> >     Do we really need both of these constructors for Executor?

It was to support recover(). I removed the old one and change the call-site in recover.


- Niklas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18403/#review37519
-----------------------------------------------------------


On March 19, 2014, 12:31 p.m., Niklas Nielsen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18403/
> -----------------------------------------------------------
> 
> (Updated March 19, 2014, 12:31 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Ian Downes, Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-922
>     https://issues.apache.org/jira/browse/MESOS-922
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This patch delegates the choice of executor to the containerizer by removing executorInfo dependencies up until Containerizer::launch().
> Containerizer::launch() now returns a future to the executor info that is being run and the slave creates the corresponding executor structure when launch completes.
> This means message handling from the running executor to the slave in the interim where the executor structure has not created, need to be enqueued until executor is ready. So far, registerExecutor() and reregisterExecutor() has been split into two continuations to deal with this issue.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/containerizer.hpp d9ae326 
>   src/slave/containerizer/mesos_containerizer.hpp ee1fd30 
>   src/slave/containerizer/mesos_containerizer.cpp c819c97 
>   src/slave/http.cpp 594032d 
>   src/slave/slave.hpp 01b80df 
>   src/slave/slave.cpp d8d3e0f 
>   src/tests/containerizer.hpp 5686398 
>   src/tests/containerizer.cpp bfb9341 
> 
> Diff: https://reviews.apache.org/r/18403/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Niklas Nielsen
> 
>


--===============5942866125775383078==--