[GitHub] [pulsar] eolivelli commented on a diff in pull request #17163: [fix][broker] Avoid messages being repeatedly replayed with SHARED subscriptions

[GitBox <gi...@apache.org>]

eolivelli commented on code in PR #17163:
URL: https://github.com/apache/pulsar/pull/17163#discussion_r949084305


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java:
##########
@@ -653,9 +655,12 @@ protected synchronized boolean trySendMessagesToConsumers(ReadType readType, Lis
 
             // remove positions first from replay list first : sendMessages recycles entries
             if (readType == ReadType.Replay) {
-                entriesForThisConsumer.forEach(entry -> {
-                    redeliveryMessages.remove(entry.getLedgerId(), entry.getEntryId());
-                });
+                // we need to replay messages still present on the redeliveryMessages queue.
+                // It may happen that more than one replay is scheduled for the same message but it
+                // must be actually replayed only once.
+                entriesForThisConsumer = entriesForThisConsumer.stream()

Review Comment:
   This may be a symptom that something else is broken.
   We have read messages that should not have been read.
   
   I wonder if we are replaying the messages in the wrong order.
   may we can accept that because the order is here is not a matter as we are replaying messages
   
   we should also double check the PersistentStickyKeyDispatcherMultipleConsumers, that has a different implementation of  `trySendMessagesToConsumers`



-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org