Skip to content

Commit fad06ee

Browse files
authored
fix: data race in chunk client hedging tests (#15466)
1 parent c8a4afe commit fad06ee

File tree

2 files changed

+11
-15
lines changed

2 files changed

+11
-15
lines changed

‎pkg/storage/chunk/client/hedging/hedging.go

-6
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,6 @@ func (cfg *Config) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) {
6464
f.IntVar(&cfg.MaxPerSecond, prefix+"hedge-max-per-second", 5, "The maximum of hedge requests allowed per seconds.")
6565
}
6666

67-
// Client returns a hedged http client.
68-
// The client transport will be mutated to use the hedged roundtripper.
69-
func (cfg *Config) Client(client *http.Client) (*http.Client, error) {
70-
return cfg.ClientWithRegisterer(client, prometheus.DefaultRegisterer)
71-
}
72-
7367
// ClientWithRegisterer returns a hedged http client with instrumentation registered to the provided registerer.
7468
// The client transport will be mutated to use the hedged roundtripper.
7569
func (cfg *Config) ClientWithRegisterer(client *http.Client, reg prometheus.Registerer) (*http.Client, error) {

‎pkg/storage/chunk/client/hedging/hedging_test.go

+11-9
Original file line numberDiff line numberDiff line change
@@ -18,37 +18,39 @@ func (fn RoundTripperFunc) RoundTrip(req *http.Request) (*http.Response, error)
1818
return fn(req)
1919
}
2020

21-
func resetMetrics() {
21+
func resetMetrics() *prometheus.Registry {
22+
//TODO: clean up this massive hack...
2223
reg := prometheus.NewRegistry()
2324
prometheus.DefaultRegisterer = reg
2425
prometheus.DefaultGatherer = reg
2526
initMetrics()
27+
return reg
2628
}
2729

2830
func TestHedging(t *testing.T) {
29-
resetMetrics()
31+
reg := resetMetrics()
3032
cfg := &Config{
3133
At: time.Duration(1),
3234
UpTo: 3,
3335
MaxPerSecond: 1000,
3436
}
3537
count := atomic.NewInt32(0)
36-
client, err := cfg.Client(&http.Client{
38+
client, err := cfg.ClientWithRegisterer(&http.Client{
3739
Transport: RoundTripperFunc(func(_ *http.Request) (*http.Response, error) {
3840
count.Inc()
3941
time.Sleep(200 * time.Millisecond)
4042
return &http.Response{
4143
StatusCode: http.StatusOK,
4244
}, nil
4345
}),
44-
})
46+
}, reg)
4547
if err != nil {
4648
t.Fatal(err)
4749
}
4850
_, _ = client.Get("http://example.com")
4951

5052
require.Equal(t, int32(3), count.Load())
51-
require.NoError(t, testutil.GatherAndCompare(prometheus.DefaultGatherer,
53+
require.NoError(t, testutil.GatherAndCompare(reg,
5254
strings.NewReader(`
5355
# HELP hedged_requests_rate_limited_total The total number of hedged requests rejected via rate limiting.
5456
# TYPE hedged_requests_rate_limited_total counter
@@ -61,29 +63,29 @@ hedged_requests_total 2
6163
}
6264

6365
func TestHedgingRateLimit(t *testing.T) {
64-
resetMetrics()
66+
reg := resetMetrics()
6567
cfg := &Config{
6668
At: time.Duration(1),
6769
UpTo: 20,
6870
MaxPerSecond: 1,
6971
}
7072
count := atomic.NewInt32(0)
71-
client, err := cfg.Client(&http.Client{
73+
client, err := cfg.ClientWithRegisterer(&http.Client{
7274
Transport: RoundTripperFunc(func(_ *http.Request) (*http.Response, error) {
7375
count.Inc()
7476
time.Sleep(200 * time.Millisecond)
7577
return &http.Response{
7678
StatusCode: http.StatusOK,
7779
}, nil
7880
}),
79-
})
81+
}, reg)
8082
if err != nil {
8183
t.Fatal(err)
8284
}
8385
_, _ = client.Get("http://example.com")
8486

8587
require.Equal(t, int32(2), count.Load())
86-
require.NoError(t, testutil.GatherAndCompare(prometheus.DefaultGatherer,
88+
require.NoError(t, testutil.GatherAndCompare(reg,
8789
strings.NewReader(`
8890
# HELP hedged_requests_rate_limited_total The total number of hedged requests rejected via rate limiting.
8991
# TYPE hedged_requests_rate_limited_total counter

0 commit comments

Comments
 (0)