Skip to content

[Spark] Run ValidatesRunner tests serially, runner metrics are JVM wide - #40428

Merged
Abacn merged 1 commit into
apache:masterfrom
tkaymak:spark-vr-serial-threads
Oct 6, 2026
Merged

Abacn merged 1 commit into
apache:masterfrom
tkaymak:spark-vr-serial-threads

Conversation

@tkaymak

@tkaymak tkaymak commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #40427.

Since #40324, ValidatesRunner methods run on 8 threads per test JVM. The Spark runners keep metrics in a JVM wide MetricsAccumulator and allow one SparkContext per JVM, so every Spark VR and PVR PostCommit is red since 2026-10-01.

This sets beam.validatesRunner.parallelThreads=1 for the Spark runner and Spark job server test tasks, as the Dataflow runner sets its own value. The trigger files run the eight affected suites on this PR.

Local check, :runners:spark:4:validatesStructuredStreamingRunnerBatch on PAssertTest, FlattenTest and CreateTest: 37 failures on master, 46 tests and 0 failures with this change.

R: @Abacn
cc @reuvenlax

@tkaymak

tkaymak commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

R: @Abacn

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment assign set of reviewers

@tkaymak

tkaymak commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

@Abacn

Abacn commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Thanks for identifying the issue and the fix. I didn't expect #40324 would affect Spark ValidateRunner tests, it should only affect Dataflow. I suggest we set "beam.validatesRunner.parallelThreads" as "1" by default (or treat as 1 if the property is absent), and set to "8" in Dataflow ValidateRunner tests that currently running on parallel (only batch tests?) only

@Abacn

Abacn commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

cc: @reuvenlax

@reuvenlax

reuvenlax commented Oct 6, 2026 via email

Copy link
Copy Markdown
Contributor

@reuvenlax

Copy link
Copy Markdown
Contributor

lgtm

@Abacn

Abacn commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Yeah Flink looks fine:

https://github.com/apache/beam/actions/workflows/beam_PostCommit_Java_ValidatesRunner_Flink.yml?query=event%3Aschedule

Spark has a well known limitation that is only one active SparkContext is allowed / honored in a JVM at a time, and caused the cross-test contamination.

@reuvenlax

reuvenlax commented Oct 6, 2026 via email

Copy link
Copy Markdown
Contributor

@Abacn Abacn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

then seems reasonable to just disable Spark.

SGTM and will merge after test passing

@codecov

codecov Bot commented Oct 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.74%. Comparing base (69bdf5f) to head (3c86ba8).
⚠️ Report is 24 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #40428      +/-   ##
============================================
- Coverage     59.04%   58.74%   -0.30%     
+ Complexity    15624    13903    -1721     
============================================
  Files          2797     2583     -214     
  Lines        280750   271419    -9331     
  Branches      12488    11226    -1262     
============================================
- Hits         165764   159443    -6321     
+ Misses       108540   105984    -2556     
+ Partials       6446     5992     -454     
Flag Coverage Δ
java 65.24% <ø> (-0.20%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Abacn
Abacn merged commit 32d54e8 into apache:master Oct 6, 2026
29 of 30 checks passed
@tkaymak tkaymak changed the title [Spark] Run ValidatesRunner tests serially, runner metrics are JVM wide Default beam.validatesRunner.parallelThreads to 1, runners opt in Oct 6, 2026
@tkaymak tkaymak changed the title Default beam.validatesRunner.parallelThreads to 1, runners opt in [Spark] Run ValidatesRunner tests serially, runner metrics are JVM wide Oct 6, 2026
@reuvenlax

reuvenlax commented Oct 6, 2026 via email

Copy link
Copy Markdown
Contributor

@Abacn

Abacn commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

The current PR was merged with only overriding Spark validate runner test to be sequential and we are good for now.

PreCommit Java doesn't run ValidateRunner test. There is a separate ValidateRunner Direct run at PostCommit time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Spark ValidatesRunner PostCommits red since parallel test threads (#40324)

3 participants