Skip to content

[SPARK-58021][CONNECT] Warm local server pool members before use - #57688

Open
ericm-db wants to merge 6 commits into
apache:masterfrom
ericm-db:local-connect-pool-warmup
Open

[SPARK-58021][CONNECT] Warm local server pool members before use#57688
ericm-db wants to merge 6 commits into
apache:masterfrom
ericm-db:local-connect-pool-warmup

Conversation

@ericm-db

@ericm-db ericm-db commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

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

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

Until lower layers merge, GitHub shows their cumulative diff. The review unit introduced here is
commit 2bb06f0127e.

This layer adds optional JIT warmup for newly booted pool members:

  • attendants launch a bounded child process against each published server;
  • fixed synthetic queries warm JVM JIT and Spark codegen paths without persistent catalog state;
  • warmup stops immediately if the member is claimed and is killed after its timeout;
  • SPARK_LOCAL_CONNECT_POOL_WARMUP=0 disables it;
  • the internal --warm entry point runs the query set; and
  • the pool documentation explains the latency and memory trade-off.

Why are the changes needed?

A booted server avoids JVM and SparkContext startup, but its first real query can still pay JIT and
code-generation costs. Keeping warmup separate from acquisition makes this optimization and its
subprocess failure boundary optional and independently reviewable.

Does this PR introduce any user-facing change?

Yes, only for the experimental pool opt-in introduced by #57102. Pool members warm themselves by
default before use. Set SPARK_LOCAL_CONNECT_POOL_WARMUP=0 to retain booted-server behavior
without synthetic queries. Behavior is unchanged when the pool opt-in is disabled.

How was this patch tested?

Added three focused tests at this layer, bringing the pool suite to 56 tests. They cover environment
parsing, termination of a warmup child after claim, and execution plus cleanup of the fixed query
set.

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)

@uros-b

uros-b commented Jul 31, 2026

Copy link
Copy Markdown
Member

Thank you @ericm-db! cc @HyukjinKwon

dtenedor pushed a commit that referenced this pull request Aug 10, 2026
### 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>
dtenedor pushed a commit that referenced this pull request Aug 10, 2026
### 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>
@ericm-db
ericm-db force-pushed the local-connect-pool-warmup branch from 275235c to 0e18358 Compare August 10, 2026 20:15
dtenedor pushed a commit that referenced this pull request Aug 12, 2026
### 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>
dtenedor pushed a commit that referenced this pull request Aug 12, 2026
### 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>
dtenedor pushed a commit that referenced this pull request Aug 21, 2026
### 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>
dtenedor pushed a commit that referenced this pull request Aug 21, 2026
### 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>
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