thetumbled commented on code in PR #23600:
URL: https://github.com/apache/pulsar/pull/23600#discussion_r1849826926


##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/NegativeAcksTracker.java:
##########
@@ -108,39 +134,56 @@ public synchronized void add(Message<?> message) {
         add(message.getMessageId(), message.getRedeliveryCount());
     }
 
+    static long trimLowerBit(long timestamp, int bits) {
+        return timestamp & (-1L << bits);
+    }
+
     private synchronized void add(MessageId messageId, int redeliveryCount) {
         if (nackedMessages == null) {
-            nackedMessages = ConcurrentLongLongPairHashMap.newBuilder()
-                    .autoShrink(true)
-                    .concurrencyLevel(1)
-                    .build();
+            nackedMessages = new Long2ObjectAVLTreeMap<>();
         }
 
-        long backoffNs;
+        long backoffMs;

Review Comment:
   - Nano is not convenient for calculating, as we need to estimate the 
accuracy of timestamp. 
   For example, with `negativeAckPrecisionBitCnt = 10`, we know that the 
redelivery time may be earlier at most 2^10=1024ms~=1s. We just trim the lower 
8 bit to bucket the time. 
   But with nano(`1ms=1000000ns`).  we can't get a suitable conf value easily.
   - It is unnecessary to use timestamp in `ns` unit. As the tick time of the 
timer is 1ms, and the latency of message delivery is at `ms` level.



-- 
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]

Reply via email to