Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inline resiliency policies for PubSub Subscription #46

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Inline resiliency policies for PubSub Subscription #46

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
955c0ed
Create R-inline-resiliency-policies-in-pubsub-subscribers.md
olitomlinson Nov 18, 2023
3b5ce8d
Update R-inline-resiliency-policies-in-pubsub-subscribers.md
olitomlinson Nov 20, 2023
17637cb
Update R-inline-resiliency-policies-in-pubsub-subscribers.md
olitomlinson Nov 20, 2023
f5b7a5d
Update R-inline-resiliency-policies-in-pubsub-subscribers.md
olitomlinson Nov 21, 2023
e33955f
Update and rename R-inline-resiliency-policies-in-pubsub-subscribers.…
olitomlinson Nov 21, 2023
fcfe41c
Update R-inline-resiliency-policies-for-pubsub-subscription.md
olitomlinson Nov 21, 2023
e0aa610
Update R-inline-resiliency-policies-for-pubsub-subscription.md
olitomlinson Nov 21, 2023
609a84d
Update R-inline-resiliency-policies-for-pubsub-subscription.md
olitomlinson Nov 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
181 changes: 181 additions & 0 deletions R-inline-resiliency-policies-in-pubsub-subscribers.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
# Inline resiliency policies in pubsub subscriber declarations

* Author(s): Oliver Tomlinson (@olitomlinson)
* State: Draft
* Updated: 18/11/2023

## Overview

I propose that declarative and programmatic PubSub subscribers should be extended specify a resiliency policy which ONLY affects the subscriber in question.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Define subscriber in question please. This is very ambiguous.

The pubsub component + app ID is a single subscriber in question.

Essentially this would override any resiliency policy for the component with regards to that very specific sidecar instance!

If you also want to include something topic specific here, this becomes a huge undertaking (requires refactoring of components, completely changing the policies we have etc)

Loading policies that are component specific from an app to overload the resiliency configuration may be possible however!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Define subscriber in question please. This is very ambiguous.

Hmmm... Maybe 'subscription' rather than 'subscriber' would be better.

"I propose that declarative and programmatic PubSub subscriptions should be extended to include a resiliency policy which is applied only to the the inbound process of delivering the message from sidecar to app. The resiliency policy is scoped to just the subscription it was declared into / against"

Does that read better?

Copy link
Author

@olitomlinson olitomlinson Nov 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm proposing a change which doesn't have to impact any of the existing component resiliency mechanics. It simply doesnt use the existing component policy (if one exists) and applies a more specialised policy (if one has been provided via dapr/subscribe or via a subscription yaml) at the time of creating the subscription onto the topic.

image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes but still a bit too ambiguous.

  1. Would these settings apply to all topics within this subscription?

Or 2) do you need to be able to configure different settings for each topic?

The latter would be an extraordinary amount of work, and could instead be accomplished by repeating 1) with a different app and configuration - but still using the same pubsub component.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't see this image earlier. As long as subscription specific resiliency configuration applies to all topics within that subscription this sounds good to me!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. No. Each subscription has the ability to provide its own unique policy, just for that subscription. They are not shared across each Topic.

The whole point of this proposal is about enabling each subscription to operate via its own specialised policy, if the component-level policy is not suitable.

I didn't see this image earlier.

Ive just created the diagram to aid our conversation :)

As long as subscription specific resiliency configuration applies to all topics within that subscription this sounds good to me!

No, unfortunately it doesn't apply to all Topics as a blanket policy, it only applies to the subscriptions that that have been configured with the inline policy.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll ping you in Discord because I think you are still misunderstanding what I'm asking :)

Copy link
Member

@berndverst berndverst Nov 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm talking about this difference:

@app.route('/dapr/subscribe', methods=['GET'])
def subscribe():
    subscriptions = [
      {
        'pubsubname': 'pubsub',
        'topic': 'checkout',
        'routes': {
          'rules': [
            {
              'match': 'event.type == "order"',
              'path': '/orders'
            },
          ],
          'default': '/orders',
          'resiliency': {
           # Resiliency Config here
          }
        }
      },
      {
        'pubsubname': 'pubsub',
        'topic': 'checkout2',
        'routes': {
          'rules': [
            {
              'match': 'event.type == "order"',
              'path': '/orders2'
            },
          ],
          'default': '/orders2',
          'resiliency': {
           # SAME Resiliency Config here
          }
        }
      }
]
    return jsonify(subscriptions)

instead of

@app.route('/dapr/subscribe', methods=['GET'])
def subscribe():
    subscriptions = [
      {
        'pubsubname': 'pubsub',
        'topic': 'checkout',
        'routes': {
          'rules': [
            {
              'match': 'event.type == "order"',
              'path': '/orders'
            },
          ],
          'default': '/orders',
          'resiliency': {
           # Resiliency Config here
          }
        }
      },
      {
        'pubsubname': 'pubsub',
        'topic': 'checkout2',
        'routes': {
          'rules': [
            {
              'match': 'event.type == "order"',
              'path': '/orders2'
            },
          ],
          'default': '/orders2',
          'resiliency': {
           # DIFFERENT resiliency config here
          }
        }
      }
]
    return jsonify(subscriptions)

We can only accept one configuration per Pubsub component / Sidecar combination.


This should only impact the runtime, but I could see potential work in the SDKs to support inline resiliency policies i.e. extending any convenience methods/attributes for registering subscribers.

## Background

### Motivation
- There isn't currently an low-friction/lightweight solution to have a specialised resiliency policy on a per subscriber basis, which means policies do not work well for some subscribers which would benefit from their own policy.

### Goals
- Empower Developers (and Operators) to provide resiliency policies which are specialised to the needs of the message handler.

### Current Shortfalls
- See motivation

## Related Items

### Related proposals

- A general proposal to introduce resiliency policies via a Callback API https://github.com/dapr/dapr/issues/5264

### Related issues

- See [here](https://github.com/dapr/dapr/issues/7184) for the problem that this proposal is aiming to address

## Expectations and alternatives

* What is in scope for this proposal?
* Support for programmatic and declarative subscribers
* What is deliberately *not* in scope?
* ???
* What alternatives have been considered, and why do they not solve the problem?
* I think it would be possible to change the existing resiliency policy yaml struture to include named topics & subscribers, but I do not think this provides the _best_ solution as it doesn't easily empower developers to craft their policies in the place in which they are needed (the subscriber code its self)
* Are there any trade-offs being made? (space for time, for example)
* Operators may loose some visibility of specialised retry policies when programmatic subscribers are used, but I think on balance this is trade-off is acceptable.
* What advantages / disadvantages does this proposal have?
* Advantages
* Reuses the existing resiliency concepts / yaml so this should feel really comfortable to users who are already familiar with resiliency policies
* Disadvantages
* Operators may not be able to _easily_ change resiliency policies which are attached to programmatic subscribers (as that would be a code change)
* Counter : Users who care about this should be encouraged to use the declarative model instead.


## Implementation Details

### Design

#### Declarative Subscriber

```yaml
apiVersion: dapr.io/v2alpha1
kind: Subscription
metadata:
name: order-subscriber
spec:
pubsubname: myPubsubComponent
topic: newOrder
routes:
default: /order
deadLetterTopic: poisonMessages
resiliency:
policies:
timeouts:
general: 5s
retries:
important:
policy: constant
duration: 5s
maxRetries: 30
circuitBreakers:
pubsubCB:
maxRequests: 1
interval: 8s
timeout: 45s
trip: consecutiveFailures > 8
components:
myPubsubComponent:
topic:
newOrder:
subscriber:
order-subscriber:
timeout: general
retry: important
circuitBreaker: pubsubCB
```

**Slim alternative** of the above. (personally, I prefer this slim version)

```yaml
apiVersion: dapr.io/v2alpha1
kind: Subscription
metadata:
name: order-subscriber
spec:
pubsubname: myPubsubComponent
topic: newOrder
routes:
default: /order
deadLetterTopic: poisonMessages
resiliency:
timeout: 5s
retry:
policy: constant
duration: 5s
maxRetries: 30
circuitBreaker:
maxRequests: 1
interval: 8s
timeout: 45s
trip: consecutiveFailures > 8
```

#### Programmatic Subscriber

continuing with the **slim** theme, this is how the `/dapr/subscribe` response could look...

```json
[
{
"pubsubname": "myPubsubComponent",
"topic": "newOrder",
"route": "/orders",
"deadLetterTopic": "poisonMessages",
"resiliency": {
"timeout": "5s",
"retry": {
"policy": "constant",
"duration": "5s",
"maxRetries": "30"
},
"circuitBreaker": {
"maxRequests": "1",
"interval": "8s",
"timeout": "45s",
"trip": "consecutiveFailures > 8"
}
}
}
]
```


### Feature lifecycle outline

* Expectations
* ???
* Compatability guarantees
* N/A
* Deprecation / co-existence with existing functionality
* This would have to co-exist with resiliency policies that are already applied to pubsub components. I would expect that the specialised policies would **override** any component-level policies.
* Feature flags
* No

### Acceptance Criteria

How will success be measured?

* By allowing specialised resiliency policies per subscriber across all topics and apps.

## Completion Checklist

What changes or actions are required to make this proposal complete? Some examples:

* Code changes
* N/A
* Tests added (e2e, unit)
* N/A
* SDK changes (if needed)
* TBD
* Documentation
* N/A