Skip to content

Commit d926e21

Browse files
authored
chore: refactor CI to have centralized SBT action (apache#4643)
1 parent 0031c60 commit d926e21

3 files changed

Lines changed: 99 additions & 22 deletions

File tree

.github/actions/setup-spark-builder/action.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,15 @@ inputs:
2828
description: 'Skip native build (when using pre-built artifact)'
2929
required: false
3030
default: 'false'
31+
skip-spark-clone:
32+
description: 'Skip cloning Spark and applying patches (when apache-spark/ is pre-staged from a build-jvm artifact)'
33+
required: false
34+
default: 'false'
3135
runs:
3236
using: "composite"
3337
steps:
3438
- name: Clone Spark repo
39+
if: ${{ inputs.skip-spark-clone != 'true' }}
3540
uses: actions/checkout@v6
3641
with:
3742
repository: apache/spark
@@ -40,6 +45,7 @@ runs:
4045
fetch-depth: 1
4146

4247
- name: Setup Spark for Comet
48+
if: ${{ inputs.skip-spark-clone != 'true' }}
4349
shell: bash
4450
run: |
4551
cd apache-spark

.github/workflows/spark_sql_test_reusable.yml

Lines changed: 93 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -52,20 +52,28 @@ env:
5252

5353
jobs:
5454

55-
# Build native library once and share with all test jobs
56-
build-native:
57-
name: Build Native Library
55+
# Build the native library AND pre-compile Spark sources + Test classes in a
56+
# single runner, then publish two artifacts the matrix consumes:
57+
# - native-lib-linux: libcomet.so (~50 MB)
58+
# - jvm-compiled-spark-<full>-jdk<N>: apache-spark.tar.gz (sources +
59+
# target/ + Zinc state, ~500 MB-1 GB)
60+
# Combining them avoids a second runner cold-start and an extra inter-job
61+
# artifact round-trip for the native lib, since the JVM build already
62+
# depends on it (the Comet Maven install bundles libcomet.so into the
63+
# Comet JAR before SBT resolves Spark's classpath).
64+
build:
65+
name: Build Native + JVM Test Classes
5866
runs-on: ubuntu-24.04
5967
container:
6068
image: amd64/rust
6169
steps:
6270
- uses: actions/checkout@v6
6371

64-
- name: Setup Rust toolchain
72+
- name: Setup Rust & Java toolchain
6573
uses: ./.github/actions/setup-builder
6674
with:
6775
rust-version: ${{ env.RUST_VERSION }}
68-
jdk-version: 17
76+
jdk-version: ${{ inputs.java }}
6977

7078
- name: Restore Cargo cache
7179
uses: actions/cache/restore@v5
@@ -85,13 +93,6 @@ jobs:
8593
env:
8694
RUSTFLAGS: "-Ctarget-cpu=x86-64-v3 -Clink-arg=-fuse-ld=bfd"
8795

88-
- name: Upload native library
89-
uses: actions/upload-artifact@v7
90-
with:
91-
name: native-lib-linux
92-
path: native/target/ci/libcomet.so
93-
retention-days: 1
94-
9596
- name: Save Cargo cache
9697
uses: actions/cache/save@v5
9798
if: github.ref == 'refs/heads/main'
@@ -102,18 +103,75 @@ jobs:
102103
native/target
103104
key: ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }}
104105

106+
- name: Upload native library
107+
uses: actions/upload-artifact@v7
108+
with:
109+
name: native-lib-linux
110+
path: native/target/ci/libcomet.so
111+
retention-days: 1
112+
113+
- name: Stage native library at release path
114+
run: |
115+
# setup-spark-builder's `mvnw install -DskipTests` (skip-native-build
116+
# path) bundles native/target/release/libcomet.so into the Comet JAR.
117+
# We built with --profile ci to avoid LTO, so the file lives at
118+
# native/target/ci/. Copy it to where the Maven build expects it.
119+
mkdir -p native/target/release
120+
cp native/target/ci/libcomet.so native/target/release/libcomet.so
121+
122+
- name: Setup Spark
123+
uses: ./.github/actions/setup-spark-builder
124+
with:
125+
spark-version: ${{ inputs.spark-full }}
126+
spark-short-version: ${{ inputs.spark-short }}
127+
skip-native-build: true
128+
129+
- name: Pre-compile Spark Test classes
130+
run: |
131+
cd apache-spark
132+
# Mirror the workaround from `Run Spark tests` below: Comet's mvn
133+
# install populates partial Parquet entries (main JAR + POM but no
134+
# `*-tests.jar` classifier). Coursier then sees the POM in
135+
# mavenLocal, declares the artifact "found locally", and refuses
136+
# to fall back to Maven Central for the missing test classifier.
137+
# Wiping the parquet cache forces a clean remote fetch.
138+
rm -rf /root/.m2/repository/org/apache/parquet
139+
# Compile Test sources for the three subprojects the matrix touches.
140+
# SBT will transitively compile Compile/compile of their dependencies
141+
# plus any Test/compile pulled in by `dependsOn(... % "test->test")`,
142+
# which is enough to satisfy `sql/testOnly`, `hive/testOnly`, and
143+
# `catalyst/test` in the downstream test jobs.
144+
NOLINT_ON_COMPILE=true build/sbt -Dsbt.log.noformat=true -mem 3072 \
145+
'catalyst/Test/compile' \
146+
'sql/Test/compile' \
147+
'hive/Test/compile'
148+
149+
- name: Pack apache-spark/ (sources + compiled output)
150+
run: |
151+
# Preserve mtimes so Zinc's incremental analysis still recognises
152+
# the compiled output as up-to-date in the test jobs. Exclude .git
153+
# because we don't need history downstream and it adds size.
154+
tar -czpf apache-spark.tar.gz \
155+
--exclude='apache-spark/.git' \
156+
apache-spark
157+
158+
- name: Upload JVM compile artifact
159+
uses: actions/upload-artifact@v7
160+
with:
161+
name: jvm-compiled-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }}
162+
path: apache-spark.tar.gz
163+
retention-days: 1
164+
105165
spark-sql-test:
106-
needs: build-native
166+
needs: build
107167
strategy:
108168
matrix:
109169
module:
110170
- {name: "catalyst", args1: "catalyst/test", args2: ""}
111171
# sql_core-* set HEAP_SIZE / METASPACE_SIZE so SparkBuild.scala caps
112-
# forked test JVMs below the Spark defaults (-Xmx4g, MaxMetaspaceSize=1300m),
113-
# leaving headroom for SBT (SBT_MEM=3072) inside the 7 GB runner budget.
114-
- {name: "sql_core-1", args1: "", args2: "sql/testOnly * -- -l org.apache.spark.tags.ExtendedSQLTest -l org.apache.spark.tags.SlowSQLTest", heap: "3g", metaspace: "1g"}
115-
- {name: "sql_core-2", args1: "", args2: "sql/testOnly * -- -n org.apache.spark.tags.ExtendedSQLTest", heap: "3g", metaspace: "1g"}
116-
- {name: "sql_core-3", args1: "", args2: "sql/testOnly * -- -n org.apache.spark.tags.SlowSQLTest", heap: "3g", metaspace: "1g"}
172+
- {name: "sql_core-1", args1: "", args2: "sql/testOnly * -- -l org.apache.spark.tags.ExtendedSQLTest -l org.apache.spark.tags.SlowSQLTest", heap: "4g", metaspace: "1g"}
173+
- {name: "sql_core-2", args1: "", args2: "sql/testOnly * -- -n org.apache.spark.tags.ExtendedSQLTest", heap: "4g", metaspace: "1g"}
174+
- {name: "sql_core-3", args1: "", args2: "sql/testOnly * -- -n org.apache.spark.tags.SlowSQLTest", heap: "4g", metaspace: "1g"}
117175
- {name: "sql_hive-1", args1: "", args2: "hive/testOnly * -- -l org.apache.spark.tags.ExtendedHiveTest -l org.apache.spark.tags.SlowHiveTest"}
118176
- {name: "sql_hive-2", args1: "", args2: "hive/testOnly * -- -n org.apache.spark.tags.ExtendedHiveTest"}
119177
- {name: "sql_hive-3", args1: "", args2: "hive/testOnly * -- -n org.apache.spark.tags.SlowHiveTest"}
@@ -134,12 +192,22 @@ jobs:
134192
with:
135193
name: native-lib-linux
136194
path: native/target/release/
195+
- name: Download JVM compile artifact
196+
uses: actions/download-artifact@v8
197+
with:
198+
name: jvm-compiled-spark-${{ inputs.spark-full }}-jdk${{ inputs.java }}
199+
- name: Extract apache-spark/ (sources + compiled output)
200+
run: |
201+
# Restore mtimes so Zinc skips compilation on the test JVM.
202+
tar -xzpf apache-spark.tar.gz
203+
rm -f apache-spark.tar.gz
137204
- name: Setup Spark
138205
uses: ./.github/actions/setup-spark-builder
139206
with:
140207
spark-version: ${{ inputs.spark-full }}
141208
spark-short-version: ${{ inputs.spark-short }}
142209
skip-native-build: true
210+
skip-spark-clone: true
143211
- name: Run Spark tests
144212
run: |
145213
cd apache-spark
@@ -176,9 +244,13 @@ jobs:
176244
fi
177245
env:
178246
LC_ALL: "C.UTF-8"
179-
# Standard GitHub runners have 7 GB RAM; cap SBT heap so forked test
180-
# JVMs fit alongside it.
181-
SBT_MEM: "3072"
247+
# The build-jvm job pre-compiled Spark sources and Test classes, so
248+
# SBT here only orchestrates `testOnly` — Zinc verifies "no changes"
249+
# against the unpacked apache-spark/ tree and skips compilation. We
250+
# cap SBT heap at 1536 MB (down from 3072 MB) so the freed RAM goes
251+
# to the forked test JVM and OS/container overhead, fixing the
252+
# cgroup-OOM SIGKILLs we saw on sql_core-* under 7 GB runners.
253+
SBT_MEM: "1536"
182254
# Mirror Spark's own JDK 21 / 25 CI workaround. apache/spark's
183255
# build_java21.yml and build_java25.yml set this same env var to
184256
# process-isolate the V1/V2 Parquet and Orc source suites because

spark/src/test/scala/org/apache/spark/sql/benchmark/CometShuffleBenchmark.scala

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ object CometShuffleBenchmark extends CometBenchmarkBase {
5555
"org.apache.spark.sql.comet.execution.shuffle.CometShuffleManager")
5656
.set("spark.comet.columnar.shuffle.async.thread.num", "7")
5757
.set("spark.comet.columnar.shuffle.spill.threshold", "30000")
58-
.set("spark.comet.memoryOverhead", "10g")
5958

6059
val sparkSession = SparkSession.builder
6160
.config(conf)

0 commit comments

Comments
 (0)