Skip to content

Commit

Permalink
sql/stats: more fully undo addOuterBuckets in stripOuterBuckets
Browse files Browse the repository at this point in the history
Near the end of `stats.(*histogram).adjustCounts`, if there is leftover
DistinctCount after adjusting each histogram bucket, we call
`addOuterBuckets`. This function creates two "outer" buckets that fully
cover the lower and upper ends of the domain with the remaining
DistinctCount.

These "outer" buckets interfere with partial stats, so we remove them
with `stripOuterBuckets` when merging partial stats. `stripOuterBuckets`
was simply slicing off the two buckets, but this could potentially leave
non-zero DistinctCount in the first bucket (which is added by
`addOuterBuckets`). `stripOuterBuckets` needs to also zero the
DistinctCount and NumRange of the first bucket to fully undo the actions
of `addOuterBuckets`.

We were only catching this when trying to forecast using the merged
histogram, which would sometimes produce a forecasted histogram with
non-zero DistinctRange in the first bucket, leading to the infamous
"histogram had first bucket with non-zero NumRange or DistinctRange"
Sentry failure.

Unfortunately, I don't think this bug fully explains all of the Sentry
failures that look like #93892 because many of those predate the
introduction of partial stats. I believe this specific failure started
with v24.3.0-alpha.0.

Informs: #93892
Fixes: #134031

(No release note because automatic collection of partial stats is not yet
enabled in any available release.)

Release note: None
  • Loading branch information
michae2 committed Nov 15, 2024
1 parent f8e69fb commit 006fbe5
Show file tree
Hide file tree
Showing 3 changed files with 206 additions and 2 deletions.
200 changes: 200 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/distsql_stats
2 changes: 1 addition & 1 deletion pkg/sql/opt/exec/execbuilder/testdata/partial_stats
6 changes: 5 additions & 1 deletion pkg/sql/stats/merge.go

0 comments on commit 006fbe5

Please sign in to comment.