[
https://issues.apache.org/jira/browse/YARN-7973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16424599#comment-16424599
]
Eric Yang commented on YARN-7973:
---------------------------------
[[email protected]] Thank you for the patch, I am able to run flex with
patch 003. A few small nitpicks:
1. LaunchType, it would be nice to declare the constants outside of
LinuxContainerExecutor. It is possible that code may evolve and
hadoop-yarn-services-core project (AM) needs to know those constants as well.
One suggested location for constant is in hadoop-yarn-api project
ApplicationConstants class is a good place to define constants.
2. docker-util.c, the get_docker_start_command. It would be easier to read
without nesting if conditions, use goto for error handling:
{code}
ret = add_to_buffer(out, outlen, DOCKER_START_COMMAND);
if (ret == 0) {
ret = add_to_buffer(out, outlen, " ");
if (ret == 0) {
ret = add_to_buffer(out, outlen, container_name);
}
free(container_name);
if (ret != 0) {
return BUFFER_TOO_SMALL;
}
return 0;
}
free(container_name);
return BUFFER_TOO_SMALL;
{code}
change to:
{code}
ret = add_to_buffer(out, outlen, DOCKER_START_COMMAND);
if (ret != 0) {
goto free_and_exit;
}
ret = add_to_buffer(out, outlen, " ");
if (ret != 0) {
goto free_and_exit;
}
ret = add_to_buffer(out, outlen, container_name);
if (ret != 0) {
goto free_and_exit;
}
free_and_exit:
free(container_name);
return ret;
{code}
3. Instead of return 0, or check for == 0. It would be nice to define 0 =
SUCCESS or SUCCESS_EXIT_CODE to improve readability for ContainerLaunch class.
> Support ContainerRelaunch for Docker containers
> -----------------------------------------------
>
> Key: YARN-7973
> URL: https://issues.apache.org/jira/browse/YARN-7973
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Shane Kumpf
> Assignee: Shane Kumpf
> Priority: Major
> Attachments: YARN-7973.001.patch, YARN-7973.002.patch,
> YARN-7973.003.patch
>
>
> Prior to YARN-5366, {{container-executor}} would remove the Docker container
> when it exited. The removal is now handled by the
> {{DockerLinuxContainerRuntime}}. {{ContainerRelaunch}} is intended to reuse
> the workdir from the previous attempt, and does not call {{cleanupContainer}}
> prior to {{launchContainer}}. The container ID is reused as well. As a
> result, the previous Docker container still exists, resulting in an error
> from Docker indicating the a container by that name already exists.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
