-----------------------------------------------------------
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
>
>
|