Skip to content

Commit

Permalink
sql/sem/builtins: deflake TestSerialNormalizationWithUniqueUnorderedID
Browse files Browse the repository at this point in the history 
This patch changes the test to run its main logic multiple times
(currently set to 10) and only fail due to non-uniformity if we see
statistically significant non-uniformity in a majority of the runs.

For context, an occasional failure is not unexpected given this is a
statistical test, but multiple failures in a row would be more
suggestive of an actual problem with the uniformity of generated IDs.

Release note: None
  • Loading branch information
@andyyang890
andyyang890 committed Aug 3, 2023
1 parent 46255c8 commit fbcd656
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 @@ -97,8 +96,8 @@ func TestMapToUniqueUnorderedID(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 @@ -107,25 +106,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 @@ -134,30 +141,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 @@ -167,55 +173,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 fbcd656

Please sign in to comment.