phillipleblanc commented on code in PR #2016:
URL:
https://github.com/apache/datafusion-ballista/pull/2016#discussion_r3606914814
##########
ballista/scheduler/src/state/mod.rs:
##########
@@ -258,7 +274,7 @@ impl<T: 'static + AsLogicalPlan, U: 'static +
AsExecutionPlan> SchedulerState<T,
async fn launch_tasks(
Review Comment:
`launch_tasks` can combine stages from multiple jobs into one executor RPC,
and then would now treat any `InvalidArgument` as a failure for every job in
that RPC. The executor will process each job sequentially and then fail the
first one with invalid argument:
`ballista/executor/src/executor_server.rs`:L850
```rust
for multi_task in multi_tasks {
let multi_task: Vec<TaskDefinition> = get_task_definition_vec(
multi_task,
// ...
)
.map_err(|e| Status::invalid_argument(format!("{e}")))?;
for task in multi_task {
task_sender
.send(CuratorTaskDefinition {
scheduler_id: scheduler_id.clone(),
task,
})
.await
.unwrap();
}
}
```
So this could cause tasks that were processed before the failing one to have
already been enqueued, while skipping any tasks after the bad one. Then this
code would mark every job as failed.
I think a better approach would be to isolate the failed jobs in
`launch_multi_task` and return it as a separate list and only mark those
specific ones as failed.
##########
ballista/scheduler/src/state/mod.rs:
##########
@@ -258,7 +274,7 @@ impl<T: 'static + AsLogicalPlan, U: 'static +
AsExecutionPlan> SchedulerState<T,
async fn launch_tasks(
Review Comment:
You could test this by having a test that sends a launch_tasks call with
multiple jobs, where only one is invalid and verify that only that is marked as
failed.
--
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]
