MarvinCai commented on pull request #9163:
URL: https://github.com/apache/pulsar/pull/9163#issuecomment-758442219
> @MarvinCai We also need to add verification in the ConsumerBuilder. With
the ConsumerBuilder, users should get a PulsarClient exception if they want to
enable DLQ on a Key_Shared subscription.
Added check in ConsumerBuilder to throw InvalidConfigurationException if set
DeadLetterTopic name for KeyShared subType.
> The same logic exists in the void messageReceived method. Is it better for
us to prohibit setting this kind of policy in the builder ?
>
> ```
> if (deadLetterPolicy != null && possibleSendToDeadLetterTopicMessages !=
null && redeliveryCount >= deadLetterPolicy.getMaxRedeliverCount()) {
>
possibleSendToDeadLetterTopicMessages.put((MessageIdImpl)message.getMessageId(),
Collections.singletonList(message));
> }
> ```
>
> There is retry logic in `deadLetterPolicy`. If the number of retries is
exceeded, how to deal with these messages? But the `retryLetterProducer` in
`consumerImpl` doesn't seem to do any processing.
For first point, there's a check `possibleSendToDeadLetterTopicMessages !=
null` and I've make possibleSendToDeadLetterTopicMessages = null if it's
KeyShared subType which is the behavior if DeadLetterPolicy is not provided.
(It's safe as we are already doing null check every time we want to use
possibleSendToDeadLetterTopicMessages)
For second point, actually we need both check. The check in ConsumerBuilder
will prevent user from setting DeadLetterTopic.
However, even if we don't set DeadLetterTopic(e in DeadLetterPolicy, in
ConsumerImpl wether to put message in DLQ is currently decided by `if
reconsumetimes > this.deadLetterPolicy.getMaxRedeliverCount()` so additional
check will still be needed here as we want retry but not DLQ for Key_Shared
subType.
Either by checking if subType == Key_Shared or if
StringUtils.isBlank(deadLetterPolicy.getDeadLetterTopic()). I just pick the
former cause it make our purpose more clear.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
