codelipenghui commented on code in PR #17318:
URL: https://github.com/apache/pulsar/pull/17318#discussion_r990615227
##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/UnAckedMessageRedeliveryTracker.java:
##########
@@ -71,9 +72,13 @@ public void run(Timeout t) throws Exception {
}
headPartition.clear();
redeliveryTimePartitions.addLast(headPartition);
- triggerRedelivery(consumerBase);
+ messageIds = getRedeliveryMessages(consumerBase);
} finally {
writeLock.unlock();
+ if (messageIds != null && !messageIds.isEmpty()) {
+ consumerBase.onAckTimeoutSend(messageIds);
+
consumerBase.redeliverUnacknowledgedMessages(messageIds);
Review Comment:
@shibd Could you please provide more details of the deadlock?
##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerBase.java:
##########
@@ -839,7 +839,7 @@ protected boolean canEnqueueMessage(Message<T> message) {
protected boolean enqueueMessageAndCheckBatchReceive(Message<T> message) {
int messageSize = message.size();
- if (canEnqueueMessage(message) && incomingMessages.offer(message)) {
+ if (isValidConsumerEpoch(message) && canEnqueueMessage(message) &&
incomingMessages.offer(message)) {
Review Comment:
And do we need to increase the available permits? Otherwise, the broker
might stop dispatching messages to the consumer due to the consumer doesn't
have available permits (the messages are skipped)
##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/UnAckedMessageRedeliveryTracker.java:
##########
@@ -71,9 +72,13 @@ public void run(Timeout t) throws Exception {
}
headPartition.clear();
redeliveryTimePartitions.addLast(headPartition);
- triggerRedelivery(consumerBase);
+ messageIds = getRedeliveryMessages(consumerBase);
} finally {
writeLock.unlock();
+ if (messageIds != null && !messageIds.isEmpty()) {
+ consumerBase.onAckTimeoutSend(messageIds);
+
consumerBase.redeliverUnacknowledgedMessages(messageIds);
Review Comment:
Oh, I think you mean you need to change triggerRedelivery ->
getRedeliveryMessages because of the deadlock issue right?
##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerBase.java:
##########
@@ -839,7 +839,7 @@ protected boolean canEnqueueMessage(Message<T> message) {
protected boolean enqueueMessageAndCheckBatchReceive(Message<T> message) {
int messageSize = message.size();
- if (canEnqueueMessage(message) && incomingMessages.offer(message)) {
+ if (isValidConsumerEpoch(message) && canEnqueueMessage(message) &&
incomingMessages.offer(message)) {
Review Comment:
If the consumer has pending receive requests. The message will be used to
complete the pending receive requests directly.
```java
internalPinnedExecutor.execute(() -> {
if (hasNextPendingReceive()) {
notifyPendingReceivedCallback(message, null);
} else if (enqueueMessageAndCheckBatchReceive(message) &&
hasPendingBatchReceive()) {
notifyPendingBatchReceivedCallBack();
}
});
```
In this case, we are not able to filter out the messages that with
unexpected epoch?
--
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]
