> On June 8, 2016, 4:54 p.m., Jiang Yan Xu wrote: > > src/tests/gc_tests.cpp, lines 948-956 > > <https://reviews.apache.org/r/47973/diff/2/?file=1400007#file1400007line948> > > > > Why do we still need `containerPaths`? We can get the sandbox path > > directly: > > > > ``` > > const string sandbox = slave::paths::getExecutorLatestRunPath( > > flags.work_dir, > > slaveId, > > frameworkId.get(), > > executor_id); > > > > ASSERT_TRUE(os::exists(sandbox)); > > ``` > > > > right?
getExecutorLatestPath(...) gives us the executor run path for latest symlink which gets collected by GC. So, this path can't be used for post GC assertions. Since we didn't create a container ourselves in the test so we didn't have a container id to call the method getExecutorRunPath(..., containerId). Here, I am getting all the run paths with getExecutorRunPaths(..) and then elimating the one with latest symlink to get the actual path. - Megha ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47973/#review136592 ----------------------------------------------------------- On June 8, 2016, 4:57 p.m., Megha Sharma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47973/ > ----------------------------------------------------------- > > (Updated June 8, 2016, 4:57 p.m.) > > > Review request for mesos and Jiang Yan Xu. > > > Bugs: MESOS-5196 > https://issues.apache.org/jira/browse/MESOS-5196 > > > Repository: mesos > > > Description > ------- > > It's possible for tasks and isolators to lay down files that are not > deletable by GC. In the face of such errors GC needs to free up disk > space wherever it can because it's already re-offered to frameworks. > > > Diffs > ----- > > src/slave/gc.cpp eedb8ca713d7f5195055bb6417f03ab70731af97 > src/tests/gc_tests.cpp 4cb7c2f612984f7f5a9378a7f972f2438bbf28c5 > > Diff: https://reviews.apache.org/r/47973/diff/ > > > Testing > ------- > > make check. > Tested with option --gtest_also_run_disabled_tests. > > > Thanks, > > Megha Sharma > >
