You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Gilbert Song <gi...@mesoshere.io> on 2015/10/15 00:54:00 UTC

Review Request 39331: Support docker local store pull image simultaneously

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

Review request for mesos, Anand Mazumdar, Jojy Varghese, and Timothy Chen.


Bugs: MESOS-3736
    https://issues.apache.org/jira/browse/MESOS-3736


Repository: mesos


Description
-------

Support docker local store pull image simultaneously


Diffs
-----

  src/slave/containerizer/provisioner/docker/metadata_manager.cpp 2b2de5245bccbd01a856b214ac6525278d794537 
  src/slave/containerizer/provisioner/docker/store.cpp 637c97c0973bda492826803a962c99635647692d 
  src/tests/containerizer/provisioner_docker_tests.cpp 9c3c45a81be6398722a37911788e347a4e91cce8 

Diff: https://reviews.apache.org/r/39331/diff/


Testing
-------

make check (ubuntu14.04 + clang-3.6)


Thanks,

Gilbert Song


Re: Review Request 39331: Support docker local store pull image simultaneously

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39331/#review102739
-----------------------------------------------------------


Patch looks great!

Reviews applied: [39331]

All tests passed.

- Mesos ReviewBot


On Oct. 14, 2015, 10:54 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39331/
> -----------------------------------------------------------
> 
> (Updated Oct. 14, 2015, 10:54 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3736
>     https://issues.apache.org/jira/browse/MESOS-3736
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Support docker local store pull image simultaneously
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/provisioner/docker/metadata_manager.cpp 2b2de5245bccbd01a856b214ac6525278d794537 
>   src/slave/containerizer/provisioner/docker/store.cpp 637c97c0973bda492826803a962c99635647692d 
>   src/tests/containerizer/provisioner_docker_tests.cpp 9c3c45a81be6398722a37911788e347a4e91cce8 
> 
> Diff: https://reviews.apache.org/r/39331/diff/
> 
> 
> Testing
> -------
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 39331: Support docker local store pull image simultaneously

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39331/#review102935
-----------------------------------------------------------



src/slave/containerizer/provisioner/docker/metadata_manager.cpp (line 80)
<https://reviews.apache.org/r/39331/#comment160767>

    Could you add comments on what the purpose of ImagePool is?



src/tests/containerizer/provisioner_docker_tests.cpp (line 920)
<https://reviews.apache.org/r/39331/#comment160769>

    How does this make sure that the same future was returned ?


- Jojy Varghese


On Oct. 14, 2015, 10:54 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39331/
> -----------------------------------------------------------
> 
> (Updated Oct. 14, 2015, 10:54 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3736
>     https://issues.apache.org/jira/browse/MESOS-3736
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Support docker local store pull image simultaneously
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/provisioner/docker/metadata_manager.cpp 2b2de5245bccbd01a856b214ac6525278d794537 
>   src/slave/containerizer/provisioner/docker/store.cpp 637c97c0973bda492826803a962c99635647692d 
>   src/tests/containerizer/provisioner_docker_tests.cpp 9c3c45a81be6398722a37911788e347a4e91cce8 
> 
> Diff: https://reviews.apache.org/r/39331/diff/
> 
> 
> Testing
> -------
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 39331: Support docker local store pull image simultaneously

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39331/#review103327
-----------------------------------------------------------


Patch looks great!

Reviews applied: [39331]

All tests passed.

- Mesos ReviewBot


On Oct. 20, 2015, 9:22 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39331/
> -----------------------------------------------------------
> 
> (Updated Oct. 20, 2015, 9:22 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3736
>     https://issues.apache.org/jira/browse/MESOS-3736
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Support docker local store pull image simultaneously
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/provisioner/docker/metadata_manager.cpp 2b2de5245bccbd01a856b214ac6525278d794537 
>   src/slave/containerizer/provisioner/docker/store.cpp 50340131d10222ac06cf21e87ad46943f171d1f6 
>   src/tests/containerizer/provisioner_docker_tests.cpp 01d3025f59d4a2714a856fe0f3a57192be023990 
> 
> Diff: https://reviews.apache.org/r/39331/diff/
> 
> 
> Testing
> -------
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> *This is not ready to be merged.
> *Still considering two question:
>  1. Handling simultaneous failure. If the first request is called and is written into the hashmap. All the other requests will be waiting for the future of the first request. But because its return type is 'Future<vector<string>>', if its future status is 'FAILED/DISCARDED', the other requests will be waiting forever. 
>  2. The current hashmap uses 'stringify(image::name)' as key, but it may not be unique because there is chance that layer_ids can be changed. One solution is to have 'stringify(image)' as key.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 39331: Support docker local store pull image simultaneously

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39331/#review103439
-----------------------------------------------------------


Patch looks great!

Reviews applied: [39331]

All tests passed.

- Mesos ReviewBot


On Oct. 21, 2015, 4:55 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39331/
> -----------------------------------------------------------
> 
> (Updated Oct. 21, 2015, 4:55 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3736
>     https://issues.apache.org/jira/browse/MESOS-3736
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Support docker local store pull image simultaneously
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/provisioner/docker/metadata_manager.cpp 2b2de5245bccbd01a856b214ac6525278d794537 
>   src/slave/containerizer/provisioner/docker/store.cpp 50340131d10222ac06cf21e87ad46943f171d1f6 
>   src/tests/containerizer/provisioner_docker_tests.cpp 01d3025f59d4a2714a856fe0f3a57192be023990 
> 
> Diff: https://reviews.apache.org/r/39331/diff/
> 
> 
> Testing
> -------
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> *This is not ready to be merged.
> *Still considering two question:
>  1. Handling simultaneous failure. If the first request is called and is written into the hashmap. All the other requests will be waiting for the future of the first request. But because its return type is 'Future<vector<string>>', if its future status is 'FAILED/DISCARDED', the other requests will be waiting forever. 
>     Solved by logic check: if it is the first call to get() Image_A, promise associate with metadateManager->get(). If not, check whether that promised future failed/discarded. If yes, over write to the hashmap.
>     
>  2. The current hashmap uses 'stringify(image::name)' as key, but it may not be unique because there is chance that layer_ids can be changed. 
>     One solution is to have 'stringify(image)' as key.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 39331: Support docker local store pull image simultaneously

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39331/#review105733
-----------------------------------------------------------


Bad patch!

Reviews applied: [39331]

Failed command: ./support/apply-review.sh -n -r 39331

Error:
 2015-11-09 19:55:37 URL:https://reviews.apache.org/r/39331/diff/raw/ [7525/7525] -> "39331.patch" [1]
error: src/slave/containerizer/mesos/provisioner/docker/store.hpp: does not exist in index
error: src/slave/containerizer/mesos/provisioner/docker/store.cpp: does not exist in index
error: src/tests/containerizer/provisioner_docker_tests.cpp: does not exist in index
Failed to apply patch

- Mesos ReviewBot


On Nov. 9, 2015, 7:26 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39331/
> -----------------------------------------------------------
> 
> (Updated Nov. 9, 2015, 7:26 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3736
>     https://issues.apache.org/jira/browse/MESOS-3736
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Support docker local store pull image simultaneously
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 95e46b9914c018b3e2472f98a54bc33ff9a46e17 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp bb02d650e16d45fcf337a7954f7a26143fb2c69f 
>   src/tests/containerizer/provisioner_docker_tests.cpp fe6a90fe32364eec8ef923a000db19183603c338 
> 
> Diff: https://reviews.apache.org/r/39331/diff/
> 
> 
> Testing
> -------
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> *This is not ready to be merged.
> *Still considering two question:
>  1. Handling simultaneous failure. If the first request is called and is written into the hashmap. All the other requests will be waiting for the future of the first request. But because its return type is 'Future<vector<string>>', if its future status is 'FAILED/DISCARDED', the other requests will be waiting forever. 
>     Solved by logic check: if it is the first call to get() Image_A, promise associate with metadateManager->get(). If not, check whether that promised future failed/discarded. If yes, over write to the hashmap.
>     
>  2. The current hashmap uses 'stringify(image::name)' as key, but it may not be unique because there is chance that layer_ids can be changed. 
>     One solution is to have 'stringify(image)' as key.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 39331: Support docker local store pull image simultaneously

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39331/#review105962
-----------------------------------------------------------



src/slave/containerizer/mesos/provisioner/docker/store.cpp (line 136)
<https://reviews.apache.org/r/39331/#comment164658>

    Actually we don't need to check exists before mkdir, so you can remove this check.



src/slave/containerizer/mesos/provisioner/docker/store.cpp (line 240)
<https://reviews.apache.org/r/39331/#comment164660>

    This is not right, you need to put the removing from the pulling hashmap in the onAny block in line 230, because this is block is not called if no one requests the same image in the same time, and will leave it in the hashmap.


- Timothy Chen


On Nov. 9, 2015, 7:26 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39331/
> -----------------------------------------------------------
> 
> (Updated Nov. 9, 2015, 7:26 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3736
>     https://issues.apache.org/jira/browse/MESOS-3736
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Support docker local store pull image simultaneously
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 95e46b9914c018b3e2472f98a54bc33ff9a46e17 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp bb02d650e16d45fcf337a7954f7a26143fb2c69f 
>   src/tests/containerizer/provisioner_docker_tests.cpp fe6a90fe32364eec8ef923a000db19183603c338 
> 
> Diff: https://reviews.apache.org/r/39331/diff/
> 
> 
> Testing
> -------
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> *This is not ready to be merged.
> *Still considering two question:
>  1. Handling simultaneous failure. If the first request is called and is written into the hashmap. All the other requests will be waiting for the future of the first request. But because its return type is 'Future<vector<string>>', if its future status is 'FAILED/DISCARDED', the other requests will be waiting forever. 
>     Solved by logic check: if it is the first call to get() Image_A, promise associate with metadateManager->get(). If not, check whether that promised future failed/discarded. If yes, over write to the hashmap.
>     
>  2. The current hashmap uses 'stringify(image::name)' as key, but it may not be unique because there is chance that layer_ids can be changed. 
>     One solution is to have 'stringify(image)' as key.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 39331: Support docker local store pull image simultaneously

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39331/#review106134
-----------------------------------------------------------


Patch looks great!

Reviews applied: [39331]

All tests passed.

- Mesos ReviewBot


On Nov. 11, 2015, 6:43 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39331/
> -----------------------------------------------------------
> 
> (Updated Nov. 11, 2015, 6:43 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3736
>     https://issues.apache.org/jira/browse/MESOS-3736
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Support docker local store pull image simultaneously
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 95e46b9914c018b3e2472f98a54bc33ff9a46e17 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 6fdb85b3d55339556b0982598d2e5258f6159466 
>   src/tests/containerizer/provisioner_docker_tests.cpp 2efe64f7896beb97f14aa2754569bd8383a67ac7 
> 
> Diff: https://reviews.apache.org/r/39331/diff/
> 
> 
> Testing
> -------
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> *This is not ready to be merged.
> *Still considering two question:
>  1. Handling simultaneous failure. If the first request is called and is written into the hashmap. All the other requests will be waiting for the future of the first request. But because its return type is 'Future<vector<string>>', if its future status is 'FAILED/DISCARDED', the other requests will be waiting forever. 
>     Solved by logic check: if it is the first call to get() Image_A, promise associate with metadateManager->get(). If not, check whether that promised future failed/discarded. If yes, over write to the hashmap.
>     
>  2. The current hashmap uses 'stringify(image::name)' as key, but it may not be unique because there is chance that layer_ids can be changed. 
>     One solution is to have 'stringify(image)' as key.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 39331: Support docker local store pull image simultaneously

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39331/#review106137
-----------------------------------------------------------


Patch looks great!

Reviews applied: [39331]

All tests passed.

- Mesos ReviewBot


On Nov. 11, 2015, 6:43 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39331/
> -----------------------------------------------------------
> 
> (Updated Nov. 11, 2015, 6:43 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3736
>     https://issues.apache.org/jira/browse/MESOS-3736
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Support docker local store pull image simultaneously
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 95e46b9914c018b3e2472f98a54bc33ff9a46e17 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 6fdb85b3d55339556b0982598d2e5258f6159466 
>   src/tests/containerizer/provisioner_docker_tests.cpp 2efe64f7896beb97f14aa2754569bd8383a67ac7 
> 
> Diff: https://reviews.apache.org/r/39331/diff/
> 
> 
> Testing
> -------
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> *This is not ready to be merged.
> *Still considering two question:
>  1. Handling simultaneous failure. If the first request is called and is written into the hashmap. All the other requests will be waiting for the future of the first request. But because its return type is 'Future<vector<string>>', if its future status is 'FAILED/DISCARDED', the other requests will be waiting forever. 
>     Solved by logic check: if it is the first call to get() Image_A, promise associate with metadateManager->get(). If not, check whether that promised future failed/discarded. If yes, over write to the hashmap.
>     
>  2. The current hashmap uses 'stringify(image::name)' as key, but it may not be unique because there is chance that layer_ids can be changed. 
>     One solution is to have 'stringify(image)' as key.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 39331: Support docker local store pull image simultaneously

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39331/#review106514
-----------------------------------------------------------



src/tests/containerizer/provisioner_docker_tests.cpp (line 1402)
<https://reviews.apache.org/r/39331/#comment165219>

    name this unmocked_pull



src/tests/containerizer/provisioner_docker_tests.cpp (line 1405)
<https://reviews.apache.org/r/39331/#comment165220>

    We cannot call a virtual method, we need to just do something by default.
    
    In this case just return empty list.



src/tests/containerizer/provisioner_docker_tests.cpp (line 1409)
<https://reviews.apache.org/r/39331/#comment165218>

    kill this



src/tests/containerizer/provisioner_docker_tests.cpp (line 1450)
<https://reviews.apache.org/r/39331/#comment165217>

    Create under the temporary directory that's created part of this test: path::join(os::getcwd(), 'rootfs1');
    
    Same for the other one.


- Timothy Chen


On Nov. 11, 2015, 6:43 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39331/
> -----------------------------------------------------------
> 
> (Updated Nov. 11, 2015, 6:43 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3736
>     https://issues.apache.org/jira/browse/MESOS-3736
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Support docker local store pull image simultaneously
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 95e46b9914c018b3e2472f98a54bc33ff9a46e17 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 6fdb85b3d55339556b0982598d2e5258f6159466 
>   src/tests/containerizer/provisioner_docker_tests.cpp 2efe64f7896beb97f14aa2754569bd8383a67ac7 
> 
> Diff: https://reviews.apache.org/r/39331/diff/
> 
> 
> Testing
> -------
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> *This is not ready to be merged.
> *Still considering two question:
>  1. Handling simultaneous failure. If the first request is called and is written into the hashmap. All the other requests will be waiting for the future of the first request. But because its return type is 'Future<vector<string>>', if its future status is 'FAILED/DISCARDED', the other requests will be waiting forever. 
>     Solved by logic check: if it is the first call to get() Image_A, promise associate with metadateManager->get(). If not, check whether that promised future failed/discarded. If yes, over write to the hashmap.
>     
>  2. The current hashmap uses 'stringify(image::name)' as key, but it may not be unique because there is chance that layer_ids can be changed. 
>     One solution is to have 'stringify(image)' as key.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 39331: Support docker local store pull image simultaneously

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39331/#review106951
-----------------------------------------------------------



src/slave/containerizer/mesos/provisioner/docker/store.cpp (line 196)
<https://reviews.apache.org/r/39331/#comment165755>

    Added () for constructor.



src/slave/containerizer/mesos/provisioner/docker/store.cpp (line 201)
<https://reviews.apache.org/r/39331/#comment165756>

    We need defer when we remove from pulling map.



src/slave/containerizer/mesos/provisioner/docker/store.cpp (line 217)
<https://reviews.apache.org/r/39331/#comment165757>

    Remove this onAny.


- Timothy Chen


On Nov. 17, 2015, 5:43 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39331/
> -----------------------------------------------------------
> 
> (Updated Nov. 17, 2015, 5:43 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3736
>     https://issues.apache.org/jira/browse/MESOS-3736
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Support docker local store pull image simultaneously
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 95e46b9914c018b3e2472f98a54bc33ff9a46e17 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 6fdb85b3d55339556b0982598d2e5258f6159466 
>   src/tests/containerizer/provisioner_docker_tests.cpp edfbb6fe932173dcbb15133e0eb685d86dd09c00 
> 
> Diff: https://reviews.apache.org/r/39331/diff/
> 
> 
> Testing
> -------
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> *This is not ready to be merged.
> *Still considering two question:
>  1. Handling simultaneous failure. If the first request is called and is written into the hashmap. All the other requests will be waiting for the future of the first request. But because its return type is 'Future<vector<string>>', if its future status is 'FAILED/DISCARDED', the other requests will be waiting forever. 
>     Solved by logic check: if it is the first call to get() Image_A, promise associate with metadateManager->get(). If not, check whether that promised future failed/discarded. If yes, over write to the hashmap.
>     
>  2. The current hashmap uses 'stringify(image::name)' as key, but it may not be unique because there is chance that layer_ids can be changed. 
>     One solution is to have 'stringify(image)' as key.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 39331: Support docker local store pull image simultaneously

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39331/#review106908
-----------------------------------------------------------


Patch looks great!

Reviews applied: [39331]

All tests passed.

- Mesos ReviewBot


On Nov. 17, 2015, 5:43 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39331/
> -----------------------------------------------------------
> 
> (Updated Nov. 17, 2015, 5:43 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3736
>     https://issues.apache.org/jira/browse/MESOS-3736
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Support docker local store pull image simultaneously
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 95e46b9914c018b3e2472f98a54bc33ff9a46e17 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 6fdb85b3d55339556b0982598d2e5258f6159466 
>   src/tests/containerizer/provisioner_docker_tests.cpp edfbb6fe932173dcbb15133e0eb685d86dd09c00 
> 
> Diff: https://reviews.apache.org/r/39331/diff/
> 
> 
> Testing
> -------
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> *This is not ready to be merged.
> *Still considering two question:
>  1. Handling simultaneous failure. If the first request is called and is written into the hashmap. All the other requests will be waiting for the future of the first request. But because its return type is 'Future<vector<string>>', if its future status is 'FAILED/DISCARDED', the other requests will be waiting forever. 
>     Solved by logic check: if it is the first call to get() Image_A, promise associate with metadateManager->get(). If not, check whether that promised future failed/discarded. If yes, over write to the hashmap.
>     
>  2. The current hashmap uses 'stringify(image::name)' as key, but it may not be unique because there is chance that layer_ids can be changed. 
>     One solution is to have 'stringify(image)' as key.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 39331: Support docker local store pull image simultaneously

Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39331/
-----------------------------------------------------------

(Updated Nov. 17, 2015, 9:43 a.m.)


Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Timothy Chen.


Bugs: MESOS-3736
    https://issues.apache.org/jira/browse/MESOS-3736


Repository: mesos


Description
-------

Support docker local store pull image simultaneously


Diffs (updated)
-----

  src/slave/containerizer/mesos/provisioner/docker/store.hpp 95e46b9914c018b3e2472f98a54bc33ff9a46e17 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 6fdb85b3d55339556b0982598d2e5258f6159466 
  src/tests/containerizer/provisioner_docker_tests.cpp edfbb6fe932173dcbb15133e0eb685d86dd09c00 

Diff: https://reviews.apache.org/r/39331/diff/


Testing
-------

make check (ubuntu14.04 + clang-3.6)

*This is not ready to be merged.
*Still considering two question:
 1. Handling simultaneous failure. If the first request is called and is written into the hashmap. All the other requests will be waiting for the future of the first request. But because its return type is 'Future<vector<string>>', if its future status is 'FAILED/DISCARDED', the other requests will be waiting forever. 
    Solved by logic check: if it is the first call to get() Image_A, promise associate with metadateManager->get(). If not, check whether that promised future failed/discarded. If yes, over write to the hashmap.
    
 2. The current hashmap uses 'stringify(image::name)' as key, but it may not be unique because there is chance that layer_ids can be changed. 
    One solution is to have 'stringify(image)' as key.


Thanks,

Gilbert Song


Re: Review Request 39331: Support docker local store pull image simultaneously

Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39331/
-----------------------------------------------------------

(Updated Nov. 11, 2015, 10:43 a.m.)


Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Timothy Chen.


Bugs: MESOS-3736
    https://issues.apache.org/jira/browse/MESOS-3736


Repository: mesos


Description
-------

Support docker local store pull image simultaneously


Diffs (updated)
-----

  src/slave/containerizer/mesos/provisioner/docker/store.hpp 95e46b9914c018b3e2472f98a54bc33ff9a46e17 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 6fdb85b3d55339556b0982598d2e5258f6159466 
  src/tests/containerizer/provisioner_docker_tests.cpp 2efe64f7896beb97f14aa2754569bd8383a67ac7 

Diff: https://reviews.apache.org/r/39331/diff/


Testing
-------

make check (ubuntu14.04 + clang-3.6)

*This is not ready to be merged.
*Still considering two question:
 1. Handling simultaneous failure. If the first request is called and is written into the hashmap. All the other requests will be waiting for the future of the first request. But because its return type is 'Future<vector<string>>', if its future status is 'FAILED/DISCARDED', the other requests will be waiting forever. 
    Solved by logic check: if it is the first call to get() Image_A, promise associate with metadateManager->get(). If not, check whether that promised future failed/discarded. If yes, over write to the hashmap.
    
 2. The current hashmap uses 'stringify(image::name)' as key, but it may not be unique because there is chance that layer_ids can be changed. 
    One solution is to have 'stringify(image)' as key.


Thanks,

Gilbert Song


Re: Review Request 39331: Support docker local store pull image simultaneously

Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39331/
-----------------------------------------------------------

(Updated Nov. 11, 2015, 9:49 a.m.)


Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Timothy Chen.


Bugs: MESOS-3736
    https://issues.apache.org/jira/browse/MESOS-3736


Repository: mesos


Description
-------

Support docker local store pull image simultaneously


Diffs (updated)
-----

  src/slave/containerizer/mesos/provisioner/docker/store.hpp 95e46b9914c018b3e2472f98a54bc33ff9a46e17 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 6fdb85b3d55339556b0982598d2e5258f6159466 
  src/tests/containerizer/provisioner_docker_tests.cpp 2efe64f7896beb97f14aa2754569bd8383a67ac7 

Diff: https://reviews.apache.org/r/39331/diff/


Testing
-------

make check (ubuntu14.04 + clang-3.6)

*This is not ready to be merged.
*Still considering two question:
 1. Handling simultaneous failure. If the first request is called and is written into the hashmap. All the other requests will be waiting for the future of the first request. But because its return type is 'Future<vector<string>>', if its future status is 'FAILED/DISCARDED', the other requests will be waiting forever. 
    Solved by logic check: if it is the first call to get() Image_A, promise associate with metadateManager->get(). If not, check whether that promised future failed/discarded. If yes, over write to the hashmap.
    
 2. The current hashmap uses 'stringify(image::name)' as key, but it may not be unique because there is chance that layer_ids can be changed. 
    One solution is to have 'stringify(image)' as key.


Thanks,

Gilbert Song


Re: Review Request 39331: Support docker local store pull image simultaneously

Posted by Gilbert Song <so...@gmail.com>.

> On Nov. 10, 2015, 1:59 p.m., Jojy Varghese wrote:
> > src/slave/containerizer/mesos/provisioner/docker/store.cpp, line 136
> > <https://reviews.apache.org/r/39331/diff/5/?file=1120529#file1120529line136>
> >
> >     Wondering if you need to pass "recursive" (true) to mkdir. This flag ensures that the the whole path of dierctory is created as needed. If its already existing, it wont create one.

If "recursive" is set as false explicitly, for the case to mkdir 
/abc/ef/ghi
it would return an error if we do not have /abc/ef existed.
We should want it true to create directory tree.


- Gilbert


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


On Nov. 11, 2015, 9:49 a.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39331/
> -----------------------------------------------------------
> 
> (Updated Nov. 11, 2015, 9:49 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3736
>     https://issues.apache.org/jira/browse/MESOS-3736
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Support docker local store pull image simultaneously
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 95e46b9914c018b3e2472f98a54bc33ff9a46e17 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 6fdb85b3d55339556b0982598d2e5258f6159466 
>   src/tests/containerizer/provisioner_docker_tests.cpp 2efe64f7896beb97f14aa2754569bd8383a67ac7 
> 
> Diff: https://reviews.apache.org/r/39331/diff/
> 
> 
> Testing
> -------
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> *This is not ready to be merged.
> *Still considering two question:
>  1. Handling simultaneous failure. If the first request is called and is written into the hashmap. All the other requests will be waiting for the future of the first request. But because its return type is 'Future<vector<string>>', if its future status is 'FAILED/DISCARDED', the other requests will be waiting forever. 
>     Solved by logic check: if it is the first call to get() Image_A, promise associate with metadateManager->get(). If not, check whether that promised future failed/discarded. If yes, over write to the hashmap.
>     
>  2. The current hashmap uses 'stringify(image::name)' as key, but it may not be unique because there is chance that layer_ids can be changed. 
>     One solution is to have 'stringify(image)' as key.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 39331: Support docker local store pull image simultaneously

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39331/#review105965
-----------------------------------------------------------



src/slave/containerizer/mesos/provisioner/docker/store.cpp (line 136)
<https://reviews.apache.org/r/39331/#comment164661>

    Wondering if you need to pass "recursive" (true) to mkdir. This flag ensures that the the whole path of dierctory is created as needed. If its already existing, it wont create one.


- Jojy Varghese


On Nov. 9, 2015, 7:26 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39331/
> -----------------------------------------------------------
> 
> (Updated Nov. 9, 2015, 7:26 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3736
>     https://issues.apache.org/jira/browse/MESOS-3736
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Support docker local store pull image simultaneously
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 95e46b9914c018b3e2472f98a54bc33ff9a46e17 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp bb02d650e16d45fcf337a7954f7a26143fb2c69f 
>   src/tests/containerizer/provisioner_docker_tests.cpp fe6a90fe32364eec8ef923a000db19183603c338 
> 
> Diff: https://reviews.apache.org/r/39331/diff/
> 
> 
> Testing
> -------
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> *This is not ready to be merged.
> *Still considering two question:
>  1. Handling simultaneous failure. If the first request is called and is written into the hashmap. All the other requests will be waiting for the future of the first request. But because its return type is 'Future<vector<string>>', if its future status is 'FAILED/DISCARDED', the other requests will be waiting forever. 
>     Solved by logic check: if it is the first call to get() Image_A, promise associate with metadateManager->get(). If not, check whether that promised future failed/discarded. If yes, over write to the hashmap.
>     
>  2. The current hashmap uses 'stringify(image::name)' as key, but it may not be unique because there is chance that layer_ids can be changed. 
>     One solution is to have 'stringify(image)' as key.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 39331: Support docker local store pull image simultaneously

Posted by Gilbert Song <so...@gmail.com>.

> On Nov. 9, 2015, 4:33 p.m., Timothy Chen wrote:
> > src/tests/containerizer/provisioner_docker_tests.cpp, line 1193
> > <https://reviews.apache.org/r/39331/diff/5/?file=1120530#file1120530line1193>
> >
> >     Owned<MockPuller> puller(new MockPuller())

I search through all tests that use mock object. Seems like none of them use Owned<MockerClass>. And actually it would return an error that converting from <MockPuller> to <Puller> when we pass the Owned<MockPuller> to store::create().

>From those examples in code, when they use '*' pointer to new an object, they do not delete at the end. Does it mean gtest would clean up when finished?


- Gilbert


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


On Nov. 11, 2015, 9:49 a.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39331/
> -----------------------------------------------------------
> 
> (Updated Nov. 11, 2015, 9:49 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3736
>     https://issues.apache.org/jira/browse/MESOS-3736
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Support docker local store pull image simultaneously
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 95e46b9914c018b3e2472f98a54bc33ff9a46e17 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 6fdb85b3d55339556b0982598d2e5258f6159466 
>   src/tests/containerizer/provisioner_docker_tests.cpp 2efe64f7896beb97f14aa2754569bd8383a67ac7 
> 
> Diff: https://reviews.apache.org/r/39331/diff/
> 
> 
> Testing
> -------
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> *This is not ready to be merged.
> *Still considering two question:
>  1. Handling simultaneous failure. If the first request is called and is written into the hashmap. All the other requests will be waiting for the future of the first request. But because its return type is 'Future<vector<string>>', if its future status is 'FAILED/DISCARDED', the other requests will be waiting forever. 
>     Solved by logic check: if it is the first call to get() Image_A, promise associate with metadateManager->get(). If not, check whether that promised future failed/discarded. If yes, over write to the hashmap.
>     
>  2. The current hashmap uses 'stringify(image::name)' as key, but it may not be unique because there is chance that layer_ids can be changed. 
>     One solution is to have 'stringify(image)' as key.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 39331: Support docker local store pull image simultaneously

Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39331/#review105773
-----------------------------------------------------------



src/slave/containerizer/mesos/provisioner/docker/store.cpp (line 131)
<https://reviews.apache.org/r/39331/#comment164428>

    Let's call this Store::create, but comment in the header that this is visible just for testing.



src/slave/containerizer/mesos/provisioner/docker/store.cpp (line 240)
<https://reviews.apache.org/r/39331/#comment164409>

    just 4 space indent from aligning with the "return" keyword



src/tests/containerizer/provisioner_docker_tests.cpp (line 1180)
<https://reviews.apache.org/r/39331/#comment164410>

    two spaces



src/tests/containerizer/provisioner_docker_tests.cpp (line 1181)
<https://reviews.apache.org/r/39331/#comment164420>

    Comment what are you testing here



src/tests/containerizer/provisioner_docker_tests.cpp (line 1193)
<https://reviews.apache.org/r/39331/#comment164416>

    Owned<MockPuller> puller(new MockPuller())



src/tests/containerizer/provisioner_docker_tests.cpp (line 1209)
<https://reviews.apache.org/r/39331/#comment164418>

    Fix all snake case as we use camelCase variables


- Timothy Chen


On Nov. 9, 2015, 7:26 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39331/
> -----------------------------------------------------------
> 
> (Updated Nov. 9, 2015, 7:26 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3736
>     https://issues.apache.org/jira/browse/MESOS-3736
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Support docker local store pull image simultaneously
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 95e46b9914c018b3e2472f98a54bc33ff9a46e17 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp bb02d650e16d45fcf337a7954f7a26143fb2c69f 
>   src/tests/containerizer/provisioner_docker_tests.cpp fe6a90fe32364eec8ef923a000db19183603c338 
> 
> Diff: https://reviews.apache.org/r/39331/diff/
> 
> 
> Testing
> -------
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> *This is not ready to be merged.
> *Still considering two question:
>  1. Handling simultaneous failure. If the first request is called and is written into the hashmap. All the other requests will be waiting for the future of the first request. But because its return type is 'Future<vector<string>>', if its future status is 'FAILED/DISCARDED', the other requests will be waiting forever. 
>     Solved by logic check: if it is the first call to get() Image_A, promise associate with metadateManager->get(). If not, check whether that promised future failed/discarded. If yes, over write to the hashmap.
>     
>  2. The current hashmap uses 'stringify(image::name)' as key, but it may not be unique because there is chance that layer_ids can be changed. 
>     One solution is to have 'stringify(image)' as key.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 39331: Support docker local store pull image simultaneously

Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39331/
-----------------------------------------------------------

(Updated Nov. 9, 2015, 11:26 a.m.)


Review request for mesos, Anand Mazumdar, Jie Yu, Jojy Varghese, and Timothy Chen.


Bugs: MESOS-3736
    https://issues.apache.org/jira/browse/MESOS-3736


Repository: mesos


Description
-------

Support docker local store pull image simultaneously


Diffs (updated)
-----

  src/slave/containerizer/mesos/provisioner/docker/store.hpp 95e46b9914c018b3e2472f98a54bc33ff9a46e17 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp bb02d650e16d45fcf337a7954f7a26143fb2c69f 
  src/tests/containerizer/provisioner_docker_tests.cpp fe6a90fe32364eec8ef923a000db19183603c338 

Diff: https://reviews.apache.org/r/39331/diff/


Testing
-------

make check (ubuntu14.04 + clang-3.6)

*This is not ready to be merged.
*Still considering two question:
 1. Handling simultaneous failure. If the first request is called and is written into the hashmap. All the other requests will be waiting for the future of the first request. But because its return type is 'Future<vector<string>>', if its future status is 'FAILED/DISCARDED', the other requests will be waiting forever. 
    Solved by logic check: if it is the first call to get() Image_A, promise associate with metadateManager->get(). If not, check whether that promised future failed/discarded. If yes, over write to the hashmap.
    
 2. The current hashmap uses 'stringify(image::name)' as key, but it may not be unique because there is chance that layer_ids can be changed. 
    One solution is to have 'stringify(image)' as key.


Thanks,

Gilbert Song


Re: Review Request 39331: Support docker local store pull image simultaneously

Posted by Gilbert Song <gi...@mesoshere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39331/
-----------------------------------------------------------

(Updated Oct. 21, 2015, 9:55 a.m.)


Review request for mesos, Anand Mazumdar, Jojy Varghese, and Timothy Chen.


Bugs: MESOS-3736
    https://issues.apache.org/jira/browse/MESOS-3736


Repository: mesos


Description
-------

Support docker local store pull image simultaneously


Diffs
-----

  src/slave/containerizer/provisioner/docker/metadata_manager.cpp 2b2de5245bccbd01a856b214ac6525278d794537 
  src/slave/containerizer/provisioner/docker/store.cpp 50340131d10222ac06cf21e87ad46943f171d1f6 
  src/tests/containerizer/provisioner_docker_tests.cpp 01d3025f59d4a2714a856fe0f3a57192be023990 

Diff: https://reviews.apache.org/r/39331/diff/


Testing (updated)
-------

make check (ubuntu14.04 + clang-3.6)

*This is not ready to be merged.
*Still considering two question:
 1. Handling simultaneous failure. If the first request is called and is written into the hashmap. All the other requests will be waiting for the future of the first request. But because its return type is 'Future<vector<string>>', if its future status is 'FAILED/DISCARDED', the other requests will be waiting forever. 
    Solved by logic check: if it is the first call to get() Image_A, promise associate with metadateManager->get(). If not, check whether that promised future failed/discarded. If yes, over write to the hashmap.
    
 2. The current hashmap uses 'stringify(image::name)' as key, but it may not be unique because there is chance that layer_ids can be changed. 
    One solution is to have 'stringify(image)' as key.


Thanks,

Gilbert Song


Re: Review Request 39331: Support docker local store pull image simultaneously

Posted by Gilbert Song <gi...@mesoshere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39331/
-----------------------------------------------------------

(Updated Oct. 21, 2015, 9:48 a.m.)


Review request for mesos, Anand Mazumdar, Jojy Varghese, and Timothy Chen.


Bugs: MESOS-3736
    https://issues.apache.org/jira/browse/MESOS-3736


Repository: mesos


Description
-------

Support docker local store pull image simultaneously


Diffs (updated)
-----

  src/slave/containerizer/provisioner/docker/metadata_manager.cpp 2b2de5245bccbd01a856b214ac6525278d794537 
  src/slave/containerizer/provisioner/docker/store.cpp 50340131d10222ac06cf21e87ad46943f171d1f6 
  src/tests/containerizer/provisioner_docker_tests.cpp 01d3025f59d4a2714a856fe0f3a57192be023990 

Diff: https://reviews.apache.org/r/39331/diff/


Testing
-------

make check (ubuntu14.04 + clang-3.6)

*This is not ready to be merged.
*Still considering two question:
 1. Handling simultaneous failure. If the first request is called and is written into the hashmap. All the other requests will be waiting for the future of the first request. But because its return type is 'Future<vector<string>>', if its future status is 'FAILED/DISCARDED', the other requests will be waiting forever. 
 2. The current hashmap uses 'stringify(image::name)' as key, but it may not be unique because there is chance that layer_ids can be changed. One solution is to have 'stringify(image)' as key.


Thanks,

Gilbert Song


Re: Review Request 39331: Support docker local store pull image simultaneously

Posted by Gilbert Song <gi...@mesoshere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39331/
-----------------------------------------------------------

(Updated Oct. 20, 2015, 2:22 p.m.)


Review request for mesos, Anand Mazumdar, Jojy Varghese, and Timothy Chen.


Bugs: MESOS-3736
    https://issues.apache.org/jira/browse/MESOS-3736


Repository: mesos


Description
-------

Support docker local store pull image simultaneously


Diffs (updated)
-----

  src/slave/containerizer/provisioner/docker/metadata_manager.cpp 2b2de5245bccbd01a856b214ac6525278d794537 
  src/slave/containerizer/provisioner/docker/store.cpp 50340131d10222ac06cf21e87ad46943f171d1f6 
  src/tests/containerizer/provisioner_docker_tests.cpp 01d3025f59d4a2714a856fe0f3a57192be023990 

Diff: https://reviews.apache.org/r/39331/diff/


Testing
-------

make check (ubuntu14.04 + clang-3.6)

*This is not ready to be merged.
*Still considering two question:
 1. Handling simultaneous failure. If the first request is called and is written into the hashmap. All the other requests will be waiting for the future of the first request. But because its return type is 'Future<vector<string>>', if its future status is 'FAILED/DISCARDED', the other requests will be waiting forever. 
 2. The current hashmap uses 'stringify(image::name)' as key, but it may not be unique because there is chance that layer_ids can be changed. One solution is to have 'stringify(image)' as key.


Thanks,

Gilbert Song


Re: Review Request 39331: Support docker local store pull image simultaneously

Posted by Gilbert Song <gi...@mesoshere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39331/
-----------------------------------------------------------

(Updated Oct. 20, 2015, 10:48 a.m.)


Review request for Anand Mazumdar, Jojy Varghese and Timothy Chen.


Bugs: MESOS-3736
    https://issues.apache.org/jira/browse/MESOS-3736


Repository: mesos


Description
-------

Support docker local store pull image simultaneously


Diffs (updated)
-----

  src/slave/containerizer/provisioner/docker/metadata_manager.cpp 2b2de5245bccbd01a856b214ac6525278d794537 
  src/slave/containerizer/provisioner/docker/store.cpp 50340131d10222ac06cf21e87ad46943f171d1f6 
  src/tests/containerizer/provisioner_docker_tests.cpp 01d3025f59d4a2714a856fe0f3a57192be023990 

Diff: https://reviews.apache.org/r/39331/diff/


Testing (updated)
-------

make check (ubuntu14.04 + clang-3.6)

*This is not ready to be merged.
*Still considering two question:
 1. Handling simultaneous failure. If the first request is called and is written into the hashmap. All the other requests will be waiting for the future of the first request. But because its return type is 'Future<vector<string>>', if its future status is 'FAILED/DISCARDED', the other requests will be waiting forever. 
 2. The current hashmap uses 'stringify(image::name)' as key, but it may not be unique because there is chance that layer_ids can be changed. One solution is to have 'stringify(image)' as key.


Thanks,

Gilbert Song


Re: Review Request 39331: Support docker local store pull image simultaneously

Posted by Jojy Varghese <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39331/#review102945
-----------------------------------------------------------



src/slave/containerizer/provisioner/docker/store.cpp (line 169)
<https://reviews.apache.org/r/39331/#comment160771>

    Does this mean that any request to get an image A will always return the same image? 
    
    I thought the idea was to prevent simultaneous downloads/fetching of the same image.


- Jojy Varghese


On Oct. 14, 2015, 10:54 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39331/
> -----------------------------------------------------------
> 
> (Updated Oct. 14, 2015, 10:54 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jojy Varghese, and Timothy Chen.
> 
> 
> Bugs: MESOS-3736
>     https://issues.apache.org/jira/browse/MESOS-3736
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Support docker local store pull image simultaneously
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/provisioner/docker/metadata_manager.cpp 2b2de5245bccbd01a856b214ac6525278d794537 
>   src/slave/containerizer/provisioner/docker/store.cpp 637c97c0973bda492826803a962c99635647692d 
>   src/tests/containerizer/provisioner_docker_tests.cpp 9c3c45a81be6398722a37911788e347a4e91cce8 
> 
> Diff: https://reviews.apache.org/r/39331/diff/
> 
> 
> Testing
> -------
> 
> make check (ubuntu14.04 + clang-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>