From 5f051bd4d706bcec7883f16a079f884249c5429e Mon Sep 17 00:00:00 2001 From: I867318 Date: Wed, 13 Oct 2021 15:10:27 -0600 Subject: [PATCH 1/7] delivery.retryAfter experimental-feature --- docs/eventing/experimental-features.md | 45 ++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/docs/eventing/experimental-features.md b/docs/eventing/experimental-features.md index c6ae058168e..5681c9b2c99 100644 --- a/docs/eventing/experimental-features.md +++ b/docs/eventing/experimental-features.md @@ -103,6 +103,51 @@ affecting existing resources. At the moment this feature is implemented only for `Subscription.Spec.Subscriber.Ref` and `Subscription.Spec.Channel`. +### DeliverySpec.RetryAfter field + +**Flag name**: `delivery-retryafter` + +**Stage**: Alpha, disabled by default + +**Tracking issue**: [#5811](https://github.com/knative/eventing/issues/5811) + +**Persona**: Developer + +When using the `delivery` spec to configure event delivery parameters, you can +use the `retryAfter` fields to specify whether to respect **Retry-After** +headers when calculating the backoff time for retrying 429 and 503 response +codes. The optional `maxDuration` field provides an override to prevent large +**Retry-After** durations from impacting throughput, and must be specified using +the [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601#Times) format. + +The following example shows a Subscription that retries sending an event 3 +times, and respects **Retry-After** headers while imposing a max backoff of +120 seconds: + +```yaml +apiVersion: messaging.knative.dev/v1 +kind: Subscription +metadata: + name: example-subscription + namespace: example-namespace +spec: + subscriber: + ref: + apiVersion: serving.knative.dev/v1 + kind: Service + name: example-sink + delivery: + backoffDelay: PT2S + backoffPolicy: linear + retry: 3 + retryAfter: + enabled: true + maxDuration: PT120S +``` + +You can specify a `delivery` spec for Channels, Subscriptions, Brokers, +Triggers, and any other resource spec that accepts the `delivery` field. + ### DeliverySpec.Timeout field **Flag name**: `delivery-timeout` From 64817d18fe862087a732ea5b96e10a1cfcb0fc5c Mon Sep 17 00:00:00 2001 From: I867318 Date: Tue, 9 Nov 2021 10:11:56 -0700 Subject: [PATCH 2/7] Refactor implementation for new design based on community feedback. --- docs/eventing/experimental-features.md | 74 ++++++++++++++++++++++---- 1 file changed, 63 insertions(+), 11 deletions(-) diff --git a/docs/eventing/experimental-features.md b/docs/eventing/experimental-features.md index 5417168f86d..d67a42b95d7 100644 --- a/docs/eventing/experimental-features.md +++ b/docs/eventing/experimental-features.md @@ -103,7 +103,7 @@ affecting existing resources. At the moment this feature is implemented only for `Subscription.Spec.Subscriber.Ref` and `Subscription.Spec.Channel`. -### DeliverySpec.RetryAfter field +### DeliverySpec.RetryAfterMax field **Flag name**: `delivery-retryafter` @@ -114,11 +114,57 @@ affecting existing resources. **Persona**: Developer When using the `delivery` spec to configure event delivery parameters, you can -use the `retryAfter` fields to specify whether to respect **Retry-After** -headers when calculating the backoff time for retrying 429 and 503 response -codes. The optional `maxDuration` field provides an override to prevent large -**Retry-After** durations from impacting throughput, and must be specified using -the [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601#Times) format. +use the `retryAfterMax` field to specify how +HTTP [Retry-After](https://datatracker.ietf.org/doc/html/rfc7231#section-7.1.3) +headers are handled when calculating backoff times for retrying **429** and +**503** responses. You can specify a `delivery` spec for Channels, +Subscriptions, Brokers, Triggers, and any other resource spec that accepts the +`delivery` field. + +The `retryAfterMax` field will only take effect if the `delivery` spec is +configured to perform retries, and will only pertain to retry attempts on +**429** and **503** response codes. + +The `retryAfterMax` field provides an override to prevent large **Retry-After** +durations from impacting throughput, and must be specified using +the [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601#Times) format. Specifying +a "zero" value of "PT0S" will effectively disable **Retry-After** support. + +Prior to this experimental feature, Knative Eventing implementations have not +supported **Retry-After** headers, and this is an attempt to provide a path +for standardizing that support. To begin the feature will be **opt-in**, but +the final state will be **opt-out** as follows... + +{% raw %} + + + + + + + + + + + + + + + + + + + + + + + + + + +
EF StageFeature FlagretryAfterMax Field AbsentretryAfterMax Field Present
Alpha / BetaDisabledAccepted by Webhook Validation & Retry-After headers NOT enforcedRejected by WebHook Validation
Alpha / BetaEnabledAccepted by Webhook Validation & Retry-After headers NOT enforcedAccepted by Webhook Validation & Retry-After headers enforced if max override > 0
Stable / GAn/aRetry-After headers enforced without max overrideRetry-After headers enforced if max override > 0
+ +{% endraw %} The following example shows a Subscription that retries sending an event 3 times, and respects **Retry-After** headers while imposing a max backoff of @@ -140,13 +186,19 @@ spec: backoffDelay: PT2S backoffPolicy: linear retry: 3 - retryAfter: - enabled: true - maxDuration: PT120S + retryAfterMax: PT120S ``` -You can specify a `delivery` spec for Channels, Subscriptions, Brokers, -Triggers, and any other resource spec that accepts the `delivery` field. +!!! note + While the experimental feature flag will enforce all DeliverySpec usage of + the `RetryAfterMax` field via Webhook validation, it does not guarantee all + implementations (Channels, Sources, etc.) will actually implement support + for the field. The shared `HTTPMessageSender.SendWithRetries()` logic + has been enhanced to support this feature, and all implementations using it + to perform retries will automatically benefit. Sandbox implementations NOT + based on this shared library (e.g. RabbitMQ, Google Pub/Sub, etc.) would + require additional development effort to respect/enforce the new + `RetryAfterMax`field. ### DeliverySpec.Timeout field From 29078a871bd4d67bdcd52f046afe860db8900da1 Mon Sep 17 00:00:00 2001 From: I867318 Date: Tue, 9 Nov 2021 11:31:04 -0700 Subject: [PATCH 3/7] formatting changes --- docs/eventing/experimental-features.md | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/docs/eventing/experimental-features.md b/docs/eventing/experimental-features.md index d67a42b95d7..ecdebfa4130 100644 --- a/docs/eventing/experimental-features.md +++ b/docs/eventing/experimental-features.md @@ -123,12 +123,11 @@ Subscriptions, Brokers, Triggers, and any other resource spec that accepts the The `retryAfterMax` field will only take effect if the `delivery` spec is configured to perform retries, and will only pertain to retry attempts on -**429** and **503** response codes. - -The `retryAfterMax` field provides an override to prevent large **Retry-After** -durations from impacting throughput, and must be specified using -the [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601#Times) format. Specifying -a "zero" value of "PT0S" will effectively disable **Retry-After** support. +**429** and **503** response codes. The field provides an override to prevent +large **Retry-After** durations from impacting throughput, and must be specified +using the [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601#Times) format. +Specifying a "zero" value of "PT0S" will effectively disable **Retry-After** +support. Prior to this experimental feature, Knative Eventing implementations have not supported **Retry-After** headers, and this is an attempt to provide a path @@ -139,7 +138,7 @@ the final state will be **opt-out** as follows... - + @@ -191,14 +190,14 @@ spec: !!! note While the experimental feature flag will enforce all DeliverySpec usage of - the `RetryAfterMax` field via Webhook validation, it does not guarantee all + the `retryAfterMax` field via Webhook validation, it does not guarantee all implementations (Channels, Sources, etc.) will actually implement support for the field. The shared `HTTPMessageSender.SendWithRetries()` logic has been enhanced to support this feature, and all implementations using it to perform retries will automatically benefit. Sandbox implementations NOT based on this shared library (e.g. RabbitMQ, Google Pub/Sub, etc.) would require additional development effort to respect/enforce the new - `RetryAfterMax`field. + `retryAfterMax`field. ### DeliverySpec.Timeout field From 39116e365168c27eed1a850a4cf6db8dbc38c8b6 Mon Sep 17 00:00:00 2001 From: I867318 Date: Fri, 12 Nov 2021 10:15:36 -0700 Subject: [PATCH 4/7] PR feedback --- docs/eventing/experimental-features.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/eventing/experimental-features.md b/docs/eventing/experimental-features.md index ecdebfa4130..5daad39411b 100644 --- a/docs/eventing/experimental-features.md +++ b/docs/eventing/experimental-features.md @@ -125,9 +125,10 @@ The `retryAfterMax` field will only take effect if the `delivery` spec is configured to perform retries, and will only pertain to retry attempts on **429** and **503** response codes. The field provides an override to prevent large **Retry-After** durations from impacting throughput, and must be specified -using the [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601#Times) format. -Specifying a "zero" value of "PT0S" will effectively disable **Retry-After** -support. +using the [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601#Times) format. The +larger of the normal backoff duration and the Retry-After header value will be +used for the subsequent retry attempt. Specifying a "zero" value of "PT0S" will +effectively disable **Retry-After** support. Prior to this experimental feature, Knative Eventing implementations have not supported **Retry-After** headers, and this is an attempt to provide a path From c2b74a537a021ee85debed63efb870aa6afb1c52 Mon Sep 17 00:00:00 2001 From: I867318 Date: Wed, 24 Nov 2021 12:37:50 -0700 Subject: [PATCH 5/7] rework html table --- docs/eventing/experimental-features.md | 56 +++++++++++++------------- 1 file changed, 27 insertions(+), 29 deletions(-) diff --git a/docs/eventing/experimental-features.md b/docs/eventing/experimental-features.md index 5daad39411b..8c7d54a38ec 100644 --- a/docs/eventing/experimental-features.md +++ b/docs/eventing/experimental-features.md @@ -126,7 +126,7 @@ configured to perform retries, and will only pertain to retry attempts on **429** and **503** response codes. The field provides an override to prevent large **Retry-After** durations from impacting throughput, and must be specified using the [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601#Times) format. The -larger of the normal backoff duration and the Retry-After header value will be +largest of the normal backoff duration and the Retry-After header value will be used for the subsequent retry attempt. Specifying a "zero" value of "PT0S" will effectively disable **Retry-After** support. @@ -135,37 +135,35 @@ supported **Retry-After** headers, and this is an attempt to provide a path for standardizing that support. To begin the feature will be **opt-in**, but the final state will be **opt-out** as follows... -{% raw %} -
EF StageFeature Stage Feature Flag retryAfterMax Field Absent retryAfterMax Field Present
- - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + +
Feature StageFeature FlagretryAfterMax Field AbsentretryAfterMax Field Present
Alpha / BetaDisabledAccepted by Webhook Validation & Retry-After headers NOT enforcedRejected by WebHook Validation
Alpha / BetaEnabledAccepted by Webhook Validation & Retry-After headers NOT enforcedAccepted by Webhook Validation & Retry-After headers enforced if max override > 0
Stable / GAn/aRetry-After headers enforced without max overrideRetry-After headers enforced if max override > 0
Feature StageFeature FlagretryAfterMax Field AbsentretryAfterMax Field Present
Alpha / BetaDisabledAccepted by Webhook Validation & Retry-After headers NOT enforcedRejected by WebHook Validation
Alpha / BetaEnabledAccepted by Webhook Validation & Retry-After headers NOT enforcedAccepted by Webhook Validation & Retry-After headers enforced if max override > 0
Stable / GAn/aRetry-After headers enforced without max overrideRetry-After headers enforced if max override > 0
-{% endraw %} - The following example shows a Subscription that retries sending an event 3 times, and respects **Retry-After** headers while imposing a max backoff of 120 seconds: From 031faada8afe6caaac8321404038cde32386e53d Mon Sep 17 00:00:00 2001 From: I867318 Date: Mon, 29 Nov 2021 15:02:35 -0700 Subject: [PATCH 6/7] convert html table to markdown --- docs/eventing/experimental-features.md | 33 ++++---------------------- 1 file changed, 5 insertions(+), 28 deletions(-) diff --git a/docs/eventing/experimental-features.md b/docs/eventing/experimental-features.md index 8c7d54a38ec..c49e6bfe9f6 100644 --- a/docs/eventing/experimental-features.md +++ b/docs/eventing/experimental-features.md @@ -135,34 +135,11 @@ supported **Retry-After** headers, and this is an attempt to provide a path for standardizing that support. To begin the feature will be **opt-in**, but the final state will be **opt-out** as follows... - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Feature StageFeature FlagretryAfterMax Field AbsentretryAfterMax Field Present
Alpha / BetaDisabledAccepted by Webhook Validation & Retry-After headers NOT enforcedRejected by WebHook Validation
Alpha / BetaEnabledAccepted by Webhook Validation & Retry-After headers NOT enforcedAccepted by Webhook Validation & Retry-After headers enforced if max override > 0
Stable / GAn/aRetry-After headers enforced without max overrideRetry-After headers enforced if max override > 0
+| Feature Stage | Feature Flag | retryAfterMax Field Absent | retryAfterMax Field Present | +| --- | --- | --- | --- | +| Alpha / Beta | Disabled | Accepted by Webhook Validation & Retry-After headers NOT enforced | Rejected by WebHook Validation | +| Alpha / Beta | Enabled | Accepted by Webhook Validation & Retry-After headers NOT enforced | Accepted by Webhook Validation & Retry-After headers enforced if max override > 0 | +| Stable / GA | n/a | Retry-After headers enforced without max override | Retry-After headers enforced if max override > 0 | The following example shows a Subscription that retries sending an event 3 times, and respects **Retry-After** headers while imposing a max backoff of From a97448929ab5dbf6540256d44825f056a4911648 Mon Sep 17 00:00:00 2001 From: I867318 Date: Tue, 30 Nov 2021 12:05:10 -0700 Subject: [PATCH 7/7] PR Feedback --- docs/eventing/experimental-features.md | 41 +++++++++++++------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/docs/eventing/experimental-features.md b/docs/eventing/experimental-features.md index c49e6bfe9f6..a93565f80ea 100644 --- a/docs/eventing/experimental-features.md +++ b/docs/eventing/experimental-features.md @@ -121,19 +121,19 @@ headers are handled when calculating backoff times for retrying **429** and Subscriptions, Brokers, Triggers, and any other resource spec that accepts the `delivery` field. -The `retryAfterMax` field will only take effect if the `delivery` spec is -configured to perform retries, and will only pertain to retry attempts on -**429** and **503** response codes. The field provides an override to prevent -large **Retry-After** durations from impacting throughput, and must be specified -using the [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601#Times) format. The -largest of the normal backoff duration and the Retry-After header value will be -used for the subsequent retry attempt. Specifying a "zero" value of "PT0S" will -effectively disable **Retry-After** support. +The `retryAfterMax` field only takes effect if you configure the `delivery` spec +to perform retries, and only pertains to retry attempts on **429** and **503** +response codes. The field provides an override to prevent large **Retry-After** +durations from impacting throughput, and must be specified using +the [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601#Times) format. The largest +of the normal backoff duration and the Retry-After header value will be used for +the subsequent retry attempt. Specifying a "zero" value of `PT0S` effectively +disables **Retry-After** support. Prior to this experimental feature, Knative Eventing implementations have not supported **Retry-After** headers, and this is an attempt to provide a path -for standardizing that support. To begin the feature will be **opt-in**, but -the final state will be **opt-out** as follows... +for standardizing that support. To begin, the feature is **opt-in**, but the +final state will be **opt-out** as follows: | Feature Stage | Feature Flag | retryAfterMax Field Absent | retryAfterMax Field Present | | --- | --- | --- | --- | @@ -141,8 +141,8 @@ the final state will be **opt-out** as follows... | Alpha / Beta | Enabled | Accepted by Webhook Validation & Retry-After headers NOT enforced | Accepted by Webhook Validation & Retry-After headers enforced if max override > 0 | | Stable / GA | n/a | Retry-After headers enforced without max override | Retry-After headers enforced if max override > 0 | -The following example shows a Subscription that retries sending an event 3 -times, and respects **Retry-After** headers while imposing a max backoff of +The following example shows a Subscription that retries sending an event three +times, and respects **Retry-After** headers while imposing a maximum backoff of 120 seconds: ```yaml @@ -165,15 +165,14 @@ spec: ``` !!! note - While the experimental feature flag will enforce all DeliverySpec usage of - the `retryAfterMax` field via Webhook validation, it does not guarantee all - implementations (Channels, Sources, etc.) will actually implement support - for the field. The shared `HTTPMessageSender.SendWithRetries()` logic - has been enhanced to support this feature, and all implementations using it - to perform retries will automatically benefit. Sandbox implementations NOT - based on this shared library (e.g. RabbitMQ, Google Pub/Sub, etc.) would - require additional development effort to respect/enforce the new - `retryAfterMax`field. + While the experimental feature flag enforces all DeliverySpec usage of the + `retryAfterMax` field through Webhook validation, it does not guarantee all + implementations, such as Channels or Sources, actually implement support + for the field. The shared `HTTPMessageSender.SendWithRetries()` logic has + been enhanced to support this feature, and all implementations using it to + perform retries will automatically benefit. Sandbox implementations not + based on this shared library, for example RabbitMQ or Google Pub/Sub, would + require additional development effort to respect the `retryAfterMax` field. ### DeliverySpec.Timeout field