> On 2012-05-29 22:49:04, Benjamin Hindman wrote: > > src/slave/slave.cpp, line 1448 > > <https://reviews.apache.org/r/5072/diff/3/?file=108835#file108835line1448> > > > > 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 garbageCollectExecutorDir. > On 2012-05-29 22:49:04, Benjamin Hindman wrote: > > src/tests/slave_tests.cpp, line 82 > > <https://reviews.apache.org/r/5072/diff/3/?file=108838#file108838line82> > > > > 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 do > 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 > > <https://reviews.apache.org/r/5072/diff/3/?file=108838#file108838line85> > > > > 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 > > <https://reviews.apache.org/r/5072/diff/3/?file=108838#file108838line217> > > > > A comment as to why you're starting the slave after the driver might 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, if 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 > > <https://reviews.apache.org/r/5072/diff/3/?file=108838#file108838line254> > > > > In this case, the launchTask will not wait for resourceOffersCall > > because 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 the > 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 Wickman. > > > 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 for > 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 > >
