andygrove opened a new pull request, #2197: URL: https://github.com/apache/datafusion-ballista/pull/2197
# Which issue does this PR close? Part of #2092. # Rationale for this change The TPC-DS gate runs exactly one configuration: the static planner at 16 partitions. Single-partition execution has never been covered, and it turns out to be where the interesting failures hide. #2184 is the case in point. At `target_partitions=1` a `UnionExec` whose branches could not be restricted away left every task running every branch, inflating q5 by 4x and q80 by 2x and crashing q49 with `FileStreamBuilder invalid partition index: 0`. That bug survived because #2070 verified its fix "at every partition count tried (4, 5, 6, 8, 16, 20, 32)" and 1 was never tried. #2186 fixed it, but nothing in CI stops it or anything like it from coming back. Adding a `--partitions 1` leg makes that class of bug a gate failure rather than something found by hand months later. # What changes are included in this PR? Replaces the single job with a two-leg matrix over `--partitions`, following the shape `tpch.yml` already uses. | Leg | Planner | `--partitions` | |---|---|---| | `p16` | static | 16 (existing coverage) | | `p1` | static | 1 (new) | - Per-leg `label` / `partitions` / `slug`, matching `tpch.yml`'s convention. - `fail-fast: false`, so one red leg does not mask the other. - Both legs share one `swatinem/rust-cache` entry via `shared-key: tpcds-sf1`, so the binaries are compiled once and restored by the second leg rather than built twice. This mirrors `tpch.yml`'s `shared-key: tpch-sf10`. - The `run_suite` shell helper is inlined, since each leg now runs exactly one invocation. - The cluster-log artifact is named per leg so failures do not collide. Adaptive-planner coverage is deliberately out of scope here; it is tracked separately in #2182 and still fails on known bugs. # Expected result of the new leg With #2186 on `main`, q5, q49, q77, q80 and q61 all verify at one partition. The one query left is **q72**, which exhausts its memory share rather than returning a wrong answer, so the `p1` leg is expected to be red on that query alone. That is a workflow-tuning problem, not a correctness one. The executor builds a per-task `FairSpillPool` of `memory_pool_size / concurrent_tasks` (`ballista/executor/src/executor_process.rs`), so `--memory-pool-size 2GB --concurrent-tasks 4` gives each task 500 MB — and collapsing to one partition pushes the whole table through a single task. At `--partitions 1` there is no intra-stage parallelism for the extra task slots to exploit, so dropping that leg to `--concurrent-tasks 1` would hand the single task the full 2 GB. I have left that out of this PR to keep the diff to the matrix itself. Happy to fold it in if reviewers would rather land this green in one go. # Are there any user-facing changes? No. CI coverage only. -- 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]
