----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47973/#review138791 -----------------------------------------------------------
Fix it, then Ship it! Just some minor suggestions. src/tests/gc_tests.cpp (lines 72 - 73) <https://reviews.apache.org/r/47973/#comment203982> Kill these. src/tests/gc_tests.cpp (line 924) <https://reviews.apache.org/r/47973/#comment203988> s/mountPoint/mountPoint_/ src/tests/gc_tests.cpp (line 943) <https://reviews.apache.org/r/47973/#comment203986> s/executor_id/executorId/. We generally use camelCasing unless it's from protobuf or flags, etc. src/tests/gc_tests.cpp (lines 960 - 962) <https://reviews.apache.org/r/47973/#comment203987> It doesn't look like we need the temp variable `latestRunPath`. How about just ``` Result<string> sandbox_ = os::realpath(slave::paths::getExecutorLatestRunPath( flags.work_dir, slaveId, frameworkId.get(), executor_id)); ASSERT_SOME(sandbox_); string sandbox = sandbox_.get(); ``` src/tests/gc_tests.cpp (line 963) <https://reviews.apache.org/r/47973/#comment203989> By using the name `mountPoint_` above, we can use `mountPoint` directly here. - Jiang Yan Xu On June 18, 2016, 8:51 p.m., Megha Sharma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47973/ > ----------------------------------------------------------- > > (Updated June 18, 2016, 8:51 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 > ------- > > Testing done with 'make check'. > > > Thanks, > > Megha Sharma > >
