Skip to content

Disable spill for probe only grouped execution#15592

Merged
rschlussel merged 1 commit intoprestodb:masterfrom
rschlussel:disable-probe-only-grouped
Jan 7, 2021
Merged

Disable spill for probe only grouped execution#15592
rschlussel merged 1 commit intoprestodb:masterfrom
rschlussel:disable-probe-only-grouped

Conversation

@rschlussel
Copy link
Copy Markdown
Contributor

@rschlussel rschlussel commented Jan 7, 2021

This PR is to unblock safely rolling out spill for join. Spill for joins where only the probe side uses grouped execution was broken because some code relied on having a fixed number of probe operators for each build operator. These queries would fail with errors like "5 probe operators finished out of 4 declared"

A proper fix would be to remove the dependency on a fixed number of probe operators, and instead do something similar to how JoinLifecycle works, where you keep track of how many probe operator references you have, and only destroy the build table if all probe operators have finished and the LookupJoinOperatorFactory is closed. But that fix requires more rigorous testing to ensure we're able to keep unspilling and respilling the partitions of the hash table.

Test plan - Verifier. I had trouble writing a test that would reliably finish, but I ran this with verifier on queries I had previously seen failures on, and the errors went away.

== RELEASE NOTES ==

General Changes
* Disable spill to disk for join queries where the probe side uses grouped execution and the build does not.  Previously these queries would fail with generic_internal_errors.

@rschlussel rschlussel requested review from sachdevs and wenleix January 7, 2021 17:07
spill does not work for probe only grouped execution because it expects
a constant number of probe operators, but instead each lifespan can be a
separate operator.  Previously such queries would fail with
generic_internal_errors and messages like "5 probe operators finished
out of 4 declared"
@rschlussel rschlussel force-pushed the disable-probe-only-grouped branch from 50ab366 to ed5ca47 Compare January 7, 2021 17:08
@rschlussel rschlussel merged commit dc00b9f into prestodb:master Jan 7, 2021
@rschlussel rschlussel deleted the disable-probe-only-grouped branch January 7, 2021 22:37
@caithagoras caithagoras mentioned this pull request Jan 11, 2021
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants