fix(11397): surface proper errors in ParquetSink #11399
Merged
Conversation
This file contains 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
…t the task join errors be surfaced
wiedld
commented
Jul 10, 2024
alamb
reviewed
Jul 10, 2024
|
I agree with @alamb early return is way better than ignoring error. |
Lordworms
pushed a commit
to Lordworms/arrow-datafusion
that referenced
this pull request
Jul 12, 2024
* fix(11397): do not surface errors for closed channels, and instead let the task join errors be surfaced * fix(11397): terminate early on channel send failure
wiedld
added a commit
to influxdata/arrow-datafusion
that referenced
this pull request
Jul 12, 2024
* fix(11397): do not surface errors for closed channels, and instead let the task join errors be surfaced * fix(11397): terminate early on channel send failure
appletreeisyellow
pushed a commit
to influxdata/arrow-datafusion
that referenced
this pull request
Jul 12, 2024
* fix(11397): do not surface errors for closed channels, and instead let the task join errors be surfaced * fix(11397): terminate early on channel send failure
findepi
pushed a commit
to findepi/datafusion
that referenced
this pull request
Jul 16, 2024
* fix(11397): do not surface errors for closed channels, and instead let the task join errors be surfaced * fix(11397): terminate early on channel send failure
xinlifoobar
pushed a commit
to xinlifoobar/datafusion
that referenced
this pull request
Jul 17, 2024
* fix(11397): do not surface errors for closed channels, and instead let the task join errors be surfaced * fix(11397): terminate early on channel send failure
appletreeisyellow
pushed a commit
to influxdata/arrow-datafusion
that referenced
this pull request
Jul 17, 2024
* fix(11397): do not surface errors for closed channels, and instead let the task join errors be surfaced * fix(11397): terminate early on channel send failure Add Optimizer Sanity Checker, improve sortedness equivalence properties (apache#11196) * Initial optimizer sanity checker. Only includes sort reqs, docs will be added. * Add distro and pipeline friendly checks * Also check the plans we create are correct. * Add distribution test cases using global limit exec. * Add test for multiple children using SortMergeJoinExec. * Move PipelineChecker to SanityCheckPlan * Fix some tests and add docs * Add some test docs and fix clippy diagnostics. * Fix some failing tests * Replace PipelineChecker with SanityChecker in .slt files. * Initial commit * Slt tests pass * Resolve linter errors * Minor changes * Minor changes * Minor changes * Minor changes * Sort PreservingMerge clear per partition * Minor changes * Update output_requirements.rs * Address reviews * Update datafusion/core/src/physical_optimizer/optimizer.rs Co-authored-by: Mehmet Ozan Kabak <[email protected]> * Update datafusion/core/src/physical_optimizer/sanity_checker.rs Co-authored-by: Mehmet Ozan Kabak <[email protected]> * Address reviews * Minor changes * Apply suggestions from code review Co-authored-by: Andrew Lamb <[email protected]> * Update comment * Add map implementation --------- Co-authored-by: Erman Yafay <[email protected]> Co-authored-by: berkaysynnada <[email protected]> Co-authored-by: Mehmet Ozan Kabak <[email protected]> Co-authored-by: Andrew Lamb <[email protected]>
xinlifoobar
pushed a commit
to xinlifoobar/datafusion
that referenced
this pull request
Jul 18, 2024
* fix(11397): do not surface errors for closed channels, and instead let the task join errors be surfaced * fix(11397): terminate early on channel send failure
appletreeisyellow
pushed a commit
to influxdata/arrow-datafusion
that referenced
this pull request
Jul 22, 2024
* fix(11397): do not surface errors for closed channels, and instead let the task join errors be surfaced * fix(11397): terminate early on channel send failure
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Which issue does this PR close?
Closes #11397
Rationale for this change
During the parallel writes in ParquetSink, we spawn a series of parallel tasks and then message pass the outcome from one task to the next. In abstraction:
read_batches => channel =>
Vec<col_write_tasks>=> channel =>Vec<serialize_rowgroup_tasks>When we encounter an error in one of the
Vec<x_tasks>we are first surfacing an error on the channel.send() rather than on the task join.What changes are included in this PR?
Don't surface the errors on the channel send.
This results in the proper error returned, as can be seen on the updated test.
Are these changes tested?
Yes.
Are there any user-facing changes?
No.