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



src/docker/docker.cpp
<https://reviews.apache.org/r/35330/#comment139853>

    Also add a comment to say why we're batching



src/docker/docker.cpp
<https://reviews.apache.org/r/35330/#comment139847>

    Move the containers parameter to next line.
    
    Let's also rename __psCollect same reason I stated for __psBatch.



src/docker/docker.cpp
<https://reviews.apache.org/r/35330/#comment139848>

    We usually prefix underscores on a method when it's a continuation of a 
mehtod, but in this case it's really a helper method to collect batches of 
futures.
    
    Can you rename it to collectInspectBatch or something similiar?



src/docker/docker.cpp
<https://reviews.apache.org/r/35330/#comment139849>

    space between if and (



src/docker/docker.cpp
<https://reviews.apache.org/r/35330/#comment139850>

    space between foreach and (, and everywhere else basically.
    
    And curly braces please.



src/docker/docker.cpp
<https://reviews.apache.org/r/35330/#comment139851>

    Even a single line if needs curly braces, and above too.



src/docker/docker.cpp
<https://reviews.apache.org/r/35330/#comment139852>

    Curly braces



src/slave/constants.hpp
<https://reviews.apache.org/r/35330/#comment139854>

    I'm not sure maximum instances makes sense, as it's quite ambigious.
    How about update the comment to say it's the maximum number of docker 
inspect that docker ps will try to invoke?


- Timothy Chen


On June 11, 2015, 12:04 a.m., Lily Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35330/
> -----------------------------------------------------------
> 
> (Updated June 11, 2015, 12:04 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-2797
>     https://issues.apache.org/jira/browse/MESOS-2797
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Capped number of parallel inspect instances on a docker ps call.
> 
> 
> Diffs
> -----
> 
>   src/docker/docker.hpp 7790d0ff9a6b085025f595533c5f46b702447927 
>   src/docker/docker.cpp 71383294c6234d08b2156565b170ada329b8e30f 
>   src/slave/constants.hpp 84927e589499e989249c217db571bbeb84a88af1 
> 
> Diff: https://reviews.apache.org/r/35330/diff/
> 
> 
> Testing
> -------
> 
> Tweaked /mesos/src/slave/containerizer/docker.cpp 
> DockerContainerizerProcess::recover function to call a 'docker ps -a' without 
> a prefix. Then ran 'docker run busybox' 1300 times to recreate failure, 
> failing DockerContainerizerTest.ROOT_DOCKER_Recover and 
> DockerContainerizerTest.ROOT_DOCKER_SkipRecoverNonDocker, also causing slave 
> to fail when launched. Also tested fix after clearing 'docker ps -a' log and 
> with 300 instances in 'docker ps -a' log to make sure original functionality 
> not broken.
> 
> 
> Thanks,
> 
> Lily Chen
> 
>

Reply via email to