TakaHiR07 commented on code in PR #25076:
URL: https://github.com/apache/pulsar/pull/25076#discussion_r2622629862
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTracker.java:
##########
@@ -126,10 +126,13 @@ public boolean addMessage(long ledgerId, long entryId,
long deliverAt) {
}
long timestamp = trimLowerBit(deliverAt, timestampPrecisionBitCnt);
- delayedMessageMap.computeIfAbsent(timestamp, k -> new
Long2ObjectRBTreeMap<>())
- .computeIfAbsent(ledgerId, k -> new Roaring64Bitmap())
- .add(entryId);
- delayedMessagesCount.incrementAndGet();
+ Roaring64Bitmap roaring64Bitmap = delayedMessageMap
+ .computeIfAbsent(timestamp, k -> new Long2ObjectRBTreeMap<>())
+ .computeIfAbsent(ledgerId, k -> new Roaring64Bitmap());
+ if (!roaring64Bitmap.contains(entryId)) {
+ roaring64Bitmap.add(entryId);
Review Comment:
Yes, unfortunately.
"add" or "addLong" I guess is the same after compile, sure it may be better
to use "addLong".
Besides, I think there is no need to consider concurrent situation in
InMemoryDelayedDeliveryTracker. I don't see any code point out that concurrent
situation would occur.
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTracker.java:
##########
@@ -126,10 +126,13 @@ public boolean addMessage(long ledgerId, long entryId,
long deliverAt) {
}
long timestamp = trimLowerBit(deliverAt, timestampPrecisionBitCnt);
- delayedMessageMap.computeIfAbsent(timestamp, k -> new
Long2ObjectRBTreeMap<>())
- .computeIfAbsent(ledgerId, k -> new Roaring64Bitmap())
- .add(entryId);
- delayedMessagesCount.incrementAndGet();
+ Roaring64Bitmap roaring64Bitmap = delayedMessageMap
+ .computeIfAbsent(timestamp, k -> new Long2ObjectRBTreeMap<>())
+ .computeIfAbsent(ledgerId, k -> new Roaring64Bitmap());
+ if (!roaring64Bitmap.contains(entryId)) {
+ roaring64Bitmap.add(entryId);
Review Comment:
Yes, unfortunately.
"add" or "addLong" I guess is the same after compile, sure it may be better
to use "addLong" directly.
Besides, I think there is no need to consider concurrent situation in
InMemoryDelayedDeliveryTracker. I don't see any code point out that concurrent
situation would occur.
--
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]
