Skip to content

Commit

Permalink
Merge #108069
Browse files Browse the repository at this point in the history 
108069: sql/sem/builtins: deflake TestSerialNormalizationWithUniqueUnorderedID r=rafiss a=andyyang890

This patch increases the number of retries for this test to 3 and increases the number of rows inserted to an even larger number to hopefully reduce the number of flakes.

Fixes #107899

Release note: None

Co-authored-by: Andy Yang <[email protected]>
  • Loading branch information
@andyyang890
craig[bot] and andyyang890 committed Aug 4, 2023
2 parents c5c821a + fbcd656 commit 142b971
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 78 deletions.
2 changes: 0 additions & 2 deletions pkg/sql/sem/builtins/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,10 +214,8 @@ go_test(
"//pkg/util/log",
"//pkg/util/mon",
"//pkg/util/randutil",
"//pkg/util/retry",
"//pkg/util/syncutil",
"//pkg/util/timeutil",
"@com_github_cockroachdb_errors//:errors",
"@com_github_lib_pq//:pq",
"@com_github_lib_pq//oid",
"@com_github_stretchr_testify//assert",
Expand Down
164 changes: 88 additions & 76 deletions pkg/sql/sem/builtins/builtins_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"bytes"
"context"
"fmt"
"math"
"math/bits"
"math/rand"
"testing"
Expand All @@ -32,9 +33,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/duration"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/retry"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"github.com/cockroachdb/errors"
"github.com/lib/pq"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -131,8 +130,8 @@ func TestMapToUnorderedUniqueInt(t *testing.T) {
}

// TestSerialNormalizationWithUniqueUnorderedID makes sure that serial
// normalization can use unique_unordered_id() and a split in a table followed
// by insertions guarantees a (somewhat) uniform distribution of the data.
// normalization can use the unordered_rowid mode and a large number of
// insertions (80k) results in a (somewhat) uniform distribution of the data.
func TestSerialNormalizationWithUniqueUnorderedID(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)
Expand All @@ -141,25 +140,33 @@ func TestSerialNormalizationWithUniqueUnorderedID(t *testing.T) {
"assumes large N")

ctx := context.Background()
const attempts = 2
// This test can flake when the random data is not distributed evenly.
err := retry.WithMaxAttempts(ctx, retry.Options{}, attempts, func() error {

params := base.TestServerArgs{
Knobs: base.TestingKnobs{
SQLEvalContext: &eval.TestingKnobs{
// We disable the randomization of some batch sizes because
// with some low values the test takes much longer.
ForceProductionValues: true,

// Since this is a statistical test, an occasional observation of non-uniformity
// is not in and of itself a huge concern. As such, we'll only fail the test if
// a majority of our observations show statistically significant amounts of
// non-uniformity.
const numObservations = 10
numNonUniformObservations := 0
for i := 0; i < numObservations; i++ {
// We use an anonymous function because of the defer in each iteration.
func() {
t.Logf("starting observation %d", i)

params := base.TestServerArgs{
Knobs: base.TestingKnobs{
SQLEvalContext: &eval.TestingKnobs{
// We disable the randomization of some batch sizes because
// with some low values the test takes much longer.
ForceProductionValues: true,
},
},
},
}
s, db, _ := serverutils.StartServer(t, params)
defer s.Stopper().Stop(ctx)
}
s, db, _ := serverutils.StartServer(t, params)
defer s.Stopper().Stop(ctx)

tdb := sqlutils.MakeSQLRunner(db)
// Create a new table with serial primary key i (unordered_rowid) and int j (index).
tdb.Exec(t, `
tdb := sqlutils.MakeSQLRunner(db)
// Create a new table with serial primary key i (unordered_rowid) and int j (index).
tdb.Exec(t, `
SET serial_normalization TO 'unordered_rowid';
CREATE DATABASE t;
USE t;
Expand All @@ -168,30 +175,29 @@ CREATE TABLE t (
j INT
)`)

numberOfRows := 100000

// Insert rows.
tdb.Exec(t, fmt.Sprintf(`
// Insert rows.
numberOfRows := 80000
tdb.Exec(t, fmt.Sprintf(`
INSERT INTO t(j) SELECT * FROM generate_series(1, %d);
`, numberOfRows))

// Build an equi-width histogram over the key range. The below query will
// generate the key bounds for each high-order bit pattern as defined by
// prefixBits. For example, if this were 3, we'd get the following groups:
//
// low | high
// ----------------------+----------------------
// 0 | 2305843009213693952
// 2305843009213693952 | 4611686018427387904
// 4611686018427387904 | 6917529027641081856
// 6917529027641081856 | 9223372036854775807
//
const prefixBits = 4
var keyCounts pq.Int64Array
tdb.QueryRow(t, `
// Build an equi-width histogram over the key range. The below query will
// generate the key bounds for each high-order bit pattern as defined by
// prefixBits. For example, if this were 3, we'd get the following groups:
//
// low | high
// ----------------------+----------------------
// 0 | 2305843009213693952
// 2305843009213693952 | 4611686018427387904
// 4611686018427387904 | 6917529027641081856
// 6917529027641081856 | 9223372036854775807
//
const prefixBits = 4
var keyCounts pq.Int64Array
tdb.QueryRow(t, `
WITH boundaries AS (
SELECT i << (64 - $1) AS p FROM ROWS FROM (generate_series(0, (1<<($1-1)) -1)) AS t (i)
UNION ALL SELECT (((1 << 62) - 1) << 1)+1 -- int63 max value
SELECT i << (64 - $1) AS p FROM ROWS FROM (generate_series(0, (1 << ($1 - 1)) - 1)) AS t (i)
UNION ALL SELECT (((1 << 62) - 1) << 1) + 1 -- int63 max value
),
groups AS (
SELECT *
Expand All @@ -201,55 +207,61 @@ INSERT INTO t(j) SELECT * FROM generate_series(1, %d);
counts AS (
SELECT count(i) AS c
FROM t, groups
WHERE low <= i AND high > i
WHERE low < i AND i <= high
GROUP BY (low, high)
)
SELECT array_agg(c)
FROM counts;`, prefixBits).Scan(&keyCounts)

t.Log("Key counts in each split range")
for i, keyCount := range keyCounts {
t.Logf("range %d: %d\n", i, keyCount)
}
require.Len(t, keyCounts, 1<<(prefixBits-1))

// To check that the distribution over ranges is not uniform, we use a
// chi-square goodness of fit statistic. We'll set our null hypothesis as
// 'each range in the distribution should have the same probability of getting
// a row inserted' and we'll check if we can reject the null hypothesis if
// chi-square is greater than the critical value we currently set as 35.2585,
// a deliberate choice that gives us a p-value of 0.00001 according to
// https://www.fourmilab.ch/rpkp/experiments/analysis/chiCalc.html. If we are
// able to reject the null hypothesis, then the distribution is not uniform,
// and we raise an error. This test has 7 degrees of freedom (categories - 1).
chiSquared := discreteUniformChiSquared(keyCounts)
criticalValue := 35.2585
if chiSquared >= criticalValue {
return errors.Newf("chiSquared value of %f must be"+
" less than criticalVal %f to guarantee distribution is relatively uniform",
chiSquared, criticalValue)
}
return nil
})
require.NoError(t, err)
t.Log("Key counts in each split range")
for i, keyCount := range keyCounts {
t.Logf("range %d: %d\n", i, keyCount)
}
require.Len(t, keyCounts, 1<<(prefixBits-1))

// To check that the distribution over ranges is not uniform, we use a
// chi-square goodness of fit statistic. We'll set our null hypothesis as
// 'each range in the distribution should have the same probability of getting
// a row inserted' and we'll check if we can reject the null hypothesis if
// chi-squared is greater than the critical value we currently set as 35.2585,
// a deliberate choice that gives us a p-value of 0.00001 according to
// https://www.fourmilab.ch/rpkp/experiments/analysis/chiCalc.html. If we are
// able to reject the null hypothesis, then the distribution is not uniform,
// and we raise an error. This test has 7 degrees of freedom (categories - 1).
chiSquared := discreteUniformChiSquared(keyCounts)
criticalValue := 35.2585
if chiSquared > criticalValue {
t.Logf("chiSquared value of %f > criticalVal %f indicates that the distribution "+
"is non-uniform (statistically significant, p < 0.00001)",
chiSquared, criticalValue)
numNonUniformObservations += 1
} else {
t.Logf("chiSquared value of %f <= criticalVal %f indicates that the distribution "+
"is relatively uniform",
chiSquared, criticalValue)
}
}()
}

if numNonUniformObservations >= numObservations/2 {
t.Fatalf("a majority of our observations indicate the distribution is non-uniform")
}
}

// discreteUniformChiSquared calculates the chi-squared statistic (ref:
// https://www.itl.nist.gov/div898/handbook/eda/section3/eda35f.htm) to be used
// in our hypothesis testing for the distribution of rows among ranges.
func discreteUniformChiSquared(counts []int64) float64 {
func discreteUniformChiSquared(buckets []int64) float64 {
var n int64
for _, c := range counts {
for _, c := range buckets {
n += c
}
p := float64(1) / float64(len(counts))
var stat float64
for _, c := range counts {
oSubE := float64(c)/float64(n) - p
stat += (oSubE * oSubE) / p
expectedPerBucket := float64(n) / float64(len(buckets))
var chiSquared float64
for _, c := range buckets {
chiSquared += math.Pow(float64(c)-expectedPerBucket, 2) / expectedPerBucket
}
stat *= float64(n)
return stat
return chiSquared
}

func TestStringToArrayAndBack(t *testing.T) {
Expand Down

0 comments on commit 142b971

Please sign in to comment.