Return-Path: X-Original-To: apmail-incubator-mesos-dev-archive@minotaur.apache.org Delivered-To: apmail-incubator-mesos-dev-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 23AB8CDD9 for ; Wed, 30 May 2012 17:55:13 +0000 (UTC) Received: (qmail 37345 invoked by uid 500); 30 May 2012 17:55:13 -0000 Delivered-To: apmail-incubator-mesos-dev-archive@incubator.apache.org Received: (qmail 37318 invoked by uid 500); 30 May 2012 17:55:13 -0000 Mailing-List: contact mesos-dev-help@incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: mesos-dev@incubator.apache.org Delivered-To: mailing list mesos-dev@incubator.apache.org Received: (qmail 37296 invoked by uid 99); 30 May 2012 17:55:13 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 30 May 2012 17:55:13 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id D4C2C1C4114; Wed, 30 May 2012 17:55:10 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============3481095712696645865==" MIME-Version: 1.0 Subject: Re: Review Request: Slave Garbage Collection From: "Benjamin Hindman" To: "Benjamin Hindman" , "Brian Wickman" , "John Sirois" Date: Wed, 30 May 2012 17:55:10 -0000 Message-ID: <20120530175510.22577.74230@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org X-ReviewRequest-URL: https://reviews.apache.org/r/5072/ Cc: "Vinod Kone" , "mesos" In-Reply-To: <20120529224904.22577.70427@reviews.apache.org> References: <20120529224904.22577.70427@reviews.apache.org> --===============3481095712696645865== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable > On 2012-05-29 22:49:04, Benjamin Hindman wrote: > > src/slave/slave.cpp, line 1448 > > > > > > Why do you need the copy here? > = > Vinod Kone wrote: > because framework->destroyExecutor(), below, deletes executor The string will get copied where/when it needs to get copied automagically.= In this case, it will happen when you do 'result.push_back(dir)' inside ga= rbageCollectExecutorDir. > On 2012-05-29 22:49:04, Benjamin Hindman wrote: > > src/tests/slave_tests.cpp, line 82 > > > > > > Why '/var/tmp'? What's the problem with the default? > = > Vinod Kone wrote: > this hoop is to get access to the work directory. it has nothing to d= o with the directory name. makes sense? No, it doesn't. :( Why aren't you just using '/tmp/mesos'? > On 2012-05-29 22:49:04, Benjamin Hindman wrote: > > src/tests/slave_tests.cpp, line 85 > > > > > > Can all this be setup once in the constructor versus for each test? > = > Vinod Kone wrote: > IIUC, constructor is also called per test!? did u mean SetupTestCase(= )? Yes, that is what I meant. > On 2012-05-29 22:49:04, Benjamin Hindman wrote: > > src/tests/slave_tests.cpp, line 217 > > > > > > A comment as to why you're starting the slave after the driver migh= t be illuminating. > = > Vinod Kone wrote: > no particular reason. does it matter? No, but all of our other tests do it differently, so it stands out. Thus, i= f it doesn't matter, I'd probably start the slave before the driver. > On 2012-05-29 22:49:04, Benjamin Hindman wrote: > > src/tests/slave_tests.cpp, line 254 > > > > > > In this case, the launchTask will not wait for resourceOffersCall b= ecause it has already occurred, hence the reason to not abstract that away.= If you want to get multiple resourceOfferCalls, consider using a volatile = int that you increment each time a resourceOffer call occurs. > = > Vinod Kone wrote: > umm, the line above resets the resourceOffersCall trigger, to force t= he wait. am i missing something? Oh woah yeah, totally missed that! - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5072/#review8178 ----------------------------------------------------------- On 2012-05-30 01:33:27, Vinod Kone wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5072/ > ----------------------------------------------------------- > = > (Updated 2012-05-30 01:33:27) > = > = > Review request for mesos, Benjamin Hindman, John Sirois, and Brian Wickma= n. > = > = > Summary > ------- > = > This is the first cut for GC inside the slave. = > = > There are 2 kinds of gc going on > = > --> Executor work dirs -- These get deleted whenever (after a timeout) an= executor exits/shutdown > --> Old slave dirs -- These get deleted when the slave gets registered fo= r the first time on a startup. > = > = > Diffs > ----- > = > src/Makefile.am 96cb4c6 = > src/common/seconds.hpp 5ae088a = > src/common/time.hpp PRE-CREATION = > src/common/timer.hpp 71dc493 = > src/common/utils.hpp 09d278a = > src/log/coordinator.hpp b881c6a = > src/log/log.hpp 79bb738 = > src/log/network.hpp 9499c63 = > src/master/frameworks_manager.hpp 31d708b = > src/slave/constants.hpp f0c8679 = > src/slave/slave.hpp 08a29d8 = > src/slave/slave.cpp 8585230 = > src/state/state.hpp 6166414 = > src/state/zookeeper.cpp e31fff7 = > src/tests/base_zookeeper_test.hpp 2f5747e = > src/tests/slave_tests.cpp PRE-CREATION = > src/zookeeper/group.hpp 8646202 = > src/zookeeper/zookeeper.hpp 5043a64 = > = > Diff: https://reviews.apache.org/r/5072/diff > = > = > Testing > ------- > = > make check scucceeds. > = > Yet to write GC specific tests. > = > = > Thanks, > = > Vinod > = > --===============3481095712696645865==--