[SPARK-59821][CORE] Make Maven utility tests safe for concurrent runs - #59096
Closed
qianlan717 wants to merge 3 commits into
Closed
qianlan717 wants to merge 3 commits into
qianlan717 wants to merge 3 commits into
Conversation
HyukjinKwon
approved these changes
Sep 28, 2026
qianlan717
force-pushed
the
SPARK-59553-concurrent-ivy-test-isolation
branch
from
September 28, 2026 09:07
292652c to
5085c7b
Compare
HyukjinKwon
approved these changes
Sep 28, 2026
HyukjinKwon
left a comment
Member
There was a problem hiding this comment.
0 addressed, 0 remaining, 0 new. No findings in this same-commit re-review.
Verification
Independently traced every createCompiledClass and m2Path caller, including packaged-class output and Maven repository deletion/recreation. A direct eight-thread javac probe completed 400/400 same-name compilations successfully and preserved the flat destination contract. Focused SBT suites were blocked during project loading because this machine redirects Maven Central to localhost; upstream CI currently reports no failures.
uros-b
approved these changes
Sep 28, 2026
Member
|
Thank you @qianlan717 and @HyukjinKwon! Please try to get CI run complete & green |
Contributor
Author
|
@uros-b and @HyukjinKwon CI is green. Can you help merge this PR to master and branch-4.x? Thanks! |
HyukjinKwon
pushed a commit
that referenced
this pull request
Sep 28, 2026
### What changes were proposed in this pull request? This follow-up to #58874 makes the Maven test utilities safe when the same suite runs concurrently. - Compile generated Java classes in an invocation-specific temporary directory instead of the process working directory. - Give each test JVM its own stable fake local Maven repository. - Add a regression test that compiles the same class name concurrently into separate destinations. ### Why are the changes needed? `SparkTestUtils.createCompiledClass` currently asks javac to emit a fixed class name, such as `MyLib.class`, in the process working directory before moving it to the requested destination. `MavenUtils.m2Path` also uses the fixed relative path `dummy/.m2/repository` during tests. When a test target is repeated concurrently, the JVMs can move or delete files needed by another JVM. This causes failures such as: ``` java.nio.file.NoSuchFileException: MyLib.class -> dummy/.m2/repository/.../MyLib.class ``` Using isolated temporary paths removes this cross-JVM interference without changing production Maven resolution. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Added a `SparkTestUtilsSuite` regression test that starts eight concurrent compilations of the same class name and verifies every output exists. `git diff --check` passes. The following focused command was attempted locally: ``` /usr/bin/sbt \ "common-utils/testOnly org.apache.spark.util.SparkTestUtilsSuite" \ "common-utils/testOnly org.apache.spark.util.MavenUtilsSuite" ``` The suites could not start because the local environment could not resolve Maven Central or the SBT plugin repositories (`UnknownHostException`). GitHub Actions will provide the executable test result. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: OpenAI Codex (GPT-5) Closes #59096 from qianlan717/SPARK-59553-concurrent-ivy-test-isolation. Authored-by: qianlan717 <qianlan.chen@databricks.com> Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com> (cherry picked from commit aeda065) Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com>
Member
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?
This follow-up to #58874 makes the Maven test utilities safe when the same suite runs concurrently.
Why are the changes needed?
SparkTestUtils.createCompiledClasscurrently asks javac to emit a fixed class name, such asMyLib.class, in the process working directory before moving it to the requested destination.MavenUtils.m2Pathalso uses the fixed relative pathdummy/.m2/repositoryduring tests.When a test target is repeated concurrently, the JVMs can move or delete files needed by another JVM. This causes failures such as:
Using isolated temporary paths removes this cross-JVM interference without changing production Maven resolution.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Added a
SparkTestUtilsSuiteregression test that starts eight concurrent compilations of the same class name and verifies every output exists.git diff --checkpasses.The following focused command was attempted locally:
The suites could not start because the local environment could not resolve Maven Central or the SBT plugin repositories (
UnknownHostException). GitHub Actions will provide the executable test result.Was this patch authored or co-authored using generative AI tooling?
Generated-by: OpenAI Codex (GPT-5)