felipepessoto commented on PR #12388: URL: https://github.com/apache/gluten/pull/12388#issuecomment-5027637160
> > For the maintainers @philo-he @zhztheplayer : this runs on every velox_backend_x86 trigger. Fair reasoning, but with #12288 going on it should be a deliberate choice — every PR, Delta paths only, or nightly + label. Any works for me. > > @felipepessoto, we should avoid unnecessary GHA resource consumption, since our usage is still running high. That sometimes means making tradeoffs, particularly for failure cases that are rare. With that in mind, I'd suggest scheduling a nightly job that runs the full test set against the latest main, and narrowing the per-PR Delta tests to the most relevant cases that guard the key functionality. The per-PR job could also be triggered only by changes to the Delta module and a few core-related modules, so PRs that don't touch those paths skip it entirely. This keeps fast, focused signal on every PR while still catching the rarer regressions once a day. Thanks @philo-he, that makes sense - I've implemented it in a82ab8945. **Nightly full run.** `delta_spark_ut.yml` now has a `schedule` trigger (05:00 UTC) that runs the full suite against the latest `main`. It builds its own native lib (no caller to provide one) and runs with `fail_on_fixed=true`, so if a baselined test starts passing the nightly goes red, our signal to refresh `known-failures.txt`. **Narrowed per-PR trigger.** A small `delta-changes` job now gates the Delta suite in `velox_backend_x86.yml`: it runs only when the PR touches a high-signal Delta path, or carries a `run-delta-ci` opt-in label (for PRs that change general code but still want to check Delta). To pick where to draw the per-PR line, I grouped the candidate paths: - **Group A - Delta-specific** (rare, ~11% of commits): `gluten-delta/**`, `delta_spark_ut.yml`, `util/delta-spark-ut/**`, `velox_backend_x86.yml`. - **Group B - core offload code** (touched on ~60% of commits): `backends-velox/**`, `gluten-core/**`, `gluten-substrait/**`, `gluten-arrow/**`, `cpp/**`, `shims/**`. - **Group C - build/infra** (broad, indirect): `dev/**`, `ep/build-velox/**`, `package/**`, `pom.xml`, `build/mvn`. Keeping B+C would have triggered the suite on ~60% of PRs, barely a saving. So, I went with the **aggressive** option: **Group A plus the Delta integration code `backends-velox/src-delta*`** (the `src-delta`, `src-delta33`, `src-delta40` sources), and dropped B and C. That takes the per-PR trigger rate to ~17% of recent commits. General Velox/core/native changes that can affect Delta offload are covered by the nightly run and the `run-delta-ci` label rather than running on every PR. One thing to flag: if you'd like the Delta checks to be required for merge, note they won't post on PRs where the suite is intentionally skipped, so they'd need to be non-required, or backed by an always-green shim job. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
