mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jiang Yan Xu <...@jxu.me>
Subject Re: Review Request 47973: Updated gc to prevent early exit in case of error.
Date Tue, 21 Jun 2016 16:39:17 GMT

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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message