AnonHxy commented on code in PR #16605:
URL: https://github.com/apache/pulsar/pull/16605#discussion_r931026850
##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/BatchMessageContainerImpl.java:
##########
@@ -197,6 +197,26 @@ public boolean isMultiBatches() {
@Override
public OpSendMsg createOpSendMsg() throws IOException {
+ if (messages.size() == 1) {
+ MessageImpl<?> msg = messages.get(0);
+ messageMetadata = msg.getMessageBuilder();
+ ByteBuf encryptedPayload = producer.encryptMessage(
+ messageMetadata,
+ producer.applyCompression(msg.getDataBuffer()));
+ if (encryptedPayload.readableBytes() >
ClientCnx.getMaxMessageSize()) {
+ discard(new PulsarClientException.InvalidMessageException(
+ "Message size is bigger than " +
ClientCnx.getMaxMessageSize() + " bytes"));
+ return null;
+ }
+ ByteBufPair cmd = producer.sendMessage(producer.producerId,
messageMetadata.getSequenceId(),
+ 1, messageMetadata, encryptedPayload);
+ final OpSendMsg op;
+ op = OpSendMsg.create(msg, cmd, messageMetadata.getSequenceId(),
firstCallback);
+ op.setNumMessagesInBatch(1);
Review Comment:
Same as `OpSendMsg#batchSizeByte` below. But the because the
`numMessagesInBatch` default value is `1`, it seems that it doesn't matter if
we set it or not.
##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/BatchMessageContainerImpl.java:
##########
@@ -197,6 +197,26 @@ public boolean isMultiBatches() {
@Override
public OpSendMsg createOpSendMsg() throws IOException {
+ if (messages.size() == 1) {
+ MessageImpl<?> msg = messages.get(0);
+ messageMetadata = msg.getMessageBuilder();
+ ByteBuf encryptedPayload = producer.encryptMessage(
+ messageMetadata,
+ producer.applyCompression(msg.getDataBuffer()));
+ if (encryptedPayload.readableBytes() >
ClientCnx.getMaxMessageSize()) {
+ discard(new PulsarClientException.InvalidMessageException(
+ "Message size is bigger than " +
ClientCnx.getMaxMessageSize() + " bytes"));
+ return null;
+ }
+ ByteBufPair cmd = producer.sendMessage(producer.producerId,
messageMetadata.getSequenceId(),
+ 1, messageMetadata, encryptedPayload);
+ final OpSendMsg op;
+ op = OpSendMsg.create(msg, cmd, messageMetadata.getSequenceId(),
firstCallback);
+ op.setNumMessagesInBatch(1);
+ op.setBatchSizeByte(encryptedPayload.readableBytes());
Review Comment:
`OpSendMsg#batchSizeByte` will not be serialized to he binnary cmd. It's
useful for the `ProducerStats`(line2156), as well as
`OpSendMsg#numMessagesInBatch`. So I think we should set it here. @eolivelli
[[ProducerStats](https://github.com/apache/pulsar/blob/5df15dd2edd7eeab309fea35828915c8698ea339/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java#L2129-L2157)](https://github.com/apache/pulsar/blob/5df15dd2edd7eeab309fea35828915c8698ea339/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java#L2129-L2157)
##########
pulsar-client-tools-test/src/test/java/org/apache/pulsar/client/cli/PulsarClientToolTest.java:
##########
@@ -261,7 +261,6 @@ public void testDisableBatching() throws Exception {
Assert.assertNotNull(msg);
if (i < numberOfMessages) {
Assert.assertEquals(new String(msg.getData()), "batched");
- Assert.assertTrue(msg.getMessageId() instanceof
BatchMessageIdImpl);
Review Comment:
OK, I will rework this test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
