From ad6778e1836d9a2607e21f658383196dd13aeb10 Mon Sep 17 00:00:00 2001 From: David Grant Date: Fri, 24 Jan 2025 09:23:42 -0800 Subject: [PATCH] Fix panics in DurationWithJitter utils when computed variance is <= zero. (#10507) Fixes a panic in DurationWithJitter, DurationWithPositiveJitter, DurationWithNegativeJitter when small inputs result in a variance of zero, because rand.Int63n panics on zero. --- CHANGELOG.md | 1 + pkg/util/time.go | 15 ++++++--------- pkg/util/time_test.go | 20 ++++++++++---------- 3 files changed, 17 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b1c6fab3175..65f665d4f09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ * [BUGFIX] Distributor: return HTTP status 415 Unsupported Media Type instead of 200 Success for Remote Write 2.0 until we support it. #10423 * [BUGFIX] Query-frontend: Add flag `-query-frontend.prom2-range-compat` and corresponding YAML to rewrite queries with ranges that worked in Prometheus 2 but are invalid in Prometheus 3. #10445 #10461 #10502 * [BUGFIX] Distributor: Fix edge case at the HA-tracker with memberlist as KVStore, where when a replica in the KVStore is marked as deleted but not yet removed, it fails to update the KVStore. #10443 +* [BUGFIX] Distributor: Fix panics in `DurationWithJitter` util functions when computed variance is zero. #10507 * [BUGFIX] Ingester: Fixed a race condition in the `PostingsForMatchers` cache that may have infrequently returned expired cached postings. #10500 ### Mixin diff --git a/pkg/util/time.go b/pkg/util/time.go index 4b93313f521..800412a4b5f 100644 --- a/pkg/util/time.go +++ b/pkg/util/time.go @@ -66,12 +66,11 @@ func ParseDurationMS(s string) (int64, error) { // DurationWithJitter returns random duration from "input - input*variance" to "input + input*variance" interval. func DurationWithJitter(input time.Duration, variancePerc float64) time.Duration { - // No duration? No jitter. - if input == 0 { + variance := int64(float64(input) * variancePerc) + if variance <= 0 { return 0 } - variance := int64(float64(input) * variancePerc) jitter := rand.Int63n(variance*2) - variance return input + time.Duration(jitter) @@ -79,12 +78,11 @@ func DurationWithJitter(input time.Duration, variancePerc float64) time.Duration // DurationWithPositiveJitter returns random duration from "input" to "input + input*variance" interval. func DurationWithPositiveJitter(input time.Duration, variancePerc float64) time.Duration { - // No duration? No jitter. - if input == 0 { + variance := int64(float64(input) * variancePerc) + if variance <= 0 { return 0 } - variance := int64(float64(input) * variancePerc) jitter := rand.Int63n(variance) return input + time.Duration(jitter) @@ -92,12 +90,11 @@ func DurationWithPositiveJitter(input time.Duration, variancePerc float64) time. // DurationWithNegativeJitter returns random duration from "input - input*variance" to "input" interval. func DurationWithNegativeJitter(input time.Duration, variancePerc float64) time.Duration { - // No duration? No jitter. - if input == 0 { + variance := int64(float64(input) * variancePerc) + if variance <= 0 { return 0 } - variance := int64(float64(input) * variancePerc) jitter := rand.Int63n(variance) return input - time.Duration(jitter) diff --git a/pkg/util/time_test.go b/pkg/util/time_test.go index 60a5d663809..e06f328e0e5 100644 --- a/pkg/util/time_test.go +++ b/pkg/util/time_test.go @@ -68,10 +68,6 @@ func TestDurationWithJitter(t *testing.T) { } } -func TestDurationWithJitter_ZeroInputDuration(t *testing.T) { - assert.Equal(t, time.Duration(0), DurationWithJitter(time.Duration(0), 0.5)) -} - func TestDurationWithPositiveJitter(t *testing.T) { const numRuns = 1000 @@ -82,10 +78,6 @@ func TestDurationWithPositiveJitter(t *testing.T) { } } -func TestDurationWithPositiveJitter_ZeroInputDuration(t *testing.T) { - assert.Equal(t, time.Duration(0), DurationWithPositiveJitter(time.Duration(0), 0.5)) -} - func TestDurationWithNegativeJitter(t *testing.T) { const numRuns = 1000 @@ -96,8 +88,16 @@ func TestDurationWithNegativeJitter(t *testing.T) { } } -func TestDurationWithNegativeJitter_ZeroInputDuration(t *testing.T) { - assert.Equal(t, time.Duration(0), DurationWithNegativeJitter(time.Duration(0), 0.5)) +func TestDurationWithJitterFamily_ZeroOutputs(t *testing.T) { + // Make sure all of these functions return zero when the computed variance is <= 0. + + type durFunc func(time.Duration, float64) time.Duration + for _, f := range []durFunc{DurationWithJitter, DurationWithNegativeJitter, DurationWithPositiveJitter} { + assert.Equal(t, time.Duration(0), f(time.Duration(0), 0.5)) + assert.Equal(t, time.Duration(0), f(time.Duration(7), 0.1)) + assert.Equal(t, time.Duration(0), f(time.Duration(7), -0.1)) + assert.Equal(t, time.Duration(0), f(10*time.Minute, -0.1)) + } } func TestParseTime(t *testing.T) {