poorbarcode commented on code in PR #20179:
URL: https://github.com/apache/pulsar/pull/20179#discussion_r1208606556


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentStickyKeyDispatcherMultipleConsumers.java:
##########
@@ -65,23 +67,22 @@ public class PersistentStickyKeyDispatcherMultipleConsumers 
extends PersistentDi
     private final KeySharedMode keySharedMode;
 
     /**
-     * When a consumer joins, it will be added to this map with the current 
read position.
-     * This means that, in order to preserve ordering, new consumers can only 
receive old
-     * messages, until the mark-delete position will move past this point.
+     * When a consumer joins, it will be added to this map with the current 
last sent position per the message key.
+     * This means that, in order to preserve ordering per the message key, new 
consumers can only receive old
+     * messages, until the mark-delete position will move past this point in 
the key. New consumers can receive
+     * any new messages with the message key that is not in the last sent 
position.
      */
-    private final LinkedHashMap<Consumer, PositionImpl> 
recentlyJoinedConsumers;
-
-    private final Set<Consumer> stuckConsumers;
-    private final Set<Consumer> nextStuckConsumers;
+    private final LinkedHashMap<Consumer, LastSentPositions> 
recentlyJoinedConsumers;
+    // The lastSentPosition is not thread-safe
+    private final LastSentPositions lastSentPositions;
 
     PersistentStickyKeyDispatcherMultipleConsumers(PersistentTopic topic, 
ManagedCursor cursor,

Review Comment:
   @equanz Sorry, my reply has been delayed again
   
   Let me summarize this scenario:
   1. a message(`Msg_1`) was put into the Replay Queue caused by the target 
consumer(`C_1`) having no permits.
   2. at this moment, the new Consumer(`C_2`) takes over the key of `Msg_1` 
above.
   3. in the next round, `Msg_1` was delivered to `C_2`.
   
   Is this explain correct?<sup>[1]</sup>
   
   ---
   
   If yes<sup>[1]</sup>, I have some question: If we revert the PR 
https://github.com/apache/pulsar/pull/20026, the `step 1` and `step 2`  can not 
be executed concurrently, right?<sup>[2]</sup>
   
   ---
   
   If yes <sup>[2]</sup>, there have 2 case:
   #### Case 1: `step 1` runs before `step 2`
   In this case, the `max read position` of `C_2` should be larger than 
`Msg-1`, then `Msg_1` will not be sent to `C_2` in the next round.
   
   #### Case 1: `step 2` runs before `step 1`
   In this case, the key of `Msg_1` above will still be taken over by `C_1`, 
then `Msg_1` will not be sent to `C_2` in the next round.



-- 
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-unsubscr...@pulsar.apache.org

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

Reply via email to