Skip to content

Avoid blocking on a split source in fault tolerant task scheduling#12311

Merged
losipiuk merged 2 commits intotrinodb:masterfrom
raunaqmorarka:batch-future
May 21, 2022
Merged

Avoid blocking on a split source in fault tolerant task scheduling#12311
losipiuk merged 2 commits intotrinodb:masterfrom
raunaqmorarka:batch-future

Conversation

@raunaqmorarka
Copy link
Copy Markdown
Member

@raunaqmorarka raunaqmorarka commented May 10, 2022

It's possible for the probe side table scan of a join to block
producing splits until the build side collects dynamic filters.
In such a scenario, we need to avoid waiting on the blocked splits
synchronously and allow other stages to schedule.

Description

Is this change a fix, improvement, new feature, refactoring, or other?

fix

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

core query engine

How would you describe this change to a non-technical end user or system administrator?

Related issues, pull requests, and links

Documentation

( x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

( x) No release notes entries required.
( ) Release notes entries required with the following suggested text:

@cla-bot cla-bot bot added the cla-signed label May 10, 2022
@raunaqmorarka raunaqmorarka marked this pull request as ready for review May 10, 2022 15:41
@raunaqmorarka raunaqmorarka force-pushed the batch-future branch 3 times, most recently from 050fa2b to 79a2ae9 Compare May 11, 2022 09:33
@raunaqmorarka raunaqmorarka requested a review from arhimondr May 11, 2022 09:52
Copy link
Copy Markdown
Contributor

@arhimondr arhimondr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation looks good to me.

I think we should probably improve our test cases (Sorry for such a late afterthought, I should've thought about it earlier and comment).

When the components (TaskSource / FaultTolerantScheduler) were simply waiting for futures to resolve there wasn't any difference between a synchronous and asynchronous SplitSource, but now there is. Thus I wonder if it makes sense to add additional test cases that verify correctness when the futures returned (by either a TaskSource or a SplitSource) are no immediately "done").

@raunaqmorarka raunaqmorarka force-pushed the batch-future branch 5 times, most recently from ab0afdd to c433caf Compare May 18, 2022 04:56
@raunaqmorarka raunaqmorarka requested a review from arhimondr May 18, 2022 04:59
Copy link
Copy Markdown
Member

@losipiuk losipiuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - one comment

It's possible for the probe side table scan of a join to block
producing splits until the build side collects dynamic filters.
In such a scenario, we need to avoid waiting on the blocked splits
synchronously and allow other stages to schedule.
@losipiuk losipiuk merged commit cb18890 into trinodb:master May 21, 2022
@github-actions github-actions bot added this to the 382 milestone May 21, 2022
@raunaqmorarka raunaqmorarka deleted the batch-future branch October 11, 2023 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants