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 2018/08/29 07:22:18 UTC
Review Request 68555: Made checker library retry to remove the
previous check container.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68555/
-----------------------------------------------------------
Review request for mesos, Andrei Budnik, Alexander Rukletsov, Gastón Kleiman, and Gilbert Song.
Bugs: MESOS-8568
https://issues.apache.org/jira/browse/MESOS-8568
Repository: mesos
Description
-------
Previously when checker library fails to remove the previous check
container, it will discard the promise and launch a new check container
which will cause two problems:
1. The discarded promise is used to launch the new check container,
that means even the new check container is launched successfully,
we still have no chance to process its check result since the
promise has already been discarded.
2. The previous check container will never get a chance to be removed
which is leak, i.e., its runtime directory and sandbox directory
will not be removed.
Now in this patch, when checker library fails to remove the previous
check container, we make it remove the previous check container again.
Diffs
-----
src/checks/checker_process.cpp 77a76f465fe57eab89f027b5acb74c2339551678
Diff: https://reviews.apache.org/r/68555/diff/1/
Testing
-------
sudo make check
Thanks,
Qian Zhang
Re: Review Request 68555: Made checker library retry to remove the
previous check container.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68555/#review208259
-----------------------------------------------------------
Ship it!
Good catch and good fix, Qian!
- Alexander Rukletsov
On Aug. 29, 2018, 7:22 a.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68555/
> -----------------------------------------------------------
>
> (Updated Aug. 29, 2018, 7:22 a.m.)
>
>
> Review request for mesos, Andrei Budnik, Alexander Rukletsov, Gastón Kleiman, and Gilbert Song.
>
>
> Bugs: MESOS-8568
> https://issues.apache.org/jira/browse/MESOS-8568
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Previously when checker library fails to remove the previous check
> container, it will discard the promise and launch a new check container
> which will cause two problems:
> 1. The discarded promise is used to launch the new check container,
> that means even the new check container is launched successfully,
> we still have no chance to process its check result since the
> promise has already been discarded.
> 2. The previous check container will never get a chance to be removed
> which is leak, i.e., its runtime directory and sandbox directory
> will not be removed.
>
> Now in this patch, when checker library fails to remove the previous
> check container, we make it remove the previous check container again.
>
>
> Diffs
> -----
>
> src/checks/checker_process.cpp 77a76f465fe57eab89f027b5acb74c2339551678
>
>
> Diff: https://reviews.apache.org/r/68555/diff/1/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 68555: Made checker library retry to remove the
previous check container.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68555/#review208205
-----------------------------------------------------------
Patch looks great!
Reviews applied: [68495, 68555]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Aug. 29, 2018, 7:22 a.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68555/
> -----------------------------------------------------------
>
> (Updated Aug. 29, 2018, 7:22 a.m.)
>
>
> Review request for mesos, Andrei Budnik, Alexander Rukletsov, Gastón Kleiman, and Gilbert Song.
>
>
> Bugs: MESOS-8568
> https://issues.apache.org/jira/browse/MESOS-8568
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Previously when checker library fails to remove the previous check
> container, it will discard the promise and launch a new check container
> which will cause two problems:
> 1. The discarded promise is used to launch the new check container,
> that means even the new check container is launched successfully,
> we still have no chance to process its check result since the
> promise has already been discarded.
> 2. The previous check container will never get a chance to be removed
> which is leak, i.e., its runtime directory and sandbox directory
> will not be removed.
>
> Now in this patch, when checker library fails to remove the previous
> check container, we make it remove the previous check container again.
>
>
> Diffs
> -----
>
> src/checks/checker_process.cpp 77a76f465fe57eab89f027b5acb74c2339551678
>
>
> Diff: https://reviews.apache.org/r/68555/diff/1/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 68555: Made checker library retry to remove the
previous check container.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68555/#review208084
-----------------------------------------------------------
PASS: Mesos patch 68555 was successfully built and tested.
Reviews applied: `['68495', '68555']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2257/mesos-review-68555
- Mesos Reviewbot Windows
On Aug. 29, 2018, 7:22 a.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68555/
> -----------------------------------------------------------
>
> (Updated Aug. 29, 2018, 7:22 a.m.)
>
>
> Review request for mesos, Andrei Budnik, Alexander Rukletsov, Gastón Kleiman, and Gilbert Song.
>
>
> Bugs: MESOS-8568
> https://issues.apache.org/jira/browse/MESOS-8568
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Previously when checker library fails to remove the previous check
> container, it will discard the promise and launch a new check container
> which will cause two problems:
> 1. The discarded promise is used to launch the new check container,
> that means even the new check container is launched successfully,
> we still have no chance to process its check result since the
> promise has already been discarded.
> 2. The previous check container will never get a chance to be removed
> which is leak, i.e., its runtime directory and sandbox directory
> will not be removed.
>
> Now in this patch, when checker library fails to remove the previous
> check container, we make it remove the previous check container again.
>
>
> Diffs
> -----
>
> src/checks/checker_process.cpp 77a76f465fe57eab89f027b5acb74c2339551678
>
>
> Diff: https://reviews.apache.org/r/68555/diff/1/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 68555: Made checker library retry to remove the
previous check container.
Posted by Gastón Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68555/#review208144
-----------------------------------------------------------
Ship it!
I would have probably added a `return` instead of an `else` statement, but there is no functional difference, so LGTM! =)
- Gastón Kleiman
On Aug. 29, 2018, 12:22 a.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68555/
> -----------------------------------------------------------
>
> (Updated Aug. 29, 2018, 12:22 a.m.)
>
>
> Review request for mesos, Andrei Budnik, Alexander Rukletsov, Gastón Kleiman, and Gilbert Song.
>
>
> Bugs: MESOS-8568
> https://issues.apache.org/jira/browse/MESOS-8568
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Previously when checker library fails to remove the previous check
> container, it will discard the promise and launch a new check container
> which will cause two problems:
> 1. The discarded promise is used to launch the new check container,
> that means even the new check container is launched successfully,
> we still have no chance to process its check result since the
> promise has already been discarded.
> 2. The previous check container will never get a chance to be removed
> which is leak, i.e., its runtime directory and sandbox directory
> will not be removed.
>
> Now in this patch, when checker library fails to remove the previous
> check container, we make it remove the previous check container again.
>
>
> Diffs
> -----
>
> src/checks/checker_process.cpp 77a76f465fe57eab89f027b5acb74c2339551678
>
>
> Diff: https://reviews.apache.org/r/68555/diff/1/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 68555: Made checker library retry to remove the
previous check container.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68555/#review208373
-----------------------------------------------------------
Ship it!
Ship It!
- Gilbert Song
On Aug. 29, 2018, 12:22 a.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68555/
> -----------------------------------------------------------
>
> (Updated Aug. 29, 2018, 12:22 a.m.)
>
>
> Review request for mesos, Andrei Budnik, Alexander Rukletsov, Gastón Kleiman, and Gilbert Song.
>
>
> Bugs: MESOS-8568
> https://issues.apache.org/jira/browse/MESOS-8568
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Previously when checker library fails to remove the previous check
> container, it will discard the promise and launch a new check container
> which will cause two problems:
> 1. The discarded promise is used to launch the new check container,
> that means even the new check container is launched successfully,
> we still have no chance to process its check result since the
> promise has already been discarded.
> 2. The previous check container will never get a chance to be removed
> which is leak, i.e., its runtime directory and sandbox directory
> will not be removed.
>
> Now in this patch, when checker library fails to remove the previous
> check container, we make it remove the previous check container again.
>
>
> Diffs
> -----
>
> src/checks/checker_process.cpp 77a76f465fe57eab89f027b5acb74c2339551678
>
>
> Diff: https://reviews.apache.org/r/68555/diff/1/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Qian Zhang
>
>