Skip to content

Commit 5f61f75

Browse files
committed
bench/rttanalysis: shard TestBenchmarkExpectation to avoid timeouts
The TestBenchmarkExpectation benchmark has been frequently timing out after 15 minutes. This appears to be caused by slow CI machines rather than issues with the test logic itself. To address this, the test is now split into four shards. Each shard is executed separately and receives the full 15-minute timeout budget. This should reduce the likelihood of timeout test failures. Fixes #148384 Release note: none Epic: none
1 parent f5ae7cb commit 5f61f75

File tree

3 files changed

+99
-12
lines changed

3 files changed

+99
-12
lines changed

pkg/bench/rttanalysis/BUILD.bazel

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ go_library(
1313
visibility = ["//visibility:public"],
1414
deps = [
1515
"//pkg/base",
16+
"//pkg/jobs",
17+
"//pkg/jobs/jobspb",
1618
"//pkg/kv/kvclient/kvcoord",
1719
"//pkg/sql",
1820
"//pkg/sql/parser",
@@ -56,9 +58,9 @@ go_test(
5658
data = glob(["testdata/**"]),
5759
embed = [":rttanalysis"],
5860
exec_properties = {"test.Pool": "large"},
61+
shard_count = 4,
5962
deps = [
6063
"//pkg/base",
61-
"//pkg/jobs",
6264
"//pkg/jobs/jobspb",
6365
"//pkg/security/securityassets",
6466
"//pkg/security/securitytest",
@@ -68,6 +70,7 @@ go_test(
6870
"//pkg/testutils/pgurlutils",
6971
"//pkg/testutils/serverutils",
7072
"//pkg/testutils/testcluster",
73+
"//pkg/util/envutil",
7174
"//pkg/util/protoutil",
7275
"//pkg/util/randutil",
7376
],

pkg/bench/rttanalysis/registry.go

Lines changed: 59 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99
"strings"
1010
"testing"
1111

12+
"github.com/cockroachdb/cockroach/pkg/jobs"
13+
"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
1214
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
1315
"github.com/cockroachdb/errors"
1416
"github.com/stretchr/testify/require"
@@ -51,15 +53,66 @@ func (r *Registry) Run(b *testing.B) {
5153
// benchmarks can be filtered by passing the usual test filters underneath
5254
// this test's name.
5355
//
54-
// It takes a long time and thus is skipped under stress, race
55-
// and short.
56+
// It takes a long time and thus is skipped under duress and short.
5657
func (r *Registry) RunExpectations(t *testing.T) {
57-
skip.UnderStress(t)
58-
skip.UnderRace(t)
58+
r.RunExpectationsSharded(t, 1, 1)
59+
}
60+
61+
// RunExpectationsSharded runs all the benchmarks for one iteration
62+
// and validates that the number of RPCs meets the expectation. If run
63+
// with the --rewrite flag, it will rewrite the run benchmarks. The
64+
// benchmarks can be filtered by passing the usual test filters underneath
65+
// this test's name.
66+
//
67+
// It takes a long time and thus is skipped under duress and short.
68+
//
69+
// When shard and totalShards are provided (> 1), only a subset of benchmarks
70+
// assigned to the specific shard will be run, enabling parallel execution.
71+
// Test groups are distributed across shards using round-robin assignment.
72+
func (r *Registry) RunExpectationsSharded(t *testing.T, shard, totalShards int) {
73+
defer jobs.TestingSetIDsToIgnore(map[jobspb.JobID]struct{}{3001: {}, 3002: {}})()
74+
skip.UnderDuress(t)
5975
skip.UnderShort(t)
60-
skip.UnderDeadlock(t)
6176

62-
runBenchmarkExpectationTests(t, r)
77+
// If totalShards is 1, run all tests; otherwise shard them
78+
var registryToUse *Registry
79+
if totalShards <= 1 {
80+
// Run all test groups
81+
registryToUse = r
82+
} else {
83+
// Create a registry with only the test groups assigned to this shard
84+
shardRegistry := &Registry{
85+
numNodes: r.numNodes,
86+
cc: r.cc,
87+
r: make(map[string][]RoundTripBenchTestCase),
88+
}
89+
90+
// Distribute test groups across shards using round-robin assignment
91+
// First, get all group names and sort them for consistent ordering
92+
groupNames := make([]string, 0, len(r.r))
93+
for groupName := range r.r {
94+
groupNames = append(groupNames, groupName)
95+
}
96+
// Sort for deterministic assignment across runs
97+
for i := 0; i < len(groupNames); i++ {
98+
for j := i + 1; j < len(groupNames); j++ {
99+
if groupNames[i] > groupNames[j] {
100+
groupNames[i], groupNames[j] = groupNames[j], groupNames[i]
101+
}
102+
}
103+
}
104+
105+
// Assign groups to shards using round-robin
106+
for i, groupName := range groupNames {
107+
assignedShard := (i % totalShards) + 1
108+
if assignedShard == shard {
109+
shardRegistry.r[groupName] = r.r[groupName]
110+
}
111+
}
112+
registryToUse = shardRegistry
113+
}
114+
115+
runBenchmarkExpectationTests(t, registryToUse)
63116
}
64117

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

pkg/bench/rttanalysis/validate_benchmark_data_test.go

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,44 @@
66
package rttanalysis
77

88
import (
9+
"strconv"
910
"testing"
1011

11-
"github.com/cockroachdb/cockroach/pkg/jobs"
12-
"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
12+
"github.com/cockroachdb/cockroach/pkg/util/envutil"
1313
)
1414

15-
func TestBenchmarkExpectation(t *testing.T) {
16-
defer jobs.TestingSetIDsToIgnore(map[jobspb.JobID]struct{}{3001: {}, 3002: {}})()
17-
reg.RunExpectations(t)
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)
1849
}

0 commit comments

Comments
 (0)