Skip to content

Commit 2ec926e

Browse files
authored
Merge pull request #157958 from rickystewart/backport24.3-157847
release-24.3: bench/rttanalysis: shard TestBenchmarkExpectation to avoid timeouts
2 parents b6d410b + 3bb57f2 commit 2ec926e

File tree

3 files changed

+103
-8
lines changed

3 files changed

+103
-8
lines changed

pkg/bench/rttanalysis/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ go_test(
5555
data = glob(["testdata/**"]),
5656
embed = [":rttanalysis"],
5757
exec_properties = {"test.Pool": "large"},
58+
shard_count = 4,
5859
deps = [
5960
"//pkg/base",
6061
"//pkg/jobs/jobspb",
@@ -66,6 +67,7 @@ go_test(
6667
"//pkg/testutils/serverutils",
6768
"//pkg/testutils/sqlutils",
6869
"//pkg/testutils/testcluster",
70+
"//pkg/util/envutil",
6971
"//pkg/util/protoutil",
7072
"//pkg/util/randutil",
7173
],

pkg/bench/rttanalysis/registry.go

Lines changed: 60 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package rttanalysis
77

88
import (
9+
"runtime"
910
"strings"
1011
"testing"
1112

@@ -51,15 +52,68 @@ func (r *Registry) Run(b *testing.B) {
5152
// benchmarks can be filtered by passing the usual test filters underneath
5253
// this test's name.
5354
//
54-
// It takes a long time and thus is skipped under stress, race
55-
// and short.
55+
// It takes a long time and thus is skipped under duress and short.
5656
func (r *Registry) RunExpectations(t *testing.T) {
57-
skip.UnderStress(t)
58-
skip.UnderRace(t)
57+
r.RunExpectationsSharded(t, 1, 1)
58+
}
59+
60+
// RunExpectationsSharded runs all the benchmarks for one iteration
61+
// and validates that the number of RPCs meets the expectation. If run
62+
// with the --rewrite flag, it will rewrite the run benchmarks. The
63+
// benchmarks can be filtered by passing the usual test filters underneath
64+
// this test's name.
65+
//
66+
// It takes a long time and thus is skipped under duress and short.
67+
//
68+
// When shard and totalShards are provided (> 1), only a subset of benchmarks
69+
// assigned to the specific shard will be run, enabling parallel execution.
70+
// Test groups are distributed across shards using round-robin assignment.
71+
func (r *Registry) RunExpectationsSharded(t *testing.T, shard, totalShards int) {
72+
skip.UnderDuress(t)
5973
skip.UnderShort(t)
60-
skip.UnderDeadlock(t)
74+
if runtime.GOARCH == "s390x" {
75+
skip.IgnoreLint(t, "test prone to crashing under s390x (see #154317)")
76+
}
77+
78+
// If totalShards is 1, run all tests; otherwise shard them
79+
var registryToUse *Registry
80+
if totalShards <= 1 {
81+
// Run all test groups
82+
registryToUse = r
83+
} else {
84+
// Create a registry with only the test groups assigned to this shard
85+
shardRegistry := &Registry{
86+
numNodes: r.numNodes,
87+
cc: r.cc,
88+
r: make(map[string][]RoundTripBenchTestCase),
89+
}
90+
91+
// Distribute test groups across shards using round-robin assignment
92+
// First, get all group names and sort them for consistent ordering
93+
groupNames := make([]string, 0, len(r.r))
94+
for groupName := range r.r {
95+
groupNames = append(groupNames, groupName)
96+
}
97+
// Sort for deterministic assignment across runs
98+
for i := 0; i < len(groupNames); i++ {
99+
for j := i + 1; j < len(groupNames); j++ {
100+
if groupNames[i] > groupNames[j] {
101+
groupNames[i], groupNames[j] = groupNames[j], groupNames[i]
102+
}
103+
}
104+
}
105+
106+
// Assign groups to shards using round-robin
107+
for i, groupName := range groupNames {
108+
assignedShard := (i % totalShards) + 1
109+
if assignedShard == shard {
110+
shardRegistry.r[groupName] = r.r[groupName]
111+
}
112+
}
113+
registryToUse = shardRegistry
114+
}
61115

62-
runBenchmarkExpectationTests(t, r)
116+
runBenchmarkExpectationTests(t, registryToUse)
63117
}
64118

65119
// Register registers a set of test cases to a given benchmark name. It is

pkg/bench/rttanalysis/validate_benchmark_data_test.go

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,45 @@
55

66
package rttanalysis
77

8-
import "testing"
8+
import (
9+
"strconv"
10+
"testing"
911

10-
func TestBenchmarkExpectation(t *testing.T) { reg.RunExpectations(t) }
12+
"github.com/cockroachdb/cockroach/pkg/util/envutil"
13+
)
14+
15+
// NOTE: If you change the number of shards, you must also update the
16+
// shard_count in BUILD.bazel to match.
17+
const shardCount = 4
18+
19+
// Validate that shardCount matches TEST_TOTAL_SHARDS environment variable at init time
20+
var _ = func() int {
21+
totalShardsStr, found := envutil.ExternalEnvString("TEST_TOTAL_SHARDS", 1)
22+
if totalShardsStr == "" || !found {
23+
return 0
24+
}
25+
totalShards, err := strconv.Atoi(totalShardsStr)
26+
if err != nil {
27+
return 0
28+
}
29+
if totalShards != shardCount {
30+
panic("shardCount mismatch: update shard_count in pkg/bench/rttanalysis/BUILD.bazel to match shardCount constant")
31+
}
32+
return 0
33+
}()
34+
35+
func TestBenchmarkExpectationShard1(t *testing.T) {
36+
reg.RunExpectationsSharded(t, 1, shardCount)
37+
}
38+
39+
func TestBenchmarkExpectationShard2(t *testing.T) {
40+
reg.RunExpectationsSharded(t, 2, shardCount)
41+
}
42+
43+
func TestBenchmarkExpectationShard3(t *testing.T) {
44+
reg.RunExpectationsSharded(t, 3, shardCount)
45+
}
46+
47+
func TestBenchmarkExpectationShard4(t *testing.T) {
48+
reg.RunExpectationsSharded(t, 4, shardCount)
49+
}

0 commit comments

Comments
 (0)