codelipenghui commented on code in PR #17326:
URL: https://github.com/apache/pulsar/pull/17326#discussion_r956952691
##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java:
##########
@@ -1559,6 +1561,14 @@ void
receiveIndividualMessagesFromBatch(BrokerEntryMetadata brokerEntryMetadata,
}
if (possibleToDeadLetter != null) {
possibleToDeadLetter.add(message);
+ // when redeliveryCount >
deadLetterPolicy.getMaxRedeliverCount())
+ // the message will not be visible to users
Review Comment:
// Skip the message which reaches the max redelivery count.
##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java:
##########
@@ -1392,6 +1392,8 @@ void messageReceived(CommandMessage cmdMessage, ByteBuf
headersAndPayload, Clien
Collections.singletonList(message));
if (redeliveryCount >
deadLetterPolicy.getMaxRedeliverCount()) {
redeliverUnacknowledgedMessages(Collections.singleton(message.getMessageId()));
+ // if send to DeadLetter we need to increase the
available permits
Review Comment:
```suggestion
// The message is skipped due to reaching the max
redelivery count, so we need to increase the available permits
```
##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java:
##########
@@ -1559,6 +1561,14 @@ void
receiveIndividualMessagesFromBatch(BrokerEntryMetadata brokerEntryMetadata,
}
if (possibleToDeadLetter != null) {
possibleToDeadLetter.add(message);
+ // when redeliveryCount >
deadLetterPolicy.getMaxRedeliverCount())
+ // the message will not be visible to users
+ if (redeliveryCount >
deadLetterPolicy.getMaxRedeliverCount()) {
+ // if send to DeadLetter we need to increase the
available permits
Review Comment:
We can remove this line, the comment is most likely duplicated with 1564:1565
##########
pulsar-broker/src/test/java/org/apache/pulsar/client/impl/TransactionEndToEndTest.java:
##########
@@ -1223,6 +1223,8 @@ public void testSendTxnAckMessageToDLQ() throws Exception
{
// the message will be sent to DLQ, can't receive
assertNull(consumer.receive(3, TimeUnit.SECONDS));
+ assertEquals(3, ((ConsumerImpl<?>) consumer).getAvailablePermits());
Review Comment:
```suggestion
assertEquals(((ConsumerImpl<?>) consumer).getAvailablePermits(), 3);
```
The second param is the expected value. We will get counterintuitive
information if the test fails here.
--
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]
