Replace AbortOnDrop / AbortDropOnMany with tokio JoinSet #6750
Conversation
aprimadi
changed the title
|
Thank you @aprimadi (cc @crepererum ) |
|
Sorry this is going to take a while @alamb I'm a bit busy these last few weeks. Will work on this whenever I have some free hours. Feel free to let me know if you need to get this done quickly and I'll get it to a reviewable state and mark it as ready for review. |
No problem @aprimadi -- we really appreciate all the help! In order to avoid this PR bitrotting, perhaps you could get it to a reviewable state (even if it doesn't port all of the uses) and we can merge that as you work on the other uses? |
There was a problem hiding this comment.
Love it 👍
BTW: I wonder if we could use clippy's disallowed_methods lint to prevent the usage of tokio::spawn altogether, since it led to so many cancellation and error propagation bugs.
|
I haven't succeed in refactoring RepartitionExec to use JoinSet. |
That is likely the most tricky one |
That's OK, I've added a subtask to #6513 so we can do that in a follow-up. Let's merge this PR here so it doesn't get stale. |
|
Thank you @aprimadi and @crepererum ❤️ |
|
Thank you @crepererum and @alamb for the review. Sorry I haven't succeed in refactoring RepartitionExec been kinda busy. I may try again once I have quite a large chunk of development and debugging time to tackle it. Or I may also not be able to do it at all due to not having enough skill 😜. |
* Use JoinSet in MemTable * Fix error handling * Refactor AbortOnDropSingle in csv physical plan * Fix csv write physical plan error propagation * Refactor json write physical plan to use JoinSet * Refactor parquet write physical plan to use JoinSet * Refactor collect_partitioned to use JoinSet * Refactor pull_from_input method to make it easier to read * Fix typo
Which issue does this PR close?
Work toward #6513
Rationale for this change
N/A
What changes are included in this PR?
N/A
Are these changes tested?
Yes, by existing tests.
Are there any user-facing changes?
No.