Skip to content

Commit 32cf4b2

Browse files
committed
Revert "bench/rttanalysis: shard TestBenchmarkExpectation to avoid timeouts"
This reverts commit 9fecc53. Fixes: #154317 Release note: none
1 parent 9e89769 commit 32cf4b2

File tree

3 files changed

+12
-99
lines changed

3 files changed

+12
-99
lines changed

pkg/bench/rttanalysis/BUILD.bazel

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@ go_library(
1313
visibility = ["//visibility:public"],
1414
deps = [
1515
"//pkg/base",
16-
"//pkg/jobs",
17-
"//pkg/jobs/jobspb",
1816
"//pkg/kv/kvclient/kvcoord",
1917
"//pkg/sql",
2018
"//pkg/sql/parser",
@@ -58,9 +56,9 @@ go_test(
5856
data = glob(["testdata/**"]),
5957
embed = [":rttanalysis"],
6058
exec_properties = {"test.Pool": "large"},
61-
shard_count = 4,
6259
deps = [
6360
"//pkg/base",
61+
"//pkg/jobs",
6462
"//pkg/jobs/jobspb",
6563
"//pkg/security/securityassets",
6664
"//pkg/security/securitytest",
@@ -71,7 +69,6 @@ go_test(
7169
"//pkg/testutils/serverutils",
7270
"//pkg/testutils/skip",
7371
"//pkg/testutils/testcluster",
74-
"//pkg/util/envutil",
7572
"//pkg/util/protoutil",
7673
"//pkg/util/randutil",
7774
],

pkg/bench/rttanalysis/registry.go

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

12-
"github.com/cockroachdb/cockroach/pkg/jobs"
13-
"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
1412
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
1513
"github.com/cockroachdb/errors"
1614
"github.com/stretchr/testify/require"
@@ -53,66 +51,15 @@ func (r *Registry) Run(b *testing.B) {
5351
// benchmarks can be filtered by passing the usual test filters underneath
5452
// this test's name.
5553
//
56-
// It takes a long time and thus is skipped under duress and short.
54+
// It takes a long time and thus is skipped under stress, race
55+
// and short.
5756
func (r *Registry) RunExpectations(t *testing.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)
57+
skip.UnderStress(t)
58+
skip.UnderRace(t)
7559
skip.UnderShort(t)
60+
skip.UnderDeadlock(t)
7661

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)
62+
runBenchmarkExpectationTests(t, r)
11663
}
11764

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

pkg/bench/rttanalysis/validate_benchmark_data_test.go

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

88
import (
9-
"strconv"
109
"testing"
1110

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

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

0 commit comments

Comments
 (0)