You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "QuakeWang (via GitHub)" <gi...@apache.org> on 2026/08/01 02:50:34 UTC
[PR] fix(scheduler): preserve stages on transition errors [datafusion-ballista]
QuakeWang opened a new pull request, #2215:
URL: https://github.com/apache/datafusion-ballista/pull/2215
# Which issue does this PR close?
Closes #2214
# Rationale for this change
`StaticExecutionGraph` removed a stage before running fallible plan transformations. If a transformation failed, the error returned before the stage was reinserted, leaving the execution graph without the original stage.
# What changes are included in this PR?
Build replacement stages before updating the stage map, so failed resolve and rollback transitions preserve the original stage. Add regression tests for all three affected transitions.
# Are there any user-facing changes?
No API changes. Failed stage transitions now preserve execution graph state.
# Testing
- `ci/scripts/rust_fmt.sh`
- Scheduler all-targets/all-features Clippy
- Scheduler library tests
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix(scheduler): preserve stages on transition errors [datafusion-ballista]
Posted by "sandugood (via GitHub)" <gi...@apache.org>.
sandugood commented on code in PR #2215:
URL: https://github.com/apache/datafusion-ballista/pull/2215#discussion_r3695347176
##########
ballista/scheduler/src/state/execution_graph.rs:
##########
@@ -1364,10 +1361,9 @@ impl ExecutionGraph for StaticExecutionGraph {
executor_id,
})
.collect();
- self.stages.insert(
- stage_id,
- ExecutionStage::UnResolved(stage.to_unresolved(failure_reasons)?),
- );
+ let unresolved_stage = stage.to_unresolved(failure_reasons)?;
Review Comment:
When this errors, the `running_tasks` was already collected, but it is now discarded. Better than a vanished stage, but it's a distinct bad state. Maybe add a line to "Are there any user facing changes"?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix(scheduler): preserve stages on transition errors [datafusion-ballista]
Posted by "milenkovicm (via GitHub)" <gi...@apache.org>.
milenkovicm merged PR #2215:
URL: https://github.com/apache/datafusion-ballista/pull/2215
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix(scheduler): preserve stages on transition errors [datafusion-ballista]
Posted by "QuakeWang (via GitHub)" <gi...@apache.org>.
QuakeWang commented on code in PR #2215:
URL: https://github.com/apache/datafusion-ballista/pull/2215#discussion_r3697587096
##########
ballista/scheduler/src/state/execution_graph.rs:
##########
@@ -1364,10 +1361,9 @@ impl ExecutionGraph for StaticExecutionGraph {
executor_id,
})
.collect();
- self.stages.insert(
- stage_id,
- ExecutionStage::UnResolved(stage.to_unresolved(failure_reasons)?),
- );
+ let unresolved_stage = stage.to_unresolved(failure_reasons)?;
Review Comment:
Good point. The running-task list was also discarded on this error path before this PR, but preserving the stage makes the resulting behavior worth stating explicitly: the stage remains `Running`, its task tracking is retained, and the error propagates before a `CancelTasks` event can be emitted. I have clarified this in the user-facing changes section. Changing cancellation or recovery policy would be outside the failure-atomic transition scope of this PR.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org