-
Notifications
You must be signed in to change notification settings - Fork 4.1k
sql: initial batch sizes too large with no row estimate hint #62524
Description
While reviewing #62282, kv95 benchmarks were shown to regress somewhat. This still holds after landing on master, with e.g. kv95/enc=false/nodes=1/cpu=32 giving 66304 ops/s before the patch and 58862 ops/s after the patch. When reverting to using 1 as the minimum capacity instead of estimatedRowCount in the ResetMaybeReallocate call the performance goes back to the baseline:
cockroach/pkg/sql/colfetcher/cfetcher.go
Line 329 in 5041b22
| rf.typs, rf.machine.batch, estimatedRowCount, rf.memoryLimit, |
While debugging, it was found that estimatedRowCount would often be 0, which would cause ResetMaybeReallocate to allocate a batch of coldata.BatchSize() (i.e. 1024) every time, instead of 1 which was the old behavior. This probably explains the performance drop.
cockroach/pkg/sql/colmem/allocator.go
Lines 168 to 170 in 5041b22
| } else if minCapacity == 0 || minCapacity > coldata.BatchSize() { | |
| minCapacity = coldata.BatchSize() | |
| } |