Skip to content

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

Open
ericm-db wants to merge 3 commits into
apache:masterfrom
ericm-db:local-connect-pool-lifecycle
Open

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

Conversation

@ericm-db

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

This is layer 3 of the local Connect pool stack: #57684 -> #57685 -> this PR. Until the lower layers merge, GitHub shows their cumulative diff; the new review unit here is commit 76def5665b6.

This layer adds process lifecycle management on top of the storage model:

  • janitor rules for dead or timed-out launches;
  • retirement of dead, unreachable, idle, and orphaned claimed servers;
  • graceful termination followed by SIGKILL escalation;
  • cleanup of stale member directories and half-started daemons;
  • idempotent release of a claimed member; and
  • a forceful pool purge escape hatch.

It does not add background acquisition or SparkSession dispatch.

Why are the changes needed?

Pool members outlive individual Python call frames and can be orphaned by crashes or SIGKILL. Their cleanup rules need to be explicit, idempotent, and testable before attendants and acquisition loops begin invoking them concurrently.

Does this PR introduce any user-facing change?

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

How was this patch tested?

Expanded LocalConnectServerPoolUnitTests with fake-process coverage for dead and live pending launches, unreachable and idle members, orphaned claims, shutdown escalation, release, and purge.

python -m unittest -v pyspark.sql.tests.connect.test_connect_local_server_pool.LocalConnectServerPoolUnitTests

All 14 tests passed. Ruff check, Ruff format check, and git diff --check also passed.

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

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

ericm-db added 3 commits July 31, 2026 16:54
Expose the per-user runtime directory and startup seed configuration, and let LocalConnectServer start isolated daemons with an ephemeral port and precomputed configuration. Keep persistent-server reuse on the same launch path and add focused unit coverage.

@HyukjinKwon HyukjinKwon 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.

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'.

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.

2 participants