Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

statistic: fix panic when building topN #47928

Merged
merged 12 commits into from
Oct 25, 2023
25 changes: 25 additions & 0 deletions pkg/statistics/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ import (
"github.com/pingcap/tidb/pkg/types"
"github.com/pingcap/tidb/pkg/util/codec"
"github.com/pingcap/tidb/pkg/util/collate"
"github.com/pingcap/tidb/pkg/util/logutil"
"github.com/pingcap/tidb/pkg/util/memory"
"go.uber.org/zap"
)

// SortedBuilder is used to build histograms for PK and index.
Expand Down Expand Up @@ -373,12 +375,35 @@ func BuildHistAndTopN(
if err != nil {
return nil, nil, errors.Trace(err)
}
foundTwice := false
for j := 0; j < len(topNList); j++ {
if bytes.Equal(sampleBytes, topNList[j].Encoded) {
// find the same value in topn: need to skip over this value in samples
copy(samples[i:], samples[uint64(i)+topNList[j].Count:])
samples = samples[:uint64(len(samples))-topNList[j].Count]
// This should never happen, but we met this panic before, so we add this check here.
// See: https://github.com/pingcap/tidb/issues/35948
if foundTwice {
allTopNByteStrings := make([][]byte, 0, len(topNList))
for _, topN := range topNList {
allTopNByteStrings = append(allTopNByteStrings, topN.Encoded)
}
logutil.BgLogger().With(
zap.String("category", "stats"),
).Warn(
"invalid sample data",
zap.ByteString("sampleBytes", sampleBytes),
zap.ByteString("topNBytes", topNList[j].Encoded),
zap.Int64("i", i),
zap.Int64("j", int64(j)),
Rustin170506 marked this conversation as resolved.
Show resolved Hide resolved
zap.ByteStrings("allTopNBytes", allTopNByteStrings),
)
Rustin170506 marked this conversation as resolved.
Show resolved Hide resolved
// NOTE: if we don't return here, we may meet panic in the following code.
// The i may decrease to a negative value.
break
}
i--
foundTwice = true
continue
}
}
Expand Down