-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PIP-189: No batching if only one message in batch #16619
Comments
I think that we should not add a configuration flag. There is no reason to turn off this feature |
@AnonHxy did you see my comment ? |
Yes. I saw the comment. And I had replied from the email. Maybe I should attatch the link to this issue. :) I agree with your point. And I have updated the PR to remove the configuration flag. But there is still some flaky-test failure and I am working on it. Thanks @eolivelli |
…16605) [improve][client]PIP-189: No batching if only one message in batch #16605 ### Motivation * See #16619 ### Modifications * See #16619 * Most of the Modifications are relevant to `BatchMessageContainerImpl` * Of course there are some tests about batching need to be modified, because batched producer can also pubulish non-batched messages when this PIP applies. The tests include: * `RGUsageMTAggrWaitForAllMsgsTest` * `BatchMessageTest` * `BrokerEntryMetadataE2ETest` * `ClientDeduplicationTest` * `TopicReaderTest` * `PulsarClientToolTest`
…pache#16605) [improve][client]PIP-189: No batching if only one message in batch apache#16605 ### Motivation * See apache#16619 ### Modifications * See apache#16619 * Most of the Modifications are relevant to `BatchMessageContainerImpl` * Of course there are some tests about batching need to be modified, because batched producer can also pubulish non-batched messages when this PIP applies. The tests include: * `RGUsageMTAggrWaitForAllMsgsTest` * `BatchMessageTest` * `BrokerEntryMetadataE2ETest` * `ClientDeduplicationTest` * `TopicReaderTest` * `PulsarClientToolTest`
…pache#16605) [improve][client]PIP-189: No batching if only one message in batch apache#16605 ### Motivation * See apache#16619 ### Modifications * See apache#16619 * Most of the Modifications are relevant to `BatchMessageContainerImpl` * Of course there are some tests about batching need to be modified, because batched producer can also pubulish non-batched messages when this PIP applies. The tests include: * `RGUsageMTAggrWaitForAllMsgsTest` * `BatchMessageTest` * `BrokerEntryMetadataE2ETest` * `ClientDeduplicationTest` * `TopicReaderTest` * `PulsarClientToolTest`
discuss mail-thread: https://lists.apache.org/thread/dbq1lrv03bhtk0lr5nwm5txo9ndjplv0
Motivation
The original discussion mail :
https://lists.apache.org/thread/5svpl5qp3bfoztf5fvtojh51zbklcht2
linked issue: #16547
Introduce the ability for producers to publish a non-batched message if there is only one message in the batch.
It is useful to save the
SingleMessageMetadata
space in entry and reduce workload of consumers to deserialize theSingleMessageMetadata
, especially when sometimes there is an amount of batched messages with only one real message.API Changes
N/A
Implementation
For
BatchMessageContainerImpl
:For
BatchMessageKeyBasedContainer
, there is no need to change, because it usesBatchMessageContainerImpl
to createOpSendMsg
Reject Alternatives
Always return
BatchMessageIdImpl
whenenableBatching
is set as true, even if publish single message with a non-batched message.Rejection reason: Consumer have to deserialize to check if there is
SingleMessageMetadata
from the payloadAdd a configuration for the producer to enable or disable this feature
Rejection reason: It is always a good idea to not create a batch message. So there is no reason to turn off this feature
The text was updated successfully, but these errors were encountered: