[SPARK-57959][SQL][TESTS] Deflake MetricsFailureInjectionSuite non-deterministic injection test#57037
Closed
HyukjinKwon wants to merge 1 commit into
Closed
Conversation
…rying until fetch-failure injection fires The 'Non-deterministic stage block failure injection - injectFailure=true' test is flaky (~1/6 in the full suite; passes 10/10 in isolation). macOS diagnostics show the failing run has stage1=300 stage2=5 (a clean run, no recompute) and finishes in ~370ms vs ~860ms on a passing run, and the test's own shuffle receives no injected FetchFailed. INJECT_SHUFFLE_FETCH_FAILURES corrupts mapper-0 of the shuffle map stage, but whether the downstream reducer observes the resulting FetchFailed - and thus forces the stage-1 recompute that inflates the raw metric - depends on task scheduling in the shared SparkContext, and it occasionally does not fire, leaving stage1Metric at exactly 300 and failing 'value > 300'. Re-run the query (resetting metrics each attempt) until the injection forces a recompute, up to 10 attempts, then run the original assertions. Test-only; does not touch the shared DAGScheduler injection machinery used by other suites. Generated-by: Claude Code
zhengruifeng
approved these changes
Jul 6, 2026
HyukjinKwon
added a commit
that referenced
this pull request
Jul 6, 2026
…erministic injection test ### What changes were proposed in this pull request? Make `MetricsFailureInjectionSuite`'s `Non-deterministic stage block failure injection - injectFailure=true` case re-run its query (resetting the metrics each attempt) until the injected shuffle fetch-failure actually forces a stage-1 recompute, up to 10 attempts, before running the existing assertions. Test-only. ### Why are the changes needed? The test is flaky. In the full suite it fails ~1 run in 6 with: ``` - Non-deterministic stage block failure injection - injectFailure=true *** FAILED *** 300 was not greater than 300 (MetricsFailureInjectionSuite.scala:...) ``` Diagnosis (repeated runs on a macOS arm64 runner, capturing the metric values and scheduler markers): - The test **passes 10/10 when run in isolation**, but flakes in the full suite - so it is a cross-test/scheduling interaction, not a problem with the query itself. - On a failing run the metrics are `stage1=300 stage2=5` (exactly the no-recompute values) and the case finishes in ~370ms versus ~860ms on a passing run, and the test's own shuffle gets **no injected `FetchFailed`**. `INJECT_SHUFFLE_FETCH_FAILURES` corrupts mapper-0 of the shuffle map stage, but whether the downstream reducer observes the resulting `FetchFailed` - and thus forces the stage-1 recompute that makes the raw `stage1Metric` exceed 300 - depends on task scheduling within the shared `SparkContext`. It occasionally does not fire, leaving `stage1Metric` at exactly 300. Retrying until the injection fires makes the test robust without weakening what it verifies (it still requires a real recompute), and without touching the shared `DAGScheduler` injection machinery that other suites (`SQLLastAttemptMetric*`, DSv2 DML metric tests) rely on. ### Does this PR introduce _any_ user-facing change? No. Test-only. ### How was this patch tested? Ran the **full** `MetricsFailureInjectionSuite` **8×** on a `macos-15` runner (where it was ~1/6 flaky before): all 8 iterations green (`Tests: succeeded 13, failed 0`). - Before (flaky on `macos-26`): https://github.com/apache/spark/actions/runs/28753698265 - After (this fix, 8/8 green on fork): https://github.com/HyukjinKwon/spark/actions/runs/28780271388 ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code Closes #57037 from HyukjinKwon/ci-fix/agent4-metrics-injection-flake-pr. Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com> (cherry picked from commit 09f08e2) Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com>
HyukjinKwon
added a commit
that referenced
this pull request
Jul 6, 2026
…erministic injection test ### What changes were proposed in this pull request? Make `MetricsFailureInjectionSuite`'s `Non-deterministic stage block failure injection - injectFailure=true` case re-run its query (resetting the metrics each attempt) until the injected shuffle fetch-failure actually forces a stage-1 recompute, up to 10 attempts, before running the existing assertions. Test-only. ### Why are the changes needed? The test is flaky. In the full suite it fails ~1 run in 6 with: ``` - Non-deterministic stage block failure injection - injectFailure=true *** FAILED *** 300 was not greater than 300 (MetricsFailureInjectionSuite.scala:...) ``` Diagnosis (repeated runs on a macOS arm64 runner, capturing the metric values and scheduler markers): - The test **passes 10/10 when run in isolation**, but flakes in the full suite - so it is a cross-test/scheduling interaction, not a problem with the query itself. - On a failing run the metrics are `stage1=300 stage2=5` (exactly the no-recompute values) and the case finishes in ~370ms versus ~860ms on a passing run, and the test's own shuffle gets **no injected `FetchFailed`**. `INJECT_SHUFFLE_FETCH_FAILURES` corrupts mapper-0 of the shuffle map stage, but whether the downstream reducer observes the resulting `FetchFailed` - and thus forces the stage-1 recompute that makes the raw `stage1Metric` exceed 300 - depends on task scheduling within the shared `SparkContext`. It occasionally does not fire, leaving `stage1Metric` at exactly 300. Retrying until the injection fires makes the test robust without weakening what it verifies (it still requires a real recompute), and without touching the shared `DAGScheduler` injection machinery that other suites (`SQLLastAttemptMetric*`, DSv2 DML metric tests) rely on. ### Does this PR introduce _any_ user-facing change? No. Test-only. ### How was this patch tested? Ran the **full** `MetricsFailureInjectionSuite` **8×** on a `macos-15` runner (where it was ~1/6 flaky before): all 8 iterations green (`Tests: succeeded 13, failed 0`). - Before (flaky on `macos-26`): https://github.com/apache/spark/actions/runs/28753698265 - After (this fix, 8/8 green on fork): https://github.com/HyukjinKwon/spark/actions/runs/28780271388 ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code Closes #57037 from HyukjinKwon/ci-fix/agent4-metrics-injection-flake-pr. Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com> (cherry picked from commit 09f08e2) Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com>
Member
Author
HyukjinKwon
added a commit
that referenced
this pull request
Jul 6, 2026
…ons in MetricsFailureInjectionSuite ### What changes were proposed in this pull request? This is a follow-up of #57037 (SPARK-57959). That PR added an explanatory comment block to `MetricsFailureInjectionSuite`, and four of those comment lines exceed the 100-character limit, breaking the `scalastyle` check on `master`: ``` [error] .../MetricsFailureInjectionSuite.scala:348: File line length exceeds 100 characters [error] .../MetricsFailureInjectionSuite.scala:349: File line length exceeds 100 characters [error] .../MetricsFailureInjectionSuite.scala:352: File line length exceeds 100 characters [error] .../MetricsFailureInjectionSuite.scala:353: File line length exceeds 100 characters ``` This PR reflows the comment block so every line fits within 100 characters. No code or test logic is changed. ### Why are the changes needed? To fix the broken `scalastyle` check on `master` (and `branch-4.x`/`branch-4.2`, where the offending commit was also backported). ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? ``` grep -nP '.{101,}' sql/core/src/test/scala/org/apache/spark/sql/execution/metric/MetricsFailureInjectionSuite.scala ``` returns no matches. The change is comments-only. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8) Closes #57039 from HyukjinKwon/fix/spark-57959-scalastyle-lines. Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com>
HyukjinKwon
added a commit
that referenced
this pull request
Jul 6, 2026
…ons in MetricsFailureInjectionSuite ### What changes were proposed in this pull request? This is a follow-up of #57037 (SPARK-57959). That PR added an explanatory comment block to `MetricsFailureInjectionSuite`, and four of those comment lines exceed the 100-character limit, breaking the `scalastyle` check on `master`: ``` [error] .../MetricsFailureInjectionSuite.scala:348: File line length exceeds 100 characters [error] .../MetricsFailureInjectionSuite.scala:349: File line length exceeds 100 characters [error] .../MetricsFailureInjectionSuite.scala:352: File line length exceeds 100 characters [error] .../MetricsFailureInjectionSuite.scala:353: File line length exceeds 100 characters ``` This PR reflows the comment block so every line fits within 100 characters. No code or test logic is changed. ### Why are the changes needed? To fix the broken `scalastyle` check on `master` (and `branch-4.x`/`branch-4.2`, where the offending commit was also backported). ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? ``` grep -nP '.{101,}' sql/core/src/test/scala/org/apache/spark/sql/execution/metric/MetricsFailureInjectionSuite.scala ``` returns no matches. The change is comments-only. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8) Closes #57039 from HyukjinKwon/fix/spark-57959-scalastyle-lines. Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com> (cherry picked from commit 4995e6f) Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com>
HyukjinKwon
added a commit
that referenced
this pull request
Jul 6, 2026
…ons in MetricsFailureInjectionSuite ### What changes were proposed in this pull request? This is a follow-up of #57037 (SPARK-57959). That PR added an explanatory comment block to `MetricsFailureInjectionSuite`, and four of those comment lines exceed the 100-character limit, breaking the `scalastyle` check on `master`: ``` [error] .../MetricsFailureInjectionSuite.scala:348: File line length exceeds 100 characters [error] .../MetricsFailureInjectionSuite.scala:349: File line length exceeds 100 characters [error] .../MetricsFailureInjectionSuite.scala:352: File line length exceeds 100 characters [error] .../MetricsFailureInjectionSuite.scala:353: File line length exceeds 100 characters ``` This PR reflows the comment block so every line fits within 100 characters. No code or test logic is changed. ### Why are the changes needed? To fix the broken `scalastyle` check on `master` (and `branch-4.x`/`branch-4.2`, where the offending commit was also backported). ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? ``` grep -nP '.{101,}' sql/core/src/test/scala/org/apache/spark/sql/execution/metric/MetricsFailureInjectionSuite.scala ``` returns no matches. The change is comments-only. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8) Closes #57039 from HyukjinKwon/fix/spark-57959-scalastyle-lines. Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com> (cherry picked from commit 4995e6f) Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com>
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
2 participants
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?
Make
MetricsFailureInjectionSuite'sNon-deterministic stage block failure injection - injectFailure=truecase re-run its query (resetting the metrics each attempt) until theinjected shuffle fetch-failure actually forces a stage-1 recompute, up to 10 attempts, before
running the existing assertions. Test-only.
Why are the changes needed?
The test is flaky. In the full suite it fails ~1 run in 6 with:
Diagnosis (repeated runs on a macOS arm64 runner, capturing the metric values and scheduler
markers):
cross-test/scheduling interaction, not a problem with the query itself.
stage1=300 stage2=5(exactly the no-recompute values) andthe case finishes in ~370ms versus ~860ms on a passing run, and the test's own shuffle gets
no injected
FetchFailed.INJECT_SHUFFLE_FETCH_FAILUREScorrupts mapper-0 of the shuffle map stage, but whether thedownstream reducer observes the resulting
FetchFailed- and thus forces the stage-1 recomputethat makes the raw
stage1Metricexceed 300 - depends on task scheduling within the sharedSparkContext. It occasionally does not fire, leavingstage1Metricat exactly 300.Retrying until the injection fires makes the test robust without weakening what it verifies (it
still requires a real recompute), and without touching the shared
DAGSchedulerinjectionmachinery that other suites (
SQLLastAttemptMetric*, DSv2 DML metric tests) rely on.Does this PR introduce any user-facing change?
No. Test-only.
How was this patch tested?
Ran the full
MetricsFailureInjectionSuite8× on amacos-15runner (where it was ~1/6flaky before): all 8 iterations green (
Tests: succeeded 13, failed 0).macos-26): https://github.com/apache/spark/actions/runs/28753698265Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code