You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Zhitao Li <zh...@gmail.com> on 2017/09/26 18:15:06 UTC
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/
-----------------------------------------------------------
(Updated Sept. 26, 2017, 6:15 p.m.)
Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
Changes
-------
Rebase.
Bugs: MESOS-4945
https://issues.apache.org/jira/browse/MESOS-4945
Repository: mesos
Description
-------
This includes the following changes:
- add a `pruneImages()` function on the chain of relevant classes;
- implement prune in docker store;
- fix mock interface to keep existing tests pass.
Diffs (updated)
-----
src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f
src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208
src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842
src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270
src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67
src/slave/containerizer/mesos/containerizer.hpp cc23b4d91be16fc95a131c09d07378b801e34d6f
src/slave/containerizer/mesos/containerizer.cpp 4d5dc13f363f5d8886983d7dd06a5cecc177c345
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac
src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a
src/tests/containerizer/mock_containerizer.hpp 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569
Diff: https://reviews.apache.org/r/56721/diff/8/
Changes: https://reviews.apache.org/r/56721/diff/7-8/
Testing
-------
Thanks,
Zhitao Li
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Zhitao Li <zh...@gmail.com>.
> On Oct. 3, 2017, 1:19 a.m., Gilbert Song wrote:
> > src/slave/containerizer/containerizer.hpp
> > Lines 156-157 (patched)
> > <https://reviews.apache.org/r/56721/diff/7/?file=1791102#file1791102line156>
> >
> > maybe return failure in this virtual function by default? so that you don't need to do that on docker containerizer.
I thought about return value when the underlying containerizer does not support `pruneImages` yet. I think returning `Nothing` will be better than failure so that users who mixes Mesos Containerizer and docker containerizer can still use the functionality without dealing with failures.
w.r.t. return something by default, I don't particularly like that because current `Containerizer` class is a pure interface class (i.e, all virtual functions are pure) and I feel keeping it this way is more consistent.
Also, it's actually possible to implement a reasonable `pruneImages` in DockerContainerizer (with something similar to https://github.com/spotify/docker-gc), so I'd like to keep this possibility there.
Please let me know if you think otherwise.
> On Oct. 3, 2017, 1:19 a.m., Gilbert Song wrote:
> > src/slave/containerizer/docker.hpp
> > Lines 112-114 (patched)
> > <https://reviews.apache.org/r/56721/diff/7/?file=1791103#file1791103line112>
> >
> > not needed.
See comments above.
> On Oct. 3, 2017, 1:19 a.m., Gilbert Song wrote:
> > src/slave/containerizer/docker.cpp
> > Lines 873-880 (patched)
> > <https://reviews.apache.org/r/56721/diff/7/?file=1791104#file1791104line873>
> >
> > not needed.
See comments above.
> On Oct. 3, 2017, 1:19 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/provisioner/docker/paths.cpp
> > Lines 115 (patched)
> > <https://reviews.apache.org/r/56721/diff/7/?file=1791110#file1791110line115>
> >
> > are you prefer namoseconds than milliseconds for precise?
Yeah nanoseconds seems less possible to have a conflict. Is there a reason to prefer milliseconds though?
> On Oct. 3, 2017, 1:19 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/provisioner/docker/store.cpp
> > Lines 536 (patched)
> > <https://reviews.apache.org/r/56721/diff/7/?file=1791112#file1791112line536>
> >
> > should be a boolean. and basically this is the lock. we should put it at the very beginning of this method, right below the check of any other pruning happening.
> >
> > also, let's comment on this lock.
Added comment.
I don't think this should be moved to before input validation above.
> On Oct. 3, 2017, 1:19 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/provisioner/docker/store.cpp
> > Lines 598 (patched)
> > <https://reviews.apache.org/r/56721/diff/7/?file=1791112#file1791112line598>
> >
> > what does this comment mean?
I thought about tracking disk space size of sweeping directory (disk usage or something else), but I realized that any prolonged non-empty sweeping directory is pretty much a bug, so I guess I'll remove this TODO.
- Zhitao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/#review186320
-----------------------------------------------------------
On Oct. 3, 2017, 5:13 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56721/
> -----------------------------------------------------------
>
> (Updated Oct. 3, 2017, 5:13 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This includes the following changes:
> - add a `pruneImages()` function on the chain of relevant classes;
> - implement prune in docker store;
> - fix mock interface to keep existing tests pass.
>
>
> Diffs
> -----
>
> src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f
> src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208
> src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842
> src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270
> src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67
> src/slave/containerizer/mesos/containerizer.hpp cc23b4d91be16fc95a131c09d07378b801e34d6f
> src/slave/containerizer/mesos/containerizer.cpp 4d5dc13f363f5d8886983d7dd06a5cecc177c345
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
> src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
> src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
> src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
> src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
> src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
> src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
> src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
> src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
> src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac
> src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a
> src/tests/containerizer/mock_containerizer.hpp 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569
>
>
> Diff: https://reviews.apache.org/r/56721/diff/9/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Zhitao Li <zh...@gmail.com>.
> On Oct. 3, 2017, 1:19 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
> > Lines 209-211 (patched)
> > <https://reviews.apache.org/r/56721/diff/7/?file=1791108#file1791108line209>
> >
> > three scenarios:
> > 1. checkpointed cache is cleaned up for some reasons
> > 2. the `activeImages` contains `excludedImages`, which the operator specifys some images that are never pulled before on this agent.
> >
> > and I just think of the 3rd case:
> > 3. if a couple big images are still being pulled (pulling started before `prune()`), it means thay are not in `storedImages` yet.
> > 4. same as #3, we just unlock the store and resume the image pulling, then the requests in queue will be executed simultanuously, but another prune() call come in from the operator for some reason.
> >
> > for the case of #3 and #4, we may need to fix:
> > https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp#L384~#L388
> >
> > because container launch may fail if any of the layers are included in those ongoing pull but still get marked from pre-existed unused images.
Case 1 means an operator or unknown manually played with checkpointed cache, so there is nothing we can really do;
Case 2 is an acceptable case IMO;
Case 3 and case 4 could be real problem. One straightfoward fix I can imagine is to require some "mutual exclusive" between any image pulling and pruning. For example, if `store` class received a `pruning` request, we simply lock the store but wait until ongoing image pulling to finish (and not admit any future image pulling until this pruning finishes its marking phase).
I understand this has some throughput implication but it seems easiest to implement. What do you think?
- Zhitao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/#review186320
-----------------------------------------------------------
On Oct. 3, 2017, 5:13 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56721/
> -----------------------------------------------------------
>
> (Updated Oct. 3, 2017, 5:13 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This includes the following changes:
> - add a `pruneImages()` function on the chain of relevant classes;
> - implement prune in docker store;
> - fix mock interface to keep existing tests pass.
>
>
> Diffs
> -----
>
> src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f
> src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208
> src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842
> src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270
> src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67
> src/slave/containerizer/mesos/containerizer.hpp cc23b4d91be16fc95a131c09d07378b801e34d6f
> src/slave/containerizer/mesos/containerizer.cpp 4d5dc13f363f5d8886983d7dd06a5cecc177c345
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
> src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
> src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
> src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
> src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
> src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
> src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
> src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
> src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
> src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac
> src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a
> src/tests/containerizer/mock_containerizer.hpp 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569
>
>
> Diff: https://reviews.apache.org/r/56721/diff/9/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/#review186320
-----------------------------------------------------------
src/slave/containerizer/containerizer.hpp
Lines 156 (patched)
<https://reviews.apache.org/r/56721/#comment262815>
Could we add some comments?
src/slave/containerizer/containerizer.hpp
Lines 156-157 (patched)
<https://reviews.apache.org/r/56721/#comment263289>
maybe return failure in this virtual function by default? so that you don't need to do that on docker containerizer.
src/slave/containerizer/docker.hpp
Lines 112-114 (patched)
<https://reviews.apache.org/r/56721/#comment263290>
not needed.
src/slave/containerizer/docker.cpp
Lines 873-880 (patched)
<https://reviews.apache.org/r/56721/#comment263291>
not needed.
src/slave/containerizer/mesos/containerizer.cpp
Lines 2628 (patched)
<https://reviews.apache.org/r/56721/#comment263693>
Need to comment here since it depends on the checkpointed containerconfig introduced recently.
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
Lines 145-147 (patched)
<https://reviews.apache.org/r/56721/#comment263766>
two more spaces?
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
Lines 209-211 (patched)
<https://reviews.apache.org/r/56721/#comment263776>
three scenarios:
1. checkpointed cache is cleaned up for some reasons
2. the `activeImages` contains `excludedImages`, which the operator specifys some images that are never pulled before on this agent.
and I just think of the 3rd case:
3. if a couple big images are still being pulled (pulling started before `prune()`), it means thay are not in `storedImages` yet.
4. same as #3, we just unlock the store and resume the image pulling, then the requests in queue will be executed simultanuously, but another prune() call come in from the operator for some reason.
for the case of #3 and #4, we may need to fix:
https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp#L384~#L388
because container launch may fail if any of the layers are included in those ongoing pull but still get marked from pre-existed unused images.
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
Lines 212 (patched)
<https://reviews.apache.org/r/56721/#comment263717>
extra space?
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
Lines 217 (patched)
<https://reviews.apache.org/r/56721/#comment263786>
It means your cache is always empty.
`retainedImages` instead?
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
Lines 219 (patched)
<https://reviews.apache.org/r/56721/#comment263787>
s/image.get().layer_ids()/image->layer_ids()/g
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
Lines 228 (patched)
<https://reviews.apache.org/r/56721/#comment263788>
rename it as `unusedLayers`?
src/slave/containerizer/mesos/provisioner/docker/paths.hpp
Lines 30-42 (original), 30-42 (patched)
<https://reviews.apache.org/r/56721/#comment263708>
Could you fix the comment as well?
src/slave/containerizer/mesos/provisioner/docker/paths.cpp
Lines 106 (patched)
<https://reviews.apache.org/r/56721/#comment263714>
Understand this dir is used for sweeping only since marking is already done. but as a helper, we should follow whatever named for this dir, e.g., `gc`.
maybe call it `getGcPath()`? (we should use the same patern in this file e.g., xxxxPath(), the `getStagingDir()` is a tech debt).
src/slave/containerizer/mesos/provisioner/docker/paths.cpp
Lines 112 (patched)
<https://reviews.apache.org/r/56721/#comment263715>
getGcLayerPath()?
src/slave/containerizer/mesos/provisioner/docker/paths.cpp
Lines 114-115 (patched)
<https://reviews.apache.org/r/56721/#comment263712>
avoid this extra var?
src/slave/containerizer/mesos/provisioner/docker/paths.cpp
Lines 115 (patched)
<https://reviews.apache.org/r/56721/#comment263718>
are you prefer namoseconds than milliseconds for precise?
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 133 (patched)
<https://reviews.apache.org/r/56721/#comment263730>
what do you think renaming this struct to `ImagePullingInfo`? since we are having a queue of `Queued` sounds a little weird.
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 145 (patched)
<https://reviews.apache.org/r/56721/#comment263807>
any reason it is not a boolean?
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 146 (patched)
<https://reviews.apache.org/r/56721/#comment263731>
s/queued/imagePullingQueue/g
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 280 (patched)
<https://reviews.apache.org/r/56721/#comment263729>
VLOG(1) << "Queuing to get the image '" << name << "' since images are being pruned";
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 281 (patched)
<https://reviews.apache.org/r/56721/#comment263733>
newline above.
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 518 (patched)
<https://reviews.apache.org/r/56721/#comment263740>
s/Another pruning is already happening!/Another image pruning is happening/g
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 536 (patched)
<https://reviews.apache.org/r/56721/#comment263811>
should be a boolean. and basically this is the lock. we should put it at the very beginning of this method, right below the check of any other pruning happening.
also, let's comment on this lock.
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 551 (patched)
<https://reviews.apache.org/r/56721/#comment263819>
The store is already unlocked
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 575 (patched)
<https://reviews.apache.org/r/56721/#comment263806>
ditto.
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 582 (patched)
<https://reviews.apache.org/r/56721/#comment263812>
quote the path and no `!` at the end?
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 586 (patched)
<https://reviews.apache.org/r/56721/#comment263813>
ditto.
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 589 (patched)
<https://reviews.apache.org/r/56721/#comment263814>
VLOG(1)? since there might be hundreds of layers.
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 589-590 (patched)
<https://reviews.apache.org/r/56721/#comment263815>
quote these 3 vars. basically we should quote all variables that are not generated by mesos.
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 591 (patched)
<https://reviews.apache.org/r/56721/#comment263816>
newline above.
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 598 (patched)
<https://reviews.apache.org/r/56721/#comment263818>
what does this comment mean?
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 619 (patched)
<https://reviews.apache.org/r/56721/#comment263821>
quote the path.
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 621 (patched)
<https://reviews.apache.org/r/56721/#comment263822>
ditto.
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 623 (patched)
<https://reviews.apache.org/r/56721/#comment263823>
ditto.
- Gilbert Song
On Sept. 26, 2017, 11:15 a.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56721/
> -----------------------------------------------------------
>
> (Updated Sept. 26, 2017, 11:15 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This includes the following changes:
> - add a `pruneImages()` function on the chain of relevant classes;
> - implement prune in docker store;
> - fix mock interface to keep existing tests pass.
>
>
> Diffs
> -----
>
> src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f
> src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208
> src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842
> src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270
> src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67
> src/slave/containerizer/mesos/containerizer.hpp cc23b4d91be16fc95a131c09d07378b801e34d6f
> src/slave/containerizer/mesos/containerizer.cpp 4d5dc13f363f5d8886983d7dd06a5cecc177c345
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
> src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
> src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
> src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
> src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
> src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
> src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
> src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
> src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
> src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac
> src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a
> src/tests/containerizer/mock_containerizer.hpp 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569
>
>
> Diff: https://reviews.apache.org/r/56721/diff/8/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Zhitao Li <zh...@gmail.com>.
> On Oct. 30, 2017, 6:20 p.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/provisioner/docker/store.cpp
> > Lines 575-610 (patched)
> > <https://reviews.apache.org/r/56721/diff/13/?file=1863873#file1863873line575>
> >
> > Let's move the mark and sweep to the prvovisioner::prune() so that we can explicitly unlock before sweeping.
See my comment above. I still do not see a good way to write it. I would rather document that in a comment in docker store.
Also consider the fact that the current mark and sweep is really specific to this docker store. The next generation of stores may or may not use a mark and sweep (likely not, if we consider recent conversation about the unified artifactory store which we are considering better reference counting). Leaking it to provisioner means we would have to clean that up one more time.
- Zhitao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/#review189625
-----------------------------------------------------------
On Oct. 19, 2017, 4:28 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56721/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2017, 4:28 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This includes the following changes:
> - add a `pruneImages()` function on the chain of relevant classes;
> - implement prune in docker store;
> - fix mock interface to keep existing tests pass.
>
>
> Diffs
> -----
>
> src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f
> src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208
> src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842
> src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270
> src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67
> src/slave/containerizer/mesos/containerizer.hpp 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b
> src/slave/containerizer/mesos/containerizer.cpp 78fdd21f8b7ede4beedff31ba2b488ffebd4ea31
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
> src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
> src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
> src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
> src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
> src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
> src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
> src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
> src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
> src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac
> src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a
> src/tests/containerizer/mock_containerizer.hpp 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569
>
>
> Diff: https://reviews.apache.org/r/56721/diff/13/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/#review189625
-----------------------------------------------------------
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 575-610 (patched)
<https://reviews.apache.org/r/56721/#comment266805>
Let's move the mark and sweep to the prvovisioner::prune() so that we can explicitly unlock before sweeping.
- Gilbert Song
On Oct. 19, 2017, 9:28 a.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56721/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2017, 9:28 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This includes the following changes:
> - add a `pruneImages()` function on the chain of relevant classes;
> - implement prune in docker store;
> - fix mock interface to keep existing tests pass.
>
>
> Diffs
> -----
>
> src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f
> src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208
> src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842
> src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270
> src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67
> src/slave/containerizer/mesos/containerizer.hpp 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b
> src/slave/containerizer/mesos/containerizer.cpp 78fdd21f8b7ede4beedff31ba2b488ffebd4ea31
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
> src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
> src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
> src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
> src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
> src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
> src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
> src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
> src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
> src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac
> src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a
> src/tests/containerizer/mock_containerizer.hpp 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569
>
>
> Diff: https://reviews.apache.org/r/56721/diff/13/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Zhitao Li <zh...@gmail.com>.
> On Oct. 30, 2017, 6:17 p.m., Zhitao Li wrote:
> > src/slave/containerizer/mesos/provisioner/docker/store.cpp
> > Lines 578 (patched)
> > <https://reviews.apache.org/r/56721/diff/13/?file=1863873#file1863873line578>
> >
> > To capture the discussion from offline conversation, we will return all `layerPath`s from this function, move the `sweep` related code and `executor` to provisioner, and let the provisioner to sweep all layer paths.
I spent more time and just cannot really come up a good implementation which does not leak one of:
- `flags.docker_store_dir`;
- docker store implementation detail on paths;
- more complex and potentially expensive helper functions between `store` and `provisioner`.
I think I'd rather document the behaivor in `store.hpp` comment and related place than over complicate this code.
- Zhitao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/#review189623
-----------------------------------------------------------
On Oct. 19, 2017, 4:28 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56721/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2017, 4:28 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This includes the following changes:
> - add a `pruneImages()` function on the chain of relevant classes;
> - implement prune in docker store;
> - fix mock interface to keep existing tests pass.
>
>
> Diffs
> -----
>
> src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f
> src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208
> src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842
> src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270
> src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67
> src/slave/containerizer/mesos/containerizer.hpp 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b
> src/slave/containerizer/mesos/containerizer.cpp 78fdd21f8b7ede4beedff31ba2b488ffebd4ea31
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
> src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
> src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
> src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
> src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
> src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
> src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
> src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
> src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
> src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac
> src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a
> src/tests/containerizer/mock_containerizer.hpp 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569
>
>
> Diff: https://reviews.apache.org/r/56721/diff/13/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/#review189623
-----------------------------------------------------------
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 578 (patched)
<https://reviews.apache.org/r/56721/#comment266804>
To capture the discussion from offline conversation, we will return all `layerPath`s from this function, move the `sweep` related code and `executor` to provisioner, and let the provisioner to sweep all layer paths.
- Zhitao Li
On Oct. 19, 2017, 4:28 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56721/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2017, 4:28 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This includes the following changes:
> - add a `pruneImages()` function on the chain of relevant classes;
> - implement prune in docker store;
> - fix mock interface to keep existing tests pass.
>
>
> Diffs
> -----
>
> src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f
> src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208
> src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842
> src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270
> src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67
> src/slave/containerizer/mesos/containerizer.hpp 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b
> src/slave/containerizer/mesos/containerizer.cpp 78fdd21f8b7ede4beedff31ba2b488ffebd4ea31
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
> src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
> src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
> src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
> src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
> src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
> src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
> src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
> src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
> src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac
> src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a
> src/tests/containerizer/mock_containerizer.hpp 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569
>
>
> Diff: https://reviews.apache.org/r/56721/diff/13/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Zhitao Li <zh...@gmail.com>.
> On Nov. 10, 2017, 2:30 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/containerizer.cpp
> > Lines 2793 (patched)
> > <https://reviews.apache.org/r/56721/diff/14/?file=1873108#file1873108line2793>
> >
> > Should we just use `Hashset<Image>`?
I don't think there is a built-in definition for protobuf messages yet: https://github.com/google/protobuf/issues/2066
It might be interesting to define a custom hash for protobuf message so we can use them, although we need to think a bit about proper hash function.
> On Nov. 10, 2017, 2:30 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/provisioner/docker/store.cpp
> > Lines 531 (patched)
> > <https://reviews.apache.org/r/56721/diff/14/?file=1873114#file1873114line531>
> >
> > maybe we should checkpoint the layer dir instead of the layerRootfs dir in the provisioner.
See my explanation from the other patch, but I still think provisioner should not second guess internal of image stores.
- Zhitao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/#review190636
-----------------------------------------------------------
On Nov. 10, 2017, 7:34 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56721/
> -----------------------------------------------------------
>
> (Updated Nov. 10, 2017, 7:34 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This includes the following changes:
> - add a `pruneImages()` function on the chain of relevant classes;
> - implement prune in docker store;
> - fix mock interface to keep existing tests pass.
>
>
> Diffs
> -----
>
> src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f
> src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208
> src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842
> src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270
> src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67
> src/slave/containerizer/mesos/containerizer.hpp 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b
> src/slave/containerizer/mesos/containerizer.cpp 100e3bbda543d87808da9ff6bea42da5099ea8c5
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
> src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
> src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
> src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
> src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
> src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
> src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
> src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
> src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
> src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac
> src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a
> src/tests/containerizer/mock_containerizer.hpp 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569
>
>
> Diff: https://reviews.apache.org/r/56721/diff/15/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/#review190636
-----------------------------------------------------------
src/slave/containerizer/mesos/containerizer.cpp
Lines 2790-2792 (patched)
<https://reviews.apache.org/r/56721/#comment268146>
We are not using this var, right?
src/slave/containerizer/mesos/containerizer.cpp
Lines 2793 (patched)
<https://reviews.apache.org/r/56721/#comment268185>
Should we just use `Hashset<Image>`?
src/slave/containerizer/mesos/containerizer.cpp
Lines 2797 (patched)
<https://reviews.apache.org/r/56721/#comment268149>
just a little rephasing:
Checkpointing `ContainerConfig` is introduced recently. Lagacy containers do not have the information of which image is used. Image pruning is disabled.
src/slave/containerizer/mesos/containerizer.cpp
Lines 2803-2804 (patched)
<https://reviews.apache.org/r/56721/#comment268183>
You might need foreachpair for log out the containerID:
```
return Failure(
"Container " + containerId + " does not have ContainerConfig checkpointed. Image pruning is disabled");
```
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp
Lines 103-107 (patched)
<https://reviews.apache.org/r/56721/#comment268215>
could we use `excludedImages` consistently in favor of adding that to operator API in near future.
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
Lines 83-85 (patched)
<https://reviews.apache.org/r/56721/#comment268217>
do we still need this variable?
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
Lines 213 (patched)
<https://reviews.apache.org/r/56721/#comment268216>
s/stored/cached/g?
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 82 (patched)
<https://reviews.apache.org/r/56721/#comment268220>
why do we neet to add this?
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 79-80 (original), 83-88 (patched)
<https://reviews.apache.org/r/56721/#comment268223>
could we not change the style for now in this file?
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 262-263 (patched)
<https://reviews.apache.org/r/56721/#comment268224>
why adding this?
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 531 (patched)
<https://reviews.apache.org/r/56721/#comment268228>
maybe we should checkpoint the layer dir instead of the layerRootfs dir in the provisioner.
src/slave/containerizer/mesos/provisioner/provisioner.hpp
Lines 110 (patched)
<https://reviews.apache.org/r/56721/#comment268186>
then, pass a hashset of Images to provisioner::pruneImages()?
src/slave/containerizer/mesos/provisioner/provisioner.hpp
Lines 207-210 (patched)
<https://reviews.apache.org/r/56721/#comment268203>
Could we add more comments to describe the critical section of this readwritelock? E.g., store directory and the provisioner dir are the critical section. and basically, `provision()` and `destroy` are not expected to touch the same critical section simultaneously. This is guarantteed by the mesos containerizer, e.g., a destroy will always wait for a container provisioning finishes, then do the cleanup.
src/slave/containerizer/mesos/provisioner/provisioner.hpp
Lines 209 (patched)
<https://reviews.apache.org/r/56721/#comment268202>
s/prune/destroy/g?
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 486-487 (patched)
<https://reviews.apache.org/r/56721/#comment268204>
quote destroy and provisioner, as well as pruneImages.
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 569-570 (patched)
<https://reviews.apache.org/r/56721/#comment268205>
ditto.
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 714-715 (patched)
<https://reviews.apache.org/r/56721/#comment268206>
ditto.
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 727 (patched)
<https://reviews.apache.org/r/56721/#comment268210>
one more space after foreach, e.g., `foreachXXX (`.
ditto to all others.
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 728-729 (patched)
<https://reviews.apache.org/r/56721/#comment268208>
use `info->layers.isNone()` instead?
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 728-730 (patched)
<https://reviews.apache.org/r/56721/#comment268209>
we need to add some comments here. generally, we might have 3 scenarios that we need to think about:
1. a lagacy container? no, impossible to hit this code since lagacy containers are already excluded by the containerizer.
2. the agent crashes after we call backend::provision() but before checkpointing the `layers`? yes, in this case it is fine to skip those layers sinece they are not used for any running containers yet.
3. someone deletes the checkpointed `layers` files? yes, possible but we dont expect this is allowed. it means we may delete layers that a running container is using. we need to at least `VLOG(1)` to that containerID (using `foreachpair` instead?).
Thoughts?
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 732-734 (patched)
<https://reviews.apache.org/r/56721/#comment268212>
fit in one line?
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 750-753 (patched)
<https://reviews.apache.org/r/56721/#comment268214>
fit in one line after the change?
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 751 (patched)
<https://reviews.apache.org/r/56721/#comment268213>
s/[=]/[]/g?
and do you need `const list<Nothing>&`?
src/slave/containerizer/mesos/provisioner/store.hpp
Lines 97-98 (patched)
<https://reviews.apache.org/r/56721/#comment268211>
Could we add comments for these two parameter? especially, people would ask while passing in the images, why do we need the active layers? it is because in the store (e.g., docker store) the cache is not the source of true. we need to not only keep the excluded images from the operator API, but also maintain the cache.
- Gilbert Song
On Oct. 30, 2017, 4:59 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56721/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2017, 4:59 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This includes the following changes:
> - add a `pruneImages()` function on the chain of relevant classes;
> - implement prune in docker store;
> - fix mock interface to keep existing tests pass.
>
>
> Diffs
> -----
>
> src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f
> src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208
> src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842
> src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270
> src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67
> src/slave/containerizer/mesos/containerizer.hpp 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b
> src/slave/containerizer/mesos/containerizer.cpp 100e3bbda543d87808da9ff6bea42da5099ea8c5
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
> src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
> src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
> src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
> src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
> src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
> src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
> src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
> src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
> src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac
> src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a
> src/tests/containerizer/mock_containerizer.hpp 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569
>
>
> Diff: https://reviews.apache.org/r/56721/diff/14/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/#review190497
-----------------------------------------------------------
src/slave/containerizer/composing.cpp
Lines 264-265 (patched)
<https://reviews.apache.org/r/56721/#comment267922>
fits in one line?
src/slave/containerizer/composing.cpp
Lines 710-713 (patched)
<https://reviews.apache.org/r/56721/#comment267931>
no need to pass in the `const list<Nothing>&`:
```
return collect(futures)
.then([]() { return Nothing(); });
```
- Gilbert Song
On Oct. 30, 2017, 4:59 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56721/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2017, 4:59 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This includes the following changes:
> - add a `pruneImages()` function on the chain of relevant classes;
> - implement prune in docker store;
> - fix mock interface to keep existing tests pass.
>
>
> Diffs
> -----
>
> src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f
> src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208
> src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842
> src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270
> src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67
> src/slave/containerizer/mesos/containerizer.hpp 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b
> src/slave/containerizer/mesos/containerizer.cpp 100e3bbda543d87808da9ff6bea42da5099ea8c5
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
> src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
> src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
> src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
> src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
> src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
> src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
> src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
> src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
> src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac
> src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a
> src/tests/containerizer/mock_containerizer.hpp 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569
>
>
> Diff: https://reviews.apache.org/r/56721/diff/14/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/#review189977
-----------------------------------------------------------
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 489 (patched)
<https://reviews.apache.org/r/56721/#comment267231>
Pull into a lambda! Or worse case scenario do the following:
.then(defer(self(), [this]() {
return provisionInLock(containerId, image);
});
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 490 (patched)
<https://reviews.apache.org/r/56721/#comment267230>
.onAny([rwLock]() {
rwLock.read_unlock();
});
- Gilbert Song
On Oct. 30, 2017, 4:59 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56721/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2017, 4:59 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This includes the following changes:
> - add a `pruneImages()` function on the chain of relevant classes;
> - implement prune in docker store;
> - fix mock interface to keep existing tests pass.
>
>
> Diffs
> -----
>
> src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f
> src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208
> src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842
> src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270
> src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67
> src/slave/containerizer/mesos/containerizer.hpp 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b
> src/slave/containerizer/mesos/containerizer.cpp 100e3bbda543d87808da9ff6bea42da5099ea8c5
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
> src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
> src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
> src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
> src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
> src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
> src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
> src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
> src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
> src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac
> src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a
> src/tests/containerizer/mock_containerizer.hpp 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569
>
>
> Diff: https://reviews.apache.org/r/56721/diff/14/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Zhitao Li <zh...@gmail.com>.
> On Nov. 16, 2017, 12:04 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/provisioner/provisioner.cpp
> > Lines 472 (patched)
> > <https://reviews.apache.org/r/56721/diff/14-15/?file=1873116#file1873116line492>
> >
> > is it possible to pass by value for our ReadWriteLock? so that we could s/this/rwLock/g?
pass by value requires creating local copy of the lock, or pass by reference requires c++14, so I'm sticking with this style.
- Zhitao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/#review191117
-----------------------------------------------------------
On Nov. 16, 2017, 4 a.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56721/
> -----------------------------------------------------------
>
> (Updated Nov. 16, 2017, 4 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This includes the following changes:
> - add a `pruneImages()` function on the chain of relevant classes;
> - implement prune in docker store;
> - fix mock interface to keep existing tests pass.
>
>
> Diffs
> -----
>
> src/slave/containerizer/composing.hpp c2689cf36575d1e7c6ef8bd2ab3e25681a2ecfcc
> src/slave/containerizer/composing.cpp 64919ef1e61a984956c2280ae6b1890c4d135ad1
> src/slave/containerizer/containerizer.hpp 2027bd93cc439ed9b2f544e57183765ee032603b
> src/slave/containerizer/docker.hpp 105c0680b6714aded1b3e05699930a072545e681
> src/slave/containerizer/docker.cpp 63432a99f58aab00cd8612164e00fe6c3a6cf5dd
> src/slave/containerizer/mesos/containerizer.hpp f5d5146de9abdd9804ede6a4b17a965a4b27c05a
> src/slave/containerizer/mesos/containerizer.cpp db5f044f7415386c94e4930f365dfc14d403414a
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
> src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
> src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
> src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
> src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
> src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
> src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
> src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
> src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
> src/tests/containerizer.hpp c98913f1a833aa6a60779b66463610ccd5911bbf
> src/tests/containerizer.cpp c6f1ec0b000781270b7c79d5e776575c6df778aa
> src/tests/containerizer/mock_containerizer.hpp 5befcccecdb76f3b70993642128745a0134ffa65
>
>
> Diff: https://reviews.apache.org/r/56721/diff/17/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/#review191117
-----------------------------------------------------------
Fix it, then Ship it!
LGTM! Thank you for patientlt addressing my comments, Zhitao!
src/slave/containerizer/mesos/containerizer.cpp
Line 2796 (original), 2798 (patched)
<https://reviews.apache.org/r/56721/#comment268711>
one more space
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Line 489 (original), 469 (patched)
<https://reviews.apache.org/r/56721/#comment268706>
sorry, my comment was not clear enough. just want to pull into a lambda. `this` may not be needed here?
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 472 (patched)
<https://reviews.apache.org/r/56721/#comment268709>
is it possible to pass by value for our ReadWriteLock? so that we could s/this/rwLock/g?
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 572-573 (original), 561-564 (patched)
<https://reviews.apache.org/r/56721/#comment268708>
ditto.
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 717-718 (original), 708-711 (patched)
<https://reviews.apache.org/r/56721/#comment268710>
ditto.
src/slave/containerizer/mesos/provisioner/provisioner.hpp
Lines 110 (patched)
<https://reviews.apache.org/r/56721/#comment268726>
Should we use `excludedImages` consistently in this patch? In favor of introducing it in the operator API.
- Gilbert Song
On Nov. 13, 2017, 10:21 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56721/
> -----------------------------------------------------------
>
> (Updated Nov. 13, 2017, 10:21 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This includes the following changes:
> - add a `pruneImages()` function on the chain of relevant classes;
> - implement prune in docker store;
> - fix mock interface to keep existing tests pass.
>
>
> Diffs
> -----
>
> src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f
> src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208
> src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842
> src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270
> src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67
> src/slave/containerizer/mesos/containerizer.hpp 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b
> src/slave/containerizer/mesos/containerizer.cpp 100e3bbda543d87808da9ff6bea42da5099ea8c5
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
> src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
> src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
> src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
> src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
> src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
> src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
> src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
> src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
> src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac
> src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a
> src/tests/containerizer/mock_containerizer.hpp 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569
>
>
> Diff: https://reviews.apache.org/r/56721/diff/16/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/#review191537
-----------------------------------------------------------
Fix it, then Ship it!
I will address them for you.
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 468-470 (patched)
<https://reviews.apache.org/r/56721/#comment269378>
oneline.
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 485 (patched)
<https://reviews.apache.org/r/56721/#comment269379>
fix the indentation
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 486-488 (patched)
<https://reviews.apache.org/r/56721/#comment269380>
oneline and put the code inside of the lambda as a separate line.
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 600-602 (patched)
<https://reviews.apache.org/r/56721/#comment269381>
ditto.
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 742-744 (patched)
<https://reviews.apache.org/r/56721/#comment269382>
ditto
- Gilbert Song
On Nov. 17, 2017, 8:51 a.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56721/
> -----------------------------------------------------------
>
> (Updated Nov. 17, 2017, 8:51 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This includes the following changes:
> - add a `pruneImages()` function on the chain of relevant classes;
> - implement prune in docker store;
> - fix mock interface to keep existing tests pass.
>
>
> Diffs
> -----
>
> src/slave/containerizer/composing.hpp c2689cf36575d1e7c6ef8bd2ab3e25681a2ecfcc
> src/slave/containerizer/composing.cpp 64919ef1e61a984956c2280ae6b1890c4d135ad1
> src/slave/containerizer/containerizer.hpp 2027bd93cc439ed9b2f544e57183765ee032603b
> src/slave/containerizer/docker.hpp 105c0680b6714aded1b3e05699930a072545e681
> src/slave/containerizer/docker.cpp 63432a99f58aab00cd8612164e00fe6c3a6cf5dd
> src/slave/containerizer/mesos/containerizer.hpp f5d5146de9abdd9804ede6a4b17a965a4b27c05a
> src/slave/containerizer/mesos/containerizer.cpp db5f044f7415386c94e4930f365dfc14d403414a
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
> src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
> src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
> src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
> src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
> src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
> src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
> src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
> src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
> src/tests/containerizer.hpp c98913f1a833aa6a60779b66463610ccd5911bbf
> src/tests/containerizer.cpp c6f1ec0b000781270b7c79d5e776575c6df778aa
> src/tests/containerizer/mock_containerizer.hpp 5befcccecdb76f3b70993642128745a0134ffa65
>
>
> Diff: https://reviews.apache.org/r/56721/diff/18/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/
-----------------------------------------------------------
(Updated Nov. 20, 2017, 12:39 p.m.)
Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
Bugs: MESOS-8249
https://issues.apache.org/jira/browse/MESOS-8249
Repository: mesos
Description
-------
This includes the following changes:
- add a `pruneImages()` function on the chain of relevant classes;
- implement prune in docker store;
- fix mock interface to keep existing tests pass.
Diffs
-----
src/slave/containerizer/composing.hpp c2689cf36575d1e7c6ef8bd2ab3e25681a2ecfcc
src/slave/containerizer/composing.cpp 64919ef1e61a984956c2280ae6b1890c4d135ad1
src/slave/containerizer/containerizer.hpp 2027bd93cc439ed9b2f544e57183765ee032603b
src/slave/containerizer/docker.hpp 105c0680b6714aded1b3e05699930a072545e681
src/slave/containerizer/docker.cpp 63432a99f58aab00cd8612164e00fe6c3a6cf5dd
src/slave/containerizer/mesos/containerizer.hpp f5d5146de9abdd9804ede6a4b17a965a4b27c05a
src/slave/containerizer/mesos/containerizer.cpp db5f044f7415386c94e4930f365dfc14d403414a
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
src/tests/containerizer.hpp c98913f1a833aa6a60779b66463610ccd5911bbf
src/tests/containerizer.cpp c6f1ec0b000781270b7c79d5e776575c6df778aa
src/tests/containerizer/mock_containerizer.hpp 5befcccecdb76f3b70993642128745a0134ffa65
Diff: https://reviews.apache.org/r/56721/diff/18/
Testing
-------
Thanks,
Zhitao Li
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/
-----------------------------------------------------------
(Updated Nov. 17, 2017, 4:51 p.m.)
Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
Changes
-------
Review comments.
Bugs: MESOS-4945
https://issues.apache.org/jira/browse/MESOS-4945
Repository: mesos
Description
-------
This includes the following changes:
- add a `pruneImages()` function on the chain of relevant classes;
- implement prune in docker store;
- fix mock interface to keep existing tests pass.
Diffs (updated)
-----
src/slave/containerizer/composing.hpp c2689cf36575d1e7c6ef8bd2ab3e25681a2ecfcc
src/slave/containerizer/composing.cpp 64919ef1e61a984956c2280ae6b1890c4d135ad1
src/slave/containerizer/containerizer.hpp 2027bd93cc439ed9b2f544e57183765ee032603b
src/slave/containerizer/docker.hpp 105c0680b6714aded1b3e05699930a072545e681
src/slave/containerizer/docker.cpp 63432a99f58aab00cd8612164e00fe6c3a6cf5dd
src/slave/containerizer/mesos/containerizer.hpp f5d5146de9abdd9804ede6a4b17a965a4b27c05a
src/slave/containerizer/mesos/containerizer.cpp db5f044f7415386c94e4930f365dfc14d403414a
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
src/tests/containerizer.hpp c98913f1a833aa6a60779b66463610ccd5911bbf
src/tests/containerizer.cpp c6f1ec0b000781270b7c79d5e776575c6df778aa
src/tests/containerizer/mock_containerizer.hpp 5befcccecdb76f3b70993642128745a0134ffa65
Diff: https://reviews.apache.org/r/56721/diff/18/
Changes: https://reviews.apache.org/r/56721/diff/17-18/
Testing
-------
Thanks,
Zhitao Li
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Zhitao Li <zh...@gmail.com>.
> On Nov. 17, 2017, 2:07 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/provisioner/provisioner.cpp
> > Lines 469 (patched)
> > <https://reviews.apache.org/r/56721/diff/17/?file=1894068#file1894068line469>
> >
> > you dont need `this` for this lambda
I think we need `this` here because the lambda accesses `stores`, which is a member field.
> On Nov. 17, 2017, 2:07 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/provisioner/provisioner.cpp
> > Lines 487 (patched)
> > <https://reviews.apache.org/r/56721/diff/17/?file=1894068#file1894068line488>
> >
> > this is not safe! you need to defer self.
I'll fix this but I don't see why unlock from another actor threads can cause unsafe behavior.
> On Nov. 17, 2017, 2:07 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/provisioner/provisioner.cpp
> > Lines 556 (patched)
> > <https://reviews.apache.org/r/56721/diff/17/?file=1894068#file1894068line557>
> >
> > ditto.
This also needs capturing `this` because it accesses `infos` member.
> On Nov. 17, 2017, 2:07 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/provisioner/provisioner.cpp
> > Lines 698 (patched)
> > <https://reviews.apache.org/r/56721/diff/17/?file=1894068#file1894068line699>
> >
> > ditto.
Same, this needs to access `infos` member.
- Zhitao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/#review191286
-----------------------------------------------------------
On Nov. 17, 2017, 4:51 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56721/
> -----------------------------------------------------------
>
> (Updated Nov. 17, 2017, 4:51 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This includes the following changes:
> - add a `pruneImages()` function on the chain of relevant classes;
> - implement prune in docker store;
> - fix mock interface to keep existing tests pass.
>
>
> Diffs
> -----
>
> src/slave/containerizer/composing.hpp c2689cf36575d1e7c6ef8bd2ab3e25681a2ecfcc
> src/slave/containerizer/composing.cpp 64919ef1e61a984956c2280ae6b1890c4d135ad1
> src/slave/containerizer/containerizer.hpp 2027bd93cc439ed9b2f544e57183765ee032603b
> src/slave/containerizer/docker.hpp 105c0680b6714aded1b3e05699930a072545e681
> src/slave/containerizer/docker.cpp 63432a99f58aab00cd8612164e00fe6c3a6cf5dd
> src/slave/containerizer/mesos/containerizer.hpp f5d5146de9abdd9804ede6a4b17a965a4b27c05a
> src/slave/containerizer/mesos/containerizer.cpp db5f044f7415386c94e4930f365dfc14d403414a
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
> src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
> src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
> src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
> src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
> src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
> src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
> src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
> src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
> src/tests/containerizer.hpp c98913f1a833aa6a60779b66463610ccd5911bbf
> src/tests/containerizer.cpp c6f1ec0b000781270b7c79d5e776575c6df778aa
> src/tests/containerizer/mock_containerizer.hpp 5befcccecdb76f3b70993642128745a0134ffa65
>
>
> Diff: https://reviews.apache.org/r/56721/diff/18/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/#review191286
-----------------------------------------------------------
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp
Lines 107 (patched)
<https://reviews.apache.org/r/56721/#comment269056>
could you do a grep and fix the excludedImages naming in
metadata_manager.cpp:
provisioner/docker/store.cpp:
mesos_containerizer_tests.cpp:
provisioner_docker_tests.cpp:
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 469 (patched)
<https://reviews.apache.org/r/56721/#comment269047>
you dont need `this` for this lambda
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 461-462 (original), 476-478 (patched)
<https://reviews.apache.org/r/56721/#comment269048>
make them oneline?
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 487 (patched)
<https://reviews.apache.org/r/56721/#comment269049>
this is not safe! you need to defer self.
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 556 (patched)
<https://reviews.apache.org/r/56721/#comment269051>
ditto.
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 599 (patched)
<https://reviews.apache.org/r/56721/#comment269052>
ditto
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 698 (patched)
<https://reviews.apache.org/r/56721/#comment269053>
ditto.
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 737 (patched)
<https://reviews.apache.org/r/56721/#comment269055>
why do you need defer here?
src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 739 (patched)
<https://reviews.apache.org/r/56721/#comment269054>
ditto
- Gilbert Song
On Nov. 15, 2017, 8 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56721/
> -----------------------------------------------------------
>
> (Updated Nov. 15, 2017, 8 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This includes the following changes:
> - add a `pruneImages()` function on the chain of relevant classes;
> - implement prune in docker store;
> - fix mock interface to keep existing tests pass.
>
>
> Diffs
> -----
>
> src/slave/containerizer/composing.hpp c2689cf36575d1e7c6ef8bd2ab3e25681a2ecfcc
> src/slave/containerizer/composing.cpp 64919ef1e61a984956c2280ae6b1890c4d135ad1
> src/slave/containerizer/containerizer.hpp 2027bd93cc439ed9b2f544e57183765ee032603b
> src/slave/containerizer/docker.hpp 105c0680b6714aded1b3e05699930a072545e681
> src/slave/containerizer/docker.cpp 63432a99f58aab00cd8612164e00fe6c3a6cf5dd
> src/slave/containerizer/mesos/containerizer.hpp f5d5146de9abdd9804ede6a4b17a965a4b27c05a
> src/slave/containerizer/mesos/containerizer.cpp db5f044f7415386c94e4930f365dfc14d403414a
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
> src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
> src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
> src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
> src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
> src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
> src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
> src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
> src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
> src/tests/containerizer.hpp c98913f1a833aa6a60779b66463610ccd5911bbf
> src/tests/containerizer.cpp c6f1ec0b000781270b7c79d5e776575c6df778aa
> src/tests/containerizer/mock_containerizer.hpp 5befcccecdb76f3b70993642128745a0134ffa65
>
>
> Diff: https://reviews.apache.org/r/56721/diff/17/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/
-----------------------------------------------------------
(Updated Nov. 16, 2017, 4 a.m.)
Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
Changes
-------
Rebase and review comments.
Bugs: MESOS-4945
https://issues.apache.org/jira/browse/MESOS-4945
Repository: mesos
Description
-------
This includes the following changes:
- add a `pruneImages()` function on the chain of relevant classes;
- implement prune in docker store;
- fix mock interface to keep existing tests pass.
Diffs (updated)
-----
src/slave/containerizer/composing.hpp c2689cf36575d1e7c6ef8bd2ab3e25681a2ecfcc
src/slave/containerizer/composing.cpp 64919ef1e61a984956c2280ae6b1890c4d135ad1
src/slave/containerizer/containerizer.hpp 2027bd93cc439ed9b2f544e57183765ee032603b
src/slave/containerizer/docker.hpp 105c0680b6714aded1b3e05699930a072545e681
src/slave/containerizer/docker.cpp 63432a99f58aab00cd8612164e00fe6c3a6cf5dd
src/slave/containerizer/mesos/containerizer.hpp f5d5146de9abdd9804ede6a4b17a965a4b27c05a
src/slave/containerizer/mesos/containerizer.cpp db5f044f7415386c94e4930f365dfc14d403414a
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
src/tests/containerizer.hpp c98913f1a833aa6a60779b66463610ccd5911bbf
src/tests/containerizer.cpp c6f1ec0b000781270b7c79d5e776575c6df778aa
src/tests/containerizer/mock_containerizer.hpp 5befcccecdb76f3b70993642128745a0134ffa65
Diff: https://reviews.apache.org/r/56721/diff/17/
Changes: https://reviews.apache.org/r/56721/diff/16-17/
Testing
-------
Thanks,
Zhitao Li
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/
-----------------------------------------------------------
(Updated Nov. 14, 2017, 6:21 a.m.)
Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
Bugs: MESOS-4945
https://issues.apache.org/jira/browse/MESOS-4945
Repository: mesos
Description
-------
This includes the following changes:
- add a `pruneImages()` function on the chain of relevant classes;
- implement prune in docker store;
- fix mock interface to keep existing tests pass.
Diffs (updated)
-----
src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f
src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208
src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842
src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270
src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67
src/slave/containerizer/mesos/containerizer.hpp 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b
src/slave/containerizer/mesos/containerizer.cpp 100e3bbda543d87808da9ff6bea42da5099ea8c5
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac
src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a
src/tests/containerizer/mock_containerizer.hpp 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569
Diff: https://reviews.apache.org/r/56721/diff/16/
Changes: https://reviews.apache.org/r/56721/diff/15-16/
Testing
-------
Thanks,
Zhitao Li
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Zhitao Li <zh...@gmail.com>.
> On Nov. 11, 2017, 6:57 p.m., Jason Lai wrote:
> > src/slave/containerizer/mesos/provisioner/docker/paths.hpp
> > Lines 98 (patched)
> > <https://reviews.apache.org/r/56721/diff/15/?file=1888597#file1888597line98>
> >
> > Nit: I'd suggest using `GC` over `Gc`
The other example of `Gc` for registry in mesos master is also in this case so not changing.
- Zhitao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/#review190783
-----------------------------------------------------------
On Nov. 10, 2017, 7:34 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56721/
> -----------------------------------------------------------
>
> (Updated Nov. 10, 2017, 7:34 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This includes the following changes:
> - add a `pruneImages()` function on the chain of relevant classes;
> - implement prune in docker store;
> - fix mock interface to keep existing tests pass.
>
>
> Diffs
> -----
>
> src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f
> src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208
> src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842
> src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270
> src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67
> src/slave/containerizer/mesos/containerizer.hpp 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b
> src/slave/containerizer/mesos/containerizer.cpp 100e3bbda543d87808da9ff6bea42da5099ea8c5
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
> src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
> src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
> src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
> src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
> src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
> src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
> src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
> src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
> src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac
> src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a
> src/tests/containerizer/mock_containerizer.hpp 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569
>
>
> Diff: https://reviews.apache.org/r/56721/diff/15/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Zhitao Li <zh...@gmail.com>.
> On Nov. 11, 2017, 6:57 p.m., Jason Lai wrote:
> > src/slave/containerizer/mesos/provisioner/docker/store.cpp
> > Lines 594 (patched)
> > <https://reviews.apache.org/r/56721/diff/15/?file=1888600#file1888600line594>
> >
> > Will there be a race here?
`rmdirs` is a lambda which is sent to the `executor` and executed in a different thread, so I don't see how a race condition can happen. Can you elaborate?
- Zhitao
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/#review190783
-----------------------------------------------------------
On Nov. 10, 2017, 7:34 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56721/
> -----------------------------------------------------------
>
> (Updated Nov. 10, 2017, 7:34 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This includes the following changes:
> - add a `pruneImages()` function on the chain of relevant classes;
> - implement prune in docker store;
> - fix mock interface to keep existing tests pass.
>
>
> Diffs
> -----
>
> src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f
> src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208
> src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842
> src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270
> src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67
> src/slave/containerizer/mesos/containerizer.hpp 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b
> src/slave/containerizer/mesos/containerizer.cpp 100e3bbda543d87808da9ff6bea42da5099ea8c5
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
> src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
> src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
> src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
> src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
> src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
> src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
> src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
> src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
> src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac
> src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a
> src/tests/containerizer/mock_containerizer.hpp 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569
>
>
> Diff: https://reviews.apache.org/r/56721/diff/15/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Jason Lai <ja...@jasonlai.net>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/#review190783
-----------------------------------------------------------
Fix it, then Ship it!
Great stuff! Just a few nits and a question
src/slave/containerizer/mesos/containerizer.cpp
Lines 2802 (patched)
<https://reviews.apache.org/r/56721/#comment268388>
Typo: **Legacy** instead of **Legacy**
src/slave/containerizer/mesos/provisioner/docker/paths.hpp
Lines 98 (patched)
<https://reviews.apache.org/r/56721/#comment268386>
Nit: I'd suggest using `GC` over `Gc`
src/slave/containerizer/mesos/provisioner/docker/paths.hpp
Lines 101 (patched)
<https://reviews.apache.org/r/56721/#comment268387>
Ditto `GC` over `Gc`
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 594 (patched)
<https://reviews.apache.org/r/56721/#comment268389>
Will there be a race here?
- Jason Lai
On Nov. 10, 2017, 7:34 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56721/
> -----------------------------------------------------------
>
> (Updated Nov. 10, 2017, 7:34 p.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This includes the following changes:
> - add a `pruneImages()` function on the chain of relevant classes;
> - implement prune in docker store;
> - fix mock interface to keep existing tests pass.
>
>
> Diffs
> -----
>
> src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f
> src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208
> src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842
> src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270
> src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67
> src/slave/containerizer/mesos/containerizer.hpp 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b
> src/slave/containerizer/mesos/containerizer.cpp 100e3bbda543d87808da9ff6bea42da5099ea8c5
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
> src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
> src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
> src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
> src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
> src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
> src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
> src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
> src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
> src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac
> src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a
> src/tests/containerizer/mock_containerizer.hpp 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569
>
>
> Diff: https://reviews.apache.org/r/56721/diff/15/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/
-----------------------------------------------------------
(Updated Nov. 10, 2017, 7:34 p.m.)
Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
Bugs: MESOS-4945
https://issues.apache.org/jira/browse/MESOS-4945
Repository: mesos
Description
-------
This includes the following changes:
- add a `pruneImages()` function on the chain of relevant classes;
- implement prune in docker store;
- fix mock interface to keep existing tests pass.
Diffs (updated)
-----
src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f
src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208
src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842
src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270
src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67
src/slave/containerizer/mesos/containerizer.hpp 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b
src/slave/containerizer/mesos/containerizer.cpp 100e3bbda543d87808da9ff6bea42da5099ea8c5
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac
src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a
src/tests/containerizer/mock_containerizer.hpp 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569
Diff: https://reviews.apache.org/r/56721/diff/15/
Changes: https://reviews.apache.org/r/56721/diff/14-15/
Testing
-------
Thanks,
Zhitao Li
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/
-----------------------------------------------------------
(Updated Oct. 30, 2017, 11:59 p.m.)
Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
Changes
-------
Clean up and comment fixes.
Bugs: MESOS-4945
https://issues.apache.org/jira/browse/MESOS-4945
Repository: mesos
Description
-------
This includes the following changes:
- add a `pruneImages()` function on the chain of relevant classes;
- implement prune in docker store;
- fix mock interface to keep existing tests pass.
Diffs (updated)
-----
src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f
src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208
src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842
src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270
src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67
src/slave/containerizer/mesos/containerizer.hpp 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b
src/slave/containerizer/mesos/containerizer.cpp 100e3bbda543d87808da9ff6bea42da5099ea8c5
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac
src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a
src/tests/containerizer/mock_containerizer.hpp 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569
Diff: https://reviews.apache.org/r/56721/diff/14/
Changes: https://reviews.apache.org/r/56721/diff/13-14/
Testing
-------
Thanks,
Zhitao Li
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/#review189494
-----------------------------------------------------------
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp
Lines 219-225 (patched)
<https://reviews.apache.org/r/56721/#comment266782>
why do we need this loop if `retainedLayers` is a hashset?
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 129 (patched)
<https://reviews.apache.org/r/56721/#comment266790>
did you consume it?
src/slave/containerizer/mesos/provisioner/docker/store.cpp
Lines 131-141 (patched)
<https://reviews.apache.org/r/56721/#comment266791>
ditto.
src/slave/containerizer/mesos/provisioner/provisioner.hpp
Lines 207 (patched)
<https://reviews.apache.org/r/56721/#comment266594>
We need comments for this variable in detials.
- Gilbert Song
On Oct. 19, 2017, 9:28 a.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56721/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2017, 9:28 a.m.)
>
>
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
>
>
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This includes the following changes:
> - add a `pruneImages()` function on the chain of relevant classes;
> - implement prune in docker store;
> - fix mock interface to keep existing tests pass.
>
>
> Diffs
> -----
>
> src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f
> src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208
> src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842
> src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270
> src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67
> src/slave/containerizer/mesos/containerizer.hpp 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b
> src/slave/containerizer/mesos/containerizer.cpp 78fdd21f8b7ede4beedff31ba2b488ffebd4ea31
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
> src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
> src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
> src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
> src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
> src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
> src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
> src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
> src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
> src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
> src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac
> src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a
> src/tests/containerizer/mock_containerizer.hpp 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569
>
>
> Diff: https://reviews.apache.org/r/56721/diff/13/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/
-----------------------------------------------------------
(Updated Oct. 19, 2017, 4:28 p.m.)
Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
Changes
-------
Updated name and interface for ReadWriteLock.
Bugs: MESOS-4945
https://issues.apache.org/jira/browse/MESOS-4945
Repository: mesos
Description
-------
This includes the following changes:
- add a `pruneImages()` function on the chain of relevant classes;
- implement prune in docker store;
- fix mock interface to keep existing tests pass.
Diffs (updated)
-----
src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f
src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208
src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842
src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270
src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67
src/slave/containerizer/mesos/containerizer.hpp 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b
src/slave/containerizer/mesos/containerizer.cpp 78fdd21f8b7ede4beedff31ba2b488ffebd4ea31
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac
src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a
src/tests/containerizer/mock_containerizer.hpp 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569
Diff: https://reviews.apache.org/r/56721/diff/13/
Changes: https://reviews.apache.org/r/56721/diff/12-13/
Testing
-------
Thanks,
Zhitao Li
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/
-----------------------------------------------------------
(Updated Oct. 14, 2017, 3:11 a.m.)
Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
Changes
-------
Proper implementation using:
- RWMutex in provisioner for concurrency;
- simpler implementation based on actual layers of active containers.
Bugs: MESOS-4945
https://issues.apache.org/jira/browse/MESOS-4945
Repository: mesos
Description
-------
This includes the following changes:
- add a `pruneImages()` function on the chain of relevant classes;
- implement prune in docker store;
- fix mock interface to keep existing tests pass.
Diffs (updated)
-----
src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f
src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208
src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842
src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270
src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67
src/slave/containerizer/mesos/containerizer.hpp cc23b4d91be16fc95a131c09d07378b801e34d6f
src/slave/containerizer/mesos/containerizer.cpp 4d5dc13f363f5d8886983d7dd06a5cecc177c345
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac
src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a
src/tests/containerizer/mock_containerizer.hpp 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569
Diff: https://reviews.apache.org/r/56721/diff/12/
Changes: https://reviews.apache.org/r/56721/diff/11-12/
Testing
-------
Thanks,
Zhitao Li
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/
-----------------------------------------------------------
(Updated Oct. 9, 2017, 11:17 p.m.)
Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
Changes
-------
- Remove the `excludeImages` field in `containerizer::pruneImages()`;
- Pass down `provisionInfo` for active containers to provisioner and store for a stable way to retain layers;
- In docker store, rebuild all active layers;
- Make sure we keep active layers as well as image references for all running containers in docker store.
- Make sure pruning in docker store is delayed until all active pulling finishes.
Bugs: MESOS-4945
https://issues.apache.org/jira/browse/MESOS-4945
Repository: mesos
Description
-------
This includes the following changes:
- add a `pruneImages()` function on the chain of relevant classes;
- implement prune in docker store;
- fix mock interface to keep existing tests pass.
Diffs (updated)
-----
src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f
src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208
src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842
src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270
src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67
src/slave/containerizer/mesos/containerizer.hpp cc23b4d91be16fc95a131c09d07378b801e34d6f
src/slave/containerizer/mesos/containerizer.cpp 4d5dc13f363f5d8886983d7dd06a5cecc177c345
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac
src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a
src/tests/containerizer/mock_containerizer.hpp 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569
Diff: https://reviews.apache.org/r/56721/diff/11/
Changes: https://reviews.apache.org/r/56721/diff/10-11/
Testing
-------
Thanks,
Zhitao Li
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/
-----------------------------------------------------------
(Updated Oct. 3, 2017, 8:24 p.m.)
Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
Changes
-------
Fix missing member initialization.
Bugs: MESOS-4945
https://issues.apache.org/jira/browse/MESOS-4945
Repository: mesos
Description
-------
This includes the following changes:
- add a `pruneImages()` function on the chain of relevant classes;
- implement prune in docker store;
- fix mock interface to keep existing tests pass.
Diffs (updated)
-----
src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f
src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208
src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842
src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270
src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67
src/slave/containerizer/mesos/containerizer.hpp cc23b4d91be16fc95a131c09d07378b801e34d6f
src/slave/containerizer/mesos/containerizer.cpp 4d5dc13f363f5d8886983d7dd06a5cecc177c345
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac
src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a
src/tests/containerizer/mock_containerizer.hpp 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569
Diff: https://reviews.apache.org/r/56721/diff/10/
Changes: https://reviews.apache.org/r/56721/diff/9-10/
Testing
-------
Thanks,
Zhitao Li
Re: Review Request 56721: Implemented pruneImages with a mark and
sweep in docker store.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56721/
-----------------------------------------------------------
(Updated Oct. 3, 2017, 5:13 p.m.)
Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
Changes
-------
Review comments.
Bugs: MESOS-4945
https://issues.apache.org/jira/browse/MESOS-4945
Repository: mesos
Description
-------
This includes the following changes:
- add a `pruneImages()` function on the chain of relevant classes;
- implement prune in docker store;
- fix mock interface to keep existing tests pass.
Diffs (updated)
-----
src/slave/containerizer/composing.hpp 06d68eef5de7745e32f0e808f11016bcc285dd8f
src/slave/containerizer/composing.cpp 587f009384f0c7ef87482686578dc822d3d5b208
src/slave/containerizer/containerizer.hpp 449bb5d0902936faae7bf9bae9c703b219aed842
src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270
src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67
src/slave/containerizer/mesos/containerizer.hpp cc23b4d91be16fc95a131c09d07378b801e34d6f
src/slave/containerizer/mesos/containerizer.cpp 4d5dc13f363f5d8886983d7dd06a5cecc177c345
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 954da1681778878c8aff6150002e52ecb648d1bb
src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp d86afd2a6ff0bf87e624db1c99255c85068bf6ab
src/slave/containerizer/mesos/provisioner/docker/paths.hpp 232c027f8f96da0cb30b957bce4607d3695050d2
src/slave/containerizer/mesos/provisioner/docker/paths.cpp cd684b33eb308ce1eeb4539a5b2d51985d835db7
src/slave/containerizer/mesos/provisioner/docker/store.hpp 1cf68665d33bd40a7605d26c96fb7b618407fdd0
src/slave/containerizer/mesos/provisioner/docker/store.cpp f357710cb19aec3654b0604f7909d068eaf20095
src/slave/containerizer/mesos/provisioner/provisioner.hpp 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e
src/slave/containerizer/mesos/provisioner/provisioner.cpp 450a3b32d69d2882973a6ed4e94e169a0256056b
src/slave/containerizer/mesos/provisioner/store.hpp 01ab83dca79e51b8c96d18ee65705912b0ac8324
src/slave/containerizer/mesos/provisioner/store.cpp cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f
src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac
src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a
src/tests/containerizer/mock_containerizer.hpp 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569
Diff: https://reviews.apache.org/r/56721/diff/9/
Changes: https://reviews.apache.org/r/56721/diff/8-9/
Testing
-------
Thanks,
Zhitao Li