knative / eventing

Event-driven application platform for Kubernetes
https://knative.dev/docs/eventing
Apache License 2.0
1.39k stars 586 forks source link

Add support for JSON Batch Format #6797

Open pnivanov opened 1 year ago

pnivanov commented 1 year ago

Problem CloudEvents spec v1.0.1 introduces the concept of batch events: https://github.com/cloudevents/spec/blob/v1.0.1/json-format.md#4-json-batch-format

Knative Eventing Broker should support JSON Batch format from the event source and in the reply events.

Currently a function can return a single CloudEvent or an empty response. There are use cases when the function needs to return multiple cloud events. Currently this is not possible.

Persona: Event producer

Exit Criteria The broker should accept multiple cloud events in a single payload when media type is set to application/cloudevents-batch+json.

pierDipi commented 1 year ago

Related to #6581

pierDipi commented 1 year ago

@pnivanov similarly to this comment here https://github.com/knative/eventing/issues/6581#issuecomment-1290278838, what do you expect from a delivery guarantees perspective, for example, some systems don't support sending multiple events transactionally, so the semantic wouldn't be "all events or nothing", would that be ok for you? what would you expect?

pierDipi commented 1 year ago

I'm looking for how batch support would impact the event delivery section of the spec: https://github.com/knative/specs/blob/main/specs/eventing/data-plane.md#event-delivery, the different approaches we could have and pros and cons for each.

pnivanov commented 1 year ago

@pierDipi I think the use case is a bit different here. IIUC the JSON Batch Format spec, the idea is to send a single HTTP request and inside the payload to have an array of CloudEvents (in JSON format). In this case the recipient accepts all or none of the events. With the derived (reply) events it gets more complicated, because some of the CE in the batch can be processed successfully while other might fail. For batch request I would expect "batch response". But then what should be the response code...?


Let me elaborate more on the particular use case, that I have.

pierDipi commented 1 year ago

I get the difference (that it is also why I commented with related and not duplicated), one option that you have is having a service that splits a single event into multiple events and send them to a broker for further processing, it could be your "pre-processor" or a more generic standalone intermediate service.

I think my questions still applies, if we would support JSON batch format for the response, what would be the delivery guarantees you expect for the batch?

pnivanov commented 1 year ago

the semantic wouldn't be "all events or nothing", would that be ok for you? what would you expect?

As you said "all or nothing" is possible only in systems that support transactional behaviour. But very often this is not the case. One of my functions sends a Slack message. This cannot be "undone". And this is ok.

IMO the spec should not impose "all or nothing". Events are discrete by nature and the batch should not be considered as some kind of grouping or relation. With this said, it should NOT be expected from the receiver to process the events in any particular order.

I would expect:

github-actions[bot] commented 1 year ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

pnivanov commented 1 year ago

/remove-lifecycle stale

pnivanov commented 1 year ago

/reopen

knative-prow[bot] commented 1 year ago

@pnivanov: Reopened this issue.

In response to [this](https://github.com/knative/eventing/issues/6797#issuecomment-1628649794): >/reopen Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
github-actions[bot] commented 9 months ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

pierDipi commented 9 months ago

/remove-lifecycle stale /triage accepted

sadath-12 commented 6 months ago

/assign

Cali0707 commented 2 months ago

In case SOME events in the batch are processed successfully and some fail, return different status code. Additionally the payload should contain the events, that failed to be processed. This will allow the sender to retry the operation only for the failed events.

It seems like the precedence here is to return HTTP 207, along with a response body that specifies the status for each event. See https://http.dev/207. It seems like in the past this was always XML, since it is primarily used in WebDAV. I think we should use JSON instead, and encode it something like:

{
    "statuses": [
        {
            "id": "12345",
            "status": 400,
        },
        {
            "id": "67890",
            "status": 202,
        }
    ]
}

Basically, we would have in the response an array of statuses with the id of the event (so that the sender can correlate them with what was originally sent), as well as the HTTP status that would have been returned for each individual request.

For the cases where everything succeeds or everything fails, the existing delivery codes could be reused.

Would this work @pnivanov @pierDipi ?