AnonHxy commented on code in PR #16605:
URL: https://github.com/apache/pulsar/pull/16605#discussion_r933246604
##########
pulsar-broker/src/test/java/org/apache/pulsar/client/api/TopicReaderTest.java:
##########
@@ -1096,7 +1096,6 @@ public void testHasMessageAvailableWithBatch() throws
Exception {
ReaderImpl<byte[]> reader =
(ReaderImpl<byte[]>)pulsarClient.newReader().topic(topicName)
.startMessageId(messageId).startMessageIdInclusive().create();
MessageIdImpl lastMsgId = (MessageIdImpl)
reader.getConsumer().getLastMessageId();
- assertTrue(messageId instanceof BatchMessageIdImpl);
Review Comment:
> we must rework the test in a way that we are producing batch messages
I think that we should just producing non-batch messages here. Although the
method name is `testHasMessageAvailableWithBatch`, from the above
comment(Line1093), I think this code block shoud test publishing single mesage
with batch enable~
##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java:
##########
@@ -425,7 +425,7 @@ CompletableFuture<MessageId>
internalSendWithTxnAsync(Message<?> message, Transa
* @param payload
* @return a new payload
*/
- private ByteBuf applyCompression(ByteBuf payload) {
+ protected ByteBuf applyCompression(ByteBuf payload) {
Review Comment:
It will not make the code look better now I think.
Because publishing one message in batch and publish with batch disable will
call different method.
As the new pushed commit `Fix out of order problem` in this PR, we should
call `OpSendMsg#create(List<MessageImpl<?>>, ByteBufPair, long, SendCallback)`
to create OpSendMsg, instead of using `OpSendMsg#create(MessageImpl<?>,
ByteBufPair, long, SendCallback)`.
--
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]
