[SPARK-58021][CONNECT] Add forceful local pool purge - #58248
Open
ericm-db wants to merge 5 commits into
Open
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
This was referenced Aug 24, 2026
dtenedor
pushed a commit
that referenced
this pull request
Aug 27, 2026
### 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: ```bash 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) Closes #57686 from ericm-db/local-connect-pool-lifecycle. Authored-by: Eric Marnadi <eric.marnadi@databricks.com> Signed-off-by: Daniel Tenedorio <daniel.tenedorio@databricks.com>
dtenedor
pushed a commit
that referenced
this pull request
Aug 27, 2026
### 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: ```bash 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) Closes #57686 from ericm-db/local-connect-pool-lifecycle. Authored-by: Eric Marnadi <eric.marnadi@databricks.com> Signed-off-by: Daniel Tenedorio <daniel.tenedorio@databricks.com> (cherry picked from commit a1857f5) Signed-off-by: Daniel Tenedorio <daniel.tenedorio@databricks.com>
ericm-db
force-pushed
the
local-connect-pool-purge
branch
from
196221a to
50b735d
Compare
August 28, 2026 00:36
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
This is layer 8 of the ten-PR local Connect pool stack:
#57684 -> #57685 -> #57907 -> #57686 -> #58247 ->
#58367 -> #57687 -> #58248 -> #57102 -> #57688
Until lower layers merge, GitHub shows their cumulative diff. The review unit introduced here is
commit 50b735d181d.
This layer adds the forceful escape hatch for returning the pool to a clean slate:
SparkSession integration and JIT warmup remain in later PRs.
Why are the changes needed?
Normal retirement deliberately preserves retryable state. Operators also need a bounded,
destructive recovery path when the pool itself is corrupt or wedged. Keeping purge separate makes
its stronger signalling and deletion semantics explicit and independently reviewable.
Does this PR introduce any user-facing change?
Yes. It adds python -m pyspark.sql.connect.local_server_pool --purge, which force-stops every
local pool process it can verify and empties the pool directory. SparkSession still does not select
the pool until #57102.
How was this patch tested?
All 64 focused pool tests at this stack layer passed:
At the final stack tip, the same invocation passed all 69 tests, including the two real-server
end-to-end tests. The rewritten stack also passed Python compilation, Ruff checking and formatting,
targeted mypy for the pool module, git diff --check, and changed-file ASCII and Python
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)