Uh oh!
There was an error while loading. Please reload this page.
[SPARK-49983][CORE][TESTS] Fix BarrierTaskContextSuite.successively sync with allGather and barrier test case to be robust - #48487
Closed
dongjoon-hyun wants to merge 1 commit into
Closed
Conversation
…sync with allGather and barrier` test case to be robust
xinrong-meng
approved these changes
Oct 16, 2024
xinrong-meng
commented
Oct 16, 2024
Member
LGTM, the tests in Build (pull_request_target) all passed. Thank you! |
dongjoon-hyun
commented
Oct 16, 2024
MemberAuthor
Thank you so much, @xinrong-meng . |
dongjoon-hyun added a commit
that referenced
this pull request
Oct 16, 2024
…sync with allGather and barrier` test case to be robust ### What changes were proposed in this pull request? This PR aims to fix `BarrierTaskContextSuite.successively sync with allGather and barrier` test case to be robust. ### Why are the changes needed? The test case asserts the duration of partitions. However, this is flaky because we don't know when a partition is triggered before `barrier` sync. https://github.com/apache/spark/blob/0e75d19a736aa18fe77414991ebb7e3577a43af8/core/src/test/scala/org/apache/spark/scheduler/BarrierTaskContextSuite.scala#L116-L118 Although we added `TestUtils.waitUntilExecutorsUp` at Apache Spark 3.0.0 like the following, - #28658 let's say a partition starts slowly than `38ms` and all partitions sleep `1s` exactly. Then, the test case fails like the following. - https://github.com/apache/spark/actions/runs/11298639789/job/31428018075 ``` BarrierTaskContextSuite: ... - successively sync with allGather and barrier *** FAILED *** 1038 was not less than or equal to 1000 (BarrierTaskContextSuite.scala:118) ``` According to the failure history here (SPARK-49983) and SPARK-31730, the slowness seems to be less than `200ms` when it happens. So, this PR aims to reduce the flakiness by capping the sleep up to 500ms while keeping the `1s` validation. There is no test coverage change because this test case focuses on the `successively sync with allGather and battier`. ### Does this PR introduce _any_ user-facing change? No, this is a test-only test case. ### How was this patch tested? Pass the CIs. ### Was this patch authored or co-authored using generative AI tooling? No. Closes#48487 from dongjoon-hyun/SPARK-49983. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit bcfe62b) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
dongjoon-hyun added a commit
that referenced
this pull request
Oct 16, 2024
…sync with allGather and barrier` test case to be robust ### What changes were proposed in this pull request? This PR aims to fix `BarrierTaskContextSuite.successively sync with allGather and barrier` test case to be robust. ### Why are the changes needed? The test case asserts the duration of partitions. However, this is flaky because we don't know when a partition is triggered before `barrier` sync. https://github.com/apache/spark/blob/0e75d19a736aa18fe77414991ebb7e3577a43af8/core/src/test/scala/org/apache/spark/scheduler/BarrierTaskContextSuite.scala#L116-L118 Although we added `TestUtils.waitUntilExecutorsUp` at Apache Spark 3.0.0 like the following, - #28658 let's say a partition starts slowly than `38ms` and all partitions sleep `1s` exactly. Then, the test case fails like the following. - https://github.com/apache/spark/actions/runs/11298639789/job/31428018075 ``` BarrierTaskContextSuite: ... - successively sync with allGather and barrier *** FAILED *** 1038 was not less than or equal to 1000 (BarrierTaskContextSuite.scala:118) ``` According to the failure history here (SPARK-49983) and SPARK-31730, the slowness seems to be less than `200ms` when it happens. So, this PR aims to reduce the flakiness by capping the sleep up to 500ms while keeping the `1s` validation. There is no test coverage change because this test case focuses on the `successively sync with allGather and battier`. ### Does this PR introduce _any_ user-facing change? No, this is a test-only test case. ### How was this patch tested? Pass the CIs. ### Was this patch authored or co-authored using generative AI tooling? No. Closes#48487 from dongjoon-hyun/SPARK-49983. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit bcfe62b) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit d37a8b9) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
HyukjinKwon added a commit
that referenced
this pull request
Jul 7, 2026
…skew assertions on macOS-26 ### What changes were proposed in this pull request? Widen the post-sync wall-clock skew tolerance in `BarrierTaskContextSuite` from a hard-coded `<= 1000` (ms) to a documented `maxSyncSkewMs = 3000` constant, applied to the three tests that assert how closely tasks finish a `barrier()` / `allGather()` global sync: - `global sync by barrier() call` - `successively sync with allGather and barrier` - `support multiple barrier() call within a single task` ### Why are the changes needed? The `Build / Maven (Scala 2.13, JDK 21, MacOS-26)` scheduled workflow has been failing on every recent run. One of the reproducing failures is: ``` BarrierTaskContextSuite: - successively sync with allGather and barrier *** FAILED *** 1078 was not less than or equal to 1000 (BarrierTaskContextSuite.scala:122) ``` These tests capture `System.currentTimeMillis()` in each task immediately after a barrier/allGather returns and assert `times.max - times.min <= 1000`. **That skew is bounded below by the barrier client's own polling granularity, not by sync correctness.** `BarrierTaskContext.runBarrier` waits for the coordinator RPC in a `Thread.sleep(1000)` loop ([`BarrierTaskContext.scala:102`](https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/BarrierTaskContext.scala#L102)), so two tasks can observe the release up to ~1s apart from the poll interval alone — before any thread-scheduling or GC jitter on a busy CI host is added. Evidence gathered: - SPARK-49983 (2024, PR #48487) previously saw `1038` and only *halved the pre-barrier sleep*. That reduces arrival spread but cannot reduce poll-granularity skew, so the flake persists on slower runners like `macos-26` arm64. - Reproduced locally on **macOS 26 arm64 / JDK 21** (the same OS/arch as the failing runner): measured post-sync skew up to **994 ms while the machine was idle** — i.e. essentially zero margin under the old `1000` bound, so any CI load tips it over. The new bound (twice the 1 s poll interval plus a jitter allowance) preserves the test's intent — tasks stay loosely in lockstep and none can race an entire extra sync ahead — without coupling the assertion to a razor-thin wall-clock margin. ### Scope note This PR fixes only the `BarrierTaskContextSuite` flake. The same workflow also exhibits other, independent flakes (Kafka `stress test for failOnDataLoss=false` topic-deletion timeout, mllib GMM FP-tolerance on arm64, `StateStoreSuite.maintenance` timeout). Those are out of scope here and would be separate changes. (The earlier `AF_UNIX path too long` UDF-worker flake was already fixed upstream by SPARK-57949.) ### Does this PR introduce _any_ user-facing change? No. Test-only. ### How was this patch tested? Locally on macOS 26 arm64 / JDK 21: `build/mvn -pl core test -DwildcardSuites=org.apache.spark.scheduler.BarrierTaskContextSuite` — all 13 tests pass; scalastyle clean. Also validated end-to-end on the `Build / Maven (Scala 2.13, JDK 21, MacOS-26)` workflow, running the same `core` matrix job on the identical `macos-26` arm64 runner image: | | `core,launcher,…` job (runs `BarrierTaskContextSuite`) | Result | |---|---|---| | **Before** (`apache/spark` master, without this fix) | [runs/28790673283 · job 85379029552](https://github.com/apache/spark/actions/runs/28790673283/job/85379029552) | ❌ `1078 was not less than or equal to 1000 (BarrierTaskContextSuite.scala:122)` — `Tests: succeeded 4103, failed 1` | | **After** (this branch) | [runs/28829214565 · job 85505367410](https://github.com/HyukjinKwon/spark/actions/runs/28829214565/job/85505367410) | ✅ `Tests: succeeded 4104, failed 0`; the three barrier tests all pass | (To run the guarded workflow on the fork, its `if: github.repository == 'apache/spark'` condition was relaxed on a throwaway commit that is **not** part of this PR — this PR is the single-file test change only.) ### Was this patch authored or co-authored using generative AI tooling? Yes, drafted with assistance from Claude. Closes#57057 from HyukjinKwon/deflake-barrier-macos26. 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 7, 2026
…skew assertions on macOS-26 ### What changes were proposed in this pull request? Widen the post-sync wall-clock skew tolerance in `BarrierTaskContextSuite` from a hard-coded `<= 1000` (ms) to a documented `maxSyncSkewMs = 3000` constant, applied to the three tests that assert how closely tasks finish a `barrier()` / `allGather()` global sync: - `global sync by barrier() call` - `successively sync with allGather and barrier` - `support multiple barrier() call within a single task` ### Why are the changes needed? The `Build / Maven (Scala 2.13, JDK 21, MacOS-26)` scheduled workflow has been failing on every recent run. One of the reproducing failures is: ``` BarrierTaskContextSuite: - successively sync with allGather and barrier *** FAILED *** 1078 was not less than or equal to 1000 (BarrierTaskContextSuite.scala:122) ``` These tests capture `System.currentTimeMillis()` in each task immediately after a barrier/allGather returns and assert `times.max - times.min <= 1000`. **That skew is bounded below by the barrier client's own polling granularity, not by sync correctness.** `BarrierTaskContext.runBarrier` waits for the coordinator RPC in a `Thread.sleep(1000)` loop ([`BarrierTaskContext.scala:102`](https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/BarrierTaskContext.scala#L102)), so two tasks can observe the release up to ~1s apart from the poll interval alone — before any thread-scheduling or GC jitter on a busy CI host is added. Evidence gathered: - SPARK-49983 (2024, PR #48487) previously saw `1038` and only *halved the pre-barrier sleep*. That reduces arrival spread but cannot reduce poll-granularity skew, so the flake persists on slower runners like `macos-26` arm64. - Reproduced locally on **macOS 26 arm64 / JDK 21** (the same OS/arch as the failing runner): measured post-sync skew up to **994 ms while the machine was idle** — i.e. essentially zero margin under the old `1000` bound, so any CI load tips it over. The new bound (twice the 1 s poll interval plus a jitter allowance) preserves the test's intent — tasks stay loosely in lockstep and none can race an entire extra sync ahead — without coupling the assertion to a razor-thin wall-clock margin. ### Scope note This PR fixes only the `BarrierTaskContextSuite` flake. The same workflow also exhibits other, independent flakes (Kafka `stress test for failOnDataLoss=false` topic-deletion timeout, mllib GMM FP-tolerance on arm64, `StateStoreSuite.maintenance` timeout). Those are out of scope here and would be separate changes. (The earlier `AF_UNIX path too long` UDF-worker flake was already fixed upstream by SPARK-57949.) ### Does this PR introduce _any_ user-facing change? No. Test-only. ### How was this patch tested? Locally on macOS 26 arm64 / JDK 21: `build/mvn -pl core test -DwildcardSuites=org.apache.spark.scheduler.BarrierTaskContextSuite` — all 13 tests pass; scalastyle clean. Also validated end-to-end on the `Build / Maven (Scala 2.13, JDK 21, MacOS-26)` workflow, running the same `core` matrix job on the identical `macos-26` arm64 runner image: | | `core,launcher,…` job (runs `BarrierTaskContextSuite`) | Result | |---|---|---| | **Before** (`apache/spark` master, without this fix) | [runs/28790673283 · job 85379029552](https://github.com/apache/spark/actions/runs/28790673283/job/85379029552) | ❌ `1078 was not less than or equal to 1000 (BarrierTaskContextSuite.scala:122)` — `Tests: succeeded 4103, failed 1` | | **After** (this branch) | [runs/28829214565 · job 85505367410](https://github.com/HyukjinKwon/spark/actions/runs/28829214565/job/85505367410) | ✅ `Tests: succeeded 4104, failed 0`; the three barrier tests all pass | (To run the guarded workflow on the fork, its `if: github.repository == 'apache/spark'` condition was relaxed on a throwaway commit that is **not** part of this PR — this PR is the single-file test change only.) ### Was this patch authored or co-authored using generative AI tooling? Yes, drafted with assistance from Claude. Closes#57057 from HyukjinKwon/deflake-barrier-macos26. Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com> (cherry picked from commit e887fd5) Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com>
HyukjinKwon added a commit
that referenced
this pull request
Jul 7, 2026
…skew assertions on macOS-26 ### What changes were proposed in this pull request? Widen the post-sync wall-clock skew tolerance in `BarrierTaskContextSuite` from a hard-coded `<= 1000` (ms) to a documented `maxSyncSkewMs = 3000` constant, applied to the three tests that assert how closely tasks finish a `barrier()` / `allGather()` global sync: - `global sync by barrier() call` - `successively sync with allGather and barrier` - `support multiple barrier() call within a single task` ### Why are the changes needed? The `Build / Maven (Scala 2.13, JDK 21, MacOS-26)` scheduled workflow has been failing on every recent run. One of the reproducing failures is: ``` BarrierTaskContextSuite: - successively sync with allGather and barrier *** FAILED *** 1078 was not less than or equal to 1000 (BarrierTaskContextSuite.scala:122) ``` These tests capture `System.currentTimeMillis()` in each task immediately after a barrier/allGather returns and assert `times.max - times.min <= 1000`. **That skew is bounded below by the barrier client's own polling granularity, not by sync correctness.** `BarrierTaskContext.runBarrier` waits for the coordinator RPC in a `Thread.sleep(1000)` loop ([`BarrierTaskContext.scala:102`](https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/BarrierTaskContext.scala#L102)), so two tasks can observe the release up to ~1s apart from the poll interval alone — before any thread-scheduling or GC jitter on a busy CI host is added. Evidence gathered: - SPARK-49983 (2024, PR #48487) previously saw `1038` and only *halved the pre-barrier sleep*. That reduces arrival spread but cannot reduce poll-granularity skew, so the flake persists on slower runners like `macos-26` arm64. - Reproduced locally on **macOS 26 arm64 / JDK 21** (the same OS/arch as the failing runner): measured post-sync skew up to **994 ms while the machine was idle** — i.e. essentially zero margin under the old `1000` bound, so any CI load tips it over. The new bound (twice the 1 s poll interval plus a jitter allowance) preserves the test's intent — tasks stay loosely in lockstep and none can race an entire extra sync ahead — without coupling the assertion to a razor-thin wall-clock margin. ### Scope note This PR fixes only the `BarrierTaskContextSuite` flake. The same workflow also exhibits other, independent flakes (Kafka `stress test for failOnDataLoss=false` topic-deletion timeout, mllib GMM FP-tolerance on arm64, `StateStoreSuite.maintenance` timeout). Those are out of scope here and would be separate changes. (The earlier `AF_UNIX path too long` UDF-worker flake was already fixed upstream by SPARK-57949.) ### Does this PR introduce _any_ user-facing change? No. Test-only. ### How was this patch tested? Locally on macOS 26 arm64 / JDK 21: `build/mvn -pl core test -DwildcardSuites=org.apache.spark.scheduler.BarrierTaskContextSuite` — all 13 tests pass; scalastyle clean. Also validated end-to-end on the `Build / Maven (Scala 2.13, JDK 21, MacOS-26)` workflow, running the same `core` matrix job on the identical `macos-26` arm64 runner image: | | `core,launcher,…` job (runs `BarrierTaskContextSuite`) | Result | |---|---|---| | **Before** (`apache/spark` master, without this fix) | [runs/28790673283 · job 85379029552](https://github.com/apache/spark/actions/runs/28790673283/job/85379029552) | ❌ `1078 was not less than or equal to 1000 (BarrierTaskContextSuite.scala:122)` — `Tests: succeeded 4103, failed 1` | | **After** (this branch) | [runs/28829214565 · job 85505367410](https://github.com/HyukjinKwon/spark/actions/runs/28829214565/job/85505367410) | ✅ `Tests: succeeded 4104, failed 0`; the three barrier tests all pass | (To run the guarded workflow on the fork, its `if: github.repository == 'apache/spark'` condition was relaxed on a throwaway commit that is **not** part of this PR — this PR is the single-file test change only.) ### Was this patch authored or co-authored using generative AI tooling? Yes, drafted with assistance from Claude. Closes#57057 from HyukjinKwon/deflake-barrier-macos26. Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com> (cherry picked from commit e887fd5) Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com>
HyukjinKwon added a commit
that referenced
this pull request
Jul 7, 2026
…skew assertions on macOS-26 ### What changes were proposed in this pull request? Widen the post-sync wall-clock skew tolerance in `BarrierTaskContextSuite` from a hard-coded `<= 1000` (ms) to a documented `maxSyncSkewMs = 3000` constant, applied to the three tests that assert how closely tasks finish a `barrier()` / `allGather()` global sync: - `global sync by barrier() call` - `successively sync with allGather and barrier` - `support multiple barrier() call within a single task` ### Why are the changes needed? The `Build / Maven (Scala 2.13, JDK 21, MacOS-26)` scheduled workflow has been failing on every recent run. One of the reproducing failures is: ``` BarrierTaskContextSuite: - successively sync with allGather and barrier *** FAILED *** 1078 was not less than or equal to 1000 (BarrierTaskContextSuite.scala:122) ``` These tests capture `System.currentTimeMillis()` in each task immediately after a barrier/allGather returns and assert `times.max - times.min <= 1000`. **That skew is bounded below by the barrier client's own polling granularity, not by sync correctness.** `BarrierTaskContext.runBarrier` waits for the coordinator RPC in a `Thread.sleep(1000)` loop ([`BarrierTaskContext.scala:102`](https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/BarrierTaskContext.scala#L102)), so two tasks can observe the release up to ~1s apart from the poll interval alone — before any thread-scheduling or GC jitter on a busy CI host is added. Evidence gathered: - SPARK-49983 (2024, PR #48487) previously saw `1038` and only *halved the pre-barrier sleep*. That reduces arrival spread but cannot reduce poll-granularity skew, so the flake persists on slower runners like `macos-26` arm64. - Reproduced locally on **macOS 26 arm64 / JDK 21** (the same OS/arch as the failing runner): measured post-sync skew up to **994 ms while the machine was idle** — i.e. essentially zero margin under the old `1000` bound, so any CI load tips it over. The new bound (twice the 1 s poll interval plus a jitter allowance) preserves the test's intent — tasks stay loosely in lockstep and none can race an entire extra sync ahead — without coupling the assertion to a razor-thin wall-clock margin. ### Scope note This PR fixes only the `BarrierTaskContextSuite` flake. The same workflow also exhibits other, independent flakes (Kafka `stress test for failOnDataLoss=false` topic-deletion timeout, mllib GMM FP-tolerance on arm64, `StateStoreSuite.maintenance` timeout). Those are out of scope here and would be separate changes. (The earlier `AF_UNIX path too long` UDF-worker flake was already fixed upstream by SPARK-57949.) ### Does this PR introduce _any_ user-facing change? No. Test-only. ### How was this patch tested? Locally on macOS 26 arm64 / JDK 21: `build/mvn -pl core test -DwildcardSuites=org.apache.spark.scheduler.BarrierTaskContextSuite` — all 13 tests pass; scalastyle clean. Also validated end-to-end on the `Build / Maven (Scala 2.13, JDK 21, MacOS-26)` workflow, running the same `core` matrix job on the identical `macos-26` arm64 runner image: | | `core,launcher,…` job (runs `BarrierTaskContextSuite`) | Result | |---|---|---| | **Before** (`apache/spark` master, without this fix) | [runs/28790673283 · job 85379029552](https://github.com/apache/spark/actions/runs/28790673283/job/85379029552) | ❌ `1078 was not less than or equal to 1000 (BarrierTaskContextSuite.scala:122)` — `Tests: succeeded 4103, failed 1` | | **After** (this branch) | [runs/28829214565 · job 85505367410](https://github.com/HyukjinKwon/spark/actions/runs/28829214565/job/85505367410) | ✅ `Tests: succeeded 4104, failed 0`; the three barrier tests all pass | (To run the guarded workflow on the fork, its `if: github.repository == 'apache/spark'` condition was relaxed on a throwaway commit that is **not** part of this PR — this PR is the single-file test change only.) ### Was this patch authored or co-authored using generative AI tooling? Yes, drafted with assistance from Claude. Closes#57057 from HyukjinKwon/deflake-barrier-macos26. Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com> (cherry picked from commit e887fd5) Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com>
HyukjinKwon added a commit
that referenced
this pull request
Jul 7, 2026
…skew assertions on macOS-26 ### What changes were proposed in this pull request? Widen the post-sync wall-clock skew tolerance in `BarrierTaskContextSuite` from a hard-coded `<= 1000` (ms) to a documented `maxSyncSkewMs = 3000` constant, applied to the three tests that assert how closely tasks finish a `barrier()` / `allGather()` global sync: - `global sync by barrier() call` - `successively sync with allGather and barrier` - `support multiple barrier() call within a single task` ### Why are the changes needed? The `Build / Maven (Scala 2.13, JDK 21, MacOS-26)` scheduled workflow has been failing on every recent run. One of the reproducing failures is: ``` BarrierTaskContextSuite: - successively sync with allGather and barrier *** FAILED *** 1078 was not less than or equal to 1000 (BarrierTaskContextSuite.scala:122) ``` These tests capture `System.currentTimeMillis()` in each task immediately after a barrier/allGather returns and assert `times.max - times.min <= 1000`. **That skew is bounded below by the barrier client's own polling granularity, not by sync correctness.** `BarrierTaskContext.runBarrier` waits for the coordinator RPC in a `Thread.sleep(1000)` loop ([`BarrierTaskContext.scala:102`](https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/BarrierTaskContext.scala#L102)), so two tasks can observe the release up to ~1s apart from the poll interval alone — before any thread-scheduling or GC jitter on a busy CI host is added. Evidence gathered: - SPARK-49983 (2024, PR #48487) previously saw `1038` and only *halved the pre-barrier sleep*. That reduces arrival spread but cannot reduce poll-granularity skew, so the flake persists on slower runners like `macos-26` arm64. - Reproduced locally on **macOS 26 arm64 / JDK 21** (the same OS/arch as the failing runner): measured post-sync skew up to **994 ms while the machine was idle** — i.e. essentially zero margin under the old `1000` bound, so any CI load tips it over. The new bound (twice the 1 s poll interval plus a jitter allowance) preserves the test's intent — tasks stay loosely in lockstep and none can race an entire extra sync ahead — without coupling the assertion to a razor-thin wall-clock margin. ### Scope note This PR fixes only the `BarrierTaskContextSuite` flake. The same workflow also exhibits other, independent flakes (Kafka `stress test for failOnDataLoss=false` topic-deletion timeout, mllib GMM FP-tolerance on arm64, `StateStoreSuite.maintenance` timeout). Those are out of scope here and would be separate changes. (The earlier `AF_UNIX path too long` UDF-worker flake was already fixed upstream by SPARK-57949.) ### Does this PR introduce _any_ user-facing change? No. Test-only. ### How was this patch tested? Locally on macOS 26 arm64 / JDK 21: `build/mvn -pl core test -DwildcardSuites=org.apache.spark.scheduler.BarrierTaskContextSuite` — all 13 tests pass; scalastyle clean. Also validated end-to-end on the `Build / Maven (Scala 2.13, JDK 21, MacOS-26)` workflow, running the same `core` matrix job on the identical `macos-26` arm64 runner image: | | `core,launcher,…` job (runs `BarrierTaskContextSuite`) | Result | |---|---|---| | **Before** (`apache/spark` master, without this fix) | [runs/28790673283 · job 85379029552](https://github.com/apache/spark/actions/runs/28790673283/job/85379029552) | ❌ `1078 was not less than or equal to 1000 (BarrierTaskContextSuite.scala:122)` — `Tests: succeeded 4103, failed 1` | | **After** (this branch) | [runs/28829214565 · job 85505367410](https://github.com/HyukjinKwon/spark/actions/runs/28829214565/job/85505367410) | ✅ `Tests: succeeded 4104, failed 0`; the three barrier tests all pass | (To run the guarded workflow on the fork, its `if: github.repository == 'apache/spark'` condition was relaxed on a throwaway commit that is **not** part of this PR — this PR is the single-file test change only.) ### Was this patch authored or co-authored using generative AI tooling? Yes, drafted with assistance from Claude. Closes#57057 from HyukjinKwon/deflake-barrier-macos26. Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com> (cherry picked from commit e887fd5) Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com>
HyukjinKwon added a commit
that referenced
this pull request
Jul 7, 2026
…skew assertions on macOS-26 ### What changes were proposed in this pull request? Widen the post-sync wall-clock skew tolerance in `BarrierTaskContextSuite` from a hard-coded `<= 1000` (ms) to a documented `maxSyncSkewMs = 3000` constant, applied to the three tests that assert how closely tasks finish a `barrier()` / `allGather()` global sync: - `global sync by barrier() call` - `successively sync with allGather and barrier` - `support multiple barrier() call within a single task` ### Why are the changes needed? The `Build / Maven (Scala 2.13, JDK 21, MacOS-26)` scheduled workflow has been failing on every recent run. One of the reproducing failures is: ``` BarrierTaskContextSuite: - successively sync with allGather and barrier *** FAILED *** 1078 was not less than or equal to 1000 (BarrierTaskContextSuite.scala:122) ``` These tests capture `System.currentTimeMillis()` in each task immediately after a barrier/allGather returns and assert `times.max - times.min <= 1000`. **That skew is bounded below by the barrier client's own polling granularity, not by sync correctness.** `BarrierTaskContext.runBarrier` waits for the coordinator RPC in a `Thread.sleep(1000)` loop ([`BarrierTaskContext.scala:102`](https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/BarrierTaskContext.scala#L102)), so two tasks can observe the release up to ~1s apart from the poll interval alone — before any thread-scheduling or GC jitter on a busy CI host is added. Evidence gathered: - SPARK-49983 (2024, PR #48487) previously saw `1038` and only *halved the pre-barrier sleep*. That reduces arrival spread but cannot reduce poll-granularity skew, so the flake persists on slower runners like `macos-26` arm64. - Reproduced locally on **macOS 26 arm64 / JDK 21** (the same OS/arch as the failing runner): measured post-sync skew up to **994 ms while the machine was idle** — i.e. essentially zero margin under the old `1000` bound, so any CI load tips it over. The new bound (twice the 1 s poll interval plus a jitter allowance) preserves the test's intent — tasks stay loosely in lockstep and none can race an entire extra sync ahead — without coupling the assertion to a razor-thin wall-clock margin. ### Scope note This PR fixes only the `BarrierTaskContextSuite` flake. The same workflow also exhibits other, independent flakes (Kafka `stress test for failOnDataLoss=false` topic-deletion timeout, mllib GMM FP-tolerance on arm64, `StateStoreSuite.maintenance` timeout). Those are out of scope here and would be separate changes. (The earlier `AF_UNIX path too long` UDF-worker flake was already fixed upstream by SPARK-57949.) ### Does this PR introduce _any_ user-facing change? No. Test-only. ### How was this patch tested? Locally on macOS 26 arm64 / JDK 21: `build/mvn -pl core test -DwildcardSuites=org.apache.spark.scheduler.BarrierTaskContextSuite` — all 13 tests pass; scalastyle clean. Also validated end-to-end on the `Build / Maven (Scala 2.13, JDK 21, MacOS-26)` workflow, running the same `core` matrix job on the identical `macos-26` arm64 runner image: | | `core,launcher,…` job (runs `BarrierTaskContextSuite`) | Result | |---|---|---| | **Before** (`apache/spark` master, without this fix) | [runs/28790673283 · job 85379029552](https://github.com/apache/spark/actions/runs/28790673283/job/85379029552) | ❌ `1078 was not less than or equal to 1000 (BarrierTaskContextSuite.scala:122)` — `Tests: succeeded 4103, failed 1` | | **After** (this branch) | [runs/28829214565 · job 85505367410](https://github.com/HyukjinKwon/spark/actions/runs/28829214565/job/85505367410) | ✅ `Tests: succeeded 4104, failed 0`; the three barrier tests all pass | (To run the guarded workflow on the fork, its `if: github.repository == 'apache/spark'` condition was relaxed on a throwaway commit that is **not** part of this PR — this PR is the single-file test change only.) ### Was this patch authored or co-authored using generative AI tooling? Yes, drafted with assistance from Claude. Closes#57057 from HyukjinKwon/deflake-barrier-macos26. Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Hyukjin Kwon <hyukjin.kwon@databricks.com> (cherry picked from commit e887fd5) 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 freeto 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?
This PR aims to fix
BarrierTaskContextSuite.successively sync with allGather and barriertest case to be robust.Why are the changes needed?
The test case asserts the duration of partitions. However, this is flaky because we don't know when a partition is triggered before
barriersync.spark/core/src/test/scala/org/apache/spark/scheduler/BarrierTaskContextSuite.scala
Lines 116 to 118 in 0e75d19
Although we added
TestUtils.waitUntilExecutorsUpat Apache Spark 3.0.0 like the following,let's say a partition starts slowly than
38msand all partitions sleep1sexactly. Then, the test case fails like the following.According to the failure history here (SPARK-49983) and SPARK-31730, the slowness seems to be less than
200mswhen it happens. So, this PR aims to reduce the flakiness by capping the sleep up to 500ms while keeping the1svalidation. There is no test coverage change because this test case focuses on thesuccessively sync with allGather and battier.Does this PR introduce any user-facing change?
No, this is a test-only test case.
How was this patch tested?
Pass the CIs.
Was this patch authored or co-authored using generative AI tooling?
No.