MarvinCai commented on a change in pull request #4265: 
[transaction][acknowledge] Introduce PENDING_ACK state in acknowledgement path
URL: https://github.com/apache/pulsar/pull/4265#discussion_r287212825
 
 

 ##########
 File path: 
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentSubscription.java
 ##########
 @@ -76,13 +90,38 @@
     // for connected subscriptions, message expiry will be checked if the 
backlog is greater than this threshold
     private static final int MINIMUM_BACKLOG_FOR_EXPIRY_CHECK = 1000;
 
+    // Map to keep track of message ack by each txn.
+    private final ConcurrentOpenHashMap<TxnID, 
ConcurrentOpenHashSet<Position>> pendingAckMessagesMap;
+
+    // Messages acked by ongoing transaction, pending transaction commit to 
materialize the acks. For faster look up.
+    // Using hashset as a message should only be acked once by one transaction.
+    private final ConcurrentOpenHashSet<Position> pendingAckMessages;
 
 Review comment:
   I actually used another ConcurrentOpenHashSet to keep all the unacked 
messages(Position) and it's just for conflict detection. 
   I'm not sure if there'll be performance/space benefit for using one reverse 
map vs a map + a set. Or maybe you think using one reverse map is cleaner?

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to