colexec: exhaust input in sorter benchmarks#45039
colexec: exhaust input in sorter benchmarks#45039craig[bot] merged 1 commit intocockroachdb:masterfrom asubiotto:bncl
Conversation
|
This change is |
yuzefovich
left a comment
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @asubiotto)
pkg/sql/colexec/external_sort_test.go, line 203 at r1 (raw file):
} sorter.Init() for {
[nit]: I think we tend to use form like for b := sorter.Next(ctx); b.Length() > 0; b = sorter.Next(ctx){} for this.
Some benchmarks were only calling `Next` the expected number of times and failing if a zero batch was encountered. However, operators perform cleanup when a zero batch is returned, so this cleanup step was being elided. It's also more in line with other benchmarks to exhaust the operator before finishing a benchmark. Release note: None (testing code cleanup)
asubiotto
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/colexec/external_sort_test.go, line 203 at r1 (raw file):
Previously, yuzefovich wrote…
[nit]: I think we tend to use form like
for b := sorter.Next(ctx); b.Length() > 0; b = sorter.Next(ctx){}for this.
Done.
|
bors r=yuzefovich |
|
bors was canceled for some reason bors r=yuzefovich |
Build failed |
|
roachtest flake bors r=yuzefovich |
45039: colexec: exhaust input in sorter benchmarks r=yuzefovich a=asubiotto Some benchmarks were only calling `Next` the expected number of times and failing if a zero batch was encountered. However, operators perform cleanup when a zero batch is returned, so this cleanup step was being elided. It's also more in line with other benchmarks to exhaust the operator before finishing a benchmark. Release note: None (testing code cleanup) 45080: storage/concurrency: test and bugfix for clearing the locks when the r=sumeerbhola a=sumeerbhola size limit of the lockTable is exceeded Release note: None Co-authored-by: Alfonso Subiotto Marques <alfonso@cockroachlabs.com> Co-authored-by: sumeerbhola <sumeer@cockroachlabs.com>
Build succeeded |
Some benchmarks were only calling
Nextthe expected number of times andfailing if a zero batch was encountered. However, operators perform cleanup
when a zero batch is returned, so this cleanup step was being elided. It's
also more in line with other benchmarks to exhaust the operator before
finishing a benchmark.
Release note: None (testing code cleanup)