abstractdog opened a new pull request, #2828: URL: https://github.com/apache/datafusion-ballista/pull/2828
# Which issue does this PR close? Closes #2827. # Rationale for this change A task whose operator metrics can't be merged into its stage's, because it reports a different number of metric sets than the stage's earlier tasks, makes `update_task_metrics` return an error. The execution graphs propagate it with `?`, which drops the task's status update, so the stage never completes and the job hangs. With AQE, TPC-DS q23b hits this: restricting a task's plan to its partitions replaces an empty `UnionExec` child's `EmptyExec` with a 0-partition `DataSourceExec`, which reports metrics where the `EmptyExec` reported none. The issue has the details. # What changes are included in this PR? - `RunningStage::update_task_metrics` no longer returns an error: when a task's metrics can't be merged, it logs a warning and leaves the stage's metrics as they were. `AdaptiveExecutionGraph`, `StaticExecutionGraph` and `PipelinedExecutionGraph`, its only callers, so keep processing the task's status. The fallible merge stays as the private `try_update_task_metrics`, which the existing unit tests call. - A unit test of `update_task_metrics` with a mismatched count, and a test in `aqe/test/job_success.rs` that runs an AQE job whose tasks report alternating numbers of metric sets to success. Without the fix it fails with the metrics-mismatch error. Tested on a 10-executor cluster with TPC-DS SF10: q23 with AQE now completes (it hung before), the scheduler logs the warning for stage 30's tasks, and the results match those with AQE off. Follow-up, not in this PR: the mismatch itself comes from the `MemorySourceConfig` stand-in in `task_builder.rs`, which exists because a 0-partition `EmptyExec` or `PlaceholderRowExec` does not survive a protobuf round-trip. apache/datafusion#23643 (in 55.0.0) encodes their partition count, but decodes it with `partitions.max(1)`, so 0 still comes back as 1. If DataFusion could tell an absent count from 0, the restriction could use `with_partitions(0)` instead of the stand-in, and every task of a stage would report the same metrics. This PR is still worth having on its own: whether a job completes should not depend on its tasks' metrics. # Are there any user-facing changes? No. A job that hung now completes; the stage's metrics in the plan display and the REST API leave out the tasks whose metrics couldn't be merged. If the stage's first reported task is one of those, the finished stage's plan is logged without metrics. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
