Eventing: Dead-Letter Queue or other Error Handling

[alexander-alvarez]

Problem
Brokers forward events to subscribers which in turn route traffic to revisions. Knative can work properly and get these events to the proper revision handler that is running correctly, but the execution of the events can fail for a variety of reasons (downstream system is down, application logic rejects the invalid message, a runtime exception occurs, etc) but right now there is no way to control or define the expected behavior in these situations.

Persona:
Event Consumer / System integrator

Exit Criteria

• An event consumer can define a configuration that indicates when a message fails / exceeds the retry back-off in a service, the given message is considered "dead" and can be routed to another channel (for the eventual handing by another service: the "dead-letter handler")
  
  
  
  • Possibly implemented as an abstraction similar to Sequence is where try-catch functionality is provided, allowing a branching pattern with successful events going down one branch and unsucessful ones going down another
  
  

Time Estimate (optional):
I have no idea since I'm not familiar with the codebase

Additional context (optional)
N/A

[lionelvillard]

This is an important pattern and there is a beginning of a solution in the context of Choice, which needs to be generalized to other Composable objects (like Sequence).

[mikehelmick]

related to #1555

[nachocano]

@lionelvillard have you started with some of this work? I'd be happy to help/collaborate... And also on #1493. IIRC @Harwayne mentioned you guys have started discussing this in the Channel task force group?

[lionelvillard]

I haven't started other than thinking about how to do it.

Regarding retrying (#1493), what about adding this capability into the CloudEvent SDK? @n3wscott any thoughts on that?

[akashrv]

/milestone v0.9.0

[nachocano]

/assign

[nachocano]

/unassign
as I'm not actively working on this...
just in case someone wants to grab it

[lionelvillard]

I'll grab it in about 3 weeks, unless someone wants to take a look at it before.

[mikehelmick]

@rgregg - This needs requirements / product input.

[aslom]

I am also interested in this. Let start on design/requirement document

using template:
https://docs.google.com/document/d/1QRREfL8gSVSURHMkDLjDtMh1BBsNiwfmaBiGnJUDQ2g/edit
via 0.9 plan
https://docs.google.com/document/d/1iR9Vfmgi1Dkf_ge95LXA-VdZ3shV0611C3vtIF6-7c4/edit#heading=h.ltzumxehvfdz

[rgregg]

We need to follow the Knative Feature Tracks template that @aslom linked to. Before we start writing code, someone with the insights on what we need to do here needs to complete steps 1 & 2 for review. Step 1 may be covered by the original text of this issue, but the need for a design doc that we can review seems clear here.

[lionelvillard]

/assign

I'm back

[lionelvillard]

link to the design document (WIP): https://docs.google.com/document/d/1qRrzGoHJQO-oc5p-yRK8IRfugd-FM_PXyM7lN5kcqks/edit#

[aslom]

The proposal looks good to me. I added section about impact. Should we put estimates on how much impact implementing it would have (in-memory channel, other channels)? Is it required feature? Add MUST and SHOULD? Additions to eventing tests?

[lionelvillard]

/milestone v0.10.0

[knative-prow-robot]

@lionelvillard: You must be a member of the knative/knative-milestone-maintainers GitHub team to set the milestone. If you believe you should be able to issue the /milestone command, please contact your and have them propose you as an additional delegate for this responsibility.

In response to this:

/milestone v0.10.0

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[akashrv]

/milestone v0.10.0

[lionelvillard]

I have made the following changes to the design:

• added more examples (broker and sequence)
• added more configuration parameters (backoffPolicy, backoffDelay)
• added a section about alternate designs, one about DLC for sources and one about embedding the configuration parameters in the channelable spec.

@nachocano @akashrv @vaikas-google @n3wscott @aslom @adamharwayne @matzew @alexander-alvarez

[aslom]

Added initial version about impact of implementing DLC/ErrorHandling in existing channel implementations and related work showing how DLQ is used in other products: https://docs.google.com/document/d/1qRrzGoHJQO-oc5p-yRK8IRfugd-FM_PXyM7lN5kcqks/edit#heading=h.h0cxm9mbzria

[aslom]

One issue raised today was do we need error sink separately form channel - there are several comments about it and one observation was that wrapping events may be avoided by using cloud events extensions?
https://docs.google.com/document/d/1qRrzGoHJQO-oc5p-yRK8IRfugd-FM_PXyM7lN5kcqks/edit#heading=h.j3wn3hkg5qre

[aslom]

Should we open issues to document discussion or continue using google doc comments?

[aslom]

PR proposal (day "one" solution): https://github.com/knative/eventing/pull/1949

[yolocs]

I wonder what's the status of this issue?

In addition to the current proposal, how people feel about:

1. Allow per-subscriber retry options. This comes handy when applications have very different capability of handling load.
2. Having runtime retry option returned by the subscriber. E.g. return 503 with Retry-After header. This gives applications the capability to adjust load dynamically.

[lionelvillard]

We have a design document: https://github.com/knative/eventing/tree/master/docs/delivery

As of today the in-memory-channel supports deadLetterSink (only).

1 is planned. No implementations yet though.

We haven't discussed 2. Can you give more details, ie how to you see the load being adjusted dynamically? Usually Kubernetes handles load-balancing so I wonder if we need to support Retry-After.

[yolocs]

I think a better way of saying it is that 2 could provide subscribers the flexibility to intentionally delay receiving some events. If an application is load-aware (doesn’t have to be traffic load), it can make use of this to delay the next delivery if it sees itself overloading. Or it might just want to delay handling some events because it’s not yet ready to handle them. To think, I feel this could be useful in general if we can have scheduled delivery (not necessarily related with this issue).

[lionelvillard]

I agree that can be useful. Do you have a concrete usecase?

[yolocs]

I don't have a concrete usecase at the moment. I guess 2 is just an optimization for 1 so it's less important.

[andreiac-silva]

Hi folks.
Do you have any idea about when Dead Letter feature will be supported by kafka channels?
Thanks.

[lionelvillard]

@andreiac-silva it might be supported already. Have you tried?

It's on my TODO list to try it and document it. Sorry for the delay

[andreiac-silva]

@lionelvillard I haven't tested it yet. I've seen the support information in knative documentation.
But I'm going to test it, thank you for the answer.

[vishaltak]

@lionelvillard - I added the DLQ config to my Broker(backed by Kafka). I created another trigger which routes it to a KService which uploads the event to S3(let's say upload-service). When I send an event to the Broker(from inside a Pod by executing a curl), the event is uploaded to S3 and the Kafka Topic "last offset" and "consumer offset" are in sync(as expected).

I then added a bug in upload-service to simulate a failure. Now when I send an event to the Broker, and the "upload-service" pod throws an error in the function(as expected) and the object is not uploaded to S3(as expected). However, there is no re-routing of the event to the DLQ(in this case the "event-display" service). Also, the Kafka Topic "last offset" and "consumer offset" are in sync. I expected the "consumer offset" to lag(since the event is not yet successfully consumed by the "upload-config" service. This situation persist with or without the DLQ parameters.

Is there something wrong in the following parameters? I used this as reference.

apiVersion: eventing.knative.dev/v1alpha1
kind: Broker
metadata:
  labels:
    eventing.knative.dev/namespaceInjected: "true"
  name: default
spec:
  channelTemplateSpec:
    apiVersion: messaging.knative.dev/v1alpha1
    kind: KafkaChannel
    spec:
      delivery:
        backoffDelay: 3s
        backoffPolicy: exponential
        deadLetterSink:
          apiVersion: serving.knative.dev/v1beta1
          kind: Service
          name: event-display
        retry: 5
      numPartitions: 3
      replicationFactor: 1

[lionelvillard]

@vishaltak Thanks! I'll take a look at it.

[vishaltak]

@paulrossman Can we have some resources/articles/documentation to fix/bypass this issue before it's closed?

[lionelvillard]

this is a reasonable request :-) Reopening.

[otavioprado]

@andreiac-silva it might be supported already. Have you tried?

It's on my TODO list to try it and document it. Sorry for the delay

@lionelvillard @n3wscott @grantr
I tried to use KafkaChannel with an event-display as deadLetterSink, but it does not work:

apiVersion: flows.knative.dev/v1beta1
kind: Sequence
metadata:
  name: sequence-audit
spec:
  channelTemplate:
    apiVersion: messaging.knative.dev/v1alpha1
    kind: KafkaChannel
    spec:
      numPartitions: 2
      replicationFactor: 1
      delivery:
        backoffDelay: 3s
        backoffPolicy: exponential
        deadLetterSink:
          apiVersion: serving.knative.dev/v1beta1
          kind: Service
          name: event-display-audit
        retry: 5
  steps:
    - ref:
        apiVersion: serving.knative.dev/v1
        kind: Service
        name: http-caller

In the configuration example above the event disappears if the "http-caller" sets cloud event to status code 500. Isn't the event sent to DLQ when its status is set with an error code?

[grantr]

@lionelvillard will break this issue down into smaller, more concrete tasks so we can measure progress more accurately.

[lionelvillard]

The remaining tasks are:

• https://github.com/knative/docs/issues/2043
• https://github.com/knative/eventing/issues/2977
• https://github.com/knative/eventing/issues/2210
• https://github.com/knative/eventing/issues/2979
• https://github.com/knative/eventing/issues/2980