andygrove commented on code in PR #2698:
URL:
https://github.com/apache/datafusion-ballista/pull/2698#discussion_r4431934713
##########
ballista/scheduler/src/state/execution_graph.rs:
##########
@@ -192,6 +192,15 @@ pub trait ExecutionGraph: Debug {
executor_id: &str,
) -> Result<(HashSet<usize>, Vec<RunningTaskInfo>)>;
+ /// Return a bound-but-never-launched task to pending.
+ ///
+ /// Used when `launch_tasks` cannot dispatch a batch (e.g. a transient
+ /// executor-metadata miss) so the partitions are not left stranded as
+ /// `Running` with no status updates ever arriving. See #2691.
+ ///
+ /// Returns `true` when the task was still `Running` and was reset.
+ fn reset_unlaunched_task(&mut self, key: &TaskKey) -> bool;
Review Comment:
Since `ExecutionGraph` is a public trait, adding a required method breaks
anyone implementing it outside the repo. Could we give it a default body that
returns `false`? That matches the "nothing was reset" meaning in the doc, and
both built-in graphs override it anyway, so behaviour here doesn't change.
Otherwise it'll need the `api-change` label and an upgrade-guide note.
--
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]
