[SPARK-58021][CONNECT] Add local pool server retirement by ericm-db · Pull Request #57686 · apache/spark

ericm-db · 2026-07-31T18:22:09Z

What changes were proposed in this pull request?

This is layer 4 of the nine-PR local Connect pool stack:

#57684 -> #57685 -> #57907 -> #57686 -> #58247 -> #57687 -> #58248 -> #57102 -> #57688

The three lower layers are merged, so GitHub shows only this layer's two-file diff. The review unit
introduced here is commit 37dbe4b5ae7.

This layer adds the normal server-retirement path on top of member claiming:

atomic JSON state replacement and cleanup of interrupted-write temporary files;
validated retired-state records with recovery of independently valid process IDs;
crash-safe state-to-retired renames;
graceful SIGTERM shutdown followed by bounded SIGKILL escalation; and
idempotent claimed-member release, including forked-child and retry handling.

Janitor recovery, acquisition, forceful purge, SparkSession integration, and warmup remain in later
PRs.

Why are the changes needed?

A claimed server must be released without losing the only process handle if the client crashes
during the state transition. Isolating retirement keeps atomic persistence, graceful shutdown, and
release semantics independently reviewable before orphan scanning and launch orchestration are
added.

Does this PR introduce any user-facing change?

No. The pool is not wired into SparkSession in this layer.

How was this patch tested?

Added eight focused tests at this layer, bringing the suite to 33 tests, and extended the storage
test with interrupted-write and stale-temporary-file coverage. The focused command is:

python/run-tests --testnames pyspark.sql.tests.connect.test_connect_local_server_pool

At the stack tip, the equivalent direct unittest invocation passed all 56 pool tests, including
the two real-server E2E tests.

The rebuilt commit passed Python AST parsing, git diff --check, and changed-file ASCII and
100-column checks.

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

Generated-by: Claude Code (Fable 5) and OpenAI Codex (GPT-5)

HyukjinKwon

0 blocking, 0 non-blocking, 0 nits.
Robust, carefully-designed crash-safety and lifecycle management; no issues found.

Verification

Verified the reaping runs under the exclusive flock and is idempotent; _reap_claimed never reaps the current process's own live claim (guards client_pid == os.getpid()) and only retires when the client or server pid is dead; _retire sends SIGTERM and records the retiring state so _reap_retired can escalate to a hard kill for a hung JVM and eventually give up on an un-signalable survivor. The PID-reuse exposure is the same class the existing single-server Discovery already carries (short-lived same-user daemons, uid/fingerprint-bounded). The 438-line test suite drives these paths with real SIGKILL-able and SIGTERM-stubborn subprocesses. Build/test: python/run-tests --testnames 'pyspark.sql.tests.connect.test_connect_local_server_pool'.

### What changes were proposed in this pull request? This is layer 1 of the six-PR local Connect pool stack: #57684 -> #57685 -> #57686 -> #57687 -> #57102 -> #57688 This patch extracts the reusable pieces of local Spark Connect server startup from the persistent-server reuse path: - expose the per-user runtime directory and startup seed configuration as module helpers; - add `LocalConnectServer.start()` as the common lifecycle-owned launch entry point; and - let the launcher use an ephemeral port and a precomputed startup configuration when requested. The existing reuse path now calls the same `LocalConnectServer.start()` method. Focused tests cover seed configuration and delegation of the new launch options. ### Why are the changes needed? The persistent reuse mode currently combines reusable local-server lifecycle handling with assumptions specific to its one fixed daemon. The single-use server pool needs the same secure configuration seeding, process launch, discovery, and readiness handling, but with independent runtime directories and ephemeral ports. Sharing one launch path keeps those behaviors consistent and isolates the larger feature from the already-working reuse implementation. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Added focused unit coverage to `pyspark.sql.tests.connect.test_connect_local_server.LocalConnectServerReuseTests` and ran the full suite, including real daemon startup, reuse, session isolation, and static configuration seeding: ```bash python -m unittest -v pyspark.sql.tests.connect.test_connect_local_server ``` All 15 tests passed. Ruff check, Ruff format check, and `git diff --check` passed. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Fable 5) and OpenAI Codex (GPT-5) Closes #57684 from ericm-db/local-connect-pool-refactor. Authored-by: Eric Marnadi <eric.marnadi@databricks.com> Signed-off-by: Daniel Tenedorio <daniel.tenedorio@databricks.com>

### What changes were proposed in this pull request? This is layer 1 of the six-PR local Connect pool stack: #57684 -> #57685 -> #57686 -> #57687 -> #57102 -> #57688 This patch extracts the reusable pieces of local Spark Connect server startup from the persistent-server reuse path: - expose the per-user runtime directory and startup seed configuration as module helpers; - add `LocalConnectServer.start()` as the common lifecycle-owned launch entry point; and - let the launcher use an ephemeral port and a precomputed startup configuration when requested. The existing reuse path now calls the same `LocalConnectServer.start()` method. Focused tests cover seed configuration and delegation of the new launch options. ### Why are the changes needed? The persistent reuse mode currently combines reusable local-server lifecycle handling with assumptions specific to its one fixed daemon. The single-use server pool needs the same secure configuration seeding, process launch, discovery, and readiness handling, but with independent runtime directories and ephemeral ports. Sharing one launch path keeps those behaviors consistent and isolates the larger feature from the already-working reuse implementation. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Added focused unit coverage to `pyspark.sql.tests.connect.test_connect_local_server.LocalConnectServerReuseTests` and ran the full suite, including real daemon startup, reuse, session isolation, and static configuration seeding: ```bash python -m unittest -v pyspark.sql.tests.connect.test_connect_local_server ``` All 15 tests passed. Ruff check, Ruff format check, and `git diff --check` passed. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Fable 5) and OpenAI Codex (GPT-5) Closes #57684 from ericm-db/local-connect-pool-refactor. Authored-by: Eric Marnadi <eric.marnadi@databricks.com> Signed-off-by: Daniel Tenedorio <daniel.tenedorio@databricks.com> (cherry picked from commit 343fbc3) Signed-off-by: Daniel Tenedorio <daniel.tenedorio@databricks.com>

### What changes were proposed in this pull request? This is layer 2 of the seven-PR local Connect pool stack: #57684 -> #57685 -> #57907 -> #57686 -> #57687 -> #57102 -> #57688 The review unit introduced here is commit `23f806b64ad`. This layer adds the filesystem-backed storage foundation for pool members: - stable state-file paths keyed by member ID and per-member directories; - an overridable private pool directory under the per-user runtime directory; - a per-pool cross-process POSIX file lock; - private directory, lock-file, and JSON state-file permissions; and - locked helpers for listing, reading, writing, renaming, and removing member state. Member validation, compatibility fingerprints, and atomic claiming are isolated in #57907. Process lifecycle, acquisition, SparkSession integration, and JIT warmup remain in later PRs. ### Why are the changes needed? The pool needs a small, independently reviewable state model before adding compatibility checks, claiming, and process supervision. Keeping this layer limited to path layout, locking, and state file access makes its filesystem and concurrency contract reviewable on its own. ### Does this PR introduce _any_ user-facing change? No. The storage model is internal and is not wired into SparkSession in this layer. ### How was this patch tested? Added three focused tests covering directory selection, private permissions and malformed JSON, and cross-process lock contention. ```bash python/run-tests --testnames pyspark.sql.tests.connect.test_connect_local_server_pool ``` These cases passed on Python 3.11 as part of the combined suite before the stack was split. The rebuilt commit passed `git diff --check`, Python AST parsing, and changed-line ASCII and 100-column checks. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Fable 5) and OpenAI Codex (GPT-5) Closes #57685 from ericm-db/local-connect-pool-storage. Authored-by: Eric Marnadi <eric.marnadi@databricks.com> Signed-off-by: Daniel Tenedorio <daniel.tenedorio@databricks.com>

### What changes were proposed in this pull request? This is layer 2 of the seven-PR local Connect pool stack: #57684 -> #57685 -> #57907 -> #57686 -> #57687 -> #57102 -> #57688 The review unit introduced here is commit `23f806b64ad`. This layer adds the filesystem-backed storage foundation for pool members: - stable state-file paths keyed by member ID and per-member directories; - an overridable private pool directory under the per-user runtime directory; - a per-pool cross-process POSIX file lock; - private directory, lock-file, and JSON state-file permissions; and - locked helpers for listing, reading, writing, renaming, and removing member state. Member validation, compatibility fingerprints, and atomic claiming are isolated in #57907. Process lifecycle, acquisition, SparkSession integration, and JIT warmup remain in later PRs. ### Why are the changes needed? The pool needs a small, independently reviewable state model before adding compatibility checks, claiming, and process supervision. Keeping this layer limited to path layout, locking, and state file access makes its filesystem and concurrency contract reviewable on its own. ### Does this PR introduce _any_ user-facing change? No. The storage model is internal and is not wired into SparkSession in this layer. ### How was this patch tested? Added three focused tests covering directory selection, private permissions and malformed JSON, and cross-process lock contention. ```bash python/run-tests --testnames pyspark.sql.tests.connect.test_connect_local_server_pool ``` These cases passed on Python 3.11 as part of the combined suite before the stack was split. The rebuilt commit passed `git diff --check`, Python AST parsing, and changed-line ASCII and 100-column checks. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Fable 5) and OpenAI Codex (GPT-5) Closes #57685 from ericm-db/local-connect-pool-storage. Authored-by: Eric Marnadi <eric.marnadi@databricks.com> Signed-off-by: Daniel Tenedorio <daniel.tenedorio@databricks.com> (cherry picked from commit 001e89c) Signed-off-by: Daniel Tenedorio <daniel.tenedorio@databricks.com>

### What changes were proposed in this pull request? This is layer 3 of the seven-PR local Connect pool stack: #57684 -> #57685 -> #57907 -> #57686 -> #57687 -> #57102 -> #57688 The two lower layers are now merged, so GitHub shows only this layer's two-file diff. This layer adds member compatibility and claiming on top of the filesystem state model: - compatibility fingerprints covering the master, startup conf, working directory, Python executables, PySpark and Spark paths, and `PYTHONPATH`, using the full SHA-256 digest; - strict validation of persisted member records followed by explicit typed `PoolMember` fields; - Spark-version, process-liveness (including Linux zombies), and socket-reachability checks; and - well-defined, approximately-FIFO claiming through an atomic state-file rename, including safe concurrent claims from separate processes. Ordering is by the wall-clock `created` timestamp (comparable across the independent publisher processes, unlike `time.monotonic()`) with stable `sorted()` tie-breaking, so it is well defined but only approximately FIFO, not guaranteed: a backward clock step (NTP, suspend/resume) can perturb it. Pool sizing lives with its first consumer in acquisition layer #57687. Process lifecycle, acquisition, SparkSession integration, and JIT warmup remain in later PRs. ### Why are the changes needed? The filesystem layer defines safe state storage, but a client also needs to distinguish compatible servers and claim exactly one member without racing other processes. Isolating that contract keeps record validation and the ready-to-claimed transition independently reviewable before lifecycle and launch orchestration are added. ### Does this PR introduce _any_ user-facing change? No. Claiming is internal and is not wired into SparkSession in this layer. ### How was this patch tested? Added fifteen focused tests at this layer, bringing the suite to 25 tests. They cover deterministic Linux zombie detection, fingerprint identity and Python-interpreter precedence, Python and Spark path compatibility, JVM-environment coverage, strict member-record validation, fingerprint-aware and approximately-FIFO claiming, a real two-process claim race, unreachable members, and malformed, dead, or version-incompatible records. ```bash python/run-tests --testnames pyspark.sql.tests.connect.test_connect_local_server_pool ``` All 25 focused tests passed locally. Ruff lint and format checks, targeted mypy, Python compilation, custom-error validation, `git diff --check`, and changed-file ASCII and line-length checks also passed. [GitHub Actions run 32406852918](https://github.com/ericm-db/spark/actions/runs/32406852918) is running against the latest commit (`36c787ab5d6`). ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8) and OpenAI Codex (GPT-5) Closes #57907 from ericm-db/local-connect-pool-claiming. Authored-by: Eric Marnadi <eric.marnadi@databricks.com> Signed-off-by: Daniel Tenedorio <daniel.tenedorio@databricks.com>

### What changes were proposed in this pull request? This is layer 3 of the seven-PR local Connect pool stack: #57684 -> #57685 -> #57907 -> #57686 -> #57687 -> #57102 -> #57688 The two lower layers are now merged, so GitHub shows only this layer's two-file diff. This layer adds member compatibility and claiming on top of the filesystem state model: - compatibility fingerprints covering the master, startup conf, working directory, Python executables, PySpark and Spark paths, and `PYTHONPATH`, using the full SHA-256 digest; - strict validation of persisted member records followed by explicit typed `PoolMember` fields; - Spark-version, process-liveness (including Linux zombies), and socket-reachability checks; and - well-defined, approximately-FIFO claiming through an atomic state-file rename, including safe concurrent claims from separate processes. Ordering is by the wall-clock `created` timestamp (comparable across the independent publisher processes, unlike `time.monotonic()`) with stable `sorted()` tie-breaking, so it is well defined but only approximately FIFO, not guaranteed: a backward clock step (NTP, suspend/resume) can perturb it. Pool sizing lives with its first consumer in acquisition layer #57687. Process lifecycle, acquisition, SparkSession integration, and JIT warmup remain in later PRs. ### Why are the changes needed? The filesystem layer defines safe state storage, but a client also needs to distinguish compatible servers and claim exactly one member without racing other processes. Isolating that contract keeps record validation and the ready-to-claimed transition independently reviewable before lifecycle and launch orchestration are added. ### Does this PR introduce _any_ user-facing change? No. Claiming is internal and is not wired into SparkSession in this layer. ### How was this patch tested? Added fifteen focused tests at this layer, bringing the suite to 25 tests. They cover deterministic Linux zombie detection, fingerprint identity and Python-interpreter precedence, Python and Spark path compatibility, JVM-environment coverage, strict member-record validation, fingerprint-aware and approximately-FIFO claiming, a real two-process claim race, unreachable members, and malformed, dead, or version-incompatible records. ```bash python/run-tests --testnames pyspark.sql.tests.connect.test_connect_local_server_pool ``` All 25 focused tests passed locally. Ruff lint and format checks, targeted mypy, Python compilation, custom-error validation, `git diff --check`, and changed-file ASCII and line-length checks also passed. [GitHub Actions run 32406852918](https://github.com/ericm-db/spark/actions/runs/32406852918) is running against the latest commit (`36c787ab5d6`). ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8) and OpenAI Codex (GPT-5) Closes #57907 from ericm-db/local-connect-pool-claiming. Authored-by: Eric Marnadi <eric.marnadi@databricks.com> Signed-off-by: Daniel Tenedorio <daniel.tenedorio@databricks.com> (cherry picked from commit ee11a92) Signed-off-by: Daniel Tenedorio <daniel.tenedorio@databricks.com>

HyukjinKwon reviewed Aug 5, 2026

View reviewed changes

ericm-db force-pushed the local-connect-pool-lifecycle branch from 76def56 to dcb8359 Compare August 10, 2026 20:15

This was referenced Aug 10, 2026

[SPARK-58021][CONNECT] Add local server pool filesystem storage #57685

Closed

[SPARK-58021][CONNECT] Add local server pool member claiming #57907

Closed

ericm-db force-pushed the local-connect-pool-lifecycle branch from dcb8359 to 4cd8e32 Compare August 22, 2026 00:38

[SPARK-58021][CONNECT] Add local pool server retirement

37dbe4b

ericm-db force-pushed the local-connect-pool-lifecycle branch from 7f18211 to 37dbe4b Compare August 24, 2026 17:17

This was referenced Aug 24, 2026

[SPARK-58021][CONNECT] Reap orphaned local pool members #58247

Open

[SPARK-58021][CONNECT] Add forceful local pool purge #58248

Open

ericm-db changed the title ~~[SPARK-58021][CONNECT] Add local server pool lifecycle management~~ [SPARK-58021][CONNECT] Add local pool server retirement Aug 24, 2026

[SPARK-58021][CONNECT] Harden local pool retirement recovery

503b378

Navigation Menu

Uh oh!

[SPARK-58021][CONNECT] Add local pool server retirement - #57686

[SPARK-58021][CONNECT] Add local pool server retirement#57686
ericm-db wants to merge 2 commits into
apache:masterfrom
ericm-db:local-connect-pool-lifecycle

ericm-db commented Jul 31, 2026 •

edited

Loading

Uh oh!

HyukjinKwon left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

ericm-db commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

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

Uh oh!

HyukjinKwon left a comment

Choose a reason for hiding this comment

Verification

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ericm-db commented Jul 31, 2026 •

edited

Loading