Skip to content

[SPARK-59821][CORE] Make Maven utility tests safe for concurrent runs - #59096

Closed
qianlan717 wants to merge 3 commits into
apache:masterfrom
qianlan717:SPARK-59553-concurrent-ivy-test-isolation
Closed

qianlan717 wants to merge 3 commits into
apache:masterfrom
qianlan717:SPARK-59553-concurrent-ivy-test-isolation

Conversation

@qianlan717

Copy link
Copy Markdown
Contributor

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)

@qianlan717 qianlan717 changed the title [SPARK-59553][FOLLOWUP][CONNECT] Isolate Maven utility test files [SPARK-59821][CORE] Make Maven utility tests safe for concurrent runs Sep 28, 2026
@qianlan717
qianlan717 force-pushed the SPARK-59553-concurrent-ivy-test-isolation branch from 292652c to 5085c7b Compare September 28, 2026 09:07
HyukjinKwon

This comment was marked as outdated.

@HyukjinKwon HyukjinKwon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

uros-b commented Sep 28, 2026

Copy link
Copy Markdown
Member

Thank you @qianlan717 and @HyukjinKwon! Please try to get CI run complete & green

@qianlan717

Copy link
Copy Markdown
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>
@HyukjinKwon

Copy link
Copy Markdown
Member

Merge Summary:

Posted by merge_spark_pr.py

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.

3 participants