Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 23 additions & 4 deletions .github/workflows/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,20 @@ and has to run after the commit is on main, and `pr_build_linux`, because of
`actions/cache` scoping. A pull request can only restore caches saved on its
own branch or on `main`, and the queue runs on a throwaway
`gh-readonly-queue/*` branch whose caches are deleted with it. Without a push
run, a `Cargo.lock` or `pom.xml` change would leave the cargo-registry, Maven
and TPC-H/TPC-DS caches on `main` stale until the next unrelated change.
run, a `Cargo.lock` or `pom.xml` change would leave the cargo-ci, cargo-debug,
Maven and TPC-H/TPC-DS caches on `main` stale until the next unrelated change.

Warming those caches is the only thing the push run is for, so on `push` the
Linux build runs in **cache-refresh-only** mode: `build-native`,
`linux-test-rust` and the two TPC-H/TPC-DS jobs, each stopping once its cache
entry is written, and nothing else. The lints, the 5x4 `linux-test` matrix and
the TPC query runs are skipped, which takes the push tier from 587
runner-minutes to about 73. Two POLICY outputs express this: `build_linux`
says whether the workflow runs at all, `build_linux_full` whether it runs the
lints and tests too, and `ci.yml` folds the second into the workflow's
`cache-refresh-only` input. `dev/ci/check-ci-config.py` fails if a job is added
to `pr_build_linux.yml` without either the guard or an entry in
`CACHE_REFRESH_JOBS` naming the cache it writes. See issue #5929.

```
pull_request | merge_group | push to main | workflow_dispatch
Expand All @@ -68,7 +80,7 @@ and TPC-H/TPC-DS caches on `main` stale until the next unrelated change.
v v v
PR + queue tier push to main only queue tier, or PR with label
--------------- ----------------- ---------------------------
pr_build_linux (+ push, for cache) docs pr_build_macos run-macos-tests
pr_build_linux (+ push, cache only) docs pr_build_macos run-macos-tests
spark_4_1 (catalyst + sql_core) pr_benchmark_check run-benchmark-check
iceberg_1_11 spark_4_1 sql_hive run-spark-4.1-hive-tests
spark_3_5 run-spark-3.5-tests
Expand Down Expand Up @@ -99,7 +111,7 @@ and TPC-H/TPC-DS caches on `main` stale until the next unrelated change.
| -------------------- | ---------------------------------------------------------------------------------------------------------------------- | ----------------------------------- |
| `preflight` | every PR / merge group / push / dispatch / label | none (always runs) |
| `changes` | every PR / merge group / push / dispatch / label | runs `dev/ci/compute-changes.py` |
| `pr_build_linux` | PR, merge group or push to main, paths matched | `dev/ci/compute-changes.py` |
| `pr_build_linux` | PR, merge group or push to main, paths matched; on push only the cache-writing jobs, via `build_linux_full` | `dev/ci/compute-changes.py` |
| `pr_build_macos` | merge group, **or** PR with `run-macos-tests` | `dev/ci/compute-changes.py` |
| `pr_benchmark_check` | merge group, **or** PR with `run-benchmark-check` | benchmark sources only |
| `docs` | push to main, paths matched | `.asf.yaml`, `docs/**`, `docs.yaml` |
Expand Down Expand Up @@ -200,6 +212,13 @@ in `dev/ci/compute-changes.py`:

Moving a suite between the PR and queue tiers is a one-word edit to `POLICY`.

An output does not have to map one-to-one onto a job. Two outputs can feed a
single call when part of a workflow belongs in a different tier from the rest:
`spark_4_1` / `spark_4_1_hive` select which module shards the one Spark 4.1
build runs, and `build_linux` / `build_linux_full` select whether the Linux
build runs everything or only the jobs that populate `main`'s caches. Both
share their `FILTERS` list by assignment so the two entries cannot drift.

So adding a suite, moving sources, or changing when something runs is an edit
to one of those two tables, not to ten `${{ }}` expressions. Keeping the policy
in Python is also what makes it testable: GitHub expressions cannot be
Expand Down
13 changes: 12 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@
# push to main would double the cost of every merge. Two exceptions: `docs`
# deploys to asf-site and must run after the commit is on main, and the Linux
# build runs on push so that main's actions/cache entries stay fresh (caches
# saved on the queue's throwaway branch are deleted with it).
# saved on the queue's throwaway branch are deleted with it). That second
# exception runs in `cache-refresh-only` mode: the cache writers and nothing
# else, because the queue has already tested the tree that landed.

name: CI

Expand Down Expand Up @@ -157,6 +159,7 @@ jobs:
runs-on: ubuntu-slim
outputs:
build_linux: ${{ steps.compute.outputs.build_linux }}
build_linux_full: ${{ steps.compute.outputs.build_linux_full }}
build_macos: ${{ steps.compute.outputs.build_macos }}
benchmark: ${{ steps.compute.outputs.benchmark }}
docs: ${{ steps.compute.outputs.docs }}
Expand Down Expand Up @@ -233,6 +236,14 @@ jobs:
needs: changes
if: needs.changes.outputs.build_linux == 'true'
uses: ./.github/workflows/pr_build_linux.yml
# Two POLICY outputs feed one call, the same shape as spark_4_1 below.
# `build_linux` decides whether the workflow runs at all; `build_linux_full`
# decides whether it runs the lints and the 5x4 test matrix as well as the
# jobs that populate main's actions/cache entries. Only push to main sets
# the first without the second, which is the whole point: the queue has
# already tested that tree, so the push run is there for the caches alone.
with:
cache-refresh-only: ${{ needs.changes.outputs.build_linux_full != 'true' }}

pr_build_macos:
name: PR Build (macOS)
Expand Down
60 changes: 55 additions & 5 deletions .github/workflows/pr_build_linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,33 @@ name: PR Build (Linux)

# Reusable: invoked by ci.yml. Triggering, path filters, and concurrency
# live in the umbrella workflow.
#
# Two modes, selected by `cache-refresh-only`:
#
# false the full pipeline: the lints, the 5x4 linux-test matrix, the Rust
# tests and the TPC-H/TPC-DS correctness runs. Pull requests and the
# merge queue.
# true only the jobs that write an actions/cache entry, and each only far
# enough to populate it. This is the push-to-main tier, where the
# queue has already tested the exact tree that landed and the sole
# remaining purpose of the run is to leave main's caches warm for the
# next pull request. See the `build_linux` / `build_linux_full`
# comments in dev/ci/compute-changes.py for why that tier exists.
#
# Keeping both modes in this file is deliberate. The cache keys are the whole
# point of the push tier, and a separate cache-warming workflow would have to
# repeat every one of them; here they cannot drift apart. dev/ci/check-ci-config.py
# pins which jobs survive `cache-refresh-only`, so a job added without the guard
# cannot quietly put the full pipeline back on the push tier.
on:
workflow_call:
inputs:
cache-refresh-only:
description: >-
Populate main's actions/cache entries without running any lint or
test. Set by ci.yml on push to main.
type: boolean
default: false

env:
RUST_VERSION: stable
Expand All @@ -32,7 +57,9 @@ env:

jobs:

# Fast lint check - gates all other jobs
# Fast lint check - gates all other jobs. Runs in cache-refresh-only mode too:
# it is 40 seconds, and `build-native` and `linux-test-rust` both `needs:` it,
# so skipping it would skip the two jobs the push tier exists to run.
lint:
name: Lint
runs-on: ubuntu-24.04
Expand All @@ -54,6 +81,7 @@ jobs:
# set, including the semantic rules, still runs in lint-java.
scalafix-syntactic:
name: Lint Scala (syntactic)
if: ${{ !inputs.cache-refresh-only }}
runs-on: ubuntu-24.04
steps:
- uses: actions/checkout@v7
Expand Down Expand Up @@ -94,6 +122,7 @@ jobs:
lint-java:
needs: lint
name: Lint Java (${{ matrix.profile.name }})
if: ${{ !inputs.cache-refresh-only }}
runs-on: ubuntu-24.04
container:
image: amd64/rust
Expand Down Expand Up @@ -173,6 +202,7 @@ jobs:
build-spark-4-1:
needs: lint
name: Build Spark 4.1, JDK 17
if: ${{ !inputs.cache-refresh-only }}
runs-on: ubuntu-24.04
container:
image: amd64/rust
Expand Down Expand Up @@ -204,6 +234,7 @@ jobs:
celeborn-reflection-compatibility:
needs: lint
name: Celeborn ${{ matrix.celeborn_version }} reflection compatibility
if: ${{ !inputs.cache-refresh-only }}
runs-on: ubuntu-24.04
container:
image: amd64/rust
Expand Down Expand Up @@ -247,7 +278,9 @@ jobs:
-Dsuites=org.apache.comet.shuffle.CelebornReflectionCompatibilitySuite \
-DfailIfNoTests=false

# Build native library once and share with all test jobs
# Build native library once and share with all test jobs. Also the owner of
# main's `cargo-ci` cache entry, which is why it runs in cache-refresh-only
# mode: rebuilding native/target from cold costs every pull request ~20 min.
build-native:
needs: lint
name: Build Native Library
Expand Down Expand Up @@ -299,7 +332,12 @@ jobs:
native/target
key: ${{ runner.os }}-cargo-ci-${{ hashFiles('native/**/Cargo.lock', 'native/**/Cargo.toml') }}-${{ hashFiles('native/**/*.rs') }}

# Run Rust tests (runs in parallel with build-native, uses debug builds)
# Run Rust tests (runs in parallel with build-native, uses debug builds).
# Owns main's `cargo-debug` cache entry, so it runs in cache-refresh-only mode
# as well. The tests themselves are a small slice of its runtime -- almost all
# of it is the compile that populates the cache -- so there is nothing to gain
# from a compile-only variant, and running them keeps the cache contents
# identical to what a pull request restores.
linux-test-rust:
needs: lint
name: ubuntu-latest/rust-test
Expand Down Expand Up @@ -342,6 +380,7 @@ jobs:

linux-test:
needs: build-native
if: ${{ !inputs.cache-refresh-only }}
strategy:
matrix:
# the goal with these profiles is to get coverage of all Java, Scala, and Spark
Expand Down Expand Up @@ -551,7 +590,11 @@ jobs:
upload-test-reports: true
skip-native-build: true

# TPC-H correctness test - verifies benchmark queries produce correct results
# TPC-H correctness test - verifies benchmark queries produce correct results.
# Runs in cache-refresh-only mode for its two cache entries: the SF=1 dataset,
# which is expensive to regenerate, and main's shared `java-maven` entry. Only
# the query run is skipped there; everything up to and including data
# generation still has to happen for the dataset cache to exist.
verify-benchmark-results-tpch:
needs: build-native
name: Verify TPC-H Results
Expand Down Expand Up @@ -607,11 +650,15 @@ jobs:
cd spark && MAVEN_OPTS='-Xmx20g' ../mvnw -B -Prelease exec:java -Dexec.mainClass="org.apache.spark.sql.GenTPCHData" -Dexec.classpathScope="test" -Dexec.cleanupDaemonThreads="false" -Dexec.args="--location `pwd`/.. --scaleFactor 1 --numPartitions 1 --overwrite"

- name: Run TPC-H queries
if: ${{ !inputs.cache-refresh-only }}
run: |
SPARK_HOME=`pwd` SPARK_TPCH_DATA=`pwd`/tpch/sf1_parquet ./mvnw -B -Prelease -Dsuites=org.apache.spark.sql.CometTPCHQuerySuite test

# TPC-DS correctness tests - verifies benchmark queries produce correct results.
# The three join strategies run sequentially in one job so the project is built once.
# The three join strategies run sequentially in one job so the project is built
# once. Runs in cache-refresh-only mode for the same reason as the TPC-H job;
# there the three query passes are skipped and only the dataset generation and
# the Maven cache remain.
verify-benchmark-results-tpcds:
needs: build-native
name: Verify TPC-DS Results
Expand Down Expand Up @@ -681,6 +728,7 @@ jobs:
cd spark && MAVEN_OPTS='-Xmx20g' ../mvnw -B -Prelease exec:java -Dexec.mainClass="org.apache.spark.sql.GenTPCDSData" -Dexec.classpathScope="test" -Dexec.cleanupDaemonThreads="false" -Dexec.args="--dsdgenDir `pwd`/../tpcds-kit/tools --location `pwd`/../tpcds-sf-1 --scaleFactor 1 --numPartitions 1"

- name: Run TPC-DS queries (Sort merge join)
if: ${{ !inputs.cache-refresh-only }}
run: |
SPARK_HOME=`pwd` SPARK_TPCDS_DATA=`pwd`/tpcds-sf-1 ./mvnw -B -Prelease -Dsuites=org.apache.spark.sql.CometTPCDSQuerySuite test
env:
Expand All @@ -689,13 +737,15 @@ jobs:
spark.sql.join.preferSortMergeJoin=true

- name: Run TPC-DS queries (Broadcast hash join)
if: ${{ !inputs.cache-refresh-only }}
run: |
SPARK_HOME=`pwd` SPARK_TPCDS_DATA=`pwd`/tpcds-sf-1 ./mvnw -B -Prelease -Dsuites=org.apache.spark.sql.CometTPCDSQuerySuite test
env:
SPARK_TPCDS_JOIN_CONF: |
spark.sql.autoBroadcastJoinThreshold=10485760

- name: Run TPC-DS queries (Shuffled hash join)
if: ${{ !inputs.cache-refresh-only }}
run: |
SPARK_HOME=`pwd` SPARK_TPCDS_DATA=`pwd`/tpcds-sf-1 ./mvnw -B -Prelease -Dsuites=org.apache.spark.sql.CometTPCDSQuerySuite test
env:
Expand Down
Loading
Loading