Skip to content

Commit

Permalink
sql: add error and reporting when unable to fix malformed quantile
Browse files Browse the repository at this point in the history
This PR captures quantile information for future reproduction when we
encounter a malformed quantile that we're unable to fix, instead of
inducing an inactionable panic.

Epic: None
Fixes: #109060

Release note: None
  • Loading branch information
rharding6373 committed Aug 24, 2023
1 parent 236f70a commit 120132e
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 9 deletions.
1 change: 1 addition & 0 deletions pkg/sql/stats/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ go_library(
"//pkg/util/hlc",
"//pkg/util/intsets",
"//pkg/util/log",
"//pkg/util/log/logcrash",
"//pkg/util/mon",
"//pkg/util/protoutil",
"//pkg/util/stop",
Expand Down
11 changes: 9 additions & 2 deletions pkg/sql/stats/forecast.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util/errorutil"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/log/logcrash"
"github.com/cockroachdb/errors"
"github.com/cockroachdb/redact"
)
Expand Down Expand Up @@ -272,7 +273,7 @@ func forecastColumnStatistics(
// histogram. NOTE: If any of the observed histograms were for inverted
// indexes this will produce an incorrect histogram.
if observed[0].HistogramData != nil && observed[0].HistogramData.ColumnType != nil {
hist, err := predictHistogram(ctx, observed, forecastAt, minRequiredFit, nonNullRowCount)
hist, err := predictHistogram(ctx, sv, observed, forecastAt, minRequiredFit, nonNullRowCount)
if err != nil {
// If we did not successfully predict a histogram then copy the latest
// histogram so we can adjust it.
Expand Down Expand Up @@ -358,6 +359,7 @@ func forecastColumnStatistics(
// predictHistogram tries to predict the histogram at forecast time.
func predictHistogram(
ctx context.Context,
sv *settings.Values,
observed []*TableStatistic,
forecastAt float64,
minRequiredFit float64,
Expand Down Expand Up @@ -410,7 +412,12 @@ func predictHistogram(
// Construct a linear regression model of quantile functions over time, and
// use it to predict a quantile function at the given time.
yₙ, r2 := quantileSimpleLinearRegression(createdAts, quantiles, forecastAt)
yₙ = yₙ.fixMalformed()
var err error
yₙ, err = yₙ.fixMalformed()
if err != nil {
logcrash.ReportOrPanic(ctx, sv, "unable to fix malformed stats quantile: %v", yₙ)
return histogram{}, err
}
log.VEventf(
ctx, 3, "forecast for table %v columns %v predicted quantile %v R² %v",
tableID, columnIDs, yₙ, r2,
Expand Down
11 changes: 7 additions & 4 deletions pkg/sql/stats/quantile.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ var zeroQuantile = quantile{{p: 0, v: 0}, {p: 1, v: 0}}
// If you are introducing a new histogram version, please check whether
// makeQuantile and quantile.toHistogram need to change, and then increase the
// hard-coded number here.
const _ uint = 2 - uint(histVersion)
const _ = 2 - uint(histVersion)

// canMakeQuantile returns true if a quantile function can be created for a
// histogram of the given type. Note that by not supporting BYTES we rule out
Expand Down Expand Up @@ -656,10 +656,10 @@ func (q quantile) integrateSquared() float64 {
// This function fixes the negative slope pieces by "moving" p from the
// overlapping places to where it should be. No p is lost in the making of these
// calculations.
func (q quantile) fixMalformed() quantile {
func (q quantile) fixMalformed() (quantile, error) {
// Check for the happy case where q is already well-formed.
if q.isWellFormed() {
return q
return q, nil
}

// To fix a malformed quantile function, we recalculate p for each distinct
Expand Down Expand Up @@ -803,6 +803,9 @@ func (q quantile) fixMalformed() quantile {
// "below" our horizontal line (<= v) and ends "above" our horizontal line
// (> v) and we always add two intersectionPs for "double roots" touching
// our line from above (one endpoint > v, one endpoint = v).
if len(intersectionPs) == 0 {
return q, errors.New("unable to fix malformed stats quantile")
}
lessEqP := intersectionPs[0]
for j := 1; j < len(intersectionPs); j += 2 {
lessEqP += intersectionPs[j+1] - intersectionPs[j]
Expand All @@ -818,7 +821,7 @@ func (q quantile) fixMalformed() quantile {
fixed = append(fixed, quantilePoint{p: lessEqP, v: val})
}
}
return fixed
return fixed, nil
}

// isWellFormed returns true if q is well-formed (i.e. is non-decreasing in v).
Expand Down
10 changes: 8 additions & 2 deletions pkg/sql/stats/quantile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1348,7 +1348,10 @@ func TestQuantileOps(t *testing.T) {
if intSqMult0 != 0 {
t.Errorf("test case %d incorrect intSqMult0 %v expected 0", i, intSqMult0)
}
fixed := tc.q.fixMalformed()
fixed, err := tc.q.fixMalformed()
if err != nil {
t.Errorf("test case %d has an unexpected error: %s", i, err)
}
if !reflect.DeepEqual(fixed, tc.fixed) {
t.Errorf("test case %d incorrect fixed %v expected %v", i, fixed, tc.fixed)
}
Expand Down Expand Up @@ -1407,7 +1410,10 @@ func TestQuantileOpsRandom(t *testing.T) {
if intSqMult0 != 0 {
t.Errorf("seed %v quantile %v incorrect intSqMult0 %v expected 0", seed, q, intSqMult0)
}
fixed := q.fixMalformed()
fixed, err := q.fixMalformed()
if err != nil {
t.Errorf("seed %v quantile %v had an unexpected error: %s", seed, q, err)
}
intSqFixed := fixed.integrateSquared()
// This seems like it should run into floating point errors, but it hasn't
// yet, so yay?
Expand Down
6 changes: 5 additions & 1 deletion pkg/sql/stats/simple_linear_regression_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,11 @@ func TestQuantileSimpleLinearRegression(t *testing.T) {
for i, tc := range testCases {
t.Run(strconv.Itoa(i), func(t *testing.T) {
yn, r2 := quantileSimpleLinearRegression(tc.x, tc.y, tc.xn)
yn = yn.fixMalformed()
var err error
yn, err = yn.fixMalformed()
if err != nil {
t.Errorf("test case %d had an unexpected error: %s", i, err)
}
if !reflect.DeepEqual(yn, tc.yn) {
t.Errorf("test case %d incorrect yn %v expected %v", i, yn, tc.yn)
}
Expand Down

0 comments on commit 120132e

Please sign in to comment.