From aa74f4d6c1dc3108ceccca0d92e2660fbd71c744 Mon Sep 17 00:00:00 2001 From: Erik Grinaker Date: Wed, 24 Mar 2021 15:25:28 +0100 Subject: [PATCH] sql: default to batch size 1 in allocator In #62282, the estimated row count was passed into the scan batch allocator to avoid growing the batch from 1. However, this also changed the default batch size from 1 to 1024 when no row count estimate was available, giving significant overhead when fetching small result sets. On `kv95/enc=false/nodes=1/cpu=32` this reduced performance from 66304 ops/s to 58862 ops/s (median of 5 runs), since these are single-row reads without estimates. This patch reverts the default batch size to 1 when no row count estimate is available. This fully fixes the `kv95` performance regression. YCSB/E takes a small hit going from 1895 ops/s to 1786 ops/s, but this only seems to happen because it takes a while for the statistics to update: sometime within the first minute of the test (after the 1-minute ramp-up period), throughput abruptly changes from ~700 ops/s to ~1800 ops/s, so using a 2-minute ramp-up period in roachtest would mostly eliminate any difference. Release note: None --- pkg/sql/colmem/allocator.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/sql/colmem/allocator.go b/pkg/sql/colmem/allocator.go index db1de607d764..1e9775539546 100644 --- a/pkg/sql/colmem/allocator.go +++ b/pkg/sql/colmem/allocator.go @@ -156,8 +156,8 @@ func (a *Allocator) NewMemBatchNoCols(typs []*types.T, capacity int) coldata.Bat // NOTE: if the reallocation occurs, then the memory under the old batch is // released, so it is expected that the caller will lose the references to the // old batch. -// Note: the method assumes that minCapacity is at least 1 and will "truncate" -// minCapacity if it is larger than coldata.BatchSize(). +// Note: the method assumes that minCapacity is at least 0 and will clamp +// minCapacity to be between 1 and coldata.BatchSize() inclusive. // TODO(yuzefovich): change the contract so that maxBatchMemSize takes priority // over minCapacity. func (a *Allocator) ResetMaybeReallocate( @@ -165,7 +165,9 @@ func (a *Allocator) ResetMaybeReallocate( ) (newBatch coldata.Batch, reallocated bool) { if minCapacity < 0 { colexecerror.InternalError(errors.AssertionFailedf("invalid minCapacity %d", minCapacity)) - } else if minCapacity == 0 || minCapacity > coldata.BatchSize() { + } else if minCapacity == 0 { + minCapacity = 1 + } else if minCapacity > coldata.BatchSize() { minCapacity = coldata.BatchSize() } reallocated = true