lhotari commented on code in PR #24430:
URL: https://github.com/apache/pulsar/pull/24430#discussion_r2156282609
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTracker.java:
##########
@@ -221,14 +230,13 @@ public NavigableSet<Position> getScheduledMessages(int
maxMessages) {
@Override
public CompletableFuture<Void> clear() {
this.delayedMessageMap.clear();
+ this.delayedMessagesCount.set(0);
return CompletableFuture.completedFuture(null);
}
@Override
public long getNumberOfDelayedMessages() {
- return delayedMessageMap.values().stream().mapToLong(
- ledgerMap -> ledgerMap.values().stream().mapToLong(
- Roaring64Bitmap::getLongCardinality).sum()).sum();
+ return delayedMessagesCount.get();
Review Comment:
Do we already have sufficient test coverage to ensure that the counter
provides an accurate value in all cases?
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTracker.java:
##########
@@ -221,14 +230,13 @@ public NavigableSet<Position> getScheduledMessages(int
maxMessages) {
@Override
public CompletableFuture<Void> clear() {
this.delayedMessageMap.clear();
+ this.delayedMessagesCount.set(0);
Review Comment:
On line 220 there's `if (delayedMessageMap.isEmpty()) {`. Would it make
sense to add `this.delayedMessagesCount.set(0);` also to that code block?
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTracker.java:
##########
@@ -125,6 +130,8 @@ public boolean addMessage(long ledgerId, long entryId, long
deliverAt) {
delayedMessageMap.computeIfAbsent(timestamp, k -> new
Long2ObjectRBTreeMap<>())
.computeIfAbsent(ledgerId, k -> new Roaring64Bitmap())
.add(entryId);
+ delayedMessagesCount.incrementAndGet();
Review Comment:
What if the same entryId is added multiple times with the same timestamp?
Would the counter get out of sync in that case? Would it be possible to add
test cases?
--
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]
