Skip to content

[VL][Delta] Add Delta Spark UT pipeline gated against a known-failures baseline - #12388

Merged
zhouyuan merged 28 commits into
apache:mainfrom
felipepessoto:delta_pipeline
Aug 10, 2026
Merged

[VL][Delta] Add Delta Spark UT pipeline gated against a known-failures baseline#12388
zhouyuan merged 28 commits into
apache:mainfrom
felipepessoto:delta_pipeline

Conversation

@felipepessoto

@felipepessoto felipepessoto commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Fix #9296.

What changes are proposed in this pull request?

Adds a CI pipeline that runs delta-io/delta's spark ScalaTest suite against the Gluten Velox bundle, so we can validate Gluten against a real Delta release and catch regressions over time.

Running the Delta UTs on Gluten produces many failures today. Not because Gluten declines to offload a plan -- that should fall back to vanilla Spark and the test should still pass. Some are real gaps: fallback not happening where it should, metrics that differ from vanilla, and native-side bugs. Others are expected rather than defects: a test that asserts on the query plan sees a different plan once the scan or operators are offloaded. A plain "red on any failure" gate would be useless. Instead, the pipeline keeps a committed baseline of known failures and gates each run against it:

  • regression -- a test fails that is not in the baseline -> the shard fails.
  • expected -- a failing test that is in the baseline -> ignored.
  • now-passing -- a baseline test that starts passing -> fails the shard (keeps the baseline honest), unless fail_on_fixed=false.
  • quarantined -- a flaky test (see below) -> always ignored, whether it passes or fails.

How it works

  1. Runs as its own workflow, not as part of velox_backend_x86.yml. Per PR a paths: filter limits it to Delta-relevant changes (gluten-delta/**, backends-velox/src-delta*/**, and this pipeline's own files), evaluated by GitHub before the run is created, so an unrelated PR costs nothing. A nightly schedule (05:00 UTC) runs it against the default branch so regressions from general Velox/core changes are still caught daily, and workflow_dispatch covers manual runs. It builds the Velox native lib and then assembles the gluten-velox-bundle fat jar (Spark 4.1 + Scala 2.13 + JDK 17, Delta profile).
  2. Clones delta-io/delta at a release tag (currently v4.2.0), drops the bundle onto the spark project's test classpath, patches DeltaSQLCommandTest to register GlutenPlugin, and cherry-picks two merged upstream Delta test-only fixes ([Spark] [Test] Collect scans by FileSourceScanLike in ScanReportHelper delta-io/delta#7104 + [VL] Remove a limit for BHJ in stage fallback policy #7105) that widen FileSourceScanExec checks to FileSourceScanLike so Gluten's transformed plan is recognized.
  3. Runs sbt spark/test sharded by suite across 8 shards (4 forked test JVMs each, ~32-way parallelism), with ScalaTest's JUnit XML reporter enabled, then gates each shard with compare-test-results.py against known-failures.txt. A final job aggregates all shards into a single ready-to-commit baseline and flags stale entries.

Flaky tests

A handful of Delta MERGE-with-deletion-vector tests fail non-deterministically (the same bundle passes them on one run, fails them on the next). They can't live in the baseline -- baselining them reds the gate when they pass, leaving them out reds it when they fail. Root cause is a native bug: the DV bitmap aggregator aborts on an invalid Long.MAX_VALUE row index during a DV-writing MERGE (RoaringBitmapArray.cpp addSafe), and it lands on a different *DVs*Suite MERGE test each run.

The gate quarantines such tests -- a quarantined failure is neutral (never a regression, never now-passing, excluded from the regenerated baseline) -- two ways:

  • flaky-tests.txt -- by test name (<suite-glob>#<test>), for ordinary flakes.
  • flaky-error-patterns.txt -- by error signature: a regex matched against the failed test's <failure>/<error> text. Because the DV-bitmap bug hits a different test each run, matching by name was whack-a-mole; the signature ignores it on whichever test it lands, while a different real failure in the same suite is still caught. This is an interim measure, removed once the native row-index bug is fixed.

Files

File Purpose
.github/workflows/delta_spark_ut.yml The Delta workflow: build native lib -> build bundle -> shard tests -> gate -> aggregate. Triggers on Delta paths per PR, nightly, or manually.
.github/workflows/util/delta-spark-ut/setup-delta.sh Clones Delta, injects the Gluten bundle, patches DeltaSQLCommandTest, cherry-picks the upstream test fixes.
.github/workflows/util/delta-spark-ut/run-delta-tests.sh One shard's test body: runs sbt spark/test (tuned JVM/heap flags) under a hang watchdog, prints memory forensics, then gates the results. Keeps the workflow step readable.
.github/workflows/util/delta-spark-ut/java-test-args.sh Shared JDK-17/Arrow/Netty JVM flags; sourced by the shard run and by local dev runs.
.github/workflows/util/delta-spark-ut/compare-test-results.py Parses JUnit XML and enforces / seeds / aggregates against the baseline (stdlib only).
.github/workflows/util/delta-spark-ut/known-failures.txt Committed baseline of currently-expected failures (# comments per line).
.github/workflows/util/delta-spark-ut/flaky-tests.txt Quarantine list by test name.
.github/workflows/util/delta-spark-ut/flaky-error-patterns.txt Quarantine list by error signature (regex vs failure text).
.github/workflows/util/delta-spark-ut/README.md Documents the gate, flaky quarantine, bootstrapping, and baseline refresh.

Operational hardening

  • JDK 17 + Arrow/Netty: forked test JVMs get the --add-opens set plus -Dio.netty.tryReflectionSetAccessible=true (otherwise Arrow's allocator fails to initialize). These live in java-test-args.sh so CI and local runs share one definition.
  • Heap tuning: forked-test heap and the sbt launcher's idle G1 behavior are tuned to keep the ~16 GB runner under the cgroup OOM threshold.
  • Hang watchdog: a per-shard watchdog dumps threads and kills a forked test JVM that has gone silent too long, so a wedged suite can't stall the whole job.
  • Fail-fast on bad data: the gate errors out (rather than passing silently) when a shard produced no JUnit reports, when a TEST-*.xml is truncated/corrupt, or -- in the aggregate job -- when fewer than the expected number of shards contributed gate lists (so a partial run can't shrink the baseline).
  • DeletionVectorsSuite 2B-row tests: two tests build/read/delete a 2-billion-row table and balloon the fork to ~13 GB of native memory (Velox row-index materialization), OOM-killing it and hanging the shard. They are force-failed (with a clear message) rather than silently ignored, so the gap stays visible until the native memory blow-up is fixed.

Scope / known limitations

  • Velox backend, x86 only; Delta v4.2.0 / Spark 4.1 / Scala 2.13 / JDK 17.
  • The baseline reflects the current set of known Delta-on-Gluten failures; refresh it via a workflow_dispatch run with update_baseline=true.
  • Future work -- Delta 4.3.0: attempted, but the bundle (compiled against Delta 4.1.0) hits a binary-incompatible Delta change (IdentityColumn.logTableWrite first param Snapshot -> SnapshotDescriptor), which NoSuchMethodErrors on every write. Supporting 4.3.0 needs the bundle built against 4.3.0; tracked as follow-up.

How was this patch tested?

This change is CI. The Delta suite runs as its own workflow -- per PR when Delta-relevant paths change, nightly at 05:00 UTC against the default branch, and on demand via workflow_dispatch. In the latest runs all shards pass against the committed baseline (failures limited to known-failures entries; no regressions).

19,073 Delta tests run (18,338 passed / 735 failed).

Main failures (735 baseline):

Fixed since the first draft (#12371): the MatchError List() DataSkipping-empty-stats failures (a FileSourceScanExec match) were fixed by cherry-picking merged Delta PRs 7104 + 7105 ( FileSourceScanExec → FileSourceScanLike ) during test setup.

Fixed: Variant / UDT type offload. The Velox variant / user-defined-type offload gap (Arrow Unsupported data type: variant at SparkArrowUtil.scala:60) was fixed upstream, so 74 tests (65 Variant + 9 user-defined-type) now pass and were removed from the baseline (810 -> 736). The 6 remaining variant auto compact tests are a separate root cause (auto-compact metrics) and stay in the baseline.

Delta Spark UT (Gluten) -- shard count vs test parallelism

Sharding is by suite (MurmurHash3(suiteName) % NUM_SHARDS), so total test work is fixed. The runners are 4-core / ~16 GB. The committed config is 8 shards x 4 forks, which is also what delta-io/delta's own spark_test.yaml uses for these suites.

Config Runner jobs Forks/shard Max shard Billed job-hrs Outcome
16 shards x 1 fork 16 1 ~110 min ~29 green, but far too expensive
4 shards x 4 forks 4 4 163 min 9.6 green
4 shards x 1 fork 4 1 360 min (hit cap) -- cancelled
8 shards x 4 forks (committed) 8 4 102-104 min 10.1-11.0 green

Both 8-shard figures are measured, not projected. Packing 4 forks into each shard is what keeps the cost down: 16 single-fork shards cost ~3x the billed hours for a similar wall-clock. Sharding on top of that is comparatively cheap, because a shard is only ~9 min of fixed setup (clone Delta, patch, compile the test sources) against ~135 min of tests. Across the two 8-shard runs so far, going 4 -> 8 cut the slowest shard by 36-37% (163 -> 102-104 min) for +5-14% billed hours.

The wall-clock gain is smaller than the halving that the fixed-vs-variable split alone suggests, because suites are distributed by MurmurHash3(suiteName) % NUM_SHARDS and then packed into 4 forks within each shard: with fewer suites per shard the packing is lumpier, so the slowest fork dominates for longer. Measured shard durations were 65/73/76/78/79/89/97/104 min in the first run and 62/63/70/73/74/78/83/102 in the second -- the tail is where the extra billed hours go, and it is also why the billed-hours delta varies noticeably between runs.

Memory is unaffected, since each shard is its own runner. No baseline regeneration is needed for a reshard -- the gate compares (suite, test) sets, so it does not depend on how suites are distributed. (The first 8-shard run did surface two DeltaUpdateCatalogSuite tests that had only been failing because of how suites were grouped into forks; they now pass and were dropped from the baseline.)

Was this patch authored or co-authored using generative AI tooling?

Generated-by: GitHub Copilot CLI

Copilot AI review requested due to automatic review settings June 28, 2026 07:33

Copilot AI 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.

Pull request overview

This PR adds a Delta Lake spark module unit-test CI pipeline for the Velox backend, integrating it into the existing velox_backend_x86.yml workflow and gating results against a committed “known failures” baseline so regressions are detected without failing on expected gaps.

Changes:

  • Adds a reusable GitHub Actions workflow to build the Gluten Velox bundle and run sharded Delta ScalaTest suites, with baseline enforcement/aggregation.
  • Adds Delta setup + patching utilities (clone, inject bundle on test classpath, patch tests, apply upstream test fixes, and force-fail known OOM-inducing tests).
  • Adds a committed known-failures.txt baseline plus documentation for maintaining it.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
.github/workflows/velox_backend_x86.yml Invokes the reusable Delta UT workflow and uploads native + Arrow artifacts for reuse.
.github/workflows/delta_spark_ut.yml Implements the reusable (and dispatchable) Delta UT pipeline: build bundle, shard tests, gate, aggregate.
.github/workflows/util/delta-spark-ut/setup-delta.sh Prepares a Delta clone for testing with the Gluten bundle; applies targeted patches/cherry-picks.
.github/workflows/util/delta-spark-ut/compare-test-results.py Parses JUnit XML and enforces/seeds/aggregates results against the known-failures baseline.
.github/workflows/util/delta-spark-ut/known-failures.txt Baseline list of currently expected failing Delta tests under Gluten.
.github/workflows/util/delta-spark-ut/README.md Documents baseline seeding, enforcement behavior, and refresh workflows.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/delta_spark_ut.yml Outdated
Comment thread .github/workflows/util/delta-spark-ut/setup-delta.sh
Comment thread .github/workflows/util/delta-spark-ut/compare-test-results.py Outdated
Copilot AI review requested due to automatic review settings June 28, 2026 09:02

Copilot AI 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.

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

Comment thread .github/workflows/delta_spark_ut.yml Outdated
Comment thread .github/workflows/delta_spark_ut.yml Outdated
Comment thread .github/workflows/util/delta-spark-ut/compare-test-results.py
Copilot AI review requested due to automatic review settings June 28, 2026 22:15

Copilot AI 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.

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

Comment thread .github/workflows/util/delta-spark-ut/compare-test-results.py Outdated
Comment thread .github/workflows/delta_spark_ut.yml Outdated
Comment thread .github/workflows/util/delta-spark-ut/known-failures.txt Outdated
Copilot AI review requested due to automatic review settings June 29, 2026 19:01

Copilot AI 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.

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

Comment thread .github/workflows/util/delta-spark-ut/setup-delta.sh Outdated
Comment thread .github/workflows/util/delta-spark-ut/compare-test-results.py
Copilot AI review requested due to automatic review settings June 30, 2026 05:42

Copilot AI 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.

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.

Comment thread .github/workflows/util/delta-spark-ut/compare-test-results.py
Comment thread .github/workflows/util/delta-spark-ut/compare-test-results.py
Comment thread .github/workflows/util/delta-spark-ut/setup-delta.sh Outdated
Comment thread .github/workflows/velox_backend_x86.yml Outdated

Copilot AI 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.

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings July 3, 2026 09:28
felipepessoto and others added 13 commits July 30, 2026 22:34
…te keys

Addresses three review comments on the Delta Spark UT pipeline:

* Hang watchdog: when it kills a wedged test fork, the running suite plus
  every suite queued behind it in that JVM never run and never write a
  report; since we ignore sbt's exit code, the gate only judged the suites
  that reported and the shard could go green. The watchdog now touches a
  marker on kill and the shard fails afterwards if it exists.

* Hang watchdog: only KILL matched sbt.ForkMain fork(s), never the sbt
  launcher. Before any fork exists (dependency resolution / cold-cache
  compile) sbt can be silent for >15 min; killing the launcher then wasted
  the slot on a confusing "compile/launch failure". All JVMs are still
  dumped for diagnostics. The per-episode dump/kill budget resets when
  output resumes so a transient pre-fork stall can't starve a later fork
  hang.

* Gate: normalize (suite, test) keys parsed from JUnit XML the same way
  baseline/flaky entries are (write_entries collapses CR/LF; parse_entry
  strips the line), so a test name with a trailing newline or surrounding
  whitespace is suppressible by a line pasted from the gate's REGRESSION
  output.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 66a9e40f-3ac8-45be-8fee-a606a22fa098
Reduce GitHub Actions usage for the Delta Spark UT suite (per review feedback
on the pipeline PR):

* Per PR (velox_backend_x86.yml): a new `delta-changes` job runs the suite only
  when the PR touches high-signal Delta paths -- the Delta integration code
  (backends-velox/src-delta*), the gluten-delta module, or this pipeline's own
  files -- or carries the `run-delta-ci` opt-in label. Changes to general
  Velox/core/native code (touched on most PRs) skip it; this drops the per-PR
  trigger rate from ~60% to ~17% of recent commits.

* Nightly (delta_spark_ut.yml): a `schedule` (05:00 UTC) runs the full suite
  against the latest default branch, so regressions from the skipped-per-PR
  paths are still caught daily. It builds its own native lib (no caller) and
  uses fail_on_fixed=true, so baseline drift surfaces as a red nightly -- the
  signal to refresh known-failures.txt.

* Docs: README "When it runs" section documents the per-PR path gate, the
  opt-in label, and the nightly run.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 66a9e40f-3ac8-45be-8fee-a606a22fa098
The comment above the Delta Spark UT job still said it "runs on every trigger
like the other spark-test jobs", which contradicted the per-PR gating added by
the delta-changes job. Remove the stale block; keep the accurate gating comment
and add a concise native-lib-reuse note on the delta-spark-ut job.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 66a9e40f-3ac8-45be-8fee-a606a22fa098
…low failure

After the latest rebase, ImplicitStreamingMergeCastingSuite "Streaming MERGE
overflow sourceType: BIGINT, targetType: DECIMAL(7,2) followAnsiEnabled: false,
ansiEnabled: true, storeAssignmentPolicy: LEGACY" fails deterministically:
Velox raises `VeloxUserError INVALID_ARGUMENT: Cannot cast BIGINT
'9223372036854775807' to DECIMAL(7,2)` (rescaleInt, DecimalUtil.h) where vanilla
Spark handles the overflow per the ANSI/LEGACY policy. Its followAnsiEnabled:true
siblings are already baselined; add this variant so the gate stays green
(735 -> 736 entries, still sorted).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 66a9e40f-3ac8-45be-8fee-a606a22fa098
setup-delta.sh runs under `set -euo pipefail`, so cherry-picking a Delta
FileSourceScanLike fix that the pinned DELTA_REF already contains exits
non-zero (empty/conflicting patch) and aborts the whole setup. This is a
latent break the moment DELTA_REF is bumped past delta-io/delta#7104/apache#7105.

Make cherry_pick_delta_fix attempt the cherry-pick and, on failure, recover
only the paths that fix touches (git diff-tree -> per-file reset + checkout,
then cherry-pick --quit) and continue -- leaving the DeltaSQLCommandTest
patch and bundle jar intact. It is self-correcting: a genuinely missing fix
resurfaces as gate regressions instead of a hard abort.

Ancestry can't distinguish "already contained" from a real conflict here
because the Delta clone is shallow (depth 1) and merge-base --is-ancestor
can't see past the graft; a reverse-apply-check is fragile when the newer
ref carries the fix plus adjacent edits. Recover-on-failure avoids both.

Generated-by: GitHub Copilot CLI
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 66a9e40f-3ac8-45be-8fee-a606a22fa098
…mpotent

Two fixes from review feedback on the Delta gate.

1. compare-test-results.py treated a skipped test as "not seen this run", so a
   baseline entry that merely got skipped was reported under "Stale baseline
   entries (suite/test gone)" and silently dropped from the regenerated
   baseline -- only to return as a regression the next time it executed.

   Each shard now writes a --skipped-out list alongside --ran-out, and the
   aggregate job reports those entries as "Skipped this run" instead of stale
   and carries them over into the regenerated baseline. Genuinely removed tests
   appear in neither list and are still reported as stale.

   Skips are deliberately NOT folded into --ran-out: "now-passing" is derived
   from `ran - failed`, so counting a skip as a run would report it as fixed
   and, under fail_on_fixed, demand its removal from the baseline. Missing
   skipped-*.txt (older artifacts) degrades to the previous behaviour, and the
   shard-completeness guard still keys off failures-*/ran-* only.

2. setup-delta.sh could not be re-run over an existing DELTA_DIR: `git remote
   add origin` exits 3 when origin already exists, which aborts the script
   under `set -euo pipefail` and forces manual cleanup. Drop the remote first
   and force the checkout so a partial previous run is recovered rather than
   fatal. No `rm -rf` is reintroduced; the bundle jar and source patches are
   applied after this block, so nothing worth keeping is discarded.

Generated-by: GitHub Copilot CLI
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 66a9e40f-3ac8-45be-8fee-a606a22fa098
…ttern

Follow-ups from review feedback on the Delta gate.

- load_patterns() compiled each line of flaky-error-patterns.txt with no error
  handling, so a typo in that hand-edited file surfaced as a bare re.error
  traceback naming neither the file nor the offending line. Raise
  BadPatternError with path, line number and pattern, and exit 2 from the gate:
  "<path>:4: invalid regex '[unclosed': unterminated character set at position 0".

- The negative-row-index signature used `-?\d+`, which would also match a
  positive index. The native check is `value >= 0`, so the reported index is
  always negative; match the sign explicitly, per this file's own rule of
  binding patterns to the exact error string so a quarantine can't swallow an
  unrelated failure.

- Note in the DeletionVectorsSuite block that the sed depends on the clone
  step's `checkout -f`. The sed appends after the test-declaration line, so
  without that per-run reset a re-run injects duplicate `fail` lines and trips
  the INJECTED != 2 check (verified: 2, 4, 6 without `-f`; 2, 2, 2 with it).

Generated-by: GitHub Copilot CLI
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 66a9e40f-3ac8-45be-8fee-a606a22fa098
The delta-changes job intends to run the Delta suite whenever it cannot
determine what the PR touched -- "never silently skip coverage". But the
detection itself was fail-closed:

  if git diff --name-only "$BASE" "$HEAD_SHA" | grep -Eq '<delta paths>'; then

When git diff fails (missing objects after a force-push race, an unfetched
fork head, or the `git merge-base ... || echo "$BASE_SHA"` fallback above
handing it an unusable sha), grep gets empty input and exits 1 -- exactly as
it does for "no Delta paths changed" -- so the else branch set run_delta=false
and the suite was skipped. `set -euo pipefail` does not help here: a command
used as an `if` condition is allowed to fail.

Capture the diff first and treat a git failure as an explicit fail-open, then
match against the captured output.

Verified over 8 scenarios: bad base+head, valid base + bad head and empty shas
now all yield run_delta=true, while gluten-delta/, backends-velox/src-delta40,
docs-only, cpp/velox-only and no-change diffs are unchanged. The previous
logic fails the first three.

Generated-by: GitHub Copilot CLI
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 66a9e40f-3ac8-45be-8fee-a606a22fa098
Shard 2 of run 30324192115 reports one regression:

  ImplicitMergeCastingSuite#MERGE overflow in WHEN MATCHED THEN UPDATE SET
  t.value = s.value sourceType: BIGINT, targetType: DECIMAL(7,2)
  followAnsiEnabled: false, ansiEnabled: true, storeAssignmentPolicy: LEGACY

Under storeAssignmentPolicy LEGACY the overflowing value is expected to be
stored without error, but the offloaded Velox cast raises instead:

  VeloxUserError INVALID_ARGUMENT
  Reason: Cannot cast BIGINT '9223372036854775807' to DECIMAL(7, 2)
  Function: rescaleInt  File: velox/type/DecimalUtil.h:218

This is the non-streaming sibling of the ImplicitStreamingMergeCastingSuite
case for the same BIGINT -> DECIMAL(7,2) / LEGACY combination that was
baselined earlier; both surfaced after the rebase. It went unnoticed one run
longer because shard 2 of run 30186097028 died on a Docker pull before the
gate ran, so the test never executed there.

Baselined rather than quarantined in flaky-tests.txt: it failed in both runs
where it actually executed, with an identical error, and the failure is a pure
expression-evaluation overflow with no dependence on scheduling or runtime
plan -- unlike the DV bitmap row-index bug that flaky-error-patterns.txt
covers. Keeping it in the baseline means the gate will tell us to remove it
once the LEGACY cast semantics are fixed, which a flaky entry would not.

736 -> 737 entries.

Generated-by: GitHub Copilot CLI
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 66a9e40f-3ac8-45be-8fee-a606a22fa098
run-delta-tests.sh is parameterized by SHARD_ID and already scopes its sbt log
and watchdog kill marker per shard, but the marker that tells the watchdog sbt
has finished was the global /tmp/sbt-done. With a shared /tmp -- parallel local
runs of several shards -- the first shard to finish creates it and every other
shard's watchdog exits its wait loop early, silently losing hang detection for
the shards still running.

Use /tmp/sbt-done-shard-${SHARD_ID} via a named SBT_DONE_MARKER, matching the
existing SBT_LOG / WATCHDOG_KILL_MARKER convention.

Verified with a reproduction of the arm/disarm handshake, running a fast shard
alongside a slow one: with the global marker the slow shard's watchdog was
disarmed after 3 ticks instead of its full 10; with the shard-scoped marker it
stays armed for the whole run. CI is unaffected (each shard is its own
container), so this only matters for local parallel runs.

Generated-by: GitHub Copilot CLI
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 66a9e40f-3ac8-45be-8fee-a606a22fa098
The Delta suite ran as a reusable workflow called by velox_backend_x86.yml, so
its own pipeline files had to be in that workflow's `paths:` filter for a change
to them to be tested. The side effect was that a Delta-CI-only change pulled in
the entire Velox matrix: on this branch's last run, a commit touching a single
shell script produced 65 jobs and 2319 runner-minutes, of which 1750 (75%) were
TPC-H/DS and Spark UT jobs that the change could not affect.

Make delta_spark_ut.yml standalone. It already built its own native library for
the workflow_dispatch/schedule path, so this mostly means dropping
`workflow_call` and the `native_lib_artifact` input, adding a `pull_request`
trigger with a Delta `paths:` filter, and deleting the `delta-changes` gate and
`delta-spark-ut` jobs from velox_backend_x86.yml along with the Delta entries in
its `paths:`.

The `paths:` filter now IS the per-PR gate, evaluated before the run is created,
so an unrelated PR costs nothing at all -- this replaces the ~60-line
`delta-changes` script. That also drops the `run-delta-ci` label opt-in, which
cannot be expressed as a path filter: the label does not exist on apache/gluten
and so was never functional, and `workflow_dispatch` covers the same need (a
Velox/core author can run the suite against a branch, including on a fork).

Tradeoff: `gluten-delta/**` and `backends-velox/src-delta*/**` still match
velox_backend_x86.yml's filter -- it builds with -Pdelta -- so a change there now
runs both workflows and builds the native library twice (~10 min) instead of
sharing it. That is small next to the case above, and sharing it again is a
separate change.

Two conditions had to move with the split:

- `concurrency:` is now set. It was deliberately absent because, as a reusable
  workflow, `github.workflow` resolved to the caller and the group would have
  collided with the caller's own. Standalone it is needed, or every push to a
  Delta PR stacks another full run.
- delta-spark-aggregate no longer uses `always()`. That evaluates true while a
  run is being cancelled, so with `cancel-in-progress` a superseded push would
  cancel the shards but still start the aggregation, which then fails on finding
  no gate lists; the same happened when a failed bundle build skipped the shards.
  `!cancelled() && needs.delta-spark-test.result != 'skipped'` keeps the intended
  behaviour of publishing a baseline when shards go red, without the spurious
  second failure.

Also fix FAIL_ON_FIXED, which the split would otherwise have silently flipped:
`inputs` now only exists for workflow_dispatch, so the old expression resolved to
false on pull_request and schedule, where the removed workflow_call input had
supplied `default: true`. It now defaults to true for those events and honours
the input only on workflow_dispatch.

Generated-by: GitHub Copilot CLI
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 66a9e40f-3ac8-45be-8fee-a606a22fa098
Each Delta shard was taking ~2.5 hours, which sets the wall clock for the whole
pipeline. Breaking down one shard of a 4-shard run: 9.1 min of fixed setup
(clone Delta, apply the patches, compile the test sources) and 134.5 min actually
running tests, so ~91% of a shard is work that shards away.

Doubling to 8 shards therefore takes the slowest shard from ~148 min to ~74 min
for about +7% total runner-minutes, since the fixed setup is paid 8 times instead
of 4. This also matches delta-io/delta's own spark_test.yaml, which runs these
same suites with NUM_SHARDS: 8, shard: [0..7] and TEST_PARALLELISM_COUNT=4.

Memory is unaffected: each shard is a separate job, so it is still 4 forked test
JVMs plus the sbt launcher against the runner's ~16G, and TEST_PARALLELISM_COUNT
stays at 4.

No baseline regeneration is needed. The gate compares (suite, test) sets against
known-failures.txt -- regressions as `failed - baseline`, now-passing as
`baseline & passed`, stale as `baseline - ran - skipped` -- so it does not depend
on how suites are distributed across shards.

Sharding further would hit a floor at the longest single suite, currently ~18 min
(DeleteSQLSQLPathBasedDVPredPushOffSuite); 8 shards stays well clear of it, and
with ~180 suites per shard there is enough granularity for the split to balance.

Generated-by: GitHub Copilot CLI
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 66a9e40f-3ac8-45be-8fee-a606a22fa098
…line

Shard 3 of the first 8-shard run went red on the now-passing check, not on a
regression: DeltaUpdateCatalogSuite's "convert to delta with partitioning
change" and "partitioned convert to delta with schema change" are in the
baseline but now pass. The aggregate job agrees globally -- 0 regressions,
2 now-passing, 0 stale.

Both tests previously failed with

  IllegalStateException: TaskResourceRegistry is not initialized

from ColumnarCachedBatchSerializer -> RowToVeloxColumnarExec ->
Runtimes.contextInstance, i.e. a task-listener setup problem on the cached-batch
path, which depends on what else is running in the same forked test JVM.
Resharding from 4 to 8 changed how suites are grouped into forks, and the
interleaving that triggered it no longer occurs.

This is a baseline update rather than a flaky quarantine: both tests failed in
each of the last two 4-shard runs and have been in the baseline since it was
bootstrapped, so there is no evidence of nondeterminism within a given
configuration -- the outcome tracked the shard count.

Verified by re-running the gate against that run's own gate-list artifacts with
this baseline: all 8 shards report 0 regressions and 0 now-passing, and the
aggregate's "distinct failing tests" (735) now matches the baseline exactly.

Generated-by: GitHub Copilot CLI
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 66a9e40f-3ac8-45be-8fee-a606a22fa098

Copilot AI 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.

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated no new comments.

@felipepessoto

Copy link
Copy Markdown
Contributor Author

@philo-he @zhouyuan This is green now and I think it's ready — the pipeline is fully self-contained and the last run passed end to end (8/8 shards, slowest 102 min, ~10 job-hours).

What's in this PR:

  • Own workflow. Delta CI no longer runs inside velox_backend_x86.yml, so a change to the Delta pipeline no longer drags in the ~50-job TPC-H/DS + Spark-UT matrix (~1750 runner-min). Per-PR it's gated by a paths: filter, plus a nightly run at 05:00 UTC for everything the filter skips.
  • 8 shards x 4 forks, matching delta-io/delta's own spark_test.yaml. Measured: slowest shard 163 → 102 min (-37%) for +5% billed hours.
  • Baseline at 735 known failures, 0 regressions.

Proposed follow-ups, happy to file issues for these:

  1. Share the native lib between the two workflows (~10 min on gluten-delta PRs, which currently build it twice). Needs a cache key covering cpp/**, build flags and the base image — a stale hit there means silently testing the wrong binary.
  2. Spark 3.5 leg, nightly-only at first. The blocker is the baseline: each Spark/Delta combination needs its own file and a bootstrap run. Once 3.5 is covered here, the imported Delta suite copies @malinjawi identified become removable.
  3. run-delta-ci label, if you want the opt-in back. I dropped it since it can't be expressed as a paths: filter and the label didn't exist on the repo; workflow_dispatch covers the same need today.

Does this look right to you both?

@felipepessoto

Copy link
Copy Markdown
Contributor Author

@philo-he @zhouyuan gentle reminder

@philo-he philo-he left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Thanks. cc @zhouyuan @zhztheplayer

@felipepessoto

Copy link
Copy Markdown
Contributor Author

LGTM. Thanks. cc @zhouyuan @zhztheplayer

@zhouyuan @zhztheplayer, do you have any additional concerns?

Comment thread .github/workflows/delta_spark_ut.yml
@felipepessoto

Copy link
Copy Markdown
Contributor Author

@zhztheplayer, @philo-he does it also need @zhouyuan approval before merging it?
Thanks

@zhouyuan

Copy link
Copy Markdown
Member

@felipepessoto thanks for the improvement

@felipepessoto

Copy link
Copy Markdown
Contributor Author

Thanks all. The follow-ups are tracked at #12743

@zhouyuan

Copy link
Copy Markdown
Member

@felipepessoto could you please move the cache to apache stash based? GHA only have 10GB avaiable, and it's occupying too many space, thus the cache on main branch is evicted and introducing cold build
https://github.com/apache/gluten/actions/caches

image Cc: @philo-he

@felipepessoto

Copy link
Copy Markdown
Contributor Author

@zhouyuan Thanks for pointing this out. I opened #12820 to move all Delta Spark UT caches (ccache, Maven, and sbt/Ivy/Coursier) from actions/cache to Apache Stash. The full Delta workflow passed: ccache restored successfully from the main-branch stash, and the Maven and sbt Stash artifacts were uploaded successfully.

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.

[Enhancement] [Build] Run Delta unit tests during PR validation

7 participants