mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Timothy Chen" <tnac...@apache.org>
Subject Re: Review Request 39331: Support docker local store pull image simultaneously
Date Tue, 10 Nov 2015 00:33:11 GMT

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


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