HTTP Transport Binding for batching JSON #370

cneijenhuis · 2019-01-15T08:52:15Z

In the discussion from last week about #360 we agreed that a batching option for the HTTP Transport Binding is useful. This is an attempt to introduce it.

During the call, @clemensv suggested to build the batching into the structured mode. I couldn't figure out how to do this generically for any event format. Instead, I did it for the JSON format only.

I also did not include the batching mode as one of the modes the receiver SHOULD support. Take a Function as a Service: Usually, each event should be processed in on an own instance of a function. This is trivial to implement when a HTTP Request always contains a single event. A batch doesn't map nicely onto the core FaaS abstractions.

The HTTP Content-Type is a bit tricky - we want that the receiver can distinguish between the events based on the header, so I've created application/cloudevents-batch+json. However, I'm not sure if this clutters the namespace too much.

Signed-off-by: Christoph Neijenhuis christoph.neijenhuis@commercetools.de

duglin · 2019-01-15T12:58:26Z

travis isn't happy :-( I wonder if it's the case of the "B"

cneijenhuis · 2019-01-15T13:57:30Z

Yep, that was it. Thanks, fixed! :)

Signed-off-by: Christoph Neijenhuis <christoph.neijenhuis@commercetools.de>

deissnerk · 2019-01-24T13:09:18Z

LGTM
How about adding support for batching to the web hook spec? In addition to Webhook-Request-Rate one could think of Webhook-Batch-Size or Webhook-CE-Batch-Size. I know that @clemensv wanted to keep the web hook spec independent from CloudEvents. So we might add this somehow on top of the basic web hook spec.

Thoughts?

clemensv · 2019-01-29T11:24:27Z

@deissnerk @cneijenhuis batching is about structuring the message payload (one or multiple). The WebHook spec is about how you deliver the payload, independent of how it's structured.

Event format: How to encode one event or a batch of events
Transport: How to bind the encoded message/batch to a transport message
WebHook (etc): Application contract to deliver the transport message to an endpoint and handle related errors.

clemensv · 2019-01-29T11:35:29Z

http-transport-binding.md

+
+In the *batched* content mode several events are batched into a single HTTP
+request or response body. The chosen [event format](#14-event-formats) MUST
+define how a batch is represented. Currently, the only format supporting


You have to change "currently, the only" if/when that changes. It makes sense to make an absolute statement like "The JSON event format that MUST be supported by any compliant implementation supports batching."

I didn't implement your suggestion exactly, because it seems to imply that batching MUST be supported by any compliant implementation. In the current proposal, it is however optional, and I think it should stay optional, as I wrote in the PR description:

I also did not include the batching mode as one of the modes the receiver SHOULD support. Take a Function as a Service: Usually, each event should be processed in on an own instance of a function. This is trivial to implement when a HTTP Request always contains a single event. A batch doesn't map nicely onto the core FaaS abstractions.

Is what I added in 1191dd2 fine?

clemensv · 2019-01-29T11:41:21Z

http-transport-binding.md

+The batch of events is then rendered in accordance with the event format
+specification and the resulting data becomes the HTTP message body.
+
+The batch MAY be empty (typically used in a HTTP response).


Strike "(typically used in a HTTP response)". There might be reasons to push an empty (zero elements) batch.

clemensv · 2019-01-29T11:45:34Z

json-format.md

+This section defines how a batch of CloudEvents is mapped to JSON.
+
+The outermost JSON element is a [JSON Array][JSON-array], which contains
+CloudEvents rendered in accordance with the [JSON event format][JSON-format]


... as elements ...

I like this approach with the use of a JSON array, however I'd like to see an alternate JSON event format that supports a 2D array.

Time series events are most efficiently contained within a grid (tabular) structure which can be represented by a two-dimensional "array" as an "object". A CloudEvents "message-level" Batch object can comprise this 2D array.

The column values within the 2D array can correspond to CloudEvent "event-level" attributes (e.g., id, time, type, data), which can include attributes from one or more extensions (e.g., sequence, object, attribute).

Another "message-level" Schema attribute (type: array as object) can map the column indices to the names of "event-level" attributes:

0, "time"
1, "type"
2, "object"
3, "attribute"
4, "data"

The Schema array could also include constraint columns (e.g. Required) allowing flexibility among producer/consumer ecosystems.

This design has 2 benefits:

It compresses a multi-event payload by removing repetitive "names" in name (attribute)-value pairs.

It streamlines a consumer's parsing a multi-event payload.

You are proposing a different data layout model. That's not a batch, it's a wholly different encoding. I discuss this here as "Record Sequence with Metadata Preamble"

https://vasters.com/blog/data-encodings-and-layout/

If you care about compact time series data transfers, we should make an Apache Avro encoding, because Avro is really good at that.

Signed-off-by: Christoph Neijenhuis <christoph.neijenhuis@commercetools.de>

deissnerk · 2019-01-30T16:33:57Z

Event format: How to encode one event or a batch of events
Transport: How to bind the encoded message/batch to a transport message
WebHook (etc): Application contract to deliver the transport message to an endpoint and handle related errors.

@clemensv I understand that distinction between transport and web hook. That's why I was suggesting to define the header fields separate from the core web hook spec. Technically I think it makes sense, that the delivery target can specify the (maximum) number of events each message is supposed to contain in addition to the request rate,
One more thing that might be interesting, is to have additional fields like "Accept: application/cloudevents-batch+json" in the validation response to signal the support of batching.

cneijenhuis · 2019-01-30T21:35:59Z

I do agree that there is an interesting interaction between request rate and batch size. I even wonder if specifying both statically is useful. E.g. if the request rate is 50/minute, and the batch size is 50:

I send 1 msg per sec. After 50 sec, I've send 50 msgs and have to stop.
I send 50 msgs every two sec. After 50 sec, I've send 1250 msgs, and can send 1250 more messages.

Maybe it is better to interpret the request rate as event rate. E.g. for a request rate of 10, you can send me 10 events in 10 requests, or you can send me 10 events in a single batch.

One more thing that might be interesting, is to have additional fields like "Accept: application/cloudevents-batch+json" in the validation response to signal the support of batching.

My take is that there is an initial registration step (which is not standardized), and - if the sender offers a choice - the delivery target (administrator) should choose the format and mode.

duglin · 2019-01-31T01:54:56Z

http-transport-binding.md

+In the *batched* content mode several events are batched into a single HTTP
+request or response body using an [event format](#14-event-formats) that
+supports batching.
+
 ### 1.4. Event Formats

 Event formats, used with the *structured* content mode, define how an event is
 expressed in a particular data format. All implementations of this
 specification MUST support the [JSON event format][JSON-format], but MAY


do we need to make this clearer w.r.t. which json format we mean? single vs batched - or do they have to support both?

@cneijenhuis what do you think about making this:

... MUST support the non-batching [JSON event format][JSON-format], but MAY...

so just add non-batching ?

I like it! d3f2edb

duglin · 2019-01-31T02:02:18Z

http-transport-binding.md

+
+The batch MAY be empty.
+All batched CloudEvents MUST have the same `specversion` attribute. Other
+attributes MAY differ, including the `contenttype` attribute.


datacontenttype

duglin · 2019-01-31T02:07:26Z

json-format.md

+Although the *JSON Batch Format* builds ontop of the *JSON Format*, it is
+considered as a separate format: a valid implementation of the *JSON Format*
+doesn't need to support it. The *JSON Batch Format* MUST NOT be used when only
+support for the *JSON Format* is indicated.


I wonder if this last sentence is appropriate since we don't get into how things like this are "indicated".

I'm okay with dropping it - the sentence tries to re-inforce/clarify the difference between the two formats, but the point has already been made in the sentence before.

duglin · 2019-01-31T02:08:15Z

json-format.md

+      "comexampleextension2" : {
+          "otherValue": 5
+      },
+      "contenttype" : "application/vnd.apache.thrift.binary",


datacontenttype

duglin · 2019-01-31T02:08:22Z

json-format.md

+      "comexampleextension2" : {
+          "otherValue": 5
+      },
+      "contenttype" : "application/json",


clemensv · 2019-01-31T09:00:45Z

I have an implementation of this here now: https://github.com/clemensv/cloudevents-sdk-csharp/tree/batching

The only little pitfall I've found is that a contentType.startsWith("application/cloudevents") is now the false test for probing for a CloudEvent. You must include the plus sign before the format indicator since application/cloudevents-batch also matches that test.

Signed-off-by: Christoph Neijenhuis <christoph.neijenhuis@commercetools.de>

duglin · 2019-02-04T14:23:59Z

This PR was approved on last week's call with the agreement to make the minor editorial changes that @cneijenhuis has now made. So, LGTM from me, however, I'd like more one LGTM just to make sure one more set of eyes has verified that the latest round of edits didn't introduce some minor bug by mistake.

duglin · 2019-02-04T14:27:55Z

@dmigliori we discussed your comment ( #370 (comment) ) on last week's call. The general consensus was that your proposal is independent of the "batching" feature. Would you be willing to open an issue or even better, a PR, so the group could discuss your proposal as a stand-alone feature?

deissnerk · 2019-02-04T16:30:05Z

@cneijenhuis

I do agree that there is an interesting interaction between request rate and batch size. I even wonder if specifying both statically is useful. E.g. if the request rate is 50/minute, and the batch size is 50:

I send 1 msg per sec. After 50 sec, I've send 50 msgs and have to stop.

I send 50 msgs every two sec. After 50 sec, I've send 1250 msgs, and can send 1250 more messages.

My original intention was that with a maximum batch size you could limit memory usage on the receiver side. I agree that the overall event rate might be interesting, too.

One more thing that might be interesting, is to have additional fields like "Accept: application/cloudevents-batch+json" in the validation response to signal the support of batching.

My take is that there is an initial registration step (which is not standardized), and - if the sender offers a choice - the delivery target (administrator) should choose the format and mode.

Fair enough.

cneijenhuis · 2019-02-04T18:50:27Z

My original intention was that with a maximum batch size you could limit memory usage on the receiver side.

Ah, I see. Yes, both limits make sense in their own right.

duglin · 2019-02-04T21:37:44Z

@cneijenhuis @deissnerk anything we need to modify here before we merge?

deissnerk · 2019-02-04T22:11:13Z

@duglin No, I don't think so. This would be a separate issue or PR. I don't really know, where to put it. We wanted to keep the web hook spec CloudEvent agnostic.

duglin · 2019-02-05T00:08:02Z

@deissnerk ok thanks. Anyone wanna give it one more look-over and an LGTM?

deissnerk · 2019-02-05T15:03:31Z

@duglin LGTM

duglin · 2019-02-05T15:08:21Z

@deissnerk thanks! merging

cneijenhuis force-pushed the http-transport-batch branch from 6a6196a to 2cb54a1 Compare January 15, 2019 13:00

HTTP Transport Binding for batching JSON

f1af14d

Signed-off-by: Christoph Neijenhuis <christoph.neijenhuis@commercetools.de>

cneijenhuis force-pushed the http-transport-batch branch from 2cb54a1 to 2d460c2 Compare January 23, 2019 20:43

Move JSON Batch into JSON Format; more generic HTTP transport

d7216e2

Signed-off-by: Christoph Neijenhuis <christoph.neijenhuis@commercetools.de>

cneijenhuis force-pushed the http-transport-batch branch from 2d460c2 to d7216e2 Compare January 23, 2019 20:45

Make Travis happy: don't use lowercase should/have

a1717e5

Signed-off-by: Christoph Neijenhuis <christoph.neijenhuis@commercetools.de>

cneijenhuis force-pushed the http-transport-batch branch from 3012413 to a1717e5 Compare January 24, 2019 10:18

duglin mentioned this pull request Jan 24, 2019

Support multiple events for HTTP binding #282

Closed

clemensv reviewed Jan 29, 2019

View reviewed changes

Implement suggestions of Clemens

1191dd2

Signed-off-by: Christoph Neijenhuis <christoph.neijenhuis@commercetools.de>

duglin reviewed Jan 31, 2019

View reviewed changes

json-format.md Outdated

"comexampleextension2" : {

"otherValue": 5

},

"contenttype" : "application/json",

Copy link

Collaborator

duglin Jan 31, 2019

Choose a reason for hiding this comment

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

dct

cneijenhuis added 2 commits February 1, 2019 14:23

s/contenttype/datacontenttype/

b6ad3a6

Signed-off-by: Christoph Neijenhuis <christoph.neijenhuis@commercetools.de>

add non-batching to clarify which format MUST be supported

d3f2edb

Signed-off-by: Christoph Neijenhuis <christoph.neijenhuis@commercetools.de>

duglin merged commit 70b6522 into cloudevents:master Feb 5, 2019

cneijenhuis mentioned this pull request Apr 5, 2019

What happens when... for JSON Batch Format. #414

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HTTP Transport Binding for batching JSON #370

HTTP Transport Binding for batching JSON #370

cneijenhuis commented Jan 15, 2019

duglin commented Jan 15, 2019

cneijenhuis commented Jan 15, 2019

deissnerk commented Jan 24, 2019 •

edited

Loading

clemensv commented Jan 29, 2019

clemensv Jan 29, 2019

cneijenhuis Jan 29, 2019

clemensv Jan 29, 2019

clemensv Jan 29, 2019

dmigliori Jan 30, 2019

clemensv Jan 31, 2019

deissnerk commented Jan 30, 2019

cneijenhuis commented Jan 30, 2019

duglin Jan 31, 2019

duglin Feb 4, 2019

cneijenhuis Feb 4, 2019

duglin Feb 4, 2019

duglin Jan 31, 2019

duglin Jan 31, 2019

cneijenhuis Feb 1, 2019

duglin Jan 31, 2019

duglin Jan 31, 2019

clemensv commented Jan 31, 2019

duglin commented Feb 4, 2019

duglin commented Feb 4, 2019

deissnerk commented Feb 4, 2019

cneijenhuis commented Feb 4, 2019

duglin commented Feb 4, 2019

deissnerk commented Feb 4, 2019

duglin commented Feb 5, 2019

deissnerk commented Feb 5, 2019

duglin commented Feb 5, 2019

HTTP Transport Binding for batching JSON #370

HTTP Transport Binding for batching JSON #370

Conversation

cneijenhuis commented Jan 15, 2019

duglin commented Jan 15, 2019

cneijenhuis commented Jan 15, 2019

deissnerk commented Jan 24, 2019 • edited Loading

clemensv commented Jan 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deissnerk commented Jan 30, 2019

cneijenhuis commented Jan 30, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clemensv commented Jan 31, 2019

duglin commented Feb 4, 2019

duglin commented Feb 4, 2019

deissnerk commented Feb 4, 2019

cneijenhuis commented Feb 4, 2019

duglin commented Feb 4, 2019

deissnerk commented Feb 4, 2019

duglin commented Feb 5, 2019

deissnerk commented Feb 5, 2019

duglin commented Feb 5, 2019

deissnerk commented Jan 24, 2019 •

edited

Loading