You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Timothy Chen <tn...@apache.org> on 2014/12/23 00:57:49 UTC
Review Request 29341: Fix docker recovery when launched in container
and add more logging.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29341/
-----------------------------------------------------------
Review request for mesos and Benjamin Hindman.
Repository: mesos-git
Description
-------
Fix docker recovery when launched in container and add more logging.
Also delay removing the executor container so it can be introspected as well as the container.
Diffs
-----
src/slave/containerizer/docker.hpp 28ebc6272cd68167fc9a0898fd8eb9d53890b815
src/slave/containerizer/docker.cpp 19a6ea2b5342abe4bf10cf4ea2d5d756df9f8665
Diff: https://reviews.apache.org/r/29341/diff/
Testing
-------
make check
Thanks,
Timothy Chen
Re: Review Request 29341: Fix docker recovery when launched in
container and add more logging.
Posted by Bernd Mathiske <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29341/#review79661
-----------------------------------------------------------
src/slave/containerizer/docker.hpp
<https://reviews.apache.org/r/29341/#comment129183>
Suggestion: explain what role the executor argument plays in this.
src/slave/containerizer/docker.cpp
<https://reviews.apache.org/r/29341/#comment129184>
According to thecomment starting on line 98, the former version of this code is correct, not the new version. Is the comment stale?
src/slave/containerizer/docker.cpp
<https://reviews.apache.org/r/29341/#comment129185>
This comment now became redundant.
src/slave/containerizer/docker.cpp
<https://reviews.apache.org/r/29341/#comment129179>
contianer -> container
src/slave/containerizer/docker.cpp
<https://reviews.apache.org/r/29341/#comment129180>
Used twice => suggestion: factor it out and use a function name instead of a comment to explain what this does
src/slave/containerizer/docker.cpp
<https://reviews.apache.org/r/29341/#comment129182>
How does what the comment says relate to the statement that follows?
- Bernd Mathiske
On Jan. 16, 2015, 5:38 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29341/
> -----------------------------------------------------------
>
> (Updated Jan. 16, 2015, 5:38 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Bernd Mathiske.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fix docker recovery when launched in container and add more logging.
> Also delay removing the executor container so it can be introspected as well as the container.
>
>
> Diffs
> -----
>
> src/slave/containerizer/docker.hpp b7bf54a
> src/slave/containerizer/docker.cpp 5f4b4ce
>
> Diff: https://reviews.apache.org/r/29341/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 29341: Fix docker recovery when launched in
container and add more logging.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29341/
-----------------------------------------------------------
(Updated Jan. 17, 2015, 1:38 a.m.)
Review request for mesos, Benjamin Hindman and Bernd Mathiske.
Changes
-------
rebased.
Repository: mesos-git
Description
-------
Fix docker recovery when launched in container and add more logging.
Also delay removing the executor container so it can be introspected as well as the container.
Diffs (updated)
-----
src/slave/containerizer/docker.hpp b7bf54a
src/slave/containerizer/docker.cpp 5f4b4ce
Diff: https://reviews.apache.org/r/29341/diff/
Testing
-------
make check
Thanks,
Timothy Chen
Re: Review Request 29341: Fix docker recovery when launched in
container and add more logging.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29341/#review67788
-----------------------------------------------------------
Bad patch!
Reviews applied: [29327, 29328, 29329, 29330, 29331, 29332, 29333]
Failed command: ./support/apply-review.sh -n -r 29333
Error:
2015-01-13 02:34:03 URL:https://reviews.apache.org/r/29333/diff/raw/ [1084/1084] -> "29333.patch" [1]
error: patch failed: src/slave/flags.hpp:316
error: src/slave/flags.hpp: patch does not apply
Failed to apply patch
- Mesos ReviewBot
On Jan. 13, 2015, 2:21 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29341/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2015, 2:21 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Bernd Mathiske.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Fix docker recovery when launched in container and add more logging.
> Also delay removing the executor container so it can be introspected as well as the container.
>
>
> Diffs
> -----
>
> src/slave/containerizer/docker.hpp b7bf54a
> src/slave/containerizer/docker.cpp 5f4b4ce
>
> Diff: https://reviews.apache.org/r/29341/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 29341: Fix docker recovery when launched in
container and add more logging.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29341/
-----------------------------------------------------------
(Updated Jan. 13, 2015, 2:21 a.m.)
Review request for mesos, Benjamin Hindman and Bernd Mathiske.
Repository: mesos-git
Description
-------
Fix docker recovery when launched in container and add more logging.
Also delay removing the executor container so it can be introspected as well as the container.
Diffs (updated)
-----
src/slave/containerizer/docker.hpp b7bf54a
src/slave/containerizer/docker.cpp 5f4b4ce
Diff: https://reviews.apache.org/r/29341/diff/
Testing
-------
make check
Thanks,
Timothy Chen
Re: Review Request 29341: Fix docker recovery when launched in
container and add more logging.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29341/
-----------------------------------------------------------
(Updated Jan. 13, 2015, 2:20 a.m.)
Review request for mesos, Benjamin Hindman and Bernd Mathiske.
Repository: mesos-git
Description
-------
Fix docker recovery when launched in container and add more logging.
Also delay removing the executor container so it can be introspected as well as the container.
Diffs (updated)
-----
src/slave/containerizer/docker.hpp b7bf54a
src/slave/containerizer/docker.cpp 5f4b4ce
Diff: https://reviews.apache.org/r/29341/diff/
Testing
-------
make check
Thanks,
Timothy Chen
Re: Review Request 29341: Fix docker recovery when launched in
container and add more logging.
Posted by Bernd Mathiske <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29341/#review67650
-----------------------------------------------------------
src/slave/containerizer/docker.cpp
<https://reviews.apache.org/r/29341/#comment111714>
typo
contianer -> container
src/slave/containerizer/docker.cpp
<https://reviews.apache.org/r/29341/#comment111715>
1. What does this do? A comment would be nice.
2. Depending on what the above reveals, reusing the volume variable may not be tidy.
src/slave/containerizer/docker.cpp
<https://reviews.apache.org/r/29341/#comment111717>
1. What pid in what host? Can you be more specific?
2. How does this relate to the following statement? By it not referring to the mentioned pid? Can you make that more explicit? Or would you like the reader to look up what ResourceStatistics() does and then find it "obvious"?
- Bernd Mathiske
On Jan. 8, 2015, 5:30 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29341/
> -----------------------------------------------------------
>
> (Updated Jan. 8, 2015, 5:30 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Bernd Mathiske.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Fix docker recovery when launched in container and add more logging.
> Also delay removing the executor container so it can be introspected as well as the container.
>
>
> Diffs
> -----
>
> src/slave/containerizer/docker.hpp b7bf54a
> src/slave/containerizer/docker.cpp 5f4b4ce
>
> Diff: https://reviews.apache.org/r/29341/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 29341: Fix docker recovery when launched in
container and add more logging.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29341/#review67364
-----------------------------------------------------------
Bad patch!
Reviews applied: [29327, 29328, 29329, 29330, 29331, 29332, 29333]
Failed command: ./support/apply-review.sh -n -r 29333
Error:
2015-01-09 02:19:27 URL:https://reviews.apache.org/r/29333/diff/raw/ [1084/1084] -> "29333.patch" [1]
error: patch failed: src/slave/flags.hpp:316
error: src/slave/flags.hpp: patch does not apply
Failed to apply patch
- Mesos ReviewBot
On Jan. 9, 2015, 1:30 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29341/
> -----------------------------------------------------------
>
> (Updated Jan. 9, 2015, 1:30 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Bernd Mathiske.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Fix docker recovery when launched in container and add more logging.
> Also delay removing the executor container so it can be introspected as well as the container.
>
>
> Diffs
> -----
>
> src/slave/containerizer/docker.hpp b7bf54a
> src/slave/containerizer/docker.cpp 5f4b4ce
>
> Diff: https://reviews.apache.org/r/29341/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 29341: Fix docker recovery when launched in
container and add more logging.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29341/
-----------------------------------------------------------
(Updated Jan. 9, 2015, 1:30 a.m.)
Review request for mesos, Benjamin Hindman and Bernd Mathiske.
Changes
-------
Rebased
Repository: mesos-git
Description
-------
Fix docker recovery when launched in container and add more logging.
Also delay removing the executor container so it can be introspected as well as the container.
Diffs (updated)
-----
src/slave/containerizer/docker.hpp b7bf54a
src/slave/containerizer/docker.cpp 5f4b4ce
Diff: https://reviews.apache.org/r/29341/diff/
Testing
-------
make check
Thanks,
Timothy Chen
Re: Review Request 29341: Fix docker recovery when launched in
container and add more logging.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29341/#review65852
-----------------------------------------------------------
Bad patch!
Reviews applied: [29338, 29339]
Failed command: ./support/apply-review.sh -n -r 29339
Error:
2014-12-23 00:24:31 URL:https://reviews.apache.org/r/29339/diff/raw/ [670/670] -> "29339.patch" [1]
error: src/docker/executor.cpp: does not exist in index
Failed to apply patch
- Mesos ReviewBot
On Dec. 22, 2014, 11:57 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29341/
> -----------------------------------------------------------
>
> (Updated Dec. 22, 2014, 11:57 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Fix docker recovery when launched in container and add more logging.
> Also delay removing the executor container so it can be introspected as well as the container.
>
>
> Diffs
> -----
>
> src/slave/containerizer/docker.hpp 28ebc6272cd68167fc9a0898fd8eb9d53890b815
> src/slave/containerizer/docker.cpp 19a6ea2b5342abe4bf10cf4ea2d5d756df9f8665
>
> Diff: https://reviews.apache.org/r/29341/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>