-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6704/#review10603
-----------------------------------------------------------



src/slave/constants.hpp
<https://reviews.apache.org/r/6704/#comment22858>

    so does 1 week seem correct?
    
    using the math you have below:
    disk 50%  
    max_age 3.5 days
    
    disk 75%
    max_age 1.75 days
    
    do we actually see the disks fill up this rapidly? perhaps initially we 
should be less agressive on this age, like set 2x or 3x on the current 
gc_timeout_hours
    
    of course, ben and you know much more about the typical disk usage patterns 
than I do, so let me know your opinion please :)



src/slave/constants.hpp
<https://reviews.apache.org/r/6704/#comment22691>

    seems a bit frequent no? what does statvfs do under the hood?



src/slave/gc.hpp
<https://reviews.apache.org/r/6704/#comment22692>

    kill these new imports, and put them in the cpp file instead?



src/slave/gc.cpp
<https://reviews.apache.org/r/6704/#comment22857>

    this will just print true or false, would be nice to have the actual error 
here, so how about we LOG in os::rmdir() when anything goes wrong?
    
    



src/slave/gc.cpp
<https://reviews.apache.org/r/6704/#comment22863>

    move the log line within the if? looks really noisy



src/slave/slave.cpp
<https://reviews.apache.org/r/6704/#comment22859>

    can you nicely format this number, like %.2f



src/slave/slave.cpp
<https://reviews.apache.org/r/6704/#comment22862>

    I'm not getting the math here:
    
    current gc timeout hours: 1 week
    suppose max age: 1 day
    
    then
    
    1 week - 1 day = 6 days
    
    so we would prune everything scheduled for deletion within the next 6 
days...? 


- Ben Mahler


On Aug. 21, 2012, 11:20 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6704/
> -----------------------------------------------------------
> 
> (Updated Aug. 21, 2012, 11:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, John Sirois, and Ben Mahler.
> 
> 
> Description
> -------
> 
> Added the ability in the slave to GC executor directories based on the 
> current disk usage.
> 
> Currently, it uses a very simple algorithm to decide which directories to 
> delete.
> 
> 
> This addresses bug MESOS-254.
>     https://issues.apache.org/jira/browse/MESOS-254
> 
> 
> Diffs
> -----
> 
>   src/slave/constants.hpp ab83972 
>   src/slave/flags.hpp 0c7917f 
>   src/slave/gc.hpp 6704742 
>   src/slave/gc.cpp 9c01024 
>   src/slave/slave.hpp 10c537b 
>   src/slave/slave.cpp 4efd41e 
>   src/tests/gc_tests.cpp 2f0bdde 
>   third_party/libprocess/include/stout/os.hpp b1eceb3 
> 
> Diff: https://reviews.apache.org/r/6704/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Reply via email to