andygrove opened a new pull request, #5871:
URL: https://github.com/apache/datafusion-comet/pull/5871

   ## Which issue does this PR close?
   
   Part of #5870 (step 2 of the plan there). It does not close the issue.
   
   ## Rationale for this change
   
   #5843 left the whole `Spark SQL Tests (Spark 4.1)` workflow in the PR tier. 
Over Aug 12 to Sep 11 (1,288 executed CI runs on pull requests, 168 
push-to-main runs as the flake baseline):
   
   | Spark 4.1 modules | Runner hrs/day on PRs | PR fail | Main fail | Runs 
where it was the only failing job | Wall clock |
   |---|---|---|---|---|---|
   | `catalyst` + `sql_core-1/2/3` (+ build) | ~96 | 8.5% | 4.5% | 33 | 100 min 
|
   | `sql_hive-1/2/3` | ~65 | 1.4% | 1.3% | 7 | 110 min |
   
   The hive shards are the worst value in the PR tier by runner-hours per 
unique catch, and `sql_hive-2` at 67 minutes after the 41 minute build is what 
sets the PR tier's wall clock. Moving them to the queue saves about 65 
runner-hours a day and brings PR feedback down to about 100 minutes, while the 
33 catches a month from `sql_core` stay on pull requests.
   
   ## What changes are included in this PR?
   
   - `spark_sql_test_reusable.yml` gains a `modules` input (`all`, `core` or 
`hive`, default `all`). The matrix rows move from a workflow literal to 
`dev/ci/spark-sql-modules.py`; the `build` job runs it and publishes the 
selected rows as a job output that `spark-sql-test` reads with `fromJSON`, the 
same pattern the Iceberg reusable workflow uses for its shards. A job-level 
`if:` cannot read `matrix`, which is why the rows are left out rather than 
skipped.
   - `dev/ci/compute-changes.py` adds a `spark_4_1_hive` output with the same 
path filters as `spark_4_1` and POLICY `["queue", 
"label:run-spark-4.1-hive-tests"]`. `spark_4_1` stays `["pr", "queue"]`.
   - `ci.yml` folds the two outputs into the one `spark_4_1` call, so the queue 
still gets every module from a single build: a plain PR run gets `core`, a 
merge group run (or a PR carrying the label) gets `all`, and a `labeled` run 
for the new label gets `hive` only.
   - `dev/ci/check-ci-config.py` gains the two policy cases for the label and a 
check that `core` and `hive` partition `all`.
   - `.github/workflows/README.md` and `docs/source/contributor-guide/ci.md` 
document the new tier split and label.
   
   The `run-spark-4.1-hive-tests` label has been created in the repository.
   
   Other Spark versions are unaffected: `spark_3_4`, `spark_3_5` and 
`spark_4_0` do not pass `modules` and keep running all seven rows in the queue.
   
   ## How are these changes tested?
   
   - `python3 dev/ci/check-ci-config.py` passes, including the new label cases 
and the module partition check.
   - `actionlint` passes. It is what caught the first attempt, a job-level 
`if:` on `matrix.module.name`, which is not a valid context there.
   - `dev/ci/spark-sql-modules.py --modules core|hive|all` prints the expected 
rows and rejects anything else.
   - Simulated the routing for each event through `compute()` plus the `ci.yml` 
expression: PR `synchronize` selects `core`, PR with the label selects `all`, 
`labeled` with the new label selects `hive`, `merge_group` selects `all`, 
`push` does not call the workflow.
   - This PR's own CI run should show the four core shards and no hive shards 
under `Spark SQL Tests (Spark 4.1)`.
   


-- 
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]

Reply via email to