You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Qian Zhang <zh...@gmail.com> on 2019/02/18 14:26:30 UTC
Re: Review Request 69972: Skipped orphan container which has no
checkpointed volumes in recovery.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69972/
-----------------------------------------------------------
(Updated Feb. 18, 2019, 10:26 p.m.)
Review request for mesos, Andrei Budnik and Gilbert Song.
Changes
-------
Addressed review comments.
Summary (updated)
-----------------
Skipped orphan container which has no checkpointed volumes in recovery.
Bugs: MESOS-9507
https://issues.apache.org/jira/browse/MESOS-9507
Repository: mesos
Description (updated)
-------
There are two cases we need to handle:
1. The checkpointed docker volumes file does not exist.
2. The checkpointed docker volumes file is empty.
For both of the two cases, in the recovery of `docker/volume` isolator,
we should remove the orphan container's checkpoint directory and then
skip the container. But for recoverable containers, we return an error
since the above two cases should not happen for them.
Diffs (updated)
-----
src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 2fd0493bc3e1749c12b81b9f2d281b86cb2c410f
src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp a72fc84da6fb0f24d363dd4c635500510da675d8
Diff: https://reviews.apache.org/r/69972/diff/2/
Changes: https://reviews.apache.org/r/69972/diff/1-2/
Testing
-------
Thanks,
Qian Zhang
Re: Review Request 69972: Handled containers which has no checkpointed
volumes during recovery.
Posted by Qian Zhang <zh...@gmail.com>.
> On Feb. 18, 2019, 11:48 p.m., Andrei Budnik wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp
> > Line 92 (original), 92 (patched)
> > <https://reviews.apache.org/r/69972/diff/3/?file=2125760#file2125760line92>
> >
> > `orphaned` sounds more straightforward to me than `recoverable`. Especially, when you are not familiar with our containerization subsystem.
Had a discussion with Gilbert, we agree we'd better not to differentiate recoverable containers and orphaned containers in this patch. What you pointed out (recoverable containers must not have empty checkpoint file) actualy exists in other isolators as well (e.g., CNI), we may consider to improve them in a separate patch in future.
- Qian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69972/#review212902
-----------------------------------------------------------
On Feb. 20, 2019, 2:17 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69972/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2019, 2:17 p.m.)
>
>
> Review request for mesos, Andrei Budnik and Gilbert Song.
>
>
> Bugs: MESOS-9507
> https://issues.apache.org/jira/browse/MESOS-9507
>
>
> Repository: mesos
>
>
> Description
> -------
>
> There are two cases we need to handle:
> 1. The container's checkpointed docker volumes file does not exist.
> 2. The container's checkpointed docker volumes file is empty.
> For both of the two cases, in the recovery of `docker/volume` isolator,
> we should construct an info object with empty docker volumes for the
> container and rely on containerizer or `docker/volume` isolator's
> `recover` method to cleanup the container.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp a72fc84da6fb0f24d363dd4c635500510da675d8
>
>
> Diff: https://reviews.apache.org/r/69972/diff/4/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 69972: Skipped orphan container which has no
checkpointed volumes in recovery.
Posted by Andrei Budnik <ab...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69972/#review212902
-----------------------------------------------------------
src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp
Line 92 (original), 92 (patched)
<https://reviews.apache.org/r/69972/#comment298782>
`orphaned` sounds more straightforward to me than `recoverable`. Especially, when you are not familiar with our containerization subsystem.
- Andrei Budnik
On Feb. 18, 2019, 2:31 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69972/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2019, 2:31 p.m.)
>
>
> Review request for mesos, Andrei Budnik and Gilbert Song.
>
>
> Bugs: MESOS-9507
> https://issues.apache.org/jira/browse/MESOS-9507
>
>
> Repository: mesos
>
>
> Description
> -------
>
> There are two cases we need to handle:
> 1. The checkpointed docker volumes file does not exist.
> 2. The checkpointed docker volumes file is empty.
> For both of the two cases, in the recovery of `docker/volume` isolator,
> we should remove the orphan container's checkpoint directory and then
> skip the container. But for recoverable containers, we return an error
> since the above two cases should not happen for them.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 2fd0493bc3e1749c12b81b9f2d281b86cb2c410f
> src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp a72fc84da6fb0f24d363dd4c635500510da675d8
>
>
> Diff: https://reviews.apache.org/r/69972/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 69972: Handled containers which has no checkpointed
volumes during recovery.
Posted by Andrei Budnik <ab...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69972/#review212956
-----------------------------------------------------------
Ship it!
Ship It!
- Andrei Budnik
On Feb. 20, 2019, 6:17 a.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69972/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2019, 6:17 a.m.)
>
>
> Review request for mesos, Andrei Budnik and Gilbert Song.
>
>
> Bugs: MESOS-9507
> https://issues.apache.org/jira/browse/MESOS-9507
>
>
> Repository: mesos
>
>
> Description
> -------
>
> There are two cases we need to handle:
> 1. The container's checkpointed docker volumes file does not exist.
> 2. The container's checkpointed docker volumes file is empty.
> For both of the two cases, in the recovery of `docker/volume` isolator,
> we should construct an info object with empty docker volumes for the
> container and rely on containerizer or `docker/volume` isolator's
> `recover` method to cleanup the container.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp a72fc84da6fb0f24d363dd4c635500510da675d8
>
>
> Diff: https://reviews.apache.org/r/69972/diff/4/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 69972: Handled containers which has no checkpointed
volumes during recovery.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69972/#review212982
-----------------------------------------------------------
Ship it!
Ship It!
- Gilbert Song
On Feb. 19, 2019, 10:17 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69972/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2019, 10:17 p.m.)
>
>
> Review request for mesos, Andrei Budnik and Gilbert Song.
>
>
> Bugs: MESOS-9507
> https://issues.apache.org/jira/browse/MESOS-9507
>
>
> Repository: mesos
>
>
> Description
> -------
>
> There are two cases we need to handle:
> 1. The container's checkpointed docker volumes file does not exist.
> 2. The container's checkpointed docker volumes file is empty.
> For both of the two cases, in the recovery of `docker/volume` isolator,
> we should construct an info object with empty docker volumes for the
> container and rely on containerizer or `docker/volume` isolator's
> `recover` method to cleanup the container.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp a72fc84da6fb0f24d363dd4c635500510da675d8
>
>
> Diff: https://reviews.apache.org/r/69972/diff/4/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 69972: Handled containers which has no checkpointed
volumes during recovery.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69972/
-----------------------------------------------------------
(Updated Feb. 20, 2019, 2:17 p.m.)
Review request for mesos, Andrei Budnik and Gilbert Song.
Changes
-------
Reused `cleanup` to remove container directory rather than doing it directly in `_recover`.
Summary (updated)
-----------------
Handled containers which has no checkpointed volumes during recovery.
Bugs: MESOS-9507
https://issues.apache.org/jira/browse/MESOS-9507
Repository: mesos
Description (updated)
-------
There are two cases we need to handle:
1. The container's checkpointed docker volumes file does not exist.
2. The container's checkpointed docker volumes file is empty.
For both of the two cases, in the recovery of `docker/volume` isolator,
we should construct an info object with empty docker volumes for the
container and rely on containerizer or `docker/volume` isolator's
`recover` method to cleanup the container.
Diffs (updated)
-----
src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp a72fc84da6fb0f24d363dd4c635500510da675d8
Diff: https://reviews.apache.org/r/69972/diff/4/
Changes: https://reviews.apache.org/r/69972/diff/3-4/
Testing
-------
Thanks,
Qian Zhang
Re: Review Request 69972: Skipped orphan container which has no
checkpointed volumes in recovery.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69972/
-----------------------------------------------------------
(Updated Feb. 18, 2019, 10:31 p.m.)
Review request for mesos, Andrei Budnik and Gilbert Song.
Changes
-------
Minor changes.
Bugs: MESOS-9507
https://issues.apache.org/jira/browse/MESOS-9507
Repository: mesos
Description
-------
There are two cases we need to handle:
1. The checkpointed docker volumes file does not exist.
2. The checkpointed docker volumes file is empty.
For both of the two cases, in the recovery of `docker/volume` isolator,
we should remove the orphan container's checkpoint directory and then
skip the container. But for recoverable containers, we return an error
since the above two cases should not happen for them.
Diffs (updated)
-----
src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 2fd0493bc3e1749c12b81b9f2d281b86cb2c410f
src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp a72fc84da6fb0f24d363dd4c635500510da675d8
Diff: https://reviews.apache.org/r/69972/diff/3/
Changes: https://reviews.apache.org/r/69972/diff/2-3/
Testing
-------
Thanks,
Qian Zhang