Delta pipeline - [VL][Delta] Guard DV DML row-index scans#12601
Draft
felipepessoto wants to merge 22 commits into
Draft
Delta pipeline - [VL][Delta] Guard DV DML row-index scans#12601felipepessoto wants to merge 22 commits into
felipepessoto wants to merge 22 commits into
Conversation
…es baseline Run delta-io/delta's `spark` ScalaTest suite against a Gluten Velox bundle in CI and gate the results against a committed baseline so the many expected Delta-on- Gluten failures stay manageable and can be fixed incrementally without letting currently-passing tests silently regress. What it adds (.github/workflows/util/delta-spark-ut/): - delta_spark_ut.yml: builds the native lib + Gluten bundle, then runs the Delta spark suite sharded by suite into 4 shards x 4 forked test JVMs (~16-way), and gates each shard against the baseline. - compare-test-results.py: the gate. Per shard, regressions (failed not in the baseline) fail the build; newly-passing baselined tests are flagged so the baseline can be tightened. Also supports seed/aggregate modes. - known-failures.txt: the committed baseline of expected failures. - setup-delta.sh: clones Delta, injects the Gluten bundle, patches DeltaSQLCommandTest, and force-fails the two DeletionVectorsSuite 2B-row tests whose native row-index materialization OOM-kills the runner and hangs the shard. - README.md: how the pipeline, gating and baseline-refresh work. The workflow also carries a hang watchdog that thread-dumps and kills a wedged fork, and tunes the per-fork heap (2G) and off-heap (2G) to fit the ~16G runner. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…line Delta's data-skipping, limit-push-down, column-pruning and scan-metric tests collect file-source scans by matching the concrete `FileSourceScanExec` case class. Under the Gluten Velox bundle the scan is offloaded to DeltaScanTransformer, a sibling that implements the same `FileSourceScanLike` interface but is not FileSourceScanExec, so the match misses and the scan looks absent. This surfaced as `scala.MatchError: List()` (~56 DataSkipping*/DeltaLimitPushDown* tests), empty generated-column partition filters (~45 OptimizeGeneratedColumnSuite tests) and broken column-pruning / scan-metric checks across the Delete, Update, Merge, DeletionVectors and RowId suites and the TestsStatistics helper. Gluten copies `partitionFilters` and the other accessors these tests read verbatim onto the offloaded scan, so results are identical to vanilla -- only the test's `case` match breaks. Fix it by cherry-picking the two merged upstream Delta commits that widen these matches to the shared `FileSourceScanLike` interface (behavior-preserving for vanilla, which also implements it): * delta-io/delta#7104 -- ScanReportHelper.collectScans * delta-io/delta#7105 -- the remaining 9 test sources, its follow-up Both are merged on Delta master but land after the ref this workflow builds against (v4.2.0), so setup-delta.sh cherry-picks them onto the shallow checkout. Each fetches the fix commit at depth 2 (commit + parent) so cherry-pick can compute the parent->fix diff, and uses `cherry-pick -n` so no committer identity is required. Once the pinned DELTA_REF advances to include a commit its cherry-pick becomes a clean no-op and that block can be removed. The cherry-picks run before the DeletionVectorsSuite 2B-row force-fail step: that step sed-injects fail() into DeletionVectorsSuite.scala, which delta-io/delta#7105 also edits, and git cherry-pick refuses to apply onto a working tree with uncommitted changes to a file it touches (exit 128). Refresh known-failures.txt from run 28299900971 (the delta-spark-aggregate job output), which ran all 19073 tests across 16 shards: removes 187 now-passing tests with 0 regressions, 963 -> 776. ~147 come from the fixes above (DataSkipping*, DeltaLimitPushDown*, OptimizeGeneratedColumnSuite, MergeInto*, RowIdSuite); the remaining ~40 are other suites that now pass (e.g. HiveConvertToDeltaSuite, BitmapAggregatorE2ESuite). Verified against the per-shard ran/failed lists: every baseline entry was observed this run (0 stale), so nothing was dropped due to a crashed or incomplete shard. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Make delta_spark_ut.yml a reusable workflow (on: workflow_call) and call it from velox_backend_x86.yml so the Delta tests reuse the native lib + arrow jars that workflow already builds, instead of duplicating the build-native-lib-centos-7 job. GitHub artifacts cannot be shared across workflows, so the only way to reuse the artifact is to run the Delta jobs in the same workflow run. delta_spark_ut.yml keeps a workflow_dispatch trigger for standalone manual runs (its build-native-lib-centos-7 job is gated to that case and skipped when called); the pull_request trigger is removed so the suite no longer double-runs. velox_backend_x86.yml gains an arrow-jars upload on its native build and a delta-spark-ut job that calls the reusable workflow. That job runs on every velox trigger like the other spark-test jobs, since core/velox/substrait/cpp changes can affect Delta query offload. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address PR review feedback: - setup-delta.sh: replace the shallow-clone + full-clone fallback (which ran a destructive `rm -rf "$DELTA_DIR"`) with a single `git init` + shallow `fetch --depth 1 origin "$DELTA_REF"` + `checkout FETCH_HEAD`. This resolves a tag, branch, or commit SHA uniformly (`git clone --branch` rejects SHAs), drops the dead fallback branch, and removes the unguarded recursive delete. - compare-test-results.py: in enforce mode, a missing/typoed --known-failures path made load_entries() return an empty set, silently degrading to seed mode and passing the gate without enforcing regressions. Treat a missing baseline file as a configuration error (exit 2); an existing-but-empty file is still allowed and legitimately seeds. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address PR review feedback with four robustness fixes: - compare-test-results.py (enforce/seed): raise NoReportsError and exit 2 when no JUnit <testsuite> elements are parsed, instead of warning and returning empty sets. Otherwise a misconfiguration (wrong reports dir, broken reporter, suites crashing before writing XML) yields zero failures -> zero regressions -> a silent green gate. - compare-test-results.py (aggregate): exit 2 before writing baseline-out when no per-shard failures-*.txt / ran-*.txt inputs are found. The gate-list download is continue-on-error and aggregate runs with if: always(), so missing artifacts would otherwise produce an empty baseline that could be committed, wiping known-failures.txt. - setup-delta.sh: pass the Delta ref after `--` in git fetch so a ref starting with `-` can't be misread as a git option (the script is workflow_dispatch- runnable with a user-supplied ref). - velox_backend_x86.yml: drop secrets: inherit from the reusable Delta UT call. delta_spark_ut.yml references no secrets, so inheriting them needlessly forwards all caller secrets to a workflow that clones and runs external code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Some Delta-on-Gluten MERGE tests that write deletion vectors fail non-deterministically: the same bundle passes them on one CI run and fails them on the next (native RoaringBitmapArray addSafe aborting on an invalid Long.MAX_VALUE row index). Such tests cannot live in known-failures.txt -- baselining them reds the gate on every run where they pass, and leaving them out reds it on every run where they fail. Add a flaky-tests.txt quarantine list read by the gate. A quarantined test is neutral: it never counts as a regression when it fails nor as now-passing when it passes, and is excluded from the regenerated baseline (aggregate mode). The suite portion of each entry is an fnmatch glob so one line covers a root-cause family across generated suite variants (e.g. *DVs*Suite); the test name is matched exactly. Seed the list with the DV-merge family behind the native row-index bug. This is an interim measure -- entries should be removed once that bug is fixed in the native backend so the tests are enforced again. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…port fail-fast, baseline count Fixes three open review comments on the Delta Spark UT pipeline: 1. spark_version was redundant with the hard-coded GLUTEN_BUNDLE_SPARK_VERSION / GLUTEN_SPARK_PROFILE, and a workflow_dispatch run could set them out of sync so the Delta tests ran against a mismatched Gluten bundle. Make spark_version the single source of truth: the Gluten bundle profile (-Pspark-<v>), the bundle jar name and Delta's -DsparkVersion are all derived from it, so a mismatch is now impossible (no guard needed). Scala 2.13 / JDK 17 stay pinned. 2. Corrupt/truncated JUnit reports (compare-test-results.py). parse_reports previously warned and skipped any XML that failed to parse, so a report truncated by a killed/OOM'd fork could silently drop a suite's failures and let the gate go green on partial data. A TEST-*.xml that fails to parse now raises CorruptReportError (exit 2); other XML matched by the broad target/** glob is still skipped. 3. Hard-coded failure count in the baseline header (known-failures.txt). The header stated a fixed total that drifts every time the baseline is refreshed. Drop the number and point at the entry count / delta-spark-aggregate summary as the authoritative source instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The aggregate job downloads per-shard gate lists with continue-on-error and the
aggregator only hard-failed when *no* inputs were found. If a shard died before
writing its gate lists (OOM / watchdog) or its artifact failed to download, the
job still regenerated a baseline that silently omitted that shard's failures,
shrinking known-failures.txt and reddening the next run.
Add --expected-shards to compare-test-results.py: in aggregate mode it counts
shards that produced a complete failures-*/ran-* pair and refuses to aggregate
(exit 2, before writing --baseline-out) when fewer than expected are present. A
shard counts only when BOTH files exist, so a partial download is caught too.
The workflow passes --expected-shards ${{ env.DELTA_NUM_SHARDS }}; omitting the
flag (default 0) disables the check, preserving prior behavior.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… body Two readability/cleanup changes to the Delta Spark UT pipeline: 1. Drop redundant Arrow jar handling. Since apache#12244 (2026-06-09) Gluten depends on vanilla Apache Arrow instead of the custom 15.0.0-gluten rename, so the Arrow jars resolve from Maven Central. Remove the arrow-jar staging in both native builds, the velox-arrow-jars / delta-spark-ut-arrow-jars uploads, the caller's arrow_jars_artifact input, and the bundle job's "Download Arrow jars" step. The bundle build's Maven cache still holds Arrow across runs and resolves it from Central on a miss. 2. Extract the shard test body into run-delta-tests.sh. The "Run Delta spark module tests" step embedded ~190 lines of shell (hang watchdog, sbt invocation with JVM/heap tuning, cgroup memory forensics, report check, known-failures gate). Move that whole body into util/delta-spark-ut/run-delta-tests.sh so the workflow step is just an `env:` block plus a one-line script call, shrinking delta_spark_ut.yml by ~180 lines. The move is faithful: the script body is the previous inline block with only the GitHub `${{ }}` expressions replaced by env vars (matrix.shard -> SHARD_ID which is already job-level env; spark_version/update_baseline/fail_on_fixed added to the step env). Verified by a textual diff against the old body and by end-to-end runs with a stubbed sbt (baseline-only -> green, regression -> red, seed -> green). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Gluten/JDK17 test JVM flags (--add-opens + the Netty reflection property, mirroring root pom.xml's extraJavaTestArgs) lived only in the workflow step's env: block, so a developer running the Delta suite locally had no shared source for them. Move them into util/delta-spark-ut/java-test-args.sh, which exports JAVA_TOOL_OPTIONS. run-delta-tests.sh sources it (via a BASH_SOURCE- relative path so it also works for local runs), and the workflow env block no longer sets JAVA_TOOL_OPTIONS. A developer can source the same file before `sbt spark/test` to get identical flags; README documents it. Faithful: the sourced value is byte-identical to the previous env-block value (verified), and an end-to-end run with a stubbed sbt confirms the forked test JVM still receives all 18 flags with JAVA_TOOL_OPTIONS unset in the environment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The native Delta DV bitmap row-index bug (RoaringBitmapArray aborting on a Long.MAX_VALUE row index during a MERGE that writes deletion vectors) is intermittent and lands on a DIFFERENT *DVs*Suite MERGE test each run, so listing every test in flaky-tests.txt was whack-a-mole (6 entries and counting). Add error-signature quarantine: the gate now records each failed test's JUnit <failure>/<error> text, and flaky-error-patterns.txt holds regexes matched against it. A failure matching a pattern is treated as flaky regardless of which test it hit -- neither counted as a regression nor written to the shard's failures list (so it can't leak into the regenerated baseline). Seeded with the RoaringBitmapArray signature. This is more precise than a name glob: a different real failure in the same DV suite is still caught, because only failures carrying the signature are ignored. The six *DVs*Suite name entries are removed from flaky-tests.txt (superseded); the name mechanism stays for flakes without a distinctive error signature. Verified against a real failing shard report: the DV failure is a regression without the patterns file and quarantined (gate green) with it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Run 29042495519 regressed on another DV-merge test whose failure was NOT the RoaringBitmapArray "exceeds max representable value" signature but a second, distinct native error from the same DV bitmap aggregation -- Reason: Delta bitmap row index cannot be negative: -6254810385378525259 Expression: value >= 0 Function: addRowIndex File: .../delta/DeltaBitmapAggregator.cc:44 vs the previously-seen too-large variant (value <= kMaxRepresentableValue, RoaringBitmapArray.cpp addSafe). Same root cause (garbage row index into the DV bitmap aggregator), different bounds check and different message. Add a second explicit pattern for it rather than broadening the existing one -- the two are distinct native error strings, so keeping each pattern tightly bound to its message avoids masking an unrelated failure that merely mentions a bitmap row index. Verified against the real failing report: the negative-index failure is a regression without the pattern and quarantined with it; the suite's five other (non-bitmap) baseline failures and benign controls are not matched. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Velox Variant/UDT type-offload gap was fixed upstream, so 74 Delta UT tests (65 Variant + 9 user-defined-type) that were expected failures now pass and were tripping the fail-on-fixed gate as "now-passing". Remove them from known-failures.txt (810 -> 736). The 6 remaining `variant auto compact` entries are a different root cause (auto-compact metrics) and correctly stay in the baseline. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 66a9e40f-3ac8-45be-8fee-a606a22fa098
DeltaFastDropFeatureSuite "We do not create redundant DV tombstones after cloning isShallowClone: true" now passes consistently (green on 2026-07-13 and 2026-07-14 runs); the earlier FileNotFoundException on a deletion-vector .bin during the DROP FEATURE parallel OPTIMIZE no longer reproduces. Keeping it in the baseline trips the fail-on-fixed gate on every run, so remove it (736 -> 735). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 66a9e40f-3ac8-45be-8fee-a606a22fa098
…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
Native DELETE/UPDATE/MERGE deletion-vector support deliberately keeps the target row-index scan on Spark until native row-index execution is proven for the required Delta table shapes. Detect those DML row-index scan shapes and keep the small scan subtree (and its parent filter/project) off the native path, so the offload path stays correctness-first while the write side is built out. - add DeltaDeletionVectorDmlUtils to detect and tag Delta DV DML row-index scans (row-index / file-path / bitmap-aggregator references) - gate OffloadDeltaScan to fall back on DML row-index scans unless native Delta write and native DML row-index scan are both enabled - add keepDmlRowIndexFallbackSubtreeOnSpark post-transform rule to keep the filter/project above a fallen-back DML scan on Spark, avoiding row<->columnar transitions right before Delta's JVM bitmap path - cover the fallback shape with DeltaDeletionVectorHandoffSuite cases for Spark 3.5 / Delta 3.3 and Spark 4.0 / Delta 4.0
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
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.
Delta CI PR is still pending to merge. This is a temporary PR to run the CI with #12215