goingforstudying-ctrl opened a new pull request, #2860:
URL: https://github.com/apache/datafusion-ballista/pull/2860

   # Which issue does this PR close?
   
   Closes #2859.
   
   # Rationale for this change
   
   While looking into #2856 I noticed that `TaskManager::executor_lost` runs 
the stage rollback with `?`:
   
   ```rust
   let reset = graph.reset_stages_on_lost_executor(executor_id)?;
   ```
   
   A rollback can fail (`rollback_resolved_shuffles` errors on plans whose 
nodes reject child rewrites — the graph tests already treat that as a real 
case). When it does, the error propagates out of the whole function, so the 
failing job is left half-reset with tasks still `Running` on the dead executor, 
and every job after it in the cache iteration never gets its tasks reset at 
all. `on_executor_lost` then just logs the error. All of those jobs hang 
forever even with healthy executors in the cluster, which is the "hangs instead 
of failing" shape #2856 reports.
   
   # What changes are included in this PR?
   
   - `executor_lost` now handles a rollback failure like the other "cannot 
survive the loss" cases already there (pipelined jobs, lost 
`DataFrame::cache()` data): the job is failed with a clear reason, and the loop 
keeps recovering the remaining jobs.
   - Two regression tests: one for the single-job case (the job is failed 
instead of left hanging), one proving the other jobs sharing the dead executor 
still get rolled back when one job's rollback fails.
   - A `#[cfg(test)]` `stages_mut` accessor on `StaticExecutionGraph` so the 
tests can build a stage whose plan rewrite fails.
   
   # Are there any user-facing changes?
   
   Only the failure mode: a job whose rollback fails during executor loss now 
fails with an error naming the executor and the rollback error, instead of 
hanging indefinitely. No API or config changes.
   
   Verified with `cargo test -p ballista-scheduler --lib` (640 passed, 
including the two new tests), `cargo fmt --check`, and `cargo clippy -p 
ballista-scheduler --lib --tests` (clean).
   


-- 
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]

Reply via email to