andygrove opened a new issue, #2595: URL: https://github.com/apache/datafusion-ballista/issues/2595
**Is your feature request related to a problem or challenge? Please describe what you are trying to do.** `StaticExecutionGraph` ([state/execution_graph.rs](https://github.com/apache/datafusion-ballista/blob/ea1126c0649b2bc08e1e233b1972b64318821c16/ballista/scheduler/src/state/execution_graph.rs)) and `AdaptiveExecutionGraph` ([state/aqe/mod.rs](https://github.com/apache/datafusion-ballista/blob/ea1126c0649b2bc08e1e233b1972b64318821c16/ballista/scheduler/src/state/aqe/mod.rs)) carry near-identical copies of most of their logic. Both files are among the most frequently changed in the scheduler (29 and 25 commits in the last 12 months), and the copies have already drifted: - `resolve_stage` / `rollback_*_stage` lose the stage on error in the AQE copy only (#2594). - The AQE `update_task_status` has dead `reset_running_stages` handling, lacks the UnResolved late fetch-failure path, and reports a less useful max-failures message (details in #1986). Duplicated code: | What | Static graph | Adaptive graph | |---|---|---| | `update_task_status` | [L760-L1195](https://github.com/apache/datafusion-ballista/blob/ea1126c0649b2bc08e1e233b1972b64318821c16/ballista/scheduler/src/state/execution_graph.rs#L760-L1195), 376 lines, cognitive complexity 164, nesting 9 | [L781-L1142](https://github.com/apache/datafusion-ballista/blob/ea1126c0649b2bc08e1e233b1972b64318821c16/ballista/scheduler/src/state/aqe/mod.rs#L781-L1142), 361 lines, cognitive complexity 121 | | `reset_stages_internal` | [L532-L655](https://github.com/apache/datafusion-ballista/blob/ea1126c0649b2bc08e1e233b1972b64318821c16/ballista/scheduler/src/state/execution_graph.rs#L532-L655) | [L548-L676](https://github.com/apache/datafusion-ballista/blob/ea1126c0649b2bc08e1e233b1972b64318821c16/ballista/scheduler/src/state/aqe/mod.rs#L548-L676) (adds `planner.remove_exchange_locations`) | | accessors, `revive`, `is_successful`, `completed_stages`, lost-executor reset tail | [L609-L766](https://github.com/apache/datafusion-ballista/blob/ea1126c0649b2bc08e1e233b1972b64318821c16/ballista/scheduler/src/state/execution_graph.rs#L609-L766) | [L630-L787](https://github.com/apache/datafusion-ballista/blob/ea1126c0649b2bc08e1e233b1972b64318821c16/ballista/scheduler/src/state/aqe/mod.rs#L630-L787) (identical apart from type names) | | stage transitions (`resolve_stage`, `succeed_stage`, `fail_stage`, `rollback_*`, `rerun_successful_stage`, `fail_job`, `succeed_job`) | L1307-L1438 | L1254-L1391 | | `processing_stages_update`, `get_running_stage_id` | L390-L455, L504-L530 | copied | | `pop_next_task` (test only) | L1484+ | L1440+ | **Describe the solution you'd like** Proposed sub-tasks, each a behaviour-preserving PR (apart from the bug fixes) relying on the existing execution-graph tests: - [ ] Fix the AQE stage-loss bug (#2594) so both copies behave the same before they are merged. - [ ] Extract the shared state (job/session ids, status, timestamps, `stages`, `failed_stage_attempts`, ...) into a common struct embedded by both graphs, and implement the accessors, `revive`, `is_successful`, `completed_stages` and lost-executor reset once. - [ ] Share the stage-transition methods and `processing_stages_update` / `get_running_stage_id` as default trait methods or free functions over the stages map. - [ ] Share `reset_stages_internal` as a function returning a reset plan; the AQE graph adds its `remove_exchange_locations` step afterwards. Replace the nested `for_each` closures with plain loops. - [ ] Break `update_task_status` into `handle_failed_task`, `handle_successful_task`, `handle_late_task_for_unresolved_stage` and the resubmit/reset post-passes, then share those between the graphs. This also resolves the AQE gaps described in #1986. - [ ] Share the test-only `pop_next_task`. **Describe alternatives you've considered** Keep the copies and port each fix by hand. The drift above shows this is already failing. **Additional context** Related: #1986, #2539, #2150. Line references are pinned to commit ea1126c0649b. -- 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]
