johnleuner opened a new pull request, #59827:
URL: https://github.com/apache/spark/pull/59827
[SPARK-60504][CONNECT] Fix withLocalConnectServer ignoring explicit --remote
### What changes were proposed in this pull request?
This PR extracts the condition used by `SparkSession.withLocalConnectServer`
to decide
whether to spawn an embedded local Spark Connect server into a new method,
`shouldStartLocalConnectServer`, and fixes a bug in that condition: an
explicitly
configured `sc://host:port` remote (via `--remote`, `spark.remote`, or
`SPARK_REMOTE`)
was being ignored whenever `spark.api.mode=connect` (or
`SPARK_CONNECT_MODE=1`) was also
set, causing the client to silently redirect to a spawned local server
instead of the
requested remote endpoint.
`shouldStartLocalConnectServer` now takes the resolved remote string and
whether
`spark.api.mode=connect` is in effect, and returns true in exactly two cases:
1. The remote is the local convenience master (e.g. `local`, `local[*]`,
`local[4]`).
2. `spark.api.mode=connect` is set and the remote is some other classic
master (e.g.
`yarn`, `spark://host:port`, `k8s://...`) rather than an already-explicit
Spark Connect
endpoint, letting an existing classic application switch to Connect just
by setting
`spark.api.mode=connect`, without changing its master.
A fully-specified `sc://host:port` remote is now never treated as a request
to start a
local server, regardless of `isAPIModeConnect`.
### Why are the changes needed?
Running spark-shell while specifying a remote sc://host:port/ was previously
broken.
Before this change, `shouldStartLocalConnectServer` only looked at whether
the remote
string started with `local`, but `withLocalConnectServer` could resolve
`remoteString` to
a classic master (e.g. `spark.master`) when `spark.api.mode=connect` was
set, including
when the user had also explicitly set an `sc://` remote. The explicit
`--remote` /
`spark.remote` / `SPARK_REMOTE` value was being passed on for connecting,
but the
decision of *whether to start a local server* did not account for
`isAPIModeConnect`,
so a classic master could be misclassified. This PR makes the decision
explicit and
covers it with tests so the explicit `sc://` remote is never overridden by a
spawned
local server.
### Does this PR introduce _any_ user-facing change?
Yes. When `spark.api.mode=connect` (or `SPARK_CONNECT_MODE=1`) is set
together with an
explicit `--remote` / `spark.remote` / `SPARK_REMOTE` pointing at
`sc://host:port`, the
client now connects to that endpoint as requested instead of silently
starting and
connecting to a local Spark Connect server.
### How was this patch tested?
I ran the old spark-shell passing --remote sc://host:port/ and --conf
spark.api.mode=connect to verify the broken behaviour
I then ran the new spark-shell with the same arguments and verified the
correct connection
I ran spark shell with both old and new versions passing only --conf
spark.api.mode=connect to verify a local Connect server was created
### Was this patch authored or co-authored using generative AI tooling?
Yes, I used an LLM to help me generate the code and the tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]