ccl/workloadccl/allccl: TestDeterministicInitialData failed

[cockroach-teamcity]

ccl/workloadccl/allccl.TestDeterministicInitialData failed with artifacts on master @ 61f7c816574848f804f1adf7f42f7315bfcaaeba:

=== RUN   TestDeterministicInitialData
--- FAIL: TestDeterministicInitialData (8.73s)
=== RUN   TestDeterministicInitialData/tpch
    all_test.go:305: 
        	Error Trace:	/home/roach/.cache/bazel/_bazel_roach/c5a4e7d36696d9cd970af2045211a7df/sandbox/processwrapper-sandbox/3236/execroot/com_github_cockroachdb_cockroach/bazel-out/k8-fastbuild/bin/pkg/ccl/workloadccl/allccl/allccl_test_/allccl_test.runfiles/com_github_cockroachdb_cockroach/pkg/ccl/workloadccl/allccl/all_test.go:305
        	Error:      	Not equal: 
        	            	expected: 0xe013881749bb67e8
        	            	actual  : 0x7373fe217bd5bdb
        	Test:       	TestDeterministicInitialData/tpch
    --- FAIL: TestDeterministicInitialData/tpch (5.95s)

Parameters: TAGS=bazel,gss,deadlock

Help

See also: How To Investigate a Go Test Failure (internal)

/cc @cockroachdb/test-eng @cockroachdb/sql-sessions

This test on roachdash | Improve this report!

Jira issue: CRDB-22615

[nicktrav]

Was feeling festive and charitable, so I did a bisect, which lead me to 7c9dd1b. Looks like the flake rate increased after this.

I'm using the following to reproduce:

$ ./dev test ./pkg/ccl/workloadccl/allccl --filter TestDeterministicInitialData/tpch --stress

cc: @rytaft

[srosenberg]

Non-determinism arises from namePerm which may get reset [1] when the corresponding sync.Pool item is empty [2].
The probability of the pool item getting GCed during a test run is much lower than p=0.5, hence the observed flakiness.
Consequently, P_NAME column [3] may end up with different byte values for any pair of runs with the identical PRNG seed.

It's not necessary to pass namePerm by reference across invocations of randPartName. The following uses a modified Fischer-Yates shuffle to generate an unbiased, random nPartNames-permutation. The proposed solution removes namePerm from sync.Pool and the underlying source of non-determinism.

func randPartName(rng *rand.Rand, a *bufalloc.ByteAllocator) []byte {
	namePerm := make([]int, len(randPartNames))
	for i := range namePerm {
		namePerm[i] = i
	}
	for i := 0; i < nPartNames; i++ {
                // N.B. Correctness requires that i <= j < len(namePerm)
		j := rng.Intn(len(namePerm) - i) + i
		namePerm[i], namePerm[j] = namePerm[j], namePerm[i]
	}
       // rest of the code to copy the permuted words into buf
        ...
}

[1]

cockroach/pkg/workload/tpch/tpch.go

Line 206 in 15bc0c4

namePerm[i] = i

[2]

cockroach/pkg/workload/tpch/generate.go

Line 148 in 15bc0c4

l := w.localsPool.Get().(*generateLocals)

[3]

cockroach/pkg/workload/tpch/generate.go

Line 159 in 15bc0c4

cb.ColVec(1).Bytes().Set(0, randPartName(rng, l.namePerm, a))