Repository navigation
feat(pulsar): message binding with key for ordering - #315
reinaut-krekels-acerta wants to merge 5 commits into
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
|
|
||
| Field Name | Type | Required | Description | Default value | | ||
| ---|:---:|:---:|:---|:---| | ||
| `key` | [Schema Object][schemaObject] \| [Reference Object][referenceObject] | No | The message key, determining the [order](https://pulsar.apache.org/docs/next/concepts-messaging/#ordering-guarantee) for consumers using a [Key_Shared](https://pulsar.apache.org/docs/next/concepts-messaging/#key_shared) subscription. | N/A | |
There was a problem hiding this comment.
I don't agree with the current design
Apache Pulsar allows to send messages with:
key(routing)partitionKey(may act asorderingKeyfor ordering whenorderingKeynot present)orderingKey(has priority overkeyfor ordering)
Instead of introduction only one key we MUST add them all
binding is internal representation of message properties(like config dump). So if we started adding keys, lets add them all also as provide correct documentation
All keys are of string type. Allow pass general JSON Schema without restrictions is bad idea which leads to incorrect representation
There was a problem hiding this comment.
Ok, I'll fix the type and update it to include all. Before this, I tried to keep the change as minimal as possible.
There was a problem hiding this comment.
Ok, I'll fix the type and update it to include all. Before this, I tried to keep the change as minimal as possible.
Don't worry to propose big changes :)
There was a problem hiding this comment.
They don’t enumerate available config keys. We can start from here https://github.com/api-evangelist/apache-pulsar/blob/master/json-schema/apache-pulsar-pulsar-message-schema.json
I'll double check Java SDK also as try to dump messages from Pulsar to get how they are written inside broke
There was a problem hiding this comment.
I've added orderingKey and restricted both keys to type: string.
I didn't add a separate partitionKey, as it seems to be the same as key, i.e. AFAICT there are only 2 key fields (Javadoc).
Currently I kept Schema Object rather than a strict string so the key's values can be described (e.g. format: uuid), similar to the Kafka binding's key. Is it ok like this or do you prefer the stricter plain string form?
There was a problem hiding this comment.
I've changed them to Type String and update an example to match yours.
Will you please re-review?
FYI, inside the broker they use protobuf with a partition_key field (and an ordering key): https://github.com/apache/pulsar/blob/master/pulsar-common/src/main/proto/PulsarApi.proto#L107
In the API they use key https://github.com/apache/pulsar/blob/master/pulsar-client-api/src/main/java/org/apache/pulsar/client/api/Message.java#L146 but it returns that partition_key field: https://github.com/apache/pulsar/blob/master/pulsar-client/src/main/java/org/apache/pulsar/client/impl/MessageImpl.java#L702
Add a Pulsar Message Binding Object with a `key` field, used for Key_Shared ordering.
Add `orderingKey` and restrict key and orderingKey to string schemas.
97cc994 to
be308d0
Compare
Use Type String for the fields, as Type defines a restriction on the field content, not on the field's form.
Add a Pulsar Message Binding Object with a
keyfield, used for Key_Shared ordering.Description
Adds a Message Binding Object to the Pulsar binding with an optional
keyfield, similar to the Kafka message binding.key(Schema Object | Reference Object) to the message binding. Pulsar uses the message key to guarantee ordering for consumers on a Key_Shared subscription. Documenting it lets producers and consumers agree on how messages are partially ordered.0.1.0to0.2.0.Related issue(s)
Resolves #289