andygrove opened a new issue, #2028:
URL: https://github.com/apache/datafusion-ballista/issues/2028
## Describe the bug
A genuinely retryable IO error is classified as **non-retryable** — and so
fails the job instead of being retried — whenever it reaches the classifier
wrapped inside `DataFusionError::Shared`. This happens in practice for errors
raised under a join's shared build side.
## Root cause
`ballista/core/src/error.rs` (~line 237) decides retryability with a
**shallow** match on the inner error:
```rust
BallistaError::DataFusionError(e) if matches!(*e,
DataFusionError::IoError(_)) => FailedTask {
retryable: true,
count_to_failures: true,
failed_reason: Some(FailedReason::IoError(IoError {})),
}
```
`matches!` inspects only the outermost variant. DataFusion wraps errors in
`DataFusionError::Shared` (an `Arc<DataFusionError>`) when an error is shared
across multiple consumers — which is exactly what happens on a hash join's
build side. A `DataFusionError::Shared(Arc(IoError(..)))` does not match
`DataFusionError::IoError(_)`.
The error therefore falls through to the catch-all (~line 248):
```rust
other => FailedTask {
retryable: false,
count_to_failures: false,
failed_reason: Some(FailedReason::ExecutionError(ExecutionError {})),
}
```
so the task is never retried and the job fails.
This affects **real Parquet/object-store IO errors**, not only synthetic
ones: a transient S3 or local-IO failure on a join's build side is precisely
the case Ballista's retry logic exists to absorb, and it is the case where it
silently does not apply.
## To Reproduce
Reproduced end to end on a real multi-process cluster by the chaos harness
in #PR_NUM (scenario `retryable_fault_is_retried_and_result_is_correct`, AQE-on
case).
The harness injects exactly one `DataFusionError::IoError` from a UDF on a
join input, with a cluster-wide budget of one fault, so the retry is guaranteed
to succeed if it happens at all:
- **AQE off:** the error reaches the classifier unwrapped → correctly marked
retryable → task retried → **query succeeds with the correct result.** ✅
- **AQE on:** the same error arrives wrapped in `DataFusionError::Shared`
(shared join build side) → misses the `matches!` arm → marked `retryable:
false` → **the job fails.** ❌
The difference between the two cases isolates the wrapping as the cause.
## Expected behavior
Retryability should be determined by the *root* error, not the outermost
wrapper. An `IoError` should be classified `retryable: true` regardless of
whether it arrives bare, inside `Shared`, or inside any other wrapping variant.
DataFusion provides `DataFusionError::find_root()` for exactly this;
classification in `error.rs` should unwrap before matching.
## Additional context
Sibling of the `FetchFailed` issue: **structured errors lose their type
crossing the DataFusion boundary, and retryability is decided by shallow
matches that cannot see through the wrapping.** Fixing both probably wants a
single, well-tested `classify(&BallistaError) -> FailedTask` that unwraps first.
--
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]
