Retryable IO errors misclassified when wrapped in DataFusionError::Shared - #2119
Merged
andygrove merged 1 commit intoJul 21, 2026
Merged
Conversation
A retryable IO error raised on a join's shared build side arrives at the FailedTask classifier wrapped in DataFusionError::Shared, so the shallow matches! on the outermost variant never sees the IoError inside and the task is marked non-retryable. Classify on find_root() instead so the retryability decision is based on the root error regardless of wrapping.
phillipleblanc
approved these changes
Jul 21, 2026
phillipleblanc
left a comment
phillipleblanc
left a comment
Contributor
There was a problem hiding this comment.
This makes sense to me, thanks!
martin-g
approved these changes
Jul 21, 2026
andygrove
approved these changes
Jul 21, 2026
andygrove
left a comment
andygrove
left a comment
Member
There was a problem hiding this comment.
Thanks @goingforstudying-ctrl!
andygrove
added a commit
to andygrove/datafusion-ballista
that referenced
this pull request
Jul 28, 2026
- Rename concurrent_tasks to vcores in chaos-executor for the ExecutorProcessConfig change on main - Ignore executor_killed_after_shuffle_write_is_recovered against apache#2027: CI showed the reduce stage hitting the dead executor's FetchFailed flattened inside DataFusionError::Shared, so the map stage is never resubmitted; the scenario only passes when the kill loses the race - Ignore both retryable-fault scenarios against apache#2027's error flattening: apache#2028 was fixed on main (apache#2119) but the sort-shuffle writer refactor now Debug-formats task errors into opaque Execution strings before classification, so the injected IoError is misclassified as non-retryable under both AQE settings - Serialize clusters machine-wide with an advisory flock so concurrent cargo invocations or binary-parallel runners cannot start two clusters at once - Raise the executor registration deadline from 30s to 120s and attach every child process log tail to the timeout error, making the CI startup timeouts diagnosable if they recur - Update the README findings to match
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hit this while running queries with joins against flaky object storage: a transient IO error on the build side of a hash join kills the whole job instead of getting retried.
Turned out the retry classifier in
ballista/core/src/error.rsdoes a shallowmatches!(*e, DataFusionError::IoError(_))on the outermost variant. But errors coming off a join's shared build side get wrapped inDataFusionError::Shared(anArcfor sharing across consumers), so theIoErrorinside is never seen and the task falls through to the catch-allretryable: falsearm. AQE off works fine because the error arrives unwrapped, which is how I narrowed it down to the wrapping.DataFusion already has
find_root()for exactly this, so the fix is to classify on the root error instead of the wrapper. One-line change plus unit tests covering bare,Shared-wrapped,Context-wrapped-Shared, and non-IO cases.Closes #2028.
Tested with
cargo test -p ballista-core --lib error::(5 new tests pass) andcargo clippy -p ballista-core --libis clean. I couldn't run the full cluster chaos harness from #2026 locally, but the unit tests cover the classifier paths directly.