[
https://issues.apache.org/jira/browse/FLINK-41206?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18134703#comment-18134703
]
Sangkyoon Nam commented on FLINK-41206:
---------------------------------------
I'd like to take this if you aren't already preparing a PR. I checked master
({{ebfb1b0c945}}), and {{pyflink-udf-runner.sh}} still pipes Python through
{{tee}}. I'd integrate your proposed approach onto master and leave the
callback location for review.
I haven't run your reproduction yet. I'd use it to validate the change on
master and check for a TaskManager INFO exit record with the worker ID, Python
PID and {{exitCode=137}}.
Please share the prototype branch or patch if you have one I can build on. If
the approach looks good, could a committer assign this to me?
> PyFlink loses Python worker exit status when the worker process is killed
> -------------------------------------------------------------------------
>
> Key: FLINK-41206
> URL: https://issues.apache.org/jira/browse/FLINK-41206
> Project: Flink
> Issue Type: Improvement
> Components: API / Python
> Reporter: Biao Geng
> Priority: Major
>
> h3. Problem
> When a Python worker is killed while the TaskManager remains running, Flink
> may report only errors such as:
> {code:java}
> Failed to close remote bundle
> CANCELLED: client cancelled
> {code}
> These messages do not clearly tell users that the Python worker process has
> exited. The default TaskManager logs also do not record its exit code. Users
> may therefore investigate a communication problem without realizing that the
> Python process has died.
> h3. Expected behavior
> The default TaskManager log should explicitly record that the Python worker
> has exited, including its worker ID, PID and exit code, without requiring
> DEBUG logging.
> This gives users a clear starting point: identify the worker that exited,
> then use its PID and exit code to investigate the cause. For example, exit
> 137 is consistent with SIGKILL and can help users correlate the failure with
> OS/container OOM records. It does not, by itself, prove an OOM kill.
> h3. Reproduction
> # Start a local standalone session cluster at INFO level. Run a one-row
> scalar-UDF job in process mode; have the UDF write {{os.getpid()}} and worker
> ID to a marker file, then wait in a loop.
> # Once the UDF is entered, send {{kill -9}} to that actual Python PID,
> leaving the launcher/JM/TM alone.
> # Inspect the task failure and TaskManager log.
> The task reports a gRPC/bundle failure, but no explicit worker-exit record
> with the Python PID and exit code. These tests deliberately send SIGKILL;
> they do not induce an OOM.
> h3. Why the exit information is lost
> The TaskManager starts a Bash [launcher
> script|https://github.com/apache/flink/blob/0addadb5caeead0aedaa780443cce89ae6ff0487/flink-python/pyflink/bin/pyflink-udf-runner.sh],
> which then starts Python. Java directly manages the script's process, so the
> exit status it receives is the script's.
> The script uses {{tee}} to save Python's output. By default, it returns
> {{{}tee{}}}'s exit status, so Python can fail while the script reports
> success.
> I tried changing the script to return Python's status. This corrected startup
> failures, but exposed another race during runtime failure: after the Python
> worker dies, Flink starts cleanup and can terminate the script before it
> forwards Python's status. Java then observes the script's termination code,
> 143, instead of Python's SIGKILL code, 137.
> Printing an exit message from the script also does not reliably make it
> visible in default TaskManager logs. The script's output is separate from
> Python's Beam logging channel, and Beam discards that process output unless
> Beam's ProcessManager DEBUG logging is enabled.
> h3. Alternatives tested
> ||Attempt||Result||
> |Preserve {{PIPESTATUS}} and print a shell hint|Startup SIGKILL changed exit
> 0 to 137; missing {{grpc}} changed 0 to 1. Runtime default logs still lacked
> the diagnostic.|
> |Enable ProcessManager DEBUG|Exposed the shell's 137 hint in MiniCluster JVM
> stderr, but requires DEBUG.|
> |Read the boot log once on bundle failure|Added startup identity information,
> but missed the 137 hint in both runtime tests.|
> |Enrich exceptions using a bounded 500 ms probe|Included 137 in an earlier
> MiniCluster prototype; rejected from this change because of added
> coordination/waiting.|
> |Add an INFO callback, retaining Bash|Standalone: Python PID 67309 was
> killed; the callback logged Bash PID 67307 / 143 after cleanup.|
> |{{exec}} Python, retaining the INFO callback|Two standalone kills logged
> actual Python PID / 137. Cancellation logged 143; 1,000 normal rows
> completed; missing {{grpc}} retained its traceback and reported 1.|
> h3. One possible solution
> Replace Bash with Python while retaining output mirroring and the boot log:
> {code:java}
> log="$BOOT_LOG_DIR/flink-python-udf-boot.log"
> exec > >(tee "${log}") 2>&1
> exec "${python}" -m pyflink.fn_execution.beam.beam_boot "$@"
> {code}
> Add one neutral {{Process.onExit()}} INFO record per worker exit:
> {code:java}
> Process for worker 1-1 exited: pid=38483, exitCode=137
> {code}
> This removes the separate shell that can lose Python's status during cleanup.
> No application-level waiting, polling, exit-classification flags or new
> environment factory is proposed. The prototype uses a Flink-local Beam
> ProcessManager copy; the hook's integration location remains a review
> decision.
> The combined prototype was tested on 2.4-SNAPSHOT. Unmodified masterĀ
> {{0addadb5caee}} also lacks the default exit diagnostic; integrating the
> prototype on that master remains outstanding.
> Intentional changes: background {{tee}} makes immediate boot-log snapshots
> best effort; its failures no longer determine Python's exit status; cleanup
> SIGTERM reaches Python directly; every exit adds INFO. A successful job's
> worker cleanup also logged 143, so nonzero exits must not automatically be
> classified as crashes.
> This change adds a log record; it does not guarantee 137 in the final Java
> exception, identify OOM automatically, or fix bundle/data-channel deadlocks.
> Windows handling is outside this first step. Logging also cannot be
> guaranteed if the TaskManager itself is killed.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)