Copilot commented on code in PR #8474:
URL: https://github.com/apache/hadoop/pull/8474#discussion_r3212206555
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c:
##########
@@ -653,6 +653,16 @@ static int validate_run_as_user_commands(int argc, char
**argv, int *operation)
}
}
+static int wrap_exit_code(int exit_code) {
+ if (exit_code == INVALID_CONTAINER_EXEC_PERMISSIONS || exit_code ==
INVALID_CONFIG_FILE) {
+ int wrap_code = WRAPPED_EXIT_CODE_USER_CONTAINER_FAILED;
+ fprintf(LOGFILE, "Wrapped exit code of user container from %d to %d to
avoid NodeManager unhealthy...\n", exit_code, wrap_code);
Review Comment:
The log message "to avoid NodeManager unhealthy..." is grammatically unclear
and doesn’t explain what condition is being avoided. Consider rewording to
something like "to avoid NodeManager becoming unhealthy" and optionally mention
it’s avoiding the ConfigurationException path in LinuxContainerExecutor for
exit codes 22/24.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/util.h:
##########
@@ -109,7 +109,8 @@ enum errorcodes {
TOO_LONG_EXECUTOR_PATH = 81,
CANNOT_GET_EXECUTABLE_NAME_FROM_KERNEL = 82,
CANNOT_GET_EXECUTABLE_NAME_FROM_PID = 83,
- WRONG_PATH_OF_EXECUTABLE = 84
+ WRONG_PATH_OF_EXECUTABLE = 84,
+ WRAPPED_EXIT_CODE_USER_CONTAINER_FAILED = 85
};
Review Comment:
New exit code 85 is added to util.h, but get_error_message() in util.c
doesn’t have a case for it, so container-executor will log "Unknown error code"
for this wrapped value. Also, util.h error codes appear to be mirrored by
LinuxContainerExecutor.ExitCode in Java (up to 84), so consider updating those
mappings to keep enums/messages consistent for diagnostics.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c:
##########
@@ -653,6 +653,16 @@ static int validate_run_as_user_commands(int argc, char
**argv, int *operation)
}
}
+static int wrap_exit_code(int exit_code) {
+ if (exit_code == INVALID_CONTAINER_EXEC_PERMISSIONS || exit_code ==
INVALID_CONFIG_FILE) {
+ int wrap_code = WRAPPED_EXIT_CODE_USER_CONTAINER_FAILED;
+ fprintf(LOGFILE, "Wrapped exit code of user container from %d to %d to
avoid NodeManager unhealthy...\n", exit_code, wrap_code);
+ return wrap_code;
+ } else {
+ return exit_code;
+ }
Review Comment:
wrap_exit_code() changes user container exit codes 22 and 24 to a single
wrapped value (85). This masks the original container exit code in YARN
diagnostics and makes 22 vs 24 indistinguishable; consider preserving the
original information (e.g., distinct wrapped codes per original value, or an
offset scheme) and/or surfacing the original exit code in diagnostics so users
can still debug failures accurately.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
