[ 
https://issues.apache.org/jira/browse/YARN-5366?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16248516#comment-16248516
 ] 

Shane Kumpf commented on YARN-5366:
-----------------------------------

Thanks for the feedback, [~ebadger]

{quote}
Can't we run the docker run command in the background? Wouldn't that fix this 
issue and allow us to continue along in the container-executor as planned?
{quote}
While this does work to allow the remaining docker commands in c-e to execute, 
they fail because the container is not yet started, so {{docker inspect}} is 
unable to look up the pid. One way to work around it would be to add a sleep 
and retry.

{quote}
Can you point to some examples of --rm not working? Are there issues that have 
been reported to the Docker community that we can follow? Has this been fixed 
in a newer release of Docker?
{quote}
The {{\-\-rm}} option calls the same backend as {{docker rm}}, 
{{daemon.ContainerRm}}. The one advantage that {{\-\-rm}} has, is that it is 
called on the daemon side vs the client. However, if client -> daemon 
communication is broken, we can't manage containers anyway, so we've got bigger 
problems. 

There are many reasons a {{ContainerRm}} might fail, even with force enabled. 
Here are a few that I've encountered: 

* [Driver devicemapper failed to remove root filesystem. Device is busy 
|https://github.com/moby/moby/issues/27381]. 
* [Unable to remove filesystem for xxx: remove 
/var/lib/docker/containers/xxx/shm: device or resource 
busy|https://github.com/moby/moby/issues/17902]

You'll note similar issues discussed with the change in behavior to {{\-\-rm}}. 
Here is a link to the {{\-\-rm}} implementation, see the associated discussion 
is in the PR:

* [AutoRemove 
code|https://github.com/moby/moby/blame/402540708c9a0c35dc0b279a0f330455633537b8/daemon/monitor.go#L181]
* [Do not remove containers from memory on 
error|https://github.com/moby/moby/pull/31012]

{quote}
I would generally think that the docker daemon would be better at removing 
containers since this is a docker issue, not a hadoop issue.
{quote}
You would sure think so, but in many cases it is neither; it is a kernel/OS 
issue. Docker is very dependent on the OS for everything it does. In the the 
rootfs case I mentioned above, OS played a big part in the problem, however, 1+ 
years later they have found some places for improvement on the docker side, but 
conclude _"Will the next version be perfect? Probably not... but I do expect 
this to get better."_ - so I don't expect these types of issues to go away 
anytime soon.

We actually can do better than docker in this case, as retrying the removal 
after the container is marked as "dead" does work in many cases. Not always, of 
course, in some cases the only way to clean up the dead container is to restart 
docker.

Having said the above, I'm not sold on the benefit of using {{\-\-rm}}. Given 
that we do want the ability to keep a container around for debugging purposes, 
if we adopt {{\-\-rm}}, we'll need two different paths for signaling and clean 
up, for not much benefit IMO. We already have support for {{docker inspect}} 
and {{docker rm}} in c-e, so I don't think we'll have an required c-e changes 
for the removal portion. Would you be OK if I pursued a patch that didn't use 
{{\-\-rm}} for now? We can revise that decision if the patch doesn't pan out?

This also reminds me, we should probably document suggested kernel, docker, and 
docker backing storage configs that we've tested.

> Improve handling of the Docker container life cycle
> ---------------------------------------------------
>
>                 Key: YARN-5366
>                 URL: https://issues.apache.org/jira/browse/YARN-5366
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: yarn
>            Reporter: Shane Kumpf
>            Assignee: Shane Kumpf
>              Labels: oct16-medium
>         Attachments: YARN-5366.001.patch, YARN-5366.002.patch, 
> YARN-5366.003.patch, YARN-5366.004.patch, YARN-5366.005.patch, 
> YARN-5366.006.patch
>
>
> There are several paths that need to be improved with regard to the Docker 
> container lifecycle when running Docker containers on YARN.
> 1) Provide the ability to keep a container on the NodeManager for a set 
> period of time for debugging purposes.
> 2) Support sending signals to the process in the container to allow for 
> triggering stack traces, heap dumps, etc.
> 3) Support for Docker's live restore, which means moving away from the use of 
> {{docker wait}}. (YARN-5818)
> 4) Improve the resiliency of liveliness checks (kill -0) by adding retries.
> 5) Improve the resiliency of container removal by adding retries.
> 6) Only attempt to stop, kill, and remove containers if the current container 
> state allows for it.
> 7) Better handling of short lived containers when the container is stopped 
> before the PID can be retrieved. (YARN-6305)



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to