andygrove commented on code in PR #2689:
URL:
https://github.com/apache/datafusion-ballista/pull/2689#discussion_r4408545091
##########
ballista/scheduler/src/state/mod.rs:
##########
@@ -393,10 +393,13 @@ impl<T: 'static + AsLogicalPlan, U: 'static +
AsExecutionPlan> SchedulerState<T,
error!(
"Failed to launch new task, could not get executor
metadata: {e}"
);
- state
+ // Refund like the rejected-launch branch: the executor
+ // stays registered, so its reserved vcores must go
+ // back into the budget (see #2686).
+ let freed = state
Review Comment:
Thanks, this looks right and matches the rejected-launch branch nicely.
One thing I noticed while reading it, not something to fix here: the tasks
in this batch were already recorded as `Running` on this executor at bind time,
and nothing launches, fails, or resets them on this path. So the vcores now
come back, but the job can still hang on those orphaned tasks. Also, since the
refund triggers a revive, a lookup that keeps failing would bind more pending
partitions to the same executor and orphan those too. Would you be up for
opening a follow-up issue to reset these tasks to pending (or fail the job) on
a metadata miss? Happy to merge this as is.
--
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]
