apache / pulsar

Apache Pulsar - distributed pub-sub messaging system
https://pulsar.apache.org/
Apache License 2.0
14.12k stars 3.57k forks source link

[feat] Replace the default NONE_KEY in Key_Shared implementation with producer name and producer sequence number #23212

Open lhotari opened 3 weeks ago

lhotari commented 3 weeks ago

Search before asking

Motivation

The Key_Shared implementation expects that the incoming message contain either a key/keyBytes or orderingKey. If the key isn't set, the implementation will use the bytes of the NONE_KEY string as the key. This could be a surprise to users since the implications of not setting the key is not documented.

Another related detail is that there doesn't seem to be documentation (other than TypedMessageBuilder javadoc) for orderingKey and key/keyBytes and how orderingKey is preferred over the key/keyBytes for key_shared subscriptions. There's no documentation for the purpose of 2 separate keys.

Broker side code: https://github.com/apache/pulsar/blob/6236116754472c61b2166da6d4797fc63c83f364/pulsar-common/src/main/java/org/apache/pulsar/common/protocol/Commands.java#L1969-L1987 https://github.com/apache/pulsar/blob/82237d3684fe506bcb6426b3b23f413422e6e4fb/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/EntryAndMetadata.java#L52-L61

Client code: https://github.com/apache/pulsar/blob/10f4e0248f0f985b1dc7ad38970c906b7fe629be/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerBase.java#L1186-L1196

The problem that this causes is that all messages without a key will handled by a single consumer. If any of these messages is in redelivery, it will cause all message delivery to be blocked.

Solution

Replace the default NONE_KEY with a key that is derived from the producer name and producer sequence number.

Alternatives

Document the current behavior and requirement in https://pulsar.apache.org/docs/next/concepts-messaging/#key_shared.

Anything else?

No response

Are you willing to submit a PR?

lhotari commented 2 weeks ago

mailing list discussion: https://lists.apache.org/thread/fyk0z2fsvv1fqp5dpdlzhqb7cd9qjr4j

phil-cd commented 2 weeks ago

Opened PR #23219 for this.