distsqlrun: make joinreader use limited batches#38837
distsqlrun: make joinreader use limited batches#38837craig[bot] merged 2 commits intocockroachdb:masterfrom
Conversation
|
This change is |
yuzefovich
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, 2 of 2 files at r2.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis and @solongordon)
pkg/sql/opt/exec/execbuilder/testdata/lookup_join, line 787 at r2 (raw file):
SET tracing = on; SELECT COUNT(*) FROM (SELECT * FROM b NATURAL INNER LOOKUP JOIN a); SET tracing = off let $lookupTableID
Huh, this is cool! I don't think I've seen using variables in logic tests before.
9a68b20 to
cc19926
Compare
|
pkg/sql/opt/exec/execbuilder/testdata/lookup_join, line 787 at r2 (raw file): Previously, yuzefovich wrote…
Unfortunately the test below still only passes when this is 63 (because of the output).. |
solongordon
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1.
Reviewable status:
pkg/sql/opt/exec/execbuilder/testdata/lookup_join, line 785 at r3 (raw file):
statement ok SET tracing = on; SELECT count(*) FROM (SELECT * FROM b NATURAL INNER LOOKUP JOIN a); SET tracing = off
I'd say might as well verify the count(*) result while you're at it.
Release note: None
Previously, lookup join put no limit on the number of returned rows from each of its lookups. This could be hugely problematic. Imagine a circumstance in which a lookup join was planned on a pair of tables that had 1 billion match rows on the right side for every row on the left side: in this case, the lookup join would ask the KV backend to return a single giant buffer with all 1 billion rows. Release note (bug fix): prevent OOM conditions during lookup joins between tables with a very large n:1 relationship.
cc19926 to
5368cf0
Compare
jordanlewis
left a comment
There was a problem hiding this comment.
TFTRs!
Reviewable status:
pkg/sql/opt/exec/execbuilder/testdata/lookup_join, line 787 at r2 (raw file):
Previously, RaduBerinde wrote…
Unfortunately the test below still only passes when this is 63 (because of the output)..
Yeah :( But at least it'll rewrite correctly with -rewrite without having to edit the queries.
pkg/sql/opt/exec/execbuilder/testdata/lookup_join, line 785 at r3 (raw file):
Previously, solongordon (Solon) wrote…
I'd say might as well verify the
count(*)result while you're at it.
Done.
|
bors r+ |
38837: distsqlrun: make joinreader use limited batches r=jordanlewis a=jordanlewis Closes #35950. Previously, lookup join put no limit on the number of returned rows from each of its lookups. This could be hugely problematic. Imagine a circumstance in which a lookup join was planned on a pair of tables that had 1 billion match rows on the right side for every row on the left side: in this case, the lookup join would ask the KV backend to return a single giant buffer with all 1 billion rows. Also, joinReader's tracing span collection was slightly broken - fix that while we're at it, to make sure we can test this change properly. Release note (bug fix): prevent OOM conditions during lookup joins between tables with a very large n:1 relationship. 38887: exec: check for unsupported by ArrowBatchConverter type r=yuzefovich a=yuzefovich Previously, we would create an arrow batch converter without paying attention to whether we supported the types which could lead to a panic during actual conversion. Now we do this check upfront so that we can fall back to DistSQL. Release note: None Co-authored-by: Jordan Lewis <jordanthelewis@gmail.com> Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Build succeeded |
Closes #35950.
Previously, lookup join put no limit on the number of returned rows from
each of its lookups. This could be hugely problematic. Imagine a
circumstance in which a lookup join was planned on a pair of tables that
had 1 billion match rows on the right side for every row on the left
side: in this case, the lookup join would ask the KV backend to return a
single giant buffer with all 1 billion rows.
Also, joinReader's tracing span collection was slightly broken - fix that while we're at it, to make sure we can test this change properly.
Release note (bug fix): prevent OOM conditions during lookup joins
between tables with a very large n:1 relationship.