Skip to content

[SPARK-57931][CORE] Restore worker channel blocking mode after pipelined Python UDF execution#56995

Closed
viirya wants to merge 1 commit into
apache:masterfrom
viirya:fix-pipelined-udf-channel-mode
Closed

[SPARK-57931][CORE] Restore worker channel blocking mode after pipelined Python UDF execution#56995
viirya wants to merge 1 commit into
apache:masterfrom
viirya:fix-pipelined-udf-channel-mode

Conversation

@viirya

@viirya viirya commented Jul 3, 2026

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

The pipelined Python UDF path (SPARK-56642) switches a borrowed worker's SocketChannel from non-blocking to blocking mode in createPipelinedDataIn and never restores it. With worker reuse enabled (the default), the worker is returned to the idle pool with its channel still in blocking mode.

This changes PythonWorkerFactory.create() to normalize a reused daemon worker's channel back to non-blocking before calling refresh(), so a pooled worker is always handed to the next task in the same (non-blocking) mode as a freshly created one — restoring the invariant that a daemon worker taken from the pool is non-blocking.

Why are the changes needed?

PythonWorker.refresh() only opens a selector when the channel is non-blocking. A pooled worker left in blocking mode therefore comes back with a null selector / selectionKey. Code on the non-pipelined (single-threaded NIO selector) path dereferences worker.selector / worker.selectionKey, so a worker in this corrupted state would throw a NullPointerException there.

In the current OSS code this does not surface as an end-to-end failure, because the worker-factory cache key (PythonWorkersKey) includes the worker envVars, and the pipelined path adds SPARK_PIPELINED_UDF=1 to envVars before requesting a worker (BasePythonRunner.compute). Pipelined and non-pipelined tasks therefore draw from separate idle pools: a worker left in blocking mode only returns to the pipelined pool, and the next borrower from that pool is again a pipelined task, which unconditionally re-sets the channel to blocking and does not use the selector. So the corrupted state is currently masked by pool isolation.

That masking is fragile. It relies on the two pools staying disjoint via envVars; it does not fix the broken invariant that a pooled daemon worker is non-blocking. Any worker-management layer that pools or reuses workers across that boundary — e.g. reusing a warmed worker regardless of whether the previous task was pipelined — will hand a blocking-mode worker to selector-path code and hit the NullPointerException. Fixing it at the pool boundary (create()) restores the invariant unconditionally and is robust to how workers are pooled.

The fix is applied in create() rather than in the pipelined path's task-completion listener because the worker is released back to the pool from the reader iterator when it reaches END_OF_STREAM, which runs before the task-completion listener; a restore in the listener would therefore run after the worker is already back in the pool. Normalizing at the single pool exit point (create()) is correct regardless of that ordering.

Does this PR introduce any user-facing change?

No. This hardens an invariant in an opt-in feature (spark.python.udf.pipelined.enabled) that has not been released yet; the pool-isolation behavior above means it is not an observable failure in OSS today.

How was this patch tested?

New unit test in PythonWorkerFactorySuite that constructs a PythonWorker over a loopback channel, puts it in the blocking state the pipelined path leaves behind (where refresh() opens no selector), and asserts refreshNonBlocking() — the method create() uses when reusing a pooled worker — normalizes the channel back to non-blocking and re-opens the selector. The test uses a mock channel rather than a real daemon worker so it runs in the core module's test environment (which has no pyspark on PYTHONPATH).

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Code

…ned Python UDF execution

### What changes were proposed in this pull request?

The pipelined Python UDF path (SPARK-56642) switches a borrowed worker's
SocketChannel from non-blocking to blocking mode in `createPipelinedDataIn`
and never restores it. With worker reuse enabled (the default), the worker is
returned to the idle pool with its channel still in blocking mode.

`PythonWorkerFactory.create()` normalizes a reused daemon worker's channel back
to non-blocking before calling `refresh()`, so the worker is always handed to
the next task in the same (non-blocking) mode as a freshly created one.

### Why are the changes needed?

`PythonWorker.refresh()` only opens a selector when the channel is non-blocking.
A pooled worker left in blocking mode therefore comes back with a null selector
and selection key. If the next task that reuses it runs on the non-pipelined
(single-threaded NIO selector) path, it dereferences `worker.selector` /
`worker.selectionKey` and throws a NullPointerException. This is cross-task
state pollution: one task using pipelined mode corrupts the channel state for
any later task that reuses the same pooled worker.

### Does this PR introduce _any_ user-facing change?

No. This fixes a bug in an opt-in feature (`spark.python.udf.pipelined.enabled`)
that has not been released yet.

### How was this patch tested?

New test in `PythonWorkerFactorySuite` that creates a daemon worker, simulates
the blocking channel state the pipelined path leaves behind, returns it to the
idle pool, and asserts the reused worker comes back in non-blocking mode with a
live selector.

### Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Code

Co-authored-by: Claude Code
@viirya viirya force-pushed the fix-pipelined-udf-channel-mode branch from f44cd3e to 79a38df Compare July 4, 2026 01:40

@uros-b uros-b left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @HyukjinKwon for PythonWorkerFactory and PythonRunner

HyukjinKwon added a commit that referenced this pull request Jul 6, 2026
…lined Python UDF data transfer

### What changes were proposed in this pull request?

This makes the pipelined Python UDF read path work under Unix Domain Socket (UDS) mode.

`BasePythonRunner.createPipelinedDataIn` (added in SPARK-56642 for pipelined JVM↔Python UDF
data transfer) sets up the read side via `worker.channel.socket().setSoTimeout(...)` and
`worker.channel.socket().getInputStream`. When Spark runs with Unix Domain Sockets
(`spark.python.unix.domain.socket.enabled=true`), `worker.channel` is an `AF_UNIX`
`SocketChannel`, whose `socket()` throws `java.lang.UnsupportedOperationException: Not supported`
(a Unix-domain channel has no `java.net.Socket` adapter). Every pipelined UDF task then fails:

```
java.lang.UnsupportedOperationException: Not supported
  at java.base/sun.nio.ch.SocketChannelImpl.socket(SocketChannelImpl.java:228)
  at org.apache.spark.api.python.BasePythonRunner.createPipelinedDataIn(PythonRunner.scala:447)
```

This adds a UDS branch that reads straight from the channel via
`Channels.newInputStream(worker.channel)` (with no `SO_TIMEOUT`-based idle-timeout detection, since
Unix-domain channels do not support `SO_TIMEOUT`). This mirrors how the existing sync-mode server
loop already guards its `setSoTimeout` calls behind `!isUnixDomainSock` and reads via
`Channels.newInputStream`. The default TCP path is unchanged (the new code only runs when UDS is
enabled).

### Why are the changes needed?

`build_uds.yml` (the "Build / Unix Domain Socket" scheduled workflow, one of the README CI badges)
is red: `pyspark.sql.tests.pandas.test_pipelined_udf` fails with `FAILED (failures=1, errors=12)`
because pipelined UDFs cannot run at all under UDS mode.

Note: this is distinct from #56995 (SPARK-57931), which restores the channel's blocking mode after
pipelined execution — a different bug in the same method. This PR fixes the UDS `socket()` crash on
the read path.

### Does this PR introduce _any_ user-facing change?

No. It makes an existing feature (pipelined Python UDF) work under UDS mode; behavior in the default
TCP mode is unchanged.

### How was this patch tested?

`pyspark.sql.tests.pandas.test_pipelined_udf` under UDS mode.

**Before (red — apache/spark `master`):**
- `build_uds` → `pyspark-sql` FAILED (`test_pipelined_udf`: `FAILED (failures=1, errors=12)`):
  https://github.com/apache/spark/actions/runs/28488010762

**After (green — with this change, fork verification):**
- `build_uds` (UDS mode) → `pyspark-sql` module PASSED:
  https://github.com/HyukjinKwon/spark/actions/runs/28758332630

### Was this patch authored or co-authored using generative AI tooling?

Yes.

Co-authored-by: Isaac

Closes #57024 from HyukjinKwon/ci-fix/agent2-uds-pipelined-udf.

Authored-by: Hyukjin Kwon <gurwls223@apache.org>
Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com>
HyukjinKwon added a commit that referenced this pull request Jul 6, 2026
…lined Python UDF data transfer

### What changes were proposed in this pull request?

This makes the pipelined Python UDF read path work under Unix Domain Socket (UDS) mode.

`BasePythonRunner.createPipelinedDataIn` (added in SPARK-56642 for pipelined JVM↔Python UDF
data transfer) sets up the read side via `worker.channel.socket().setSoTimeout(...)` and
`worker.channel.socket().getInputStream`. When Spark runs with Unix Domain Sockets
(`spark.python.unix.domain.socket.enabled=true`), `worker.channel` is an `AF_UNIX`
`SocketChannel`, whose `socket()` throws `java.lang.UnsupportedOperationException: Not supported`
(a Unix-domain channel has no `java.net.Socket` adapter). Every pipelined UDF task then fails:

```
java.lang.UnsupportedOperationException: Not supported
  at java.base/sun.nio.ch.SocketChannelImpl.socket(SocketChannelImpl.java:228)
  at org.apache.spark.api.python.BasePythonRunner.createPipelinedDataIn(PythonRunner.scala:447)
```

This adds a UDS branch that reads straight from the channel via
`Channels.newInputStream(worker.channel)` (with no `SO_TIMEOUT`-based idle-timeout detection, since
Unix-domain channels do not support `SO_TIMEOUT`). This mirrors how the existing sync-mode server
loop already guards its `setSoTimeout` calls behind `!isUnixDomainSock` and reads via
`Channels.newInputStream`. The default TCP path is unchanged (the new code only runs when UDS is
enabled).

### Why are the changes needed?

`build_uds.yml` (the "Build / Unix Domain Socket" scheduled workflow, one of the README CI badges)
is red: `pyspark.sql.tests.pandas.test_pipelined_udf` fails with `FAILED (failures=1, errors=12)`
because pipelined UDFs cannot run at all under UDS mode.

Note: this is distinct from #56995 (SPARK-57931), which restores the channel's blocking mode after
pipelined execution — a different bug in the same method. This PR fixes the UDS `socket()` crash on
the read path.

### Does this PR introduce _any_ user-facing change?

No. It makes an existing feature (pipelined Python UDF) work under UDS mode; behavior in the default
TCP mode is unchanged.

### How was this patch tested?

`pyspark.sql.tests.pandas.test_pipelined_udf` under UDS mode.

**Before (red — apache/spark `master`):**
- `build_uds` → `pyspark-sql` FAILED (`test_pipelined_udf`: `FAILED (failures=1, errors=12)`):
  https://github.com/apache/spark/actions/runs/28488010762

**After (green — with this change, fork verification):**
- `build_uds` (UDS mode) → `pyspark-sql` module PASSED:
  https://github.com/HyukjinKwon/spark/actions/runs/28758332630

### Was this patch authored or co-authored using generative AI tooling?

Yes.

Co-authored-by: Isaac

Closes #57024 from HyukjinKwon/ci-fix/agent2-uds-pipelined-udf.

Authored-by: Hyukjin Kwon <gurwls223@apache.org>
Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com>
(cherry picked from commit 17b11e3)
Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com>
@viirya viirya closed this in 20d8cce Jul 6, 2026
viirya added a commit that referenced this pull request Jul 6, 2026
…ned Python UDF execution

### What changes were proposed in this pull request?

The pipelined Python UDF path (SPARK-56642) switches a borrowed worker's `SocketChannel` from non-blocking to blocking mode in `createPipelinedDataIn` and never restores it. With worker reuse enabled (the default), the worker is returned to the idle pool with its channel still in blocking mode.

This changes `PythonWorkerFactory.create()` to normalize a reused daemon worker's channel back to non-blocking before calling `refresh()`, so a pooled worker is always handed to the next task in the same (non-blocking) mode as a freshly created one — restoring the invariant that a daemon worker taken from the pool is non-blocking.

### Why are the changes needed?

`PythonWorker.refresh()` only opens a selector when the channel is non-blocking. A pooled worker left in blocking mode therefore comes back with a null `selector` / `selectionKey`. Code on the non-pipelined (single-threaded NIO selector) path dereferences `worker.selector` / `worker.selectionKey`, so a worker in this corrupted state would throw a `NullPointerException` there.

In the current OSS code this does not surface as an end-to-end failure, because the worker-factory cache key (`PythonWorkersKey`) includes the worker `envVars`, and the pipelined path adds `SPARK_PIPELINED_UDF=1` to `envVars` before requesting a worker (`BasePythonRunner.compute`). Pipelined and non-pipelined tasks therefore draw from separate idle pools: a worker left in blocking mode only returns to the pipelined pool, and the next borrower from that pool is again a pipelined task, which unconditionally re-sets the channel to blocking and does not use the selector. So the corrupted state is currently masked by pool isolation.

That masking is fragile. It relies on the two pools staying disjoint via `envVars`; it does not fix the broken invariant that a pooled daemon worker is non-blocking. Any worker-management layer that pools or reuses workers across that boundary — e.g. reusing a warmed worker regardless of whether the previous task was pipelined — will hand a blocking-mode worker to selector-path code and hit the `NullPointerException`. Fixing it at the pool boundary (`create()`) restores the invariant unconditionally and is robust to how workers are pooled.

The fix is applied in `create()` rather than in the pipelined path's task-completion listener because the worker is released back to the pool from the reader iterator when it reaches `END_OF_STREAM`, which runs *before* the task-completion listener; a restore in the listener would therefore run after the worker is already back in the pool. Normalizing at the single pool exit point (`create()`) is correct regardless of that ordering.

### Does this PR introduce _any_ user-facing change?

No. This hardens an invariant in an opt-in feature (`spark.python.udf.pipelined.enabled`) that has not been released yet; the pool-isolation behavior above means it is not an observable failure in OSS today.

### How was this patch tested?

New unit test in `PythonWorkerFactorySuite` that constructs a `PythonWorker` over a loopback channel, puts it in the blocking state the pipelined path leaves behind (where `refresh()` opens no selector), and asserts `refreshNonBlocking()` — the method `create()` uses when reusing a pooled worker — normalizes the channel back to non-blocking and re-opens the selector. The test uses a mock channel rather than a real daemon worker so it runs in the core module's test environment (which has no `pyspark` on PYTHONPATH).

### Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Code

Closes #56995 from viirya/fix-pipelined-udf-channel-mode.

Authored-by: Liang-Chi Hsieh <viirya@gmail.com>
Signed-off-by: Liang-Chi Hsieh <viirya@gmail.com>
(cherry picked from commit 20d8cce)
Signed-off-by: Liang-Chi Hsieh <viirya@gmail.com>
@viirya

viirya commented Jul 6, 2026

Copy link
Copy Markdown
Member Author

Merge Summary:

Posted by merge_spark_pr.py

@viirya viirya deleted the fix-pipelined-udf-channel-mode branch July 6, 2026 07:01
@viirya

viirya commented Jul 6, 2026

Copy link
Copy Markdown
Member Author

Thanks @uros-b @HyukjinKwon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants