[SPARK-58241][PYTHON] Fix the columnar Arrow Python UDF runner writing the input schema in the legacy protocol position#57403
Conversation
…g the input schema in the legacy protocol position
The message-based worker protocol moved the SQL_ARROW_BATCHED_UDF input
schema from the UDF command section into evalConf ("input_type"), but
ColumnarArrowPythonWithNamedArgumentRunner still wrote schema.json at
the old position. WorkerInitInfo.from_stream reads that section as a
UDF count followed by UDF entries, so the schema string desynced the
entire init-message parse; the worker eventually tried to read a
garbage length past the end of the init message and, because the init
ZeroCopyByteStream was never marked finished, blocked forever instead
of failing -- hanging any Python UDF over a columnar Arrow input (first
reachable in-tree via the Arrow-cached scan, SPARK-57268).
Align the columnar runner with ArrowPythonWithNamedArgumentRunner
(schema via evalConf, writeUDF writes only the UDF list), and mark the
fully-materialized init message stream finished in
SparkSocketMessageReceiver so any future protocol mismatch fails fast
with EOFError instead of deadlocking.
Co-authored-by: Claude Code
Ruff flags the legacy self-import-star main block (F403); use pyspark.testing.main like the sibling test modules. Co-authored-by: Claude Code
dongjoon-hyun
left a comment
There was a problem hiding this comment.
+1, LGTM. Thank you, @viirya .
Co-authored-by: Claude Code
uros-b
left a comment
There was a problem hiding this comment.
Thank you @viirya and @dongjoon-hyun!
Yicong-Huang
left a comment
There was a problem hiding this comment.
I think this is a legit fix and thanks for catching that! Left one nit inline.
| df.unpersist() | ||
|
|
||
| def test_udf_on_cached_numeric_column_alongside_unselected_string(self): | ||
| # Only the UDF's input columns are serialized; unselected columns ride through the |
There was a problem hiding this comment.
The comment says unselected columns "ride through the pass-through recombination," but the assertion only checks the UDF output ([Row(2), Row(3)]) -- the unselected s column is projected away by the select and never observed. Not vacuous (the pass-through row-count assert would still fire on a size mismatch), but to actually observe the pass-through the comment names, consider also selecting s and asserting its value.
There was a problem hiding this comment.
Good point -- the comment claimed more than the assertion observed. The test now selects s alongside the UDF result and asserts [Row("a", 2), Row("b", 3)], so the pass-through values (and their row alignment with the UDF output) are observed directly rather than implied by the row-count check.
…s in the test Select the non-UDF-input column alongside the UDF result and assert its values, so the pass-through recombination the comment describes is observed directly instead of implied by the row-count check. Co-authored-by: Claude Code
…g the input schema in the legacy protocol position ### What changes were proposed in this pull request? Two fixes to the Python worker init-message protocol: 1. **Protocol fix**: `ColumnarArrowPythonWithNamedArgumentRunner` (the columnar-input Arrow Python UDF runner) still wrote `schema.json` as a UTF string at the head of the UDF command section for `SQL_ARROW_BATCHED_UDF`. The message-based worker protocol moved that schema into `evalConf` under the `"input_type"` key -- which the row-based `ArrowPythonWithNamedArgumentRunner` already does -- and the worker's `WorkerInitInfo.from_stream` therefore parses the command section as a UDF count followed by UDF entries. The stray schema string desyncs the entire parse: its length prefix is misread as the UDF count, subsequent bytes are misread as field lengths, and the worker ends up trying to read a garbage-sized value past the end of the init message. This PR aligns the columnar runner with the row-based one: schema via `evalConf`, `writeUDF` writes only the UDF list. 2. **Fail-fast hardening**: the worker blocked *forever* instead of failing because `SparkSocketMessageReceiver._do_get_init_message` materializes the entire init message but never marked the resulting `ZeroCopyByteStream` finished, so an over-read waits on a condition for a chunk that can never arrive (the worker is single-threaded at that point). The receiver now calls `finish()` on the fully-materialized stream, so any future writer/reader protocol mismatch surfaces as an immediate `EOFError` naming the short read instead of a silent, undiagnosable hang. ### Why are the changes needed? Any Python UDF whose input plan produces columnar Arrow batches deadlocks: the JVM task thread waits for worker output, the worker waits for init-message bytes that were never sent, forever. This was latent since the columnar-input runner was added because no in-tree source produced Arrow-backed `ColumnarBatch` input for it; the Arrow-cached scan (SPARK-57268) is the first, so a simple `df.cache()` (with the Arrow cache serializer) followed by any Arrow-optimized Python UDF now hangs with default configurations. Note `ArrowEvalPythonExec.doExecute` routes through the columnar evaluator whenever `child.supportsColumnar`, regardless of `spark.sql.execution.arrow.pythonUDF.columnarInput.enabled`, so the config is not a workaround. Diagnosed by instrumenting `WorkerInitInfo.from_stream`: the parse is byte-exact through the conf sections and desyncs precisely at the UDF command section, matching the extra `schema.json` write; the worker's `faulthandler` stack shows it blocked in `ZeroCopyByteStream._try_read_bytes` under `UDFInfo.from_stream`. ### Does this PR introduce _any_ user-facing change? No. It fixes a hang; the wire format for the columnar runner now matches what the worker has expected all along. ### How was this patch tested? - New e2e suite `pyspark.sql.tests.arrow.test_arrow_python_udf_cached` (own module because `spark.sql.cache.serializer` is a static conf latched into a JVM-wide singleton): Arrow-optimized Python UDFs over an Arrow-cached relation -- the exact deadlock scenario -- including an unselected pass-through column. Hung forever before the fix; passes in seconds with it. - New unit tests in `pyspark.tests.test_spark_message_receiver` for the socket receiver: declared content reads back, and an over-read past the declared init-message length raises `EOFError` instead of blocking. - Existing `pyspark.tests.test_zero_copy_byte_stream`, `pyspark.tests.test_spark_message_receiver`, and `pyspark.sql.tests.arrow.test_arrow_python_udf` all pass. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code This pull request and its description were written by Claude Code. Closes #57403 from viirya/arrow-columnar-udf-init-protocol. Authored-by: Liang-Chi Hsieh <viirya@gmail.com> Signed-off-by: Liang-Chi Hsieh <viirya@gmail.com> (cherry picked from commit e6d66ab) Signed-off-by: Liang-Chi Hsieh <viirya@gmail.com>
|
Thanks @dongjoon-hyun @uros-b @Yicong-Huang |
|
I created a backport of this to branch-4.2 #57419. |
…riting the input schema in the legacy protocol position ### What changes were proposed in this pull request? Backport of the Scala fix from #57403 (master commit e6d66ab) to branch-4.2. `ColumnarArrowPythonWithNamedArgumentRunner` still wrote `schema.json` as a UTF string at the head of the UDF command section for `SQL_ARROW_BATCHED_UDF`. SPARK-56340 -- which is in branch-4.2 -- moved that schema into `evalConf` under the `"input_type"` key (branch-4.2's worker reads it from there, and branch-4.2's row-based `ArrowPythonWithNamedArgumentRunner` already writes it there), so the worker parses the command section as a UDF count followed by UDF entries. The stray schema string desyncs the parse and the worker blocks waiting for bytes that never arrive, hanging the task forever. This aligns the columnar runner with the row-based one: schema via `evalConf`, `writeUDF` writes only the UDF list. Scope differences from the master PR, both verified against branch-4.2: - The Python-side fail-fast hardening is not applicable: the message-receiver files (`pyspark/messages/`, `worker_message.py`) do not exist on branch-4.2 (the message-based protocol refactor is 4.3+). - The end-to-end test is not portable: it drives the columnar runner through the Arrow-cached scan (SPARK-57268), which is 4.3+; branch-4.2 has no in-tree producer of Arrow-backed columnar input, so the bug there is reachable only via Arrow-backed DSv2 connectors. ### Why are the changes needed? Both origins of the bug are in branch-4.2: SPARK-56340 (protocol migration, reader side) and SPARK-56350 (the columnar runner with the legacy write). Any Python UDF whose input plan produces Arrow-backed `ColumnarBatch` (e.g. an Arrow-backed DSv2 connector) deadlocks on 4.2 exactly as on master. ### Does this PR introduce _any_ user-facing change? No. It fixes a hang; the wire format now matches what the branch-4.2 worker expects. ### How was this patch tested? The fix mirrors the `evalConf` pattern already shipping in branch-4.2's row-based runner. Compile and scalastyle pass on branch-4.2, and `pyspark.sql.tests.arrow.test_arrow_python_udf` passes against the patched build. The scenario-level end-to-end coverage lives in the master PR (#57403), where the Arrow-cached scan provides an in-tree columnar Arrow input; branch-4.2 has no in-tree way to construct that input. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code This pull request and its description were written by Claude Code. Closes #57419 from viirya/SPARK-58241-branch-4.2. Authored-by: Liang-Chi Hsieh <viirya@gmail.com> Signed-off-by: Liang-Chi Hsieh <viirya@gmail.com>
What changes were proposed in this pull request?
Two fixes to the Python worker init-message protocol:
Protocol fix:
ColumnarArrowPythonWithNamedArgumentRunner(the columnar-input Arrow Python UDF runner) still wroteschema.jsonas a UTF string at the head of the UDF command section forSQL_ARROW_BATCHED_UDF. The message-based worker protocol moved that schema intoevalConfunder the"input_type"key -- which the row-basedArrowPythonWithNamedArgumentRunneralready does -- and the worker'sWorkerInitInfo.from_streamtherefore parses the command section as a UDF count followed by UDF entries. The stray schema string desyncs the entire parse: its length prefix is misread as the UDF count, subsequent bytes are misread as field lengths, and the worker ends up trying to read a garbage-sized value past the end of the init message. This PR aligns the columnar runner with the row-based one: schema viaevalConf,writeUDFwrites only the UDF list.Fail-fast hardening: the worker blocked forever instead of failing because
SparkSocketMessageReceiver._do_get_init_messagematerializes the entire init message but never marked the resultingZeroCopyByteStreamfinished, so an over-read waits on a condition for a chunk that can never arrive (the worker is single-threaded at that point). The receiver now callsfinish()on the fully-materialized stream, so any future writer/reader protocol mismatch surfaces as an immediateEOFErrornaming the short read instead of a silent, undiagnosable hang.Why are the changes needed?
Any Python UDF whose input plan produces columnar Arrow batches deadlocks: the JVM task thread waits for worker output, the worker waits for init-message bytes that were never sent, forever. This was latent since the columnar-input runner was added because no in-tree source produced Arrow-backed
ColumnarBatchinput for it; the Arrow-cached scan (SPARK-57268) is the first, so a simpledf.cache()(with the Arrow cache serializer) followed by any Arrow-optimized Python UDF now hangs with default configurations. NoteArrowEvalPythonExec.doExecuteroutes through the columnar evaluator wheneverchild.supportsColumnar, regardless ofspark.sql.execution.arrow.pythonUDF.columnarInput.enabled, so the config is not a workaround.Diagnosed by instrumenting
WorkerInitInfo.from_stream: the parse is byte-exact through the conf sections and desyncs precisely at the UDF command section, matching the extraschema.jsonwrite; the worker'sfaulthandlerstack shows it blocked inZeroCopyByteStream._try_read_bytesunderUDFInfo.from_stream.Does this PR introduce any user-facing change?
No. It fixes a hang; the wire format for the columnar runner now matches what the worker has expected all along.
How was this patch tested?
pyspark.sql.tests.arrow.test_arrow_python_udf_cached(own module becausespark.sql.cache.serializeris a static conf latched into a JVM-wide singleton): Arrow-optimized Python UDFs over an Arrow-cached relation -- the exact deadlock scenario -- including an unselected pass-through column. Hung forever before the fix; passes in seconds with it.pyspark.tests.test_spark_message_receiverfor the socket receiver: declared content reads back, and an over-read past the declared init-message length raisesEOFErrorinstead of blocking.pyspark.tests.test_zero_copy_byte_stream,pyspark.tests.test_spark_message_receiver, andpyspark.sql.tests.arrow.test_arrow_python_udfall pass.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code
This pull request and its description were written by Claude Code.