fapaul opened a new pull request, #57247:
URL: https://github.com/apache/spark/pull/57247

   ### What changes were proposed in this pull request?
   
   Introduce a Spark-owned `PythonWorkerHandle` trait to replace direct use of 
`java.lang.ProcessHandle` in the Python worker path. The runner then depends 
only on the small set of operations it actually needs — `isAlive()`, 
`destroy()`, `terminationDiagnostics(faultHandlerEnabled)`.
   
   - New `PythonWorkerHandle` trait + `LocalPythonWorkerHandle` (wraps 
`java.lang.ProcessHandle`) and a `PythonWorkerHandle.of(pid, faultHandlerLog)` 
factory (`core/.../api/python/PythonWorkerHandle.scala`).
   - `SparkEnv.createPythonWorker`, `PythonWorkerFactory`, `PythonRunner`, 
`PythonUDFRunner`, `PythonArrowOutput`, `PythonPlannerRunner` rewired from 
`Option[ProcessHandle]` to `Option[PythonWorkerHandle]`.
   - Post-mortem diagnostics flow through 
`handle.terminationDiagnostics(faultHandlerEnabled)`: the runner supplies the 
(SQLConf-resolved) faulthandler flag, and the handle locates its own worker's 
faulthandler log via an injected closure. The reader no longer needs to know 
about faulthandler log files.
   - The OS pid is deliberately **not** part of the handle contract. Its one 
consumer — the daemon kill-by-pid protocol — is served from a factory-internal 
`daemonWorkers` map (a `DaemonWorker(handle, pid)` record), keeping the pid a 
local-process detail.
   - Adds `PythonWorkerHandleSuite`.
   
   Behavior-preserving: `LocalPythonWorkerHandle` delegates 1:1 to the JDK 
handle and the existing faulthandler read.
   
   ### Why are the changes needed?
   
   `java.lang.ProcessHandle` is a sealed JDK type hardcoded across the Python 
worker path, and the faulthandler-log reading is duplicated inline across the 
runners. Introducing a small Spark-owned interface decouples the runner from 
the concrete JDK type and centralizes "what the runner needs from a worker 
handle" — including the diagnostics read — into a single, testable place.
   
   ### Does this PR introduce _any_ user-facing change?
   
   No. Internal refactor; no config, API, or behavior change.
   
   ### How was this patch tested?
   
   - New `PythonWorkerHandleSuite` (5 tests): the injected faulthandler-log 
reader is invoked only when enabled and its result returned verbatim; the 
default reader yields `None`; `of` binds the pid into the reader; 
`isAlive`/`destroy` delegate to the underlying process (spawn a subprocess, 
assert alive → destroy → reaped → not alive); and `of` returns `None` for a pid 
with no live process.
   - `core` (main + test) and `sql/core` compile cleanly (Scala 2.13).
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   Generated-by: Claude (Anthropic)
   


-- 
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]

Reply via email to