[SPARK-58264][CONNECT][TESTS] Use OS-assigned ports in Connect server tests - #57436
Draft
Ma77Ball wants to merge 3 commits into
Draft
[SPARK-58264][CONNECT][TESTS] Use OS-assigned ports in Connect server tests#57436Ma77Ball wants to merge 3 commits into
Ma77Ball wants to merge 3 commits into
Conversation
uros-b
reviewed
Jul 27, 2026
uros-b
left a comment
Member
There was a problem hiding this comment.
cc @sarutak @Ma77Ball regarding #57560 (review).
Contributor
Author
|
@uros-b either PR should resolve this flake. The main difference is that binding to port 0 instead of a fixed guessed port also removes the potential collision with other processes or parallel JVMs more of a latent race than the observed failure but it lets us drop fixed ports from the harness entirely. It also avoids the probe-then-rebind TOCTOU (time-of-check to time-of-use) window that the fixed-port approach still leaves open. |
…d cause issues in parallel processes
Ma77Ball
force-pushed
the
SPARK-58264-connect-test-os-ports
branch
from
July 27, 2026 22:01
4f0cd9b to
27b797d
Compare
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
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?
Bind the Spark Connect server test harness to port 0 so the OS assigns a free port, instead of guessing
CONNECT_GRPC_BINDING_PORT + Random.nextInt(1000):SparkConnectServerTeststarts the service on port 0 via a newstartServicehelper and reads the real bound port fromSparkConnectService.localPortintoserverPort.SparkConnectServiceKeepAliveSuiterestarts via a newrestartServicehelper that rebinds to port 0 rather than reusing a fixed port.RemoteSparkSessionlaunches the server process with binding port 0 and passes aspark.connect.test.portFile. It reads the real bound port back from that file instead of pre-reserving one, which removes a time-of-check/time-of-use (TOCTOU) window where a reserved-then-closed port could be taken before the server binds it.SimpleSparkConnectServicewrites the actual bound port to the configuredportFile.Why are the changes needed?
The guessed fixed port races other processes and parallel test JVMs for a specific port, causing intermittent
BindException. Letting the OS pick a free port removes that race, and also removes the stop/start-same-port window since the OS never hands back a port that is still bound or closing. For the client harness, having the server bind port 0 itself and report the real port avoids pre-reserving a port (a TOCTOU race).Does this PR introduce any user-facing change?
No.
How was this patch tested?
Existing Connect server and client-jvm suites.
connectandconnect-client-jvmcompile; scalafmt, scalastyle, and ASCII/line-length lint pass.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Opus 4.8