From 821361dbc099c433bf437dbe5d77644457a5a7fa Mon Sep 17 00:00:00 2001 From: kahirokunn Date: Wed, 14 Oct 2026 02:43:19 +0900 Subject: [PATCH] fix(delivery): wait one backoff delay before the first linear retry Signed-off-by: kahirokunn --- .../resources/in-memory-channel.yaml | 4 +- config/core/resources/broker.yaml | 2 +- config/core/resources/channel.yaml | 4 +- config/core/resources/parallel.yaml | 2 +- config/core/resources/requestreply.yaml | 2 +- config/core/resources/sequence.yaml | 2 +- config/core/resources/subscription.yaml | 2 +- config/core/resources/trigger.yaml | 2 +- docs/delivery/README.md | 2 +- docs/eventing-api.md | 4 +- pkg/apis/duck/v1/delivery_types.go | 2 +- pkg/apis/duck/v1beta1/delivery_types.go | 2 +- pkg/kncloudevents/retries.go | 6 +-- pkg/kncloudevents/retries_test.go | 44 ++++++++++++++----- .../dispatcher/inmemorychannel_test.go | 3 +- 15 files changed, 54 insertions(+), 29 deletions(-) diff --git a/config/channels/in-memory-channel/resources/in-memory-channel.yaml b/config/channels/in-memory-channel/resources/in-memory-channel.yaml index be5a469d060..047af5262bc 100644 --- a/config/channels/in-memory-channel/resources/in-memory-channel.yaml +++ b/config/channels/in-memory-channel/resources/in-memory-channel.yaml @@ -43,7 +43,7 @@ spec: type: object properties: backoffDelay: - description: 'BackoffDelay is the delay before retrying. More information on Duration format: - https://www.iso.org/iso-8601-date-and-time-format.html - https://en.wikipedia.org/wiki/ISO_8601 For linear policy, backoff delay is backoffDelay*. For exponential policy, backoff delay is backoffDelay*2^.' + description: 'BackoffDelay is the delay before retrying. More information on Duration format: - https://www.iso.org/iso-8601-date-and-time-format.html - https://en.wikipedia.org/wiki/ISO_8601 For linear policy, backoff delay is backoffDelay*. For exponential policy, backoff delay is backoffDelay*2^(-1).' type: string backoffPolicy: description: BackoffPolicy is the retry backoff policy (linear, exponential). @@ -93,7 +93,7 @@ spec: type: object properties: backoffDelay: - description: 'BackoffDelay is the delay before retrying. More information on Duration format: - https://www.iso.org/iso-8601-date-and-time-format.html - https://en.wikipedia.org/wiki/ISO_8601 For linear policy, backoff delay is backoffDelay*. For exponential policy, backoff delay is backoffDelay*2^.' + description: 'BackoffDelay is the delay before retrying. More information on Duration format: - https://www.iso.org/iso-8601-date-and-time-format.html - https://en.wikipedia.org/wiki/ISO_8601 For linear policy, backoff delay is backoffDelay*. For exponential policy, backoff delay is backoffDelay*2^(-1).' type: string backoffPolicy: description: BackoffPolicy is the retry backoff policy (linear, exponential). diff --git a/config/core/resources/broker.yaml b/config/core/resources/broker.yaml index c8e3eea5f39..84a99034927 100644 --- a/config/core/resources/broker.yaml +++ b/config/core/resources/broker.yaml @@ -59,7 +59,7 @@ spec: type: object properties: backoffDelay: - description: 'BackoffDelay is the delay before retrying. More information on Duration format: - https://www.iso.org/iso-8601-date-and-time-format.html - https://en.wikipedia.org/wiki/ISO_8601 For linear policy, backoff delay is backoffDelay*. For exponential policy, backoff delay is backoffDelay*2^.' + description: 'BackoffDelay is the delay before retrying. More information on Duration format: - https://www.iso.org/iso-8601-date-and-time-format.html - https://en.wikipedia.org/wiki/ISO_8601 For linear policy, backoff delay is backoffDelay*. For exponential policy, backoff delay is backoffDelay*2^(-1).' type: string backoffPolicy: description: BackoffPolicy is the retry backoff policy (linear, exponential). diff --git a/config/core/resources/channel.yaml b/config/core/resources/channel.yaml index 4ca65afb239..a694b2374a6 100644 --- a/config/core/resources/channel.yaml +++ b/config/core/resources/channel.yaml @@ -71,7 +71,7 @@ spec: type: object properties: backoffDelay: - description: 'BackoffDelay is the delay before retrying. More information on Duration format: - https://www.iso.org/iso-8601-date-and-time-format.html - https://en.wikipedia.org/wiki/ISO_8601 For linear policy, backoff delay is backoffDelay*. For exponential policy, backoff delay is backoffDelay*2^.' + description: 'BackoffDelay is the delay before retrying. More information on Duration format: - https://www.iso.org/iso-8601-date-and-time-format.html - https://en.wikipedia.org/wiki/ISO_8601 For linear policy, backoff delay is backoffDelay*. For exponential policy, backoff delay is backoffDelay*2^(-1).' type: string backoffPolicy: description: BackoffPolicy is the retry backoff policy (linear, exponential). @@ -121,7 +121,7 @@ spec: type: object properties: backoffDelay: - description: 'BackoffDelay is the delay before retrying. More information on Duration format: - https://www.iso.org/iso-8601-date-and-time-format.html - https://en.wikipedia.org/wiki/ISO_8601 For linear policy, backoff delay is backoffDelay*. For exponential policy, backoff delay is backoffDelay*2^.' + description: 'BackoffDelay is the delay before retrying. More information on Duration format: - https://www.iso.org/iso-8601-date-and-time-format.html - https://en.wikipedia.org/wiki/ISO_8601 For linear policy, backoff delay is backoffDelay*. For exponential policy, backoff delay is backoffDelay*2^(-1).' type: string backoffPolicy: description: BackoffPolicy is the retry backoff policy (linear, exponential). diff --git a/config/core/resources/parallel.yaml b/config/core/resources/parallel.yaml index 492e3f119f4..2d4907916d4 100644 --- a/config/core/resources/parallel.yaml +++ b/config/core/resources/parallel.yaml @@ -57,7 +57,7 @@ spec: - https://en.wikipedia.org/wiki/ISO_8601 For linear policy, backoff delay is backoffDelay*. For exponential policy, backoff delay is - backoffDelay*2^.' + backoffDelay*2^(-1).' type: string backoffPolicy: description: BackoffPolicy is the retry backoff diff --git a/config/core/resources/requestreply.yaml b/config/core/resources/requestreply.yaml index 70d4c8cc3fa..65af2736057 100644 --- a/config/core/resources/requestreply.yaml +++ b/config/core/resources/requestreply.yaml @@ -62,7 +62,7 @@ spec: type: object properties: backoffDelay: - description: 'BackoffDelay is the delay before retrying. More information on Duration format: - https://www.iso.org/iso-8601-date-and-time-format.html - https://en.wikipedia.org/wiki/ISO_8601 For linear policy, backoff delay is backoffDelay*. For exponential policy, backoff delay is backoffDelay*2^.' + description: 'BackoffDelay is the delay before retrying. More information on Duration format: - https://www.iso.org/iso-8601-date-and-time-format.html - https://en.wikipedia.org/wiki/ISO_8601 For linear policy, backoff delay is backoffDelay*. For exponential policy, backoff delay is backoffDelay*2^(-1).' type: string backoffPolicy: description: BackoffPolicy is the retry backoff policy (linear, exponential). diff --git a/config/core/resources/sequence.yaml b/config/core/resources/sequence.yaml index f0021d89c88..610f93cbba7 100644 --- a/config/core/resources/sequence.yaml +++ b/config/core/resources/sequence.yaml @@ -91,7 +91,7 @@ spec: type: object properties: backoffDelay: - description: 'BackoffDelay is the delay before retrying. More information on Duration format: - https://www.iso.org/iso-8601-date-and-time-format.html - https://en.wikipedia.org/wiki/ISO_8601 For linear policy, backoff delay is backoffDelay*. For exponential policy, backoff delay is backoffDelay*2^.' + description: 'BackoffDelay is the delay before retrying. More information on Duration format: - https://www.iso.org/iso-8601-date-and-time-format.html - https://en.wikipedia.org/wiki/ISO_8601 For linear policy, backoff delay is backoffDelay*. For exponential policy, backoff delay is backoffDelay*2^(-1).' type: string backoffPolicy: description: BackoffPolicy is the retry backoff policy (linear, exponential). diff --git a/config/core/resources/subscription.yaml b/config/core/resources/subscription.yaml index 71edb78ae03..081003cf608 100644 --- a/config/core/resources/subscription.yaml +++ b/config/core/resources/subscription.yaml @@ -57,7 +57,7 @@ spec: type: object properties: backoffDelay: - description: 'BackoffDelay is the delay before retrying. More information on Duration format: - https://www.iso.org/iso-8601-date-and-time-format.html - https://en.wikipedia.org/wiki/ISO_8601 For linear policy, backoff delay is backoffDelay*. For exponential policy, backoff delay is backoffDelay*2^.' + description: 'BackoffDelay is the delay before retrying. More information on Duration format: - https://www.iso.org/iso-8601-date-and-time-format.html - https://en.wikipedia.org/wiki/ISO_8601 For linear policy, backoff delay is backoffDelay*. For exponential policy, backoff delay is backoffDelay*2^(-1).' type: string backoffPolicy: description: BackoffPolicy is the retry backoff policy (linear, exponential). diff --git a/config/core/resources/trigger.yaml b/config/core/resources/trigger.yaml index 712d3cd576b..65677b36aec 100644 --- a/config/core/resources/trigger.yaml +++ b/config/core/resources/trigger.yaml @@ -79,7 +79,7 @@ spec: x-kubernetes-preserve-unknown-fields: true # This is necessary to enable the experimental feature delivery-timeout properties: backoffDelay: - description: 'BackoffDelay is the delay before retrying. More information on Duration format: - https://www.iso.org/iso-8601-date-and-time-format.html - https://en.wikipedia.org/wiki/ISO_8601 For linear policy, backoff delay is backoffDelay*. For exponential policy, backoff delay is backoffDelay*2^.' + description: 'BackoffDelay is the delay before retrying. More information on Duration format: - https://www.iso.org/iso-8601-date-and-time-format.html - https://en.wikipedia.org/wiki/ISO_8601 For linear policy, backoff delay is backoffDelay*. For exponential policy, backoff delay is backoffDelay*2^(-1).' type: string backoffPolicy: description: BackoffPolicy is the retry backoff policy (linear, exponential). diff --git a/docs/delivery/README.md b/docs/delivery/README.md index 8026087a156..db2ce59f4ae 100644 --- a/docs/delivery/README.md +++ b/docs/delivery/README.md @@ -99,7 +99,7 @@ type DeliverySpec struct { // - https://en.wikipedia.org/wiki/ISO_8601 // // For linear policy, backoff delay is backoffDelay*. - // For exponential policy, backoff delay is backoffDelay*2^. + // For exponential policy, backoff delay is backoffDelay*2^(-1). // +optional BackoffDelay *string `json:"backoffDelay,omitempty"` } diff --git a/docs/eventing-api.md b/docs/eventing-api.md index e9e7bc425c1..515d7eb3864 100644 --- a/docs/eventing-api.md +++ b/docs/eventing-api.md @@ -479,7 +479,7 @@ More information on Duration format: - https://www.iso.org/iso-8601-date-and-time-format.html - https://en.wikipedia.org/wiki/ISO_8601

For linear policy, backoff delay is backoffDelay*. -For exponential policy, backoff delay is backoffDelay*2^.

+For exponential policy, backoff delay is backoffDelay*2^(-1).

@@ -1512,7 +1512,7 @@ More information on Duration format: - https://www.iso.org/iso-8601-date-and-time-format.html - https://en.wikipedia.org/wiki/ISO_8601

For linear policy, backoff delay is backoffDelay*. -For exponential policy, backoff delay is backoffDelay*2^.

+For exponential policy, backoff delay is backoffDelay*2^(-1).

diff --git a/pkg/apis/duck/v1/delivery_types.go b/pkg/apis/duck/v1/delivery_types.go index f0fef5fc2ae..a982878cdec 100644 --- a/pkg/apis/duck/v1/delivery_types.go +++ b/pkg/apis/duck/v1/delivery_types.go @@ -58,7 +58,7 @@ type DeliverySpec struct { // - https://en.wikipedia.org/wiki/ISO_8601 // // For linear policy, backoff delay is backoffDelay*. - // For exponential policy, backoff delay is backoffDelay*2^. + // For exponential policy, backoff delay is backoffDelay*2^(-1). // +optional BackoffDelay *string `json:"backoffDelay,omitempty"` diff --git a/pkg/apis/duck/v1beta1/delivery_types.go b/pkg/apis/duck/v1beta1/delivery_types.go index 7a444fc0e0e..fcdb8e8cf96 100644 --- a/pkg/apis/duck/v1beta1/delivery_types.go +++ b/pkg/apis/duck/v1beta1/delivery_types.go @@ -55,7 +55,7 @@ type DeliverySpec struct { // - https://en.wikipedia.org/wiki/ISO_8601 // // For linear policy, backoff delay is backoffDelay*. - // For exponential policy, backoff delay is backoffDelay*2^. + // For exponential policy, backoff delay is backoffDelay*2^(-1). // +optional BackoffDelay *string `json:"backoffDelay,omitempty"` } diff --git a/pkg/kncloudevents/retries.go b/pkg/kncloudevents/retries.go index c6199917aeb..05560293b22 100644 --- a/pkg/kncloudevents/retries.go +++ b/pkg/kncloudevents/retries.go @@ -162,17 +162,17 @@ func saturatingPeriodDuration(p period.Period) time.Duration { } func linearBackoff(delay time.Duration, attemptNum int, configuredMax *time.Duration) time.Duration { - if attemptNum <= 0 || delay <= 0 { + if attemptNum < 0 || delay <= 0 { return 0 } limit := maxBackoffDuration if configuredMax != nil { limit = *configuredMax } - if delay >= limit || time.Duration(attemptNum) > limit/delay { + if delay >= limit || time.Duration(attemptNum) >= limit/delay { return limit } - return delay * time.Duration(attemptNum) + return delay * (time.Duration(attemptNum) + 1) } func exponentialBackoff(delay time.Duration, attemptNum int, configuredMax *time.Duration) time.Duration { diff --git a/pkg/kncloudevents/retries_test.go b/pkg/kncloudevents/retries_test.go index f7d29cede2a..8b967059010 100644 --- a/pkg/kncloudevents/retries_test.go +++ b/pkg/kncloudevents/retries_test.go @@ -76,7 +76,7 @@ func TestRetryConfigBackoffMax(t *testing.T) { policy: linear, delay: "PT3S", backoffMax: ptr.String("PT10S"), - attempts: []int{1, 2, 3, 4, int(^uint(0) >> 1)}, + attempts: []int{0, 1, 2, 3, int(^uint(0) >> 1)}, wantBackoffs: []time.Duration{ 3 * time.Second, 6 * time.Second, @@ -110,7 +110,7 @@ func TestRetryConfigBackoffMax(t *testing.T) { name: "uncapped linear saturates", policy: linear, delay: "PT1S", - attempts: []int{1, int(^uint(0) >> 1)}, + attempts: []int{0, int(^uint(0) >> 1)}, wantBackoffs: []time.Duration{time.Second, maxBackoffDuration}, }, { @@ -189,6 +189,7 @@ func TestRetryConfigBackoffMax(t *testing.T) { func TestDoWithRetriesBackoffMax(t *testing.T) { tests := []struct { name string + policy v1.BackoffPolicyType delay string backoffMax *string retryAfter string @@ -197,38 +198,61 @@ func TestDoWithRetriesBackoffMax(t *testing.T) { }{ { name: "exponential backoff is capped", + policy: v1.BackoffPolicyExponential, delay: "PT1S", backoffMax: ptr.String("PT2S"), wantTimes: []time.Duration{0, time.Second, 3 * time.Second, 5 * time.Second, 7 * time.Second}, }, { name: "exponential backoff without a maximum", + policy: v1.BackoffPolicyExponential, delay: "PT1S", wantTimes: []time.Duration{0, time.Second, 3 * time.Second, 7 * time.Second, 15 * time.Second}, }, { name: "maximum below the initial delay", + policy: v1.BackoffPolicyExponential, delay: "PT3S", backoffMax: ptr.String("PT2S"), wantTimes: []time.Duration{0, 2 * time.Second, 4 * time.Second, 6 * time.Second, 8 * time.Second}, }, { name: "retry after can exceed the backoff maximum", + policy: v1.BackoffPolicyExponential, delay: "PT1S", backoffMax: ptr.String("PT2S"), retryAfter: "3", retryAfterMax: ptr.String("PT5S"), wantTimes: []time.Duration{0, 3 * time.Second, 6 * time.Second, 9 * time.Second, 12 * time.Second}, }, + { + name: "linear backoff waits one delay before the first retry", + policy: v1.BackoffPolicyLinear, + delay: "PT1S", + wantTimes: []time.Duration{0, time.Second, 3 * time.Second, 6 * time.Second, 10 * time.Second}, + }, + { + name: "linear backoff is capped", + policy: v1.BackoffPolicyLinear, + delay: "PT1S", + backoffMax: ptr.String("PT2S"), + wantTimes: []time.Duration{0, time.Second, 3 * time.Second, 5 * time.Second, 7 * time.Second}, + }, + { + name: "linear maximum below the initial delay", + policy: v1.BackoffPolicyLinear, + delay: "PT3S", + backoffMax: ptr.String("PT2S"), + wantTimes: []time.Duration{0, 2 * time.Second, 4 * time.Second, 6 * time.Second, 8 * time.Second}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { synctest.Test(t, func(t *testing.T) { - policy := v1.BackoffPolicyExponential config, err := RetryConfigFromDeliverySpec(v1.DeliverySpec{ Retry: ptr.Int32(4), - BackoffPolicy: &policy, + BackoffPolicy: &tt.policy, BackoffDelay: &tt.delay, BackoffMax: tt.backoffMax, RetryAfterMax: tt.retryAfterMax, @@ -328,22 +352,22 @@ func TestRetryConfigFromDeliverySpec(t *testing.T) { backoffPolicy: v1.BackoffPolicyExponential, backoffDelay: "PT1.5S", expectedBackoffDurations: []time.Duration{ + 1500 * time.Millisecond, 3 * time.Second, 6 * time.Second, 12 * time.Second, 24 * time.Second, - 48 * time.Second, }, }, { name: "Successful Exponential Backoff 500ms, 5 retries", backoffPolicy: v1.BackoffPolicyExponential, backoffDelay: "PT0.5S", expectedBackoffDurations: []time.Duration{ + 500 * time.Millisecond, 1 * time.Second, 2 * time.Second, 4 * time.Second, 8 * time.Second, - 16 * time.Second, }, }, { name: "Invalid Backoff Delay", @@ -356,11 +380,11 @@ func TestRetryConfigFromDeliverySpec(t *testing.T) { backoffDelay: "PT0.5S", timeout: &validISO8601DurationString, expectedBackoffDurations: []time.Duration{ + 500 * time.Millisecond, 1 * time.Second, 2 * time.Second, 4 * time.Second, 8 * time.Second, - 16 * time.Second, }, }, { name: "Invalid Timeout", @@ -374,11 +398,11 @@ func TestRetryConfigFromDeliverySpec(t *testing.T) { backoffDelay: "PT0.5S", retryAfterMax: &validISO8601DurationString, expectedBackoffDurations: []time.Duration{ + 500 * time.Millisecond, 1 * time.Second, 2 * time.Second, 4 * time.Second, 8 * time.Second, - 16 * time.Second, }, }, { name: "Invalid RetryAfterMax", @@ -422,8 +446,8 @@ func TestRetryConfigFromDeliverySpec(t *testing.T) { assert.Nil(t, retryConfig.RetryAfterMaxDuration) } - for i := 1; i < retry; i++ { - expectedBackoffDuration := tc.expectedBackoffDurations[i-1] + for i := 0; i < retry; i++ { + expectedBackoffDuration := tc.expectedBackoffDurations[i] actualBackoffDuration := retryConfig.Backoff(i, nil) assert.Equal(t, expectedBackoffDuration, actualBackoffDuration) } diff --git a/pkg/reconciler/inmemorychannel/dispatcher/inmemorychannel_test.go b/pkg/reconciler/inmemorychannel/dispatcher/inmemorychannel_test.go index c0fdca8ddd1..ca70f789504 100644 --- a/pkg/reconciler/inmemorychannel/dispatcher/inmemorychannel_test.go +++ b/pkg/reconciler/inmemorychannel/dispatcher/inmemorychannel_test.go @@ -553,7 +553,8 @@ func TestReconciler_ReconcileKind(t *testing.T) { WithInMemoryChannelEventPoliciesReady()), subs: []fanout.Subscription{*previousSubscription}, wantBackoffs: map[int]time.Duration{ - 3: 3 * time.Second, + 0: 1 * time.Second, + 2: 3 * time.Second, 20: 10 * time.Second, }, wantSubs: []fanout.Subscription{{