From fc876b729e3689dea4c293cdb3127f39d6ebdc00 Mon Sep 17 00:00:00 2001 From: Denovo1998 Date: Thu, 30 Oct 2025 20:20:22 +0800 Subject: [PATCH 01/16] [feat][broker] Implement topic-level delayed delivery tracking with in-memory manager --- ...InMemoryDelayedDeliveryTrackerFactory.java | 30 +- ...oryTopicDelayedDeliveryTrackerManager.java | 473 ++++++++++++++++++ ...MemoryTopicDelayedDeliveryTrackerView.java | 120 +++++ .../TopicDelayedDeliveryTrackerManager.java | 74 +++ .../DelayedDeliveryTrackerFactoryTest.java | 6 +- 5 files changed, 696 insertions(+), 7 deletions(-) create mode 100644 pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java create mode 100644 pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerView.java create mode 100644 pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/TopicDelayedDeliveryTrackerManager.java diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTrackerFactory.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTrackerFactory.java index f8b8f5a8ba459..555c730465c69 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTrackerFactory.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTrackerFactory.java @@ -22,6 +22,8 @@ import io.netty.util.HashedWheelTimer; import io.netty.util.Timer; import io.netty.util.concurrent.DefaultThreadFactory; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; import java.util.concurrent.TimeUnit; import org.apache.pulsar.broker.PulsarService; import org.apache.pulsar.broker.ServiceConfiguration; @@ -40,6 +42,9 @@ public class InMemoryDelayedDeliveryTrackerFactory implements DelayedDeliveryTra private long fixedDelayDetectionLookahead; + // Cache of topic-level managers: topic name -> manager instance + private final ConcurrentMap topicManagers = new ConcurrentHashMap<>(); + @Override public void initialize(PulsarService pulsarService) { ServiceConfiguration config = pulsarService.getConfig(); @@ -54,7 +59,7 @@ public void initialize(PulsarService pulsarService) { public DelayedDeliveryTracker newTracker(AbstractPersistentDispatcherMultipleConsumers dispatcher) { String topicName = dispatcher.getTopic().getName(); String subscriptionName = dispatcher.getSubscription().getName(); - DelayedDeliveryTracker tracker = DelayedDeliveryTracker.DISABLE; + DelayedDeliveryTracker tracker = DelayedDeliveryTracker.DISABLE; try { tracker = newTracker0(dispatcher); } catch (Exception e) { @@ -66,13 +71,30 @@ public DelayedDeliveryTracker newTracker(AbstractPersistentDispatcherMultipleCon } @VisibleForTesting - InMemoryDelayedDeliveryTracker newTracker0(AbstractPersistentDispatcherMultipleConsumers dispatcher) { - return new InMemoryDelayedDeliveryTracker(dispatcher, timer, tickTimeMillis, - isDelayedDeliveryDeliverAtTimeStrict, fixedDelayDetectionLookahead); + DelayedDeliveryTracker newTracker0(AbstractPersistentDispatcherMultipleConsumers dispatcher) { + String topicName = dispatcher.getTopic().getName(); + + // Get or create topic-level manager for this topic + TopicDelayedDeliveryTrackerManager manager = topicManagers.computeIfAbsent(topicName, + k -> new InMemoryTopicDelayedDeliveryTrackerManager(timer, tickTimeMillis, + isDelayedDeliveryDeliverAtTimeStrict, fixedDelayDetectionLookahead)); + + // Create a per-subscription view from the topic-level manager + return manager.createOrGetView(dispatcher); } @Override public void close() { + // Close all topic-level managers + for (TopicDelayedDeliveryTrackerManager manager : topicManagers.values()) { + try { + manager.close(); + } catch (Exception e) { + log.warn("Failed to close topic-level delayed delivery manager", e); + } + } + topicManagers.clear(); + if (timer != null) { timer.stop(); } diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java new file mode 100644 index 0000000000000..fa0575f393502 --- /dev/null +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java @@ -0,0 +1,473 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pulsar.broker.delayed; + +import io.netty.util.Timeout; +import io.netty.util.Timer; +import io.netty.util.TimerTask; +import it.unimi.dsi.fastutil.longs.Long2ObjectRBTreeMap; +import it.unimi.dsi.fastutil.longs.Long2ObjectSortedMap; +import java.time.Clock; +import java.util.HashMap; +import java.util.Map; +import java.util.NavigableSet; +import java.util.TreeSet; +import java.util.concurrent.atomic.AtomicLong; +import lombok.Getter; +import lombok.extern.slf4j.Slf4j; +import org.apache.bookkeeper.mledger.Position; +import org.apache.bookkeeper.mledger.PositionFactory; +import org.apache.pulsar.broker.service.persistent.AbstractPersistentDispatcherMultipleConsumers; +import org.roaringbitmap.longlong.Roaring64Bitmap; + +/** + * In-memory implementation of topic-level delayed delivery tracker manager. + * This manager maintains a single global delayed message index per topic that is shared by all + * subscriptions, significantly reducing memory usage in multi-subscription scenarios. + */ +@Slf4j +public class InMemoryTopicDelayedDeliveryTrackerManager implements TopicDelayedDeliveryTrackerManager, TimerTask { + + // Global delayed message index: timestamp -> ledgerId -> entryId bitmap + private final Long2ObjectSortedMap> delayedMessageMap = + new Long2ObjectRBTreeMap<>(); + + // Subscription registry: subscription name -> subscription context + private final Map subscriptionContexts = new HashMap<>(); + + // Timer for delayed delivery + private final Timer timer; + private Timeout timeout; + private long currentTimeoutTarget = -1; + + // Configuration + private final long tickTimeMillis; + private final boolean isDelayedDeliveryDeliverAtTimeStrict; + private final long fixedDelayDetectionLookahead; + private final Clock clock; + + // Statistics + private final AtomicLong delayedMessagesCount = new AtomicLong(0); + private final AtomicLong bufferMemoryBytes = new AtomicLong(0); + + // Timestamp precision for memory optimization + private final int timestampPrecisionBitCnt; + + /** + * Subscription context that holds per-subscription state. + */ + @Getter + static class SubContext { + private final AbstractPersistentDispatcherMultipleConsumers dispatcher; + private final String subscriptionName; + private final long tickTimeMillis; + private final boolean isDelayedDeliveryDeliverAtTimeStrict; + private final long fixedDelayDetectionLookahead; + private Position markDeletePosition; + + SubContext(AbstractPersistentDispatcherMultipleConsumers dispatcher, long tickTimeMillis, + boolean isDelayedDeliveryDeliverAtTimeStrict, long fixedDelayDetectionLookahead) { + this.dispatcher = dispatcher; + this.subscriptionName = dispatcher.getSubscription().getName(); + this.tickTimeMillis = tickTimeMillis; + this.isDelayedDeliveryDeliverAtTimeStrict = isDelayedDeliveryDeliverAtTimeStrict; + this.fixedDelayDetectionLookahead = fixedDelayDetectionLookahead; + } + + void updateMarkDeletePosition(Position position) { + this.markDeletePosition = position; + } + + long getCutoffTime() { + return isDelayedDeliveryDeliverAtTimeStrict ? System.currentTimeMillis() : + System.currentTimeMillis() + tickTimeMillis; + } + } + + public InMemoryTopicDelayedDeliveryTrackerManager(Timer timer, long tickTimeMillis, + boolean isDelayedDeliveryDeliverAtTimeStrict, + long fixedDelayDetectionLookahead) { + this(timer, tickTimeMillis, Clock.systemUTC(), isDelayedDeliveryDeliverAtTimeStrict, + fixedDelayDetectionLookahead); + } + + public InMemoryTopicDelayedDeliveryTrackerManager(Timer timer, long tickTimeMillis, Clock clock, + boolean isDelayedDeliveryDeliverAtTimeStrict, + long fixedDelayDetectionLookahead) { + this.timer = timer; + this.tickTimeMillis = tickTimeMillis; + this.clock = clock; + this.isDelayedDeliveryDeliverAtTimeStrict = isDelayedDeliveryDeliverAtTimeStrict; + this.fixedDelayDetectionLookahead = fixedDelayDetectionLookahead; + this.timestampPrecisionBitCnt = calculateTimestampPrecisionBitCnt(tickTimeMillis); + } + + private static int calculateTimestampPrecisionBitCnt(long tickTimeMillis) { + int bitCnt = 0; + while (tickTimeMillis > 0) { + tickTimeMillis >>= 1; + bitCnt++; + } + return bitCnt > 0 ? bitCnt - 1 : 0; + } + + private static long trimLowerBit(long timestamp, int bits) { + return timestamp & (-1L << bits); + } + + @Override + public DelayedDeliveryTracker createOrGetView(AbstractPersistentDispatcherMultipleConsumers dispatcher) { + String subscriptionName = dispatcher.getSubscription().getName(); + + synchronized (this) { + SubContext subContext = subscriptionContexts.computeIfAbsent(subscriptionName, + k -> new SubContext(dispatcher, tickTimeMillis, isDelayedDeliveryDeliverAtTimeStrict, + fixedDelayDetectionLookahead)); + + return new InMemoryTopicDelayedDeliveryTrackerView(this, subContext); + } + } + + @Override + public void unregister(AbstractPersistentDispatcherMultipleConsumers dispatcher) { + String subscriptionName = dispatcher.getSubscription().getName(); + + synchronized (this) { + subscriptionContexts.remove(subscriptionName); + + // If no more subscriptions, close the manager + if (subscriptionContexts.isEmpty() && delayedMessageMap.isEmpty()) { + close(); + } + } + } + + @Override + public void onTickTimeUpdated(long newTickTimeMillis) { + // For now, tick time updates are not supported after initialization + // This could be enhanced to update all subscription contexts + log.warn("Tick time updates are not currently supported for topic-level delayed delivery managers"); + } + + @Override + public long topicBufferMemoryBytes() { + return bufferMemoryBytes.get(); + } + + @Override + public long topicDelayedMessages() { + return delayedMessagesCount.get(); + } + + @Override + public void close() { + synchronized (this) { + if (timeout != null) { + timeout.cancel(); + timeout = null; + } + delayedMessageMap.clear(); + subscriptionContexts.clear(); + delayedMessagesCount.set(0); + bufferMemoryBytes.set(0); + } + } + + // Internal methods for subscription views + + /** + * Add a message to the global delayed message index. + */ + boolean addMessageForSub(SubContext subContext, long ledgerId, long entryId, long deliverAt) { + synchronized (this) { + if (deliverAt < 0 || deliverAt <= subContext.getCutoffTime()) { + return false; + } + + long timestamp = trimLowerBit(deliverAt, timestampPrecisionBitCnt); + Long2ObjectSortedMap ledgerMap = delayedMessageMap.computeIfAbsent( + timestamp, k -> new Long2ObjectRBTreeMap<>()); + Roaring64Bitmap entryIds = ledgerMap.computeIfAbsent(ledgerId, k -> new Roaring64Bitmap()); + + // Check if this entry already exists (deduplication) + if (!entryIds.contains(entryId)) { + entryIds.add(entryId); + delayedMessagesCount.incrementAndGet(); + updateBufferMemoryEstimate(); + } + + updateTimer(); + return true; + } + } + + /** + * Check if there are messages available for a subscription. + */ + boolean hasMessageAvailableForSub(SubContext subContext) { + synchronized (this) { + if (delayedMessageMap.isEmpty()) { + return false; + } + + long cutoffTime = subContext.getCutoffTime(); + long firstTimestamp = delayedMessageMap.firstLongKey(); + + if (firstTimestamp > cutoffTime) { + return false; + } + + // Quick check: if there's any message in the earliest time bucket that's after mark delete + Long2ObjectSortedMap ledgerMap = delayedMessageMap.get(firstTimestamp); + if (ledgerMap != null) { + Position markDelete = subContext.getMarkDeletePosition(); + if (markDelete == null) { + return true; // No mark delete means all messages are available + } + + for (var entry : ledgerMap.long2ObjectEntrySet()) { + long ledgerId = entry.getLongKey(); + Roaring64Bitmap entryIds = entry.getValue(); + + if (ledgerId > markDelete.getLedgerId()) { + return true; + } else if (ledgerId == markDelete.getLedgerId()) { + // Check if there are any entry IDs after mark delete + if (entryIds.stream().anyMatch(entryId -> entryId > markDelete.getEntryId())) { + return true; + } + } + } + } + + return false; + } + } + + /** + * Get scheduled messages for a subscription. + */ + NavigableSet getScheduledMessagesForSub(SubContext subContext, int maxMessages) { + synchronized (this) { + int remaining = maxMessages; + NavigableSet positions = new TreeSet<>(); + long cutoffTime = subContext.getCutoffTime(); + Position markDelete = subContext.getMarkDeletePosition(); + + // Iterate through time buckets + var iterator = delayedMessageMap.long2ObjectEntrySet().iterator(); + while (iterator.hasNext() && remaining > 0) { + var timeEntry = iterator.next(); + long timestamp = timeEntry.getLongKey(); + + if (timestamp > cutoffTime) { + break; + } + + Long2ObjectSortedMap ledgerMap = timeEntry.getValue(); + + // Iterate through ledgers in this time bucket + var ledgerIterator = ledgerMap.long2ObjectEntrySet().iterator(); + while (ledgerIterator.hasNext() && remaining > 0) { + var ledgerEntry = ledgerIterator.next(); + long ledgerId = ledgerEntry.getLongKey(); + Roaring64Bitmap entryIds = ledgerEntry.getValue(); + + // Filter entries based on mark delete position + long[] entryIdArray = entryIds.toArray(); + for (long entryId : entryIdArray) { + if (remaining <= 0) { + break; + } + + // Skip entries that are before or at mark delete + if (markDelete != null) { + if (ledgerId < markDelete.getLedgerId()) { + continue; + } + if (ledgerId == markDelete.getLedgerId() && entryId <= markDelete.getEntryId()) { + continue; + } + } + + positions.add(PositionFactory.create(ledgerId, entryId)); + remaining--; + } + } + } + + // Note: We don't remove messages from the global index here + // Pruning will be handled separately based on min mark delete across all subscriptions + + return positions; + } + } + + /** + * Check if deliveries should be paused for a subscription. + */ + boolean shouldPauseAllDeliveriesForSub(SubContext subContext) { + // Simplified implementation - could be enhanced with fixed delay detection + return fixedDelayDetectionLookahead > 0 + && getNumberOfVisibleDelayedMessagesForSub(subContext) >= fixedDelayDetectionLookahead + && !hasMessageAvailableForSub(subContext); + } + + /** + * Clear delayed messages for a subscription (no-op for topic-level manager). + */ + void clearForSub() { + // No-op: we don't clear global index for individual subscriptions + } + + /** + * Update mark delete position for a subscription. + */ + void updateMarkDeletePosition(SubContext subContext, Position position) { + synchronized (this) { + subContext.updateMarkDeletePosition(position); + // Trigger pruning if needed + pruneByMinMarkDelete(); + } + } + + // Private helper methods + + private void updateTimer() { + if (delayedMessageMap.isEmpty()) { + if (timeout != null) { + currentTimeoutTarget = -1; + timeout.cancel(); + timeout = null; + } + return; + } + + long nextDeliveryTime = delayedMessageMap.firstLongKey(); + if (nextDeliveryTime == currentTimeoutTarget) { + return; + } + + if (timeout != null) { + timeout.cancel(); + } + + long now = clock.millis(); + long delayMillis = nextDeliveryTime - now; + + if (delayMillis < 0) { + // Messages are ready, but we don't need to keep retriggering + return; + } + + currentTimeoutTarget = nextDeliveryTime; + timeout = timer.newTimeout(this, delayMillis, java.util.concurrent.TimeUnit.MILLISECONDS); + } + + private void updateBufferMemoryEstimate() { + // Simplified memory estimation + long estimatedBytes = delayedMessageMap.values().stream() + .mapToLong(ledgerMap -> ledgerMap.values().stream() + .mapToLong(Roaring64Bitmap::getLongSizeInBytes).sum()) + .sum(); + bufferMemoryBytes.set(estimatedBytes); + } + + private long getNumberOfVisibleDelayedMessagesForSub(SubContext subContext) { + // Simplified implementation - returns total count + // Could be enhanced to count only messages visible to this subscription + return delayedMessagesCount.get(); + } + + private void pruneByMinMarkDelete() { + // Find the minimum mark delete position across all subscriptions + Position minMarkDelete = null; + for (SubContext subContext : subscriptionContexts.values()) { + Position markDelete = subContext.getMarkDeletePosition(); + if (markDelete != null) { + if (minMarkDelete == null || markDelete.compareTo(minMarkDelete) < 0) { + minMarkDelete = markDelete; + } + } + } + + if (minMarkDelete == null) { + return; + } + + // Prune entries that are before min mark delete + var iterator = delayedMessageMap.long2ObjectEntrySet().iterator(); + while (iterator.hasNext()) { + var timeEntry = iterator.next(); + Long2ObjectSortedMap ledgerMap = timeEntry.getValue(); + + var ledgerIterator = ledgerMap.long2ObjectEntrySet().iterator(); + while (ledgerIterator.hasNext()) { + var ledgerEntry = ledgerIterator.next(); + long ledgerId = ledgerEntry.getLongKey(); + Roaring64Bitmap entryIds = ledgerEntry.getValue(); + + if (ledgerId < minMarkDelete.getLedgerId()) { + // Entire ledger can be removed + delayedMessagesCount.addAndGet(-entryIds.getLongCardinality()); + ledgerIterator.remove(); + } else if (ledgerId == minMarkDelete.getLedgerId()) { + // Remove entries <= mark delete entry ID + long removedCount = 0; + var entryIterator = entryIds.iterator(); + while (entryIterator.hasNext()) { + long entryId = entryIterator.next(); + if (entryId <= minMarkDelete.getEntryId()) { + entryIterator.remove(); + removedCount++; + } + } + delayedMessagesCount.addAndGet(-removedCount); + + if (entryIds.isEmpty()) { + ledgerIterator.remove(); + } + } + } + + if (ledgerMap.isEmpty()) { + iterator.remove(); + } + } + + updateBufferMemoryEstimate(); + } + + @Override + public void run(Timeout timeout) throws Exception { + if (timeout == null || timeout.isCancelled()) { + return; + } + + synchronized (this) { + currentTimeoutTarget = -1; + this.timeout = null; + + // Trigger read more entries for all subscriptions + for (SubContext subContext : subscriptionContexts.values()) { + subContext.getDispatcher().readMoreEntriesAsync(); + } + } + } +} \ No newline at end of file diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerView.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerView.java new file mode 100644 index 0000000000000..c7af60d50924b --- /dev/null +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerView.java @@ -0,0 +1,120 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pulsar.broker.delayed; + +import java.util.NavigableSet; +import java.util.concurrent.CompletableFuture; +import lombok.extern.slf4j.Slf4j; +import org.apache.bookkeeper.mledger.Position; + +/** + * View object for a subscription that implements DelayedDeliveryTracker interface. + * This view forwards all operations to the topic-level manager while maintaining + * compatibility with existing dispatcher logic. + */ +@Slf4j +public class InMemoryTopicDelayedDeliveryTrackerView implements DelayedDeliveryTracker { + + private final InMemoryTopicDelayedDeliveryTrackerManager manager; + private final InMemoryTopicDelayedDeliveryTrackerManager.SubContext subContext; + private boolean closed = false; + + public InMemoryTopicDelayedDeliveryTrackerView(InMemoryTopicDelayedDeliveryTrackerManager manager, + InMemoryTopicDelayedDeliveryTrackerManager.SubContext subContext) { + this.manager = manager; + this.subContext = subContext; + } + + @Override + public boolean addMessage(long ledgerId, long entryId, long deliveryAt) { + checkClosed(); + return manager.addMessageForSub(subContext, ledgerId, entryId, deliveryAt); + } + + @Override + public boolean hasMessageAvailable() { + checkClosed(); + return manager.hasMessageAvailableForSub(subContext); + } + + @Override + public long getNumberOfDelayedMessages() { + checkClosed(); + // Return an estimate of visible delayed messages for this subscription + // For now, return the total count - could be enhanced to count only visible messages + return manager.topicDelayedMessages(); + } + + @Override + public long getBufferMemoryUsage() { + checkClosed(); + // Return the topic-level memory usage (shared by all subscriptions) + return manager.topicBufferMemoryBytes(); + } + + @Override + public NavigableSet getScheduledMessages(int maxMessages) { + checkClosed(); + return manager.getScheduledMessagesForSub(subContext, maxMessages); + } + + @Override + public boolean shouldPauseAllDeliveries() { + checkClosed(); + return manager.shouldPauseAllDeliveriesForSub(subContext); + } + + @Override + public void resetTickTime(long tickTime) { + checkClosed(); + manager.onTickTimeUpdated(tickTime); + } + + @Override + public CompletableFuture clear() { + checkClosed(); + // For topic-level manager, clear is a no-op for individual subscriptions + manager.clearForSub(); + return CompletableFuture.completedFuture(null); + } + + @Override + public void close() { + if (closed) { + return; + } + closed = true; + manager.unregister(subContext.getDispatcher()); + } + + /** + * Update the mark delete position for this subscription. + * This is called by the dispatcher when messages are acknowledged. + */ + public void updateMarkDeletePosition(Position position) { + checkClosed(); + manager.updateMarkDeletePosition(subContext, position); + } + + private void checkClosed() { + if (closed) { + throw new IllegalStateException("DelayedDeliveryTracker is already closed"); + } + } +} \ No newline at end of file diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/TopicDelayedDeliveryTrackerManager.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/TopicDelayedDeliveryTrackerManager.java new file mode 100644 index 0000000000000..707bcefc76a53 --- /dev/null +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/TopicDelayedDeliveryTrackerManager.java @@ -0,0 +1,74 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pulsar.broker.delayed; + +import org.apache.pulsar.broker.service.persistent.AbstractPersistentDispatcherMultipleConsumers; + +/** + * Manager interface for topic-level delayed delivery tracking. + * This interface provides a unified abstraction for managing delayed delivery at the topic level, + * allowing different implementations (in-memory, bucket-based) to share the same contract. + *

+ * The manager maintains a single global delayed message index per topic that is shared by all + * subscriptions, and provides per-subscription "view" objects that implement DelayedDeliveryTracker + * interface for compatibility with existing dispatcher logic. + */ +public interface TopicDelayedDeliveryTrackerManager extends AutoCloseable { + + /** + * Create or get a delayed delivery tracker view for the specified subscription. + * + * @param dispatcher the dispatcher instance for the subscription + * @return a DelayedDeliveryTracker view for the subscription + */ + DelayedDeliveryTracker createOrGetView(AbstractPersistentDispatcherMultipleConsumers dispatcher); + + /** + * Unregister a subscription from the manager. + * + * @param dispatcher the dispatcher instance to unregister + */ + void unregister(AbstractPersistentDispatcherMultipleConsumers dispatcher); + + /** + * Update the tick time configuration for the topic. + * + * @param newTickTimeMillis the new tick time in milliseconds + */ + void onTickTimeUpdated(long newTickTimeMillis); + + /** + * Get the total memory usage of the topic-level delayed message index. + * + * @return memory usage in bytes + */ + long topicBufferMemoryBytes(); + + /** + * Get the total number of delayed messages in the topic-level index. + * + * @return number of delayed messages + */ + long topicDelayedMessages(); + + /** + * Close the manager and release all resources. + */ + void close(); +} \ No newline at end of file diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/DelayedDeliveryTrackerFactoryTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/DelayedDeliveryTrackerFactoryTest.java index f43b5495fac25..f01391c770d47 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/DelayedDeliveryTrackerFactoryTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/DelayedDeliveryTrackerFactoryTest.java @@ -81,7 +81,7 @@ public void testFallbackToInMemoryTracker() throws Exception { // the factory should be fallback to InMemoryDelayedDeliveryTrackerFactory @Cleanup DelayedDeliveryTracker tracker = brokerService.getDelayedDeliveryTrackerFactory().newTracker(dispatcher); - Assert.assertTrue(tracker instanceof InMemoryDelayedDeliveryTracker); + Assert.assertTrue(tracker instanceof InMemoryTopicDelayedDeliveryTrackerView); DelayedDeliveryTrackerFactory fallbackFactory = brokerService.getFallbackDelayedDeliveryTrackerFactory(); Assert.assertTrue(fallbackFactory instanceof InMemoryDelayedDeliveryTrackerFactory); @@ -223,11 +223,11 @@ public void testPublishDelayMessagesAndCreateBucketDelayDeliveryTrackerFailed() }); Optional optional = reference.get(); - Assert.assertTrue(optional.get() instanceof InMemoryDelayedDeliveryTracker); + Assert.assertTrue(optional.get() instanceof InMemoryTopicDelayedDeliveryTrackerView); // Mock DelayedDeliveryTracker and Count the number of addMessage() calls AtomicInteger counter = new AtomicInteger(0); - InMemoryDelayedDeliveryTracker tracker = (InMemoryDelayedDeliveryTracker) optional.get(); + InMemoryTopicDelayedDeliveryTrackerView tracker = (InMemoryTopicDelayedDeliveryTrackerView) optional.get(); tracker = Mockito.spy(tracker); Mockito.doAnswer(inv -> { counter.incrementAndGet(); From dee4363495a5a84edb0919cdb378b1d5b63eefa7 Mon Sep 17 00:00:00 2001 From: Denovo1998 Date: Sat, 1 Nov 2025 14:38:11 +0800 Subject: [PATCH 02/16] feat[broker] Enhance InMemoryTopicDelayedDeliveryTrackerManager with fixed-delay detection and memory optimization --- ...oryTopicDelayedDeliveryTrackerManager.java | 152 +++++--- .../InMemoryTopicDeliveryTrackerTest.java | 341 ++++++++++++++++++ 2 files changed, 452 insertions(+), 41 deletions(-) create mode 100644 pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDeliveryTrackerTest.java diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java index fa0575f393502..5e603b1c9b4b7 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java @@ -55,9 +55,11 @@ public class InMemoryTopicDelayedDeliveryTrackerManager implements TopicDelayedD private final Timer timer; private Timeout timeout; private long currentTimeoutTarget = -1; + // Last time the TimerTask was triggered + private long lastTickRun = 0L; // Configuration - private final long tickTimeMillis; + private long tickTimeMillis; private final boolean isDelayedDeliveryDeliverAtTimeStrict; private final long fixedDelayDetectionLookahead; private final Clock clock; @@ -66,8 +68,12 @@ public class InMemoryTopicDelayedDeliveryTrackerManager implements TopicDelayedD private final AtomicLong delayedMessagesCount = new AtomicLong(0); private final AtomicLong bufferMemoryBytes = new AtomicLong(0); + // Fixed-delay detection (parity with legacy behavior) + private long highestDeliveryTimeTracked = 0; + private boolean messagesHaveFixedDelay = true; + // Timestamp precision for memory optimization - private final int timestampPrecisionBitCnt; + private int timestampPrecisionBitCnt; /** * Subscription context that holds per-subscription state. @@ -76,18 +82,21 @@ public class InMemoryTopicDelayedDeliveryTrackerManager implements TopicDelayedD static class SubContext { private final AbstractPersistentDispatcherMultipleConsumers dispatcher; private final String subscriptionName; - private final long tickTimeMillis; + private long tickTimeMillis; private final boolean isDelayedDeliveryDeliverAtTimeStrict; private final long fixedDelayDetectionLookahead; + private final Clock clock; private Position markDeletePosition; SubContext(AbstractPersistentDispatcherMultipleConsumers dispatcher, long tickTimeMillis, - boolean isDelayedDeliveryDeliverAtTimeStrict, long fixedDelayDetectionLookahead) { + boolean isDelayedDeliveryDeliverAtTimeStrict, long fixedDelayDetectionLookahead, + Clock clock) { this.dispatcher = dispatcher; this.subscriptionName = dispatcher.getSubscription().getName(); this.tickTimeMillis = tickTimeMillis; this.isDelayedDeliveryDeliverAtTimeStrict = isDelayedDeliveryDeliverAtTimeStrict; this.fixedDelayDetectionLookahead = fixedDelayDetectionLookahead; + this.clock = clock; } void updateMarkDeletePosition(Position position) { @@ -95,8 +104,8 @@ void updateMarkDeletePosition(Position position) { } long getCutoffTime() { - return isDelayedDeliveryDeliverAtTimeStrict ? System.currentTimeMillis() : - System.currentTimeMillis() + tickTimeMillis; + long now = clock.millis(); + return isDelayedDeliveryDeliverAtTimeStrict ? now : now + tickTimeMillis; } } @@ -138,7 +147,7 @@ public DelayedDeliveryTracker createOrGetView(AbstractPersistentDispatcherMultip synchronized (this) { SubContext subContext = subscriptionContexts.computeIfAbsent(subscriptionName, k -> new SubContext(dispatcher, tickTimeMillis, isDelayedDeliveryDeliverAtTimeStrict, - fixedDelayDetectionLookahead)); + fixedDelayDetectionLookahead, clock)); return new InMemoryTopicDelayedDeliveryTrackerView(this, subContext); } @@ -150,9 +159,11 @@ public void unregister(AbstractPersistentDispatcherMultipleConsumers dispatcher) synchronized (this) { subscriptionContexts.remove(subscriptionName); - - // If no more subscriptions, close the manager - if (subscriptionContexts.isEmpty() && delayedMessageMap.isEmpty()) { + // If no more subscriptions, proactively free index and close the manager to release memory + if (subscriptionContexts.isEmpty()) { + delayedMessageMap.clear(); + delayedMessagesCount.set(0); + bufferMemoryBytes.set(0); close(); } } @@ -160,9 +171,24 @@ public void unregister(AbstractPersistentDispatcherMultipleConsumers dispatcher) @Override public void onTickTimeUpdated(long newTickTimeMillis) { - // For now, tick time updates are not supported after initialization - // This could be enhanced to update all subscription contexts - log.warn("Tick time updates are not currently supported for topic-level delayed delivery managers"); + synchronized (this) { + if (this.tickTimeMillis == newTickTimeMillis) { + return; + } + this.tickTimeMillis = newTickTimeMillis; + // Update precision bits for new tick time (accept old/new buckets co-exist) + this.timestampPrecisionBitCnt = calculateTimestampPrecisionBitCnt(newTickTimeMillis); + // Propagate to all subscriptions + for (SubContext sc : subscriptionContexts.values()) { + sc.tickTimeMillis = newTickTimeMillis; + } + // Re-evaluate timer scheduling with new tick time + updateTimer(); + if (log.isDebugEnabled()) { + log.debug("Updated tickTimeMillis for topic-level delayed delivery manager to {} ms", + newTickTimeMillis); + } + } } @Override @@ -205,23 +231,36 @@ boolean addMessageForSub(SubContext subContext, long ledgerId, long entryId, lon timestamp, k -> new Long2ObjectRBTreeMap<>()); Roaring64Bitmap entryIds = ledgerMap.computeIfAbsent(ledgerId, k -> new Roaring64Bitmap()); - // Check if this entry already exists (deduplication) - if (!entryIds.contains(entryId)) { + // Incremental memory accounting: measure size delta on change + long before = entryIds.getLongSizeInBytes(); + boolean existed = entryIds.contains(entryId); + if (!existed) { entryIds.add(entryId); delayedMessagesCount.incrementAndGet(); - updateBufferMemoryEstimate(); + long after = entryIds.getLongSizeInBytes(); + bufferMemoryBytes.addAndGet(after - before); } updateTimer(); + // Update global fixed-delay detection + checkAndUpdateHighest(deliverAt); return true; } } + private void checkAndUpdateHighest(long deliverAt) { + if (deliverAt < (highestDeliveryTimeTracked - tickTimeMillis)) { + messagesHaveFixedDelay = false; + } + highestDeliveryTimeTracked = Math.max(highestDeliveryTimeTracked, deliverAt); + } + /** * Check if there are messages available for a subscription. */ boolean hasMessageAvailableForSub(SubContext subContext) { synchronized (this) { + refreshMarkDeletePosition(subContext); if (delayedMessageMap.isEmpty()) { return false; } @@ -265,6 +304,7 @@ boolean hasMessageAvailableForSub(SubContext subContext) { */ NavigableSet getScheduledMessagesForSub(SubContext subContext, int maxMessages) { synchronized (this) { + refreshMarkDeletePosition(subContext); int remaining = maxMessages; NavigableSet positions = new TreeSet<>(); long cutoffTime = subContext.getCutoffTime(); @@ -289,18 +329,18 @@ NavigableSet getScheduledMessagesForSub(SubContext subContext, int max long ledgerId = ledgerEntry.getLongKey(); Roaring64Bitmap entryIds = ledgerEntry.getValue(); - // Filter entries based on mark delete position - long[] entryIdArray = entryIds.toArray(); - for (long entryId : entryIdArray) { - if (remaining <= 0) { - break; - } + // Fast skip if entire ledger is before mark-delete + if (markDelete != null && ledgerId < markDelete.getLedgerId()) { + continue; + } + + // Iterate over entry ids without materializing array + var it = entryIds.iterator(); + while (it.hasNext() && remaining > 0) { + long entryId = it.next(); // Skip entries that are before or at mark delete if (markDelete != null) { - if (ledgerId < markDelete.getLedgerId()) { - continue; - } if (ledgerId == markDelete.getLedgerId() && entryId <= markDelete.getEntryId()) { continue; } @@ -312,8 +352,8 @@ NavigableSet getScheduledMessagesForSub(SubContext subContext, int max } } - // Note: We don't remove messages from the global index here - // Pruning will be handled separately based on min mark delete across all subscriptions + // Prune global index based on min mark-delete across all subscriptions + pruneByMinMarkDelete(); return positions; } @@ -323,9 +363,10 @@ NavigableSet getScheduledMessagesForSub(SubContext subContext, int max * Check if deliveries should be paused for a subscription. */ boolean shouldPauseAllDeliveriesForSub(SubContext subContext) { - // Simplified implementation - could be enhanced with fixed delay detection - return fixedDelayDetectionLookahead > 0 - && getNumberOfVisibleDelayedMessagesForSub(subContext) >= fixedDelayDetectionLookahead + // Parity with legacy: pause if all observed delays are fixed and backlog is large enough + return subContext.getFixedDelayDetectionLookahead() > 0 + && messagesHaveFixedDelay + && getNumberOfVisibleDelayedMessagesForSub(subContext) >= subContext.getFixedDelayDetectionLookahead() && !hasMessageAvailableForSub(subContext); } @@ -372,21 +413,20 @@ private void updateTimer() { long delayMillis = nextDeliveryTime - now; if (delayMillis < 0) { - // Messages are ready, but we don't need to keep retriggering + // Messages are ready; avoid retriggering timer, dispatcher will pick them on next read return; } + // Align with tick window like AbstractDelayedDeliveryTracker + long remainingTickDelayMillis = lastTickRun + tickTimeMillis - now; + long calculatedDelayMillis = Math.max(delayMillis, remainingTickDelayMillis); + currentTimeoutTarget = nextDeliveryTime; - timeout = timer.newTimeout(this, delayMillis, java.util.concurrent.TimeUnit.MILLISECONDS); + timeout = timer.newTimeout(this, calculatedDelayMillis, java.util.concurrent.TimeUnit.MILLISECONDS); } private void updateBufferMemoryEstimate() { - // Simplified memory estimation - long estimatedBytes = delayedMessageMap.values().stream() - .mapToLong(ledgerMap -> ledgerMap.values().stream() - .mapToLong(Roaring64Bitmap::getLongSizeInBytes).sum()) - .sum(); - bufferMemoryBytes.set(estimatedBytes); + // No-op in incremental mode (kept for compatibility) } private long getNumberOfVisibleDelayedMessagesForSub(SubContext subContext) { @@ -425,11 +465,14 @@ private void pruneByMinMarkDelete() { if (ledgerId < minMarkDelete.getLedgerId()) { // Entire ledger can be removed + long bytes = entryIds.getLongSizeInBytes(); delayedMessagesCount.addAndGet(-entryIds.getLongCardinality()); + bufferMemoryBytes.addAndGet(-bytes); ledgerIterator.remove(); } else if (ledgerId == minMarkDelete.getLedgerId()) { // Remove entries <= mark delete entry ID long removedCount = 0; + long before = entryIds.getLongSizeInBytes(); var entryIterator = entryIds.iterator(); while (entryIterator.hasNext()) { long entryId = entryIterator.next(); @@ -438,7 +481,9 @@ private void pruneByMinMarkDelete() { removedCount++; } } + long after = entryIds.getLongSizeInBytes(); delayedMessagesCount.addAndGet(-removedCount); + bufferMemoryBytes.addAndGet(after - before); if (entryIds.isEmpty()) { ledgerIterator.remove(); @@ -460,14 +505,39 @@ public void run(Timeout timeout) throws Exception { return; } + java.util.ArrayList toTrigger = new java.util.ArrayList<>(); synchronized (this) { currentTimeoutTarget = -1; this.timeout = null; + lastTickRun = clock.millis(); - // Trigger read more entries for all subscriptions + // Decide which dispatchers to trigger while holding the lock for (SubContext subContext : subscriptionContexts.values()) { - subContext.getDispatcher().readMoreEntriesAsync(); + if (hasMessageAvailableForSub(subContext)) { + toTrigger.add(subContext.getDispatcher()); + } + } + } + // Invoke callbacks outside the manager lock to reduce contention + for (AbstractPersistentDispatcherMultipleConsumers d : toTrigger) { + d.readMoreEntriesAsync(); + } + } + + private void refreshMarkDeletePosition(SubContext subContext) { + try { + Position pos = subContext.getDispatcher().getCursor().getMarkDeletedPosition(); + if (pos != null) { + Position current = subContext.getMarkDeletePosition(); + if (current == null || pos.compareTo(current) > 0) { + subContext.updateMarkDeletePosition(pos); + } + } + } catch (Throwable t) { + if (log.isDebugEnabled()) { + log.debug("Failed to refresh mark-delete position for subscription {}", + subContext.getSubscriptionName(), t); } } } -} \ No newline at end of file +} diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDeliveryTrackerTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDeliveryTrackerTest.java new file mode 100644 index 0000000000000..9b6b1d3481412 --- /dev/null +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDeliveryTrackerTest.java @@ -0,0 +1,341 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pulsar.broker.delayed; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; +import io.netty.util.Timeout; +import io.netty.util.Timer; +import io.netty.util.TimerTask; +import java.time.Clock; +import java.util.NavigableMap; +import java.util.NavigableSet; +import java.util.TreeMap; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; +import org.apache.bookkeeper.mledger.ManagedCursor; +import org.apache.bookkeeper.mledger.Position; +import org.apache.bookkeeper.mledger.PositionFactory; +import org.apache.pulsar.broker.service.Subscription; +import org.apache.pulsar.broker.service.persistent.AbstractPersistentDispatcherMultipleConsumers; +import org.mockito.stubbing.Answer; +import org.testng.annotations.Test; + +@Test(groups = "broker") +public class InMemoryTopicDeliveryTrackerTest { + + private static class TestEnv { + final Timer timer; + final NavigableMap tasks; + final Clock clock; + final AtomicLong time; + + TestEnv() { + this.tasks = new TreeMap<>(); + this.time = new AtomicLong(0L); + this.clock = mock(Clock.class); + when(clock.millis()).then((Answer) invocation -> time.get()); + + this.timer = mock(Timer.class); + when(timer.newTimeout(any(), anyLong(), any())).then(invocation -> { + TimerTask task = invocation.getArgument(0, TimerTask.class); + long timeout = invocation.getArgument(1, Long.class); + TimeUnit unit = invocation.getArgument(2, TimeUnit.class); + long scheduleAt = time.get() + unit.toMillis(timeout); + tasks.put(scheduleAt, task); + Timeout t = mock(Timeout.class); + when(t.cancel()).then(i -> { + tasks.remove(scheduleAt, task); + return null; + }); + when(t.isCancelled()).thenReturn(false); + return t; + }); + } + } + + private static AbstractPersistentDispatcherMultipleConsumers newDispatcher(String subName, ManagedCursor cursor) + throws Exception { + AbstractPersistentDispatcherMultipleConsumers dispatcher = + mock(AbstractPersistentDispatcherMultipleConsumers.class); + Subscription subscription = mock(Subscription.class); + when(subscription.getName()).thenReturn(subName); + when(dispatcher.getSubscription()).thenReturn(subscription); + when(dispatcher.getCursor()).thenReturn(cursor); + return dispatcher; + } + + @Test + public void testSingleSubscriptionBasicFlow() throws Exception { + TestEnv env = new TestEnv(); + long tickMs = 100; + boolean strict = true; + long lookahead = 10; + InMemoryTopicDelayedDeliveryTrackerManager manager = + new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, tickMs, env.clock, strict, lookahead); + + ManagedCursor cursor = mock(ManagedCursor.class); + AbstractPersistentDispatcherMultipleConsumers dispatcher = newDispatcher("sub-a", cursor); + DelayedDeliveryTracker view = manager.createOrGetView(dispatcher); + + assertFalse(view.hasMessageAvailable()); + + // Add 3 messages in the future + env.time.set(1000); + assertTrue(view.addMessage(1, 1, 1200)); + assertTrue(view.addMessage(1, 2, 1300)); + assertTrue(view.addMessage(2, 1, 1400)); + + assertFalse(view.hasMessageAvailable()); + assertEquals(view.getNumberOfDelayedMessages(), 3); + + // Advance time so first 2 buckets are visible + env.time.set(1350); + assertTrue(view.hasMessageAvailable()); + NavigableSet scheduled = view.getScheduledMessages(10); + // Should include both positions from first 2 buckets + assertEquals(scheduled.size(), 2); + + // Global counter doesn't drop until mark-delete pruning + assertEquals(view.getNumberOfDelayedMessages(), 3); + + // Mark-delete beyond the scheduled positions and prune + when(cursor.getMarkDeletedPosition()).thenReturn(PositionFactory.create(1L, 2L)); + // Trigger pruning by another get + view.getScheduledMessages(10); + // Now only one entry remains in global index + assertEquals(view.getNumberOfDelayedMessages(), 1); + + // Cleanup + view.close(); + } + + @Test + public void testSharedIndexDedupAcrossSubscriptions() throws Exception { + TestEnv env = new TestEnv(); + InMemoryTopicDelayedDeliveryTrackerManager manager = + new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 1, env.clock, true, 0); + + ManagedCursor c1 = mock(ManagedCursor.class); + ManagedCursor c2 = mock(ManagedCursor.class); + AbstractPersistentDispatcherMultipleConsumers d1 = newDispatcher("sub-a", c1); + AbstractPersistentDispatcherMultipleConsumers d2 = newDispatcher("sub-b", c2); + + DelayedDeliveryTracker v1 = manager.createOrGetView(d1); + DelayedDeliveryTracker v2 = manager.createOrGetView(d2); + + env.time.set(1000); + assertTrue(v1.addMessage(10, 20, 2000)); + // Add the same message from another subscription; should be de-duplicated in global index + assertTrue(v2.addMessage(10, 20, 2000)); + + assertEquals(v1.getNumberOfDelayedMessages(), 1); + assertEquals(v2.getNumberOfDelayedMessages(), 1); + + v1.close(); + v2.close(); + } + + @Test + public void testTimerRunTriggersOnlyAvailableSubscriptions() throws Exception { + TestEnv env = new TestEnv(); + long tickMs = 100; + InMemoryTopicDelayedDeliveryTrackerManager manager = + new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, tickMs, env.clock, true, 0); + + ManagedCursor c1 = mock(ManagedCursor.class); + ManagedCursor c2 = mock(ManagedCursor.class); + AbstractPersistentDispatcherMultipleConsumers d1 = newDispatcher("sub-a", c1); + AbstractPersistentDispatcherMultipleConsumers d2 = newDispatcher("sub-b", c2); + DelayedDeliveryTracker v1 = manager.createOrGetView(d1); + DelayedDeliveryTracker v2 = manager.createOrGetView(d2); + + env.time.set(0); + // Add two buckets. Only sub-a will have messages available based on mark-delete + assertTrue(v1.addMessage(1, 1, 500)); + assertTrue(v2.addMessage(1, 2, 500)); + + // Before cutoff + assertFalse(v1.hasMessageAvailable()); + assertFalse(v2.hasMessageAvailable()); + + // Set time after cutoff and set sub-a mark-delete behind entries, sub-b beyond entries + env.time.set(600); + when(c1.getMarkDeletedPosition()).thenReturn(PositionFactory.create(0L, 0L)); // visible + when(c2.getMarkDeletedPosition()).thenReturn(PositionFactory.create(1L, 5L)); // not visible + + // Invoke manager timer task directly + manager.run(mock(Timeout.class)); + + // Only d1 should be triggered + verify(d1, times(1)).readMoreEntriesAsync(); + verify(d2, times(0)).readMoreEntriesAsync(); + + v1.close(); + v2.close(); + } + + @Test + public void testPauseWithFixedDelays() throws Exception { + TestEnv env = new TestEnv(); + long lookahead = 5; + InMemoryTopicDelayedDeliveryTrackerManager manager = + new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 10, env.clock, true, lookahead); + + ManagedCursor cursor = mock(ManagedCursor.class); + AbstractPersistentDispatcherMultipleConsumers dispatcher = newDispatcher("sub-a", cursor); + InMemoryTopicDelayedDeliveryTrackerView view = + (InMemoryTopicDelayedDeliveryTrackerView) manager.createOrGetView(dispatcher); + + // Add strictly increasing deliverAt times (fixed delay scenario) + env.time.set(0); + for (int i = 1; i <= lookahead; i++) { + assertTrue(view.addMessage(i, i, i * 100)); + } + assertTrue(view.shouldPauseAllDeliveries()); + + // Move time forward to make messages available -> pause should be lifted + env.time.set(lookahead * 100 + 1); + assertFalse(view.shouldPauseAllDeliveries()); + + view.close(); + } + + @Test + public void testDynamicTickTimeUpdateAffectsCutoff() throws Exception { + TestEnv env = new TestEnv(); + // non-strict mode: cutoff = now + tick + InMemoryTopicDelayedDeliveryTrackerManager manager = + new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 100, env.clock, false, 0); + + ManagedCursor cursor = mock(ManagedCursor.class); + AbstractPersistentDispatcherMultipleConsumers dispatcher = newDispatcher("sub-a", cursor); + DelayedDeliveryTracker view = manager.createOrGetView(dispatcher); + + env.time.set(1000); + // deliverAt within current tick window -> rejected + assertFalse(view.addMessage(1, 1, 1050)); // cutoff=1100 + assertEquals(view.getNumberOfDelayedMessages(), 0); + + // shrink tick: cutoff reduces -> same deliverAt becomes accepted + view.resetTickTime(10); + assertTrue(view.addMessage(1, 1, 1050)); // cutoff=1010 + assertEquals(view.getNumberOfDelayedMessages(), 1); + + view.close(); + } + + @Test + public void testMinMarkDeleteAcrossSubscriptions() throws Exception { + TestEnv env = new TestEnv(); + InMemoryTopicDelayedDeliveryTrackerManager manager = + new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 1, env.clock, true, 0); + + ManagedCursor c1 = mock(ManagedCursor.class); + ManagedCursor c2 = mock(ManagedCursor.class); + AbstractPersistentDispatcherMultipleConsumers d1 = newDispatcher("sub-a", c1); + AbstractPersistentDispatcherMultipleConsumers d2 = newDispatcher("sub-b", c2); + InMemoryTopicDelayedDeliveryTrackerView v1 = + (InMemoryTopicDelayedDeliveryTrackerView) manager.createOrGetView(d1); + InMemoryTopicDelayedDeliveryTrackerView v2 = + (InMemoryTopicDelayedDeliveryTrackerView) manager.createOrGetView(d2); + + env.time.set(0); + assertTrue(v1.addMessage(1, 1, 100)); + assertTrue(v1.addMessage(1, 2, 100)); + assertTrue(v1.addMessage(2, 1, 100)); + assertEquals(v1.getNumberOfDelayedMessages(), 3); + + // c1 behind, c2 ahead + when(c1.getMarkDeletedPosition()).thenReturn(PositionFactory.create(0L, 0L)); + when(c2.getMarkDeletedPosition()).thenReturn(PositionFactory.create(10L, 10L)); + + env.time.set(200); + // Trigger v2 read + prune attempt; min mark-delete still from c1 => no prune + v2.getScheduledMessages(10); + assertEquals(v1.getNumberOfDelayedMessages(), 3); + + // Advance c1 mark-delete beyond (1,2) + v1.updateMarkDeletePosition(PositionFactory.create(1L, 2L)); + v1.getScheduledMessages(10); + // Now only (2,1) should remain + assertEquals(v1.getNumberOfDelayedMessages(), 1); + + v1.close(); + v2.close(); + } + + @Test + public void testTimerSchedulingWindowAlignment() throws Exception { + TestEnv env = new TestEnv(); + long tickMs = 1000; + InMemoryTopicDelayedDeliveryTrackerManager manager = + new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, tickMs, env.clock, true, 0); + + ManagedCursor cursor = mock(ManagedCursor.class); + AbstractPersistentDispatcherMultipleConsumers dispatcher = newDispatcher("sub-a", cursor); + DelayedDeliveryTracker view = manager.createOrGetView(dispatcher); + + // Establish lastTickRun via a manual run at t=10000 + env.time.set(10000); + manager.run(mock(Timeout.class)); + + // Add with deliverAt=10001, but tick window alignment should schedule at >= 11000 + assertTrue(view.addMessage(1, 1, 10001)); + long scheduledAt = env.tasks.firstKey(); + assertTrue(scheduledAt >= 11000, "scheduledAt=" + scheduledAt); + + // If no recent tick run, deliverAt should determine + env.tasks.clear(); + env.time.set(20000); + // No run -> lastTickRun remains 10000; deliverAt=20005 < lastTickRun+tick(11000)? no, so schedule at deliverAt + assertTrue(view.addMessage(1, 2, 20005)); + long scheduledAt2 = env.tasks.firstKey(); + assertEquals(scheduledAt2, 20005); + + view.close(); + } + + @Test + public void testBufferMemoryUsageAndCleanup() throws Exception { + TestEnv env = new TestEnv(); + InMemoryTopicDelayedDeliveryTrackerManager manager = + new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 1, env.clock, true, 0); + + ManagedCursor c = mock(ManagedCursor.class); + AbstractPersistentDispatcherMultipleConsumers d = newDispatcher("sub-a", c); + DelayedDeliveryTracker v = manager.createOrGetView(d); + + env.time.set(0); + assertTrue(v.addMessage(1, 1, 10)); + assertTrue(v.getBufferMemoryUsage() > 0); + + v.close(); + // After last subscription closes, manager should clear index and memory + assertEquals(manager.topicDelayedMessages(), 0); + assertEquals(manager.topicBufferMemoryBytes(), 0); + } +} From e455512034ffe18e340f99766b99950f8017f576 Mon Sep 17 00:00:00 2001 From: Denovo1998 Date: Sat, 1 Nov 2025 16:10:55 +0800 Subject: [PATCH 03/16] [feat][broker] Improve InMemoryTopicDelayedDeliveryTrackerManager with enhanced memory management and concurrency controls --- ...InMemoryDelayedDeliveryTrackerFactory.java | 14 +- ...oryTopicDelayedDeliveryTrackerManager.java | 424 ++++++++++-------- 2 files changed, 239 insertions(+), 199 deletions(-) diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTrackerFactory.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTrackerFactory.java index 555c730465c69..7bfab427488ff 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTrackerFactory.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTrackerFactory.java @@ -22,6 +22,7 @@ import io.netty.util.HashedWheelTimer; import io.netty.util.Timer; import io.netty.util.concurrent.DefaultThreadFactory; +import java.time.Clock; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.TimeUnit; @@ -74,10 +75,15 @@ public DelayedDeliveryTracker newTracker(AbstractPersistentDispatcherMultipleCon DelayedDeliveryTracker newTracker0(AbstractPersistentDispatcherMultipleConsumers dispatcher) { String topicName = dispatcher.getTopic().getName(); - // Get or create topic-level manager for this topic - TopicDelayedDeliveryTrackerManager manager = topicManagers.computeIfAbsent(topicName, - k -> new InMemoryTopicDelayedDeliveryTrackerManager(timer, tickTimeMillis, - isDelayedDeliveryDeliverAtTimeStrict, fixedDelayDetectionLookahead)); + // Get or create topic-level manager for this topic with onEmpty callback to remove from cache + final TopicDelayedDeliveryTrackerManager[] holder = new TopicDelayedDeliveryTrackerManager[1]; + TopicDelayedDeliveryTrackerManager manager = topicManagers.computeIfAbsent(topicName, k -> { + InMemoryTopicDelayedDeliveryTrackerManager m = new InMemoryTopicDelayedDeliveryTrackerManager( + timer, tickTimeMillis, Clock.systemUTC(), isDelayedDeliveryDeliverAtTimeStrict, + fixedDelayDetectionLookahead, () -> topicManagers.remove(topicName, holder[0])); + holder[0] = m; + return m; + }); // Create a per-subscription view from the topic-level manager return manager.createOrGetView(dispatcher); diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java index 5e603b1c9b4b7..ae06e549153b8 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java @@ -21,14 +21,16 @@ import io.netty.util.Timeout; import io.netty.util.Timer; import io.netty.util.TimerTask; -import it.unimi.dsi.fastutil.longs.Long2ObjectRBTreeMap; -import it.unimi.dsi.fastutil.longs.Long2ObjectSortedMap; import java.time.Clock; -import java.util.HashMap; +import java.util.ArrayList; import java.util.Map; import java.util.NavigableSet; import java.util.TreeSet; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentSkipListMap; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.locks.ReentrantLock; import lombok.Getter; import lombok.extern.slf4j.Slf4j; import org.apache.bookkeeper.mledger.Position; @@ -45,11 +47,13 @@ public class InMemoryTopicDelayedDeliveryTrackerManager implements TopicDelayedDeliveryTrackerManager, TimerTask { // Global delayed message index: timestamp -> ledgerId -> entryId bitmap - private final Long2ObjectSortedMap> delayedMessageMap = - new Long2ObjectRBTreeMap<>(); + // Outer: sorted by timestamp for efficient finding of earliest bucket + // Inner: per-ledger bitmaps of entry-ids + private final ConcurrentSkipListMap> delayedMessageMap = + new ConcurrentSkipListMap<>(); // Subscription registry: subscription name -> subscription context - private final Map subscriptionContexts = new HashMap<>(); + private final ConcurrentHashMap subscriptionContexts = new ConcurrentHashMap<>(); // Timer for delayed delivery private final Timer timer; @@ -69,12 +73,19 @@ public class InMemoryTopicDelayedDeliveryTrackerManager implements TopicDelayedD private final AtomicLong bufferMemoryBytes = new AtomicLong(0); // Fixed-delay detection (parity with legacy behavior) - private long highestDeliveryTimeTracked = 0; - private boolean messagesHaveFixedDelay = true; + private volatile long highestDeliveryTimeTracked = 0; + private volatile boolean messagesHaveFixedDelay = true; // Timestamp precision for memory optimization private int timestampPrecisionBitCnt; + // Per-bucket locks (timestamp -> lock) for fine-grained concurrency + private final ConcurrentHashMap bucketLocks = new ConcurrentHashMap<>(); + + // Timer state guard + private final ReentrantLock timerLock = new ReentrantLock(); + + /** * Subscription context that holds per-subscription state. */ @@ -109,22 +120,32 @@ long getCutoffTime() { } } + private final Runnable onEmptyCallback; + public InMemoryTopicDelayedDeliveryTrackerManager(Timer timer, long tickTimeMillis, boolean isDelayedDeliveryDeliverAtTimeStrict, long fixedDelayDetectionLookahead) { this(timer, tickTimeMillis, Clock.systemUTC(), isDelayedDeliveryDeliverAtTimeStrict, - fixedDelayDetectionLookahead); + fixedDelayDetectionLookahead, null); } public InMemoryTopicDelayedDeliveryTrackerManager(Timer timer, long tickTimeMillis, Clock clock, boolean isDelayedDeliveryDeliverAtTimeStrict, long fixedDelayDetectionLookahead) { + this(timer, tickTimeMillis, clock, isDelayedDeliveryDeliverAtTimeStrict, fixedDelayDetectionLookahead, null); + } + + public InMemoryTopicDelayedDeliveryTrackerManager(Timer timer, long tickTimeMillis, Clock clock, + boolean isDelayedDeliveryDeliverAtTimeStrict, + long fixedDelayDetectionLookahead, + Runnable onEmptyCallback) { this.timer = timer; this.tickTimeMillis = tickTimeMillis; this.clock = clock; this.isDelayedDeliveryDeliverAtTimeStrict = isDelayedDeliveryDeliverAtTimeStrict; this.fixedDelayDetectionLookahead = fixedDelayDetectionLookahead; this.timestampPrecisionBitCnt = calculateTimestampPrecisionBitCnt(tickTimeMillis); + this.onEmptyCallback = onEmptyCallback; } private static int calculateTimestampPrecisionBitCnt(long tickTimeMillis) { @@ -144,50 +165,66 @@ private static long trimLowerBit(long timestamp, int bits) { public DelayedDeliveryTracker createOrGetView(AbstractPersistentDispatcherMultipleConsumers dispatcher) { String subscriptionName = dispatcher.getSubscription().getName(); - synchronized (this) { - SubContext subContext = subscriptionContexts.computeIfAbsent(subscriptionName, + SubContext subContext = subscriptionContexts.computeIfAbsent(subscriptionName, k -> new SubContext(dispatcher, tickTimeMillis, isDelayedDeliveryDeliverAtTimeStrict, - fixedDelayDetectionLookahead, clock)); - - return new InMemoryTopicDelayedDeliveryTrackerView(this, subContext); - } + fixedDelayDetectionLookahead, clock)); + return new InMemoryTopicDelayedDeliveryTrackerView(this, subContext); } @Override public void unregister(AbstractPersistentDispatcherMultipleConsumers dispatcher) { String subscriptionName = dispatcher.getSubscription().getName(); - synchronized (this) { - subscriptionContexts.remove(subscriptionName); - // If no more subscriptions, proactively free index and close the manager to release memory - if (subscriptionContexts.isEmpty()) { - delayedMessageMap.clear(); - delayedMessagesCount.set(0); - bufferMemoryBytes.set(0); - close(); + subscriptionContexts.remove(subscriptionName); + // If no more subscriptions, proactively free index and release memory + if (subscriptionContexts.isEmpty()) { + timerLock.lock(); + try { + if (timeout != null) { + timeout.cancel(); + timeout = null; + } + currentTimeoutTarget = -1; + } finally { + timerLock.unlock(); + } + delayedMessageMap.clear(); + bucketLocks.clear(); + delayedMessagesCount.set(0); + bufferMemoryBytes.set(0); + if (onEmptyCallback != null) { + try { + onEmptyCallback.run(); + } catch (Throwable t) { + if (log.isDebugEnabled()) { + log.debug("onEmptyCallback failed", t); + } + } } } } @Override public void onTickTimeUpdated(long newTickTimeMillis) { - synchronized (this) { - if (this.tickTimeMillis == newTickTimeMillis) { - return; - } - this.tickTimeMillis = newTickTimeMillis; - // Update precision bits for new tick time (accept old/new buckets co-exist) - this.timestampPrecisionBitCnt = calculateTimestampPrecisionBitCnt(newTickTimeMillis); - // Propagate to all subscriptions - for (SubContext sc : subscriptionContexts.values()) { - sc.tickTimeMillis = newTickTimeMillis; - } - // Re-evaluate timer scheduling with new tick time - updateTimer(); - if (log.isDebugEnabled()) { - log.debug("Updated tickTimeMillis for topic-level delayed delivery manager to {} ms", - newTickTimeMillis); - } + if (this.tickTimeMillis == newTickTimeMillis) { + return; + } + this.tickTimeMillis = newTickTimeMillis; + // Update precision bits for new tick time (accept old/new buckets co-exist) + this.timestampPrecisionBitCnt = calculateTimestampPrecisionBitCnt(newTickTimeMillis); + // Propagate to all subscriptions + for (SubContext sc : subscriptionContexts.values()) { + sc.tickTimeMillis = newTickTimeMillis; + } + // Re-evaluate timer scheduling with new tick time + timerLock.lock(); + try { + updateTimerLocked(); + } finally { + timerLock.unlock(); + } + if (log.isDebugEnabled()) { + log.debug("Updated tickTimeMillis for topic-level delayed delivery manager to {} ms", newTickTimeMillis); } } @@ -203,16 +240,21 @@ public long topicDelayedMessages() { @Override public void close() { - synchronized (this) { + timerLock.lock(); + try { if (timeout != null) { timeout.cancel(); timeout = null; } - delayedMessageMap.clear(); - subscriptionContexts.clear(); - delayedMessagesCount.set(0); - bufferMemoryBytes.set(0); + currentTimeoutTarget = -1; + } finally { + timerLock.unlock(); } + delayedMessageMap.clear(); + bucketLocks.clear(); + subscriptionContexts.clear(); + delayedMessagesCount.set(0); + bufferMemoryBytes.set(0); } // Internal methods for subscription views @@ -221,31 +263,37 @@ public void close() { * Add a message to the global delayed message index. */ boolean addMessageForSub(SubContext subContext, long ledgerId, long entryId, long deliverAt) { - synchronized (this) { - if (deliverAt < 0 || deliverAt <= subContext.getCutoffTime()) { - return false; - } + if (deliverAt < 0 || deliverAt <= subContext.getCutoffTime()) { + return false; + } - long timestamp = trimLowerBit(deliverAt, timestampPrecisionBitCnt); - Long2ObjectSortedMap ledgerMap = delayedMessageMap.computeIfAbsent( - timestamp, k -> new Long2ObjectRBTreeMap<>()); + long timestamp = trimLowerBit(deliverAt, timestampPrecisionBitCnt); + ReentrantLock bLock = bucketLocks.computeIfAbsent(timestamp, k -> new ReentrantLock()); + bLock.lock(); + try { + ConcurrentHashMap ledgerMap = + delayedMessageMap.computeIfAbsent(timestamp, k -> new ConcurrentHashMap<>()); Roaring64Bitmap entryIds = ledgerMap.computeIfAbsent(ledgerId, k -> new Roaring64Bitmap()); - - // Incremental memory accounting: measure size delta on change long before = entryIds.getLongSizeInBytes(); - boolean existed = entryIds.contains(entryId); - if (!existed) { + if (!entryIds.contains(entryId)) { entryIds.add(entryId); delayedMessagesCount.incrementAndGet(); long after = entryIds.getLongSizeInBytes(); bufferMemoryBytes.addAndGet(after - before); } + } finally { + bLock.unlock(); + } - updateTimer(); - // Update global fixed-delay detection - checkAndUpdateHighest(deliverAt); - return true; + // Timer update and fixed delay detection + timerLock.lock(); + try { + updateTimerLocked(); + } finally { + timerLock.unlock(); } + checkAndUpdateHighest(deliverAt); + return true; } private void checkAndUpdateHighest(long deliverAt) { @@ -259,104 +307,76 @@ private void checkAndUpdateHighest(long deliverAt) { * Check if there are messages available for a subscription. */ boolean hasMessageAvailableForSub(SubContext subContext) { - synchronized (this) { - refreshMarkDeletePosition(subContext); - if (delayedMessageMap.isEmpty()) { - return false; - } - - long cutoffTime = subContext.getCutoffTime(); - long firstTimestamp = delayedMessageMap.firstLongKey(); - - if (firstTimestamp > cutoffTime) { - return false; - } - - // Quick check: if there's any message in the earliest time bucket that's after mark delete - Long2ObjectSortedMap ledgerMap = delayedMessageMap.get(firstTimestamp); - if (ledgerMap != null) { - Position markDelete = subContext.getMarkDeletePosition(); - if (markDelete == null) { - return true; // No mark delete means all messages are available - } - - for (var entry : ledgerMap.long2ObjectEntrySet()) { - long ledgerId = entry.getLongKey(); - Roaring64Bitmap entryIds = entry.getValue(); - - if (ledgerId > markDelete.getLedgerId()) { - return true; - } else if (ledgerId == markDelete.getLedgerId()) { - // Check if there are any entry IDs after mark delete - if (entryIds.stream().anyMatch(entryId -> entryId > markDelete.getEntryId())) { - return true; - } - } - } - } - + if (delayedMessageMap.isEmpty()) { return false; } + Long firstKey = delayedMessageMap.firstKey(); + if (firstKey == null) { + return false; + } + long cutoffTime = subContext.getCutoffTime(); + return firstKey <= cutoffTime; } /** * Get scheduled messages for a subscription. */ NavigableSet getScheduledMessagesForSub(SubContext subContext, int maxMessages) { - synchronized (this) { - refreshMarkDeletePosition(subContext); - int remaining = maxMessages; - NavigableSet positions = new TreeSet<>(); - long cutoffTime = subContext.getCutoffTime(); - Position markDelete = subContext.getMarkDeletePosition(); - - // Iterate through time buckets - var iterator = delayedMessageMap.long2ObjectEntrySet().iterator(); - while (iterator.hasNext() && remaining > 0) { - var timeEntry = iterator.next(); - long timestamp = timeEntry.getLongKey(); - - if (timestamp > cutoffTime) { - break; + NavigableSet positions = new TreeSet<>(); + int remaining = maxMessages; + + // Refresh mark-delete once outside of any bucket lock + refreshMarkDeletePosition(subContext); + long cutoffTime = subContext.getCutoffTime(); + Position markDelete = subContext.getMarkDeletePosition(); + + // Snapshot of buckets up to cutoff and iterate per-bucket with bucket locks + java.util.List tsList = new java.util.ArrayList<>(delayedMessageMap.headMap(cutoffTime, true).keySet()); + for (Long ts : tsList) { + if (remaining <= 0) { + break; + } + ReentrantLock bLock = bucketLocks.get(ts); + if (bLock == null) { + continue; + } + bLock.lock(); + try { + ConcurrentHashMap ledgerMap = delayedMessageMap.get(ts); + if (ledgerMap == null) { + continue; } - - Long2ObjectSortedMap ledgerMap = timeEntry.getValue(); - - // Iterate through ledgers in this time bucket - var ledgerIterator = ledgerMap.long2ObjectEntrySet().iterator(); - while (ledgerIterator.hasNext() && remaining > 0) { - var ledgerEntry = ledgerIterator.next(); - long ledgerId = ledgerEntry.getLongKey(); + for (Map.Entry ledgerEntry : ledgerMap.entrySet()) { + if (remaining <= 0) { + break; + } + long ledgerId = ledgerEntry.getKey(); Roaring64Bitmap entryIds = ledgerEntry.getValue(); - - // Fast skip if entire ledger is before mark-delete if (markDelete != null && ledgerId < markDelete.getLedgerId()) { continue; } - - // Iterate over entry ids without materializing array - var it = entryIds.iterator(); + org.roaringbitmap.longlong.LongIterator it = entryIds.getLongIterator(); while (it.hasNext() && remaining > 0) { long entryId = it.next(); - - // Skip entries that are before or at mark delete - if (markDelete != null) { - if (ledgerId == markDelete.getLedgerId() && entryId <= markDelete.getEntryId()) { - continue; - } + if (markDelete != null && ledgerId == markDelete.getLedgerId() + && entryId <= markDelete.getEntryId()) { + continue; } - positions.add(PositionFactory.create(ledgerId, entryId)); remaining--; } } + } finally { + bLock.unlock(); } + } - // Prune global index based on min mark-delete across all subscriptions + // Prune global index based on min mark-delete across all subscriptions (write path) + if (!positions.isEmpty()) { pruneByMinMarkDelete(); - - return positions; } + + return positions; } /** @@ -381,16 +401,13 @@ void clearForSub() { * Update mark delete position for a subscription. */ void updateMarkDeletePosition(SubContext subContext, Position position) { - synchronized (this) { - subContext.updateMarkDeletePosition(position); - // Trigger pruning if needed - pruneByMinMarkDelete(); - } + subContext.updateMarkDeletePosition(position); + pruneByMinMarkDelete(); } // Private helper methods - private void updateTimer() { + private void updateTimerLocked() { if (delayedMessageMap.isEmpty()) { if (timeout != null) { currentTimeoutTarget = -1; @@ -399,30 +416,26 @@ private void updateTimer() { } return; } - - long nextDeliveryTime = delayedMessageMap.firstLongKey(); + Long nextKey = delayedMessageMap.firstKey(); + if (nextKey == null) { + return; + } + long nextDeliveryTime = nextKey; if (nextDeliveryTime == currentTimeoutTarget) { return; } - if (timeout != null) { timeout.cancel(); } - long now = clock.millis(); long delayMillis = nextDeliveryTime - now; - if (delayMillis < 0) { - // Messages are ready; avoid retriggering timer, dispatcher will pick them on next read return; } - - // Align with tick window like AbstractDelayedDeliveryTracker long remainingTickDelayMillis = lastTickRun + tickTimeMillis - now; long calculatedDelayMillis = Math.max(delayMillis, remainingTickDelayMillis); - currentTimeoutTarget = nextDeliveryTime; - timeout = timer.newTimeout(this, calculatedDelayMillis, java.util.concurrent.TimeUnit.MILLISECONDS); + timeout = timer.newTimeout(this, calculatedDelayMillis, TimeUnit.MILLISECONDS); } private void updateBufferMemoryEstimate() { @@ -451,74 +464,95 @@ private void pruneByMinMarkDelete() { return; } - // Prune entries that are before min mark delete - var iterator = delayedMessageMap.long2ObjectEntrySet().iterator(); - while (iterator.hasNext()) { - var timeEntry = iterator.next(); - Long2ObjectSortedMap ledgerMap = timeEntry.getValue(); - - var ledgerIterator = ledgerMap.long2ObjectEntrySet().iterator(); - while (ledgerIterator.hasNext()) { - var ledgerEntry = ledgerIterator.next(); - long ledgerId = ledgerEntry.getLongKey(); - Roaring64Bitmap entryIds = ledgerEntry.getValue(); - - if (ledgerId < minMarkDelete.getLedgerId()) { - // Entire ledger can be removed - long bytes = entryIds.getLongSizeInBytes(); - delayedMessagesCount.addAndGet(-entryIds.getLongCardinality()); - bufferMemoryBytes.addAndGet(-bytes); - ledgerIterator.remove(); - } else if (ledgerId == minMarkDelete.getLedgerId()) { - // Remove entries <= mark delete entry ID - long removedCount = 0; - long before = entryIds.getLongSizeInBytes(); - var entryIterator = entryIds.iterator(); - while (entryIterator.hasNext()) { - long entryId = entryIterator.next(); - if (entryId <= minMarkDelete.getEntryId()) { - entryIterator.remove(); + // No idempotency set to clean (Option A): rely on per-bitmap removal below + + // Prune per bucket under bucket lock + for (Long ts : new ArrayList<>(delayedMessageMap.keySet())) { + ReentrantLock bLock = bucketLocks.get(ts); + if (bLock == null) { + continue; + } + bLock.lock(); + try { + ConcurrentHashMap ledgerMap = delayedMessageMap.get(ts); + if (ledgerMap == null) { + continue; + } + ArrayList ledgersToRemove = new ArrayList<>(); + for (Map.Entry ledgerEntry : ledgerMap.entrySet()) { + long ledgerId = ledgerEntry.getKey(); + Roaring64Bitmap entryIds = ledgerEntry.getValue(); + if (ledgerId < minMarkDelete.getLedgerId()) { + long bytes = entryIds.getLongSizeInBytes(); + delayedMessagesCount.addAndGet(-entryIds.getLongCardinality()); + bufferMemoryBytes.addAndGet(-bytes); + ledgersToRemove.add(ledgerId); + } else if (ledgerId == minMarkDelete.getLedgerId()) { + long before = entryIds.getLongSizeInBytes(); + long removedCount = 0; + org.roaringbitmap.longlong.LongIterator it = entryIds.getLongIterator(); + java.util.ArrayList toRemove = new java.util.ArrayList<>(); + while (it.hasNext()) { + long e = it.next(); + if (e <= minMarkDelete.getEntryId()) { + toRemove.add(e); + } + } + for (Long e : toRemove) { + entryIds.removeLong(e); removedCount++; } - } - long after = entryIds.getLongSizeInBytes(); - delayedMessagesCount.addAndGet(-removedCount); - bufferMemoryBytes.addAndGet(after - before); - - if (entryIds.isEmpty()) { - ledgerIterator.remove(); + long after = entryIds.getLongSizeInBytes(); + delayedMessagesCount.addAndGet(-removedCount); + bufferMemoryBytes.addAndGet(after - before); + if (entryIds.isEmpty()) { + ledgersToRemove.add(ledgerId); + } } } - } - - if (ledgerMap.isEmpty()) { - iterator.remove(); + for (Long ledgerId : ledgersToRemove) { + ledgerMap.remove(ledgerId); + } + if (ledgerMap.isEmpty()) { + delayedMessageMap.remove(ts); + bucketLocks.remove(ts); + } + } finally { + bLock.unlock(); } } - - updateBufferMemoryEstimate(); } + // idempotency set removed per Option A + @Override public void run(Timeout timeout) throws Exception { if (timeout == null || timeout.isCancelled()) { return; } - java.util.ArrayList toTrigger = new java.util.ArrayList<>(); - synchronized (this) { + // Clear timer state + timerLock.lock(); + try { currentTimeoutTarget = -1; this.timeout = null; lastTickRun = clock.millis(); + } finally { + timerLock.unlock(); + } - // Decide which dispatchers to trigger while holding the lock + ArrayList toTrigger = new ArrayList<>(); + Long earliestTs = delayedMessageMap.firstKey(); + if (earliestTs != null) { for (SubContext subContext : subscriptionContexts.values()) { - if (hasMessageAvailableForSub(subContext)) { + long cutoff = subContext.getCutoffTime(); + if (earliestTs <= cutoff) { toTrigger.add(subContext.getDispatcher()); } } } - // Invoke callbacks outside the manager lock to reduce contention + + // Invoke callbacks outside of locks for (AbstractPersistentDispatcherMultipleConsumers d : toTrigger) { d.readMoreEntriesAsync(); } From 60f6bcdf242af714ffc54a25213c3d5fb4093ae8 Mon Sep 17 00:00:00 2001 From: Denovo1998 Date: Sat, 1 Nov 2025 17:03:23 +0800 Subject: [PATCH 04/16] feat[broker] Add pruning mechanism and improve timestamp handling in InMemoryTopicDelayedDeliveryTrackerManager --- ...oryTopicDelayedDeliveryTrackerManager.java | 60 +++++++++++++------ 1 file changed, 43 insertions(+), 17 deletions(-) diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java index ae06e549153b8..9e40e862c41cb 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java @@ -23,6 +23,7 @@ import io.netty.util.TimerTask; import java.time.Clock; import java.util.ArrayList; +import java.util.List; import java.util.Map; import java.util.NavigableSet; import java.util.TreeSet; @@ -36,6 +37,7 @@ import org.apache.bookkeeper.mledger.Position; import org.apache.bookkeeper.mledger.PositionFactory; import org.apache.pulsar.broker.service.persistent.AbstractPersistentDispatcherMultipleConsumers; +import org.roaringbitmap.longlong.LongIterator; import org.roaringbitmap.longlong.Roaring64Bitmap; /** @@ -72,12 +74,21 @@ public class InMemoryTopicDelayedDeliveryTrackerManager implements TopicDelayedD private final AtomicLong delayedMessagesCount = new AtomicLong(0); private final AtomicLong bufferMemoryBytes = new AtomicLong(0); + // Prune throttling + // Last pruning time + private final AtomicLong lastPruneNanos = new AtomicLong(0); + // Minimum interval between prunes + private final long minPruneIntervalNanos; + // Fixed-delay detection (parity with legacy behavior) private volatile long highestDeliveryTimeTracked = 0; private volatile boolean messagesHaveFixedDelay = true; // Timestamp precision for memory optimization - private int timestampPrecisionBitCnt; + // TODO: Due to the dynamic adjustment of tickTimeMillis, the same message Position(ledgerId, entryId) + // may be bucketed into different timestamp buckets under different time precisions, + // causing "cross-bucket duplicate indexes" and thus duplicate memory occupancy and duplicate returns. + private volatile int timestampPrecisionBitCnt; // Per-bucket locks (timestamp -> lock) for fine-grained concurrency private final ConcurrentHashMap bucketLocks = new ConcurrentHashMap<>(); @@ -85,7 +96,6 @@ public class InMemoryTopicDelayedDeliveryTrackerManager implements TopicDelayedD // Timer state guard private final ReentrantLock timerLock = new ReentrantLock(); - /** * Subscription context that holds per-subscription state. */ @@ -146,6 +156,10 @@ public InMemoryTopicDelayedDeliveryTrackerManager(Timer timer, long tickTimeMill this.fixedDelayDetectionLookahead = fixedDelayDetectionLookahead; this.timestampPrecisionBitCnt = calculateTimestampPrecisionBitCnt(tickTimeMillis); this.onEmptyCallback = onEmptyCallback; + // Default prune throttle interval: clamp to [5ms, 50ms] using tickTimeMillis as hint + // TODO: make configurable if needed + long pruneMs = Math.max(5L, Math.min(50L, tickTimeMillis)); + this.minPruneIntervalNanos = TimeUnit.MILLISECONDS.toNanos(pruneMs); } private static int calculateTimestampPrecisionBitCnt(long tickTimeMillis) { @@ -310,12 +324,14 @@ boolean hasMessageAvailableForSub(SubContext subContext) { if (delayedMessageMap.isEmpty()) { return false; } - Long firstKey = delayedMessageMap.firstKey(); - if (firstKey == null) { + // Use firstEntry() to avoid NoSuchElementException on concurrent empty map + Map.Entry> first = delayedMessageMap.firstEntry(); + if (first == null) { return false; } long cutoffTime = subContext.getCutoffTime(); - return firstKey <= cutoffTime; + long firstTs = first.getKey(); + return firstTs <= cutoffTime; } /** @@ -331,7 +347,7 @@ NavigableSet getScheduledMessagesForSub(SubContext subContext, int max Position markDelete = subContext.getMarkDeletePosition(); // Snapshot of buckets up to cutoff and iterate per-bucket with bucket locks - java.util.List tsList = new java.util.ArrayList<>(delayedMessageMap.headMap(cutoffTime, true).keySet()); + List tsList = new ArrayList<>(delayedMessageMap.headMap(cutoffTime, true).keySet()); for (Long ts : tsList) { if (remaining <= 0) { break; @@ -355,7 +371,7 @@ NavigableSet getScheduledMessagesForSub(SubContext subContext, int max if (markDelete != null && ledgerId < markDelete.getLedgerId()) { continue; } - org.roaringbitmap.longlong.LongIterator it = entryIds.getLongIterator(); + LongIterator it = entryIds.getLongIterator(); while (it.hasNext() && remaining > 0) { long entryId = it.next(); if (markDelete != null && ledgerId == markDelete.getLedgerId() @@ -408,7 +424,9 @@ void updateMarkDeletePosition(SubContext subContext, Position position) { // Private helper methods private void updateTimerLocked() { - if (delayedMessageMap.isEmpty()) { + // Use firstEntry() to avoid NoSuchElementException on concurrent empty map + Map.Entry> first = delayedMessageMap.firstEntry(); + if (first == null) { if (timeout != null) { currentTimeoutTarget = -1; timeout.cancel(); @@ -416,11 +434,7 @@ private void updateTimerLocked() { } return; } - Long nextKey = delayedMessageMap.firstKey(); - if (nextKey == null) { - return; - } - long nextDeliveryTime = nextKey; + long nextDeliveryTime = first.getKey(); if (nextDeliveryTime == currentTimeoutTarget) { return; } @@ -490,8 +504,8 @@ private void pruneByMinMarkDelete() { } else if (ledgerId == minMarkDelete.getLedgerId()) { long before = entryIds.getLongSizeInBytes(); long removedCount = 0; - org.roaringbitmap.longlong.LongIterator it = entryIds.getLongIterator(); - java.util.ArrayList toRemove = new java.util.ArrayList<>(); + LongIterator it = entryIds.getLongIterator(); + ArrayList toRemove = new ArrayList<>(); while (it.hasNext()) { long e = it.next(); if (e <= minMarkDelete.getEntryId()) { @@ -523,6 +537,16 @@ private void pruneByMinMarkDelete() { } } + private void maybePruneByTime() { + long now = System.nanoTime(); + long last = lastPruneNanos.get(); + if (now - last >= minPruneIntervalNanos) { + if (lastPruneNanos.compareAndSet(last, now)) { + pruneByMinMarkDelete(); + } + } + } + // idempotency set removed per Option A @Override @@ -542,8 +566,10 @@ public void run(Timeout timeout) throws Exception { } ArrayList toTrigger = new ArrayList<>(); - Long earliestTs = delayedMessageMap.firstKey(); - if (earliestTs != null) { + // Use firstEntry() to avoid NoSuchElementException on concurrent empty map + Map.Entry> first = delayedMessageMap.firstEntry(); + if (first != null) { + long earliestTs = first.getKey(); for (SubContext subContext : subscriptionContexts.values()) { long cutoff = subContext.getCutoffTime(); if (earliestTs <= cutoff) { From 2c30c9470c687a12aefac7871f1bcebab837bb12 Mon Sep 17 00:00:00 2001 From: Denovo1998 Date: Sat, 1 Nov 2025 17:22:50 +0800 Subject: [PATCH 05/16] [feat][broker] Refactor delayedMessageMap to use Long2ObjectRBTreeMap for improved performance and memory efficiency --- ...oryTopicDelayedDeliveryTrackerManager.java | 46 +++++++++++++------ 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java index 9e40e862c41cb..547b08bdedb01 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java @@ -21,6 +21,8 @@ import io.netty.util.Timeout; import io.netty.util.Timer; import io.netty.util.TimerTask; +import it.unimi.dsi.fastutil.longs.Long2ObjectMap; +import it.unimi.dsi.fastutil.longs.Long2ObjectRBTreeMap; import java.time.Clock; import java.util.ArrayList; import java.util.List; @@ -51,7 +53,7 @@ public class InMemoryTopicDelayedDeliveryTrackerManager implements TopicDelayedD // Global delayed message index: timestamp -> ledgerId -> entryId bitmap // Outer: sorted by timestamp for efficient finding of earliest bucket // Inner: per-ledger bitmaps of entry-ids - private final ConcurrentSkipListMap> delayedMessageMap = + private final ConcurrentSkipListMap> delayedMessageMap = new ConcurrentSkipListMap<>(); // Subscription registry: subscription name -> subscription context @@ -103,11 +105,11 @@ public class InMemoryTopicDelayedDeliveryTrackerManager implements TopicDelayedD static class SubContext { private final AbstractPersistentDispatcherMultipleConsumers dispatcher; private final String subscriptionName; - private long tickTimeMillis; + private volatile long tickTimeMillis; private final boolean isDelayedDeliveryDeliverAtTimeStrict; private final long fixedDelayDetectionLookahead; private final Clock clock; - private Position markDeletePosition; + private volatile Position markDeletePosition; SubContext(AbstractPersistentDispatcherMultipleConsumers dispatcher, long tickTimeMillis, boolean isDelayedDeliveryDeliverAtTimeStrict, long fixedDelayDetectionLookahead, @@ -285,9 +287,13 @@ boolean addMessageForSub(SubContext subContext, long ledgerId, long entryId, lon ReentrantLock bLock = bucketLocks.computeIfAbsent(timestamp, k -> new ReentrantLock()); bLock.lock(); try { - ConcurrentHashMap ledgerMap = - delayedMessageMap.computeIfAbsent(timestamp, k -> new ConcurrentHashMap<>()); - Roaring64Bitmap entryIds = ledgerMap.computeIfAbsent(ledgerId, k -> new Roaring64Bitmap()); + Long2ObjectRBTreeMap ledgerMap = + delayedMessageMap.computeIfAbsent(timestamp, k -> new Long2ObjectRBTreeMap<>()); + Roaring64Bitmap entryIds = ledgerMap.get(ledgerId); + if (entryIds == null) { + entryIds = new Roaring64Bitmap(); + ledgerMap.put(ledgerId, entryIds); + } long before = entryIds.getLongSizeInBytes(); if (!entryIds.contains(entryId)) { entryIds.add(entryId); @@ -325,7 +331,7 @@ boolean hasMessageAvailableForSub(SubContext subContext) { return false; } // Use firstEntry() to avoid NoSuchElementException on concurrent empty map - Map.Entry> first = delayedMessageMap.firstEntry(); + Map.Entry> first = delayedMessageMap.firstEntry(); if (first == null) { return false; } @@ -358,15 +364,15 @@ NavigableSet getScheduledMessagesForSub(SubContext subContext, int max } bLock.lock(); try { - ConcurrentHashMap ledgerMap = delayedMessageMap.get(ts); + Long2ObjectRBTreeMap ledgerMap = delayedMessageMap.get(ts); if (ledgerMap == null) { continue; } - for (Map.Entry ledgerEntry : ledgerMap.entrySet()) { + for (Long2ObjectMap.Entry ledgerEntry : ledgerMap.long2ObjectEntrySet()) { if (remaining <= 0) { break; } - long ledgerId = ledgerEntry.getKey(); + long ledgerId = ledgerEntry.getLongKey(); Roaring64Bitmap entryIds = ledgerEntry.getValue(); if (markDelete != null && ledgerId < markDelete.getLedgerId()) { continue; @@ -425,7 +431,7 @@ void updateMarkDeletePosition(SubContext subContext, Position position) { private void updateTimerLocked() { // Use firstEntry() to avoid NoSuchElementException on concurrent empty map - Map.Entry> first = delayedMessageMap.firstEntry(); + Map.Entry> first = delayedMessageMap.firstEntry(); if (first == null) { if (timeout != null) { currentTimeoutTarget = -1; @@ -488,13 +494,13 @@ private void pruneByMinMarkDelete() { } bLock.lock(); try { - ConcurrentHashMap ledgerMap = delayedMessageMap.get(ts); + Long2ObjectRBTreeMap ledgerMap = delayedMessageMap.get(ts); if (ledgerMap == null) { continue; } ArrayList ledgersToRemove = new ArrayList<>(); - for (Map.Entry ledgerEntry : ledgerMap.entrySet()) { - long ledgerId = ledgerEntry.getKey(); + for (Long2ObjectMap.Entry ledgerEntry : ledgerMap.long2ObjectEntrySet()) { + long ledgerId = ledgerEntry.getLongKey(); Roaring64Bitmap entryIds = ledgerEntry.getValue(); if (ledgerId < minMarkDelete.getLedgerId()) { long bytes = entryIds.getLongSizeInBytes(); @@ -567,7 +573,7 @@ public void run(Timeout timeout) throws Exception { ArrayList toTrigger = new ArrayList<>(); // Use firstEntry() to avoid NoSuchElementException on concurrent empty map - Map.Entry> first = delayedMessageMap.firstEntry(); + Map.Entry> first = delayedMessageMap.firstEntry(); if (first != null) { long earliestTs = first.getKey(); for (SubContext subContext : subscriptionContexts.values()) { @@ -576,6 +582,16 @@ public void run(Timeout timeout) throws Exception { toTrigger.add(subContext.getDispatcher()); } } + + // If a significant portion of subscriptions are eligible, opportunistically prune (throttled) + int subs = subscriptionContexts.size(); + int eligible = toTrigger.size(); + // majority by default + int threshold = Math.max(1, subs / 2); + if (eligible >= threshold) { + // Not under timerLock or any bucket lock; prune uses per-bucket locks and is safe here + maybePruneByTime(); + } } // Invoke callbacks outside of locks From 6fd6ead5e8d29244a7448aa4c71c4f696e3f8397 Mon Sep 17 00:00:00 2001 From: Denovo1998 Date: Sat, 1 Nov 2025 18:08:42 +0800 Subject: [PATCH 06/16] [feat][broker] Refactor mark-delete handling in InMemoryTopicDelayedDeliveryTrackerManager for event-driven updates and improved performance --- ...oryTopicDelayedDeliveryTrackerManager.java | 24 ++++--------------- ...PersistentDispatcherMultipleConsumers.java | 19 +++++++++++++++ ...entDispatcherMultipleConsumersClassic.java | 19 +++++++++++++++ 3 files changed, 43 insertions(+), 19 deletions(-) diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java index 547b08bdedb01..f90e0bc6e594c 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java @@ -347,8 +347,6 @@ NavigableSet getScheduledMessagesForSub(SubContext subContext, int max NavigableSet positions = new TreeSet<>(); int remaining = maxMessages; - // Refresh mark-delete once outside of any bucket lock - refreshMarkDeletePosition(subContext); long cutoffTime = subContext.getCutoffTime(); Position markDelete = subContext.getMarkDeletePosition(); @@ -393,9 +391,9 @@ NavigableSet getScheduledMessagesForSub(SubContext subContext, int max } } - // Prune global index based on min mark-delete across all subscriptions (write path) + // Throttled prune: avoid heavy prune on every hot-path call if (!positions.isEmpty()) { - pruneByMinMarkDelete(); + maybePruneByTime(); } return positions; @@ -423,8 +421,8 @@ void clearForSub() { * Update mark delete position for a subscription. */ void updateMarkDeletePosition(SubContext subContext, Position position) { + // Event-driven update from dispatcher; keep it lightweight (no prune here) subContext.updateMarkDeletePosition(position); - pruneByMinMarkDelete(); } // Private helper methods @@ -601,19 +599,7 @@ public void run(Timeout timeout) throws Exception { } private void refreshMarkDeletePosition(SubContext subContext) { - try { - Position pos = subContext.getDispatcher().getCursor().getMarkDeletedPosition(); - if (pos != null) { - Position current = subContext.getMarkDeletePosition(); - if (current == null || pos.compareTo(current) > 0) { - subContext.updateMarkDeletePosition(pos); - } - } - } catch (Throwable t) { - if (log.isDebugEnabled()) { - log.debug("Failed to refresh mark-delete position for subscription {}", - subContext.getSubscriptionName(), t); - } - } + // Deprecated: mark-delete is now updated via event-driven callbacks from dispatcher. + // Intentionally no-op to keep API surface; will be removed in a later cleanup. } } diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java index 097ab9cd0febf..6bd8c9d2ad5a0 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java @@ -53,6 +53,7 @@ import org.apache.pulsar.broker.delayed.DelayedDeliveryTracker; import org.apache.pulsar.broker.delayed.DelayedDeliveryTrackerFactory; import org.apache.pulsar.broker.delayed.InMemoryDelayedDeliveryTracker; +import org.apache.pulsar.broker.delayed.InMemoryTopicDelayedDeliveryTrackerView; import org.apache.pulsar.broker.delayed.bucket.BucketDelayedDeliveryTracker; import org.apache.pulsar.broker.loadbalance.extensions.data.BrokerLookupData; import org.apache.pulsar.broker.service.AbstractDispatcherMultipleConsumers; @@ -1476,6 +1477,24 @@ public ManagedCursor getCursor() { return cursor; } + @Override + public void markDeletePositionMoveForward() { + // Event-driven mark-delete notification: update the topic-level delayed tracker view + Optional trackerOpt; + synchronized (this) { + trackerOpt = this.delayedDeliveryTracker; + } + trackerOpt.ifPresent(tracker -> { + if (tracker instanceof InMemoryTopicDelayedDeliveryTrackerView view) { + Position md = cursor.getMarkDeletedPosition(); + if (md != null) { + // Only update the cached mark-delete; avoid heavy operations in this callback + view.updateMarkDeletePosition(md); + } + } + }); + } + protected int getStickyKeyHash(Entry entry) { // There's no need to calculate the hash for Shared subscription return STICKY_KEY_HASH_NOT_SET; diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumersClassic.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumersClassic.java index 0f496e461b85c..c1e070d89266e 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumersClassic.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumersClassic.java @@ -50,6 +50,7 @@ import org.apache.pulsar.broker.delayed.DelayedDeliveryTracker; import org.apache.pulsar.broker.delayed.DelayedDeliveryTrackerFactory; import org.apache.pulsar.broker.delayed.InMemoryDelayedDeliveryTracker; +import org.apache.pulsar.broker.delayed.InMemoryTopicDelayedDeliveryTrackerView; import org.apache.pulsar.broker.delayed.bucket.BucketDelayedDeliveryTracker; import org.apache.pulsar.broker.loadbalance.extensions.data.BrokerLookupData; import org.apache.pulsar.broker.service.AbstractDispatcherMultipleConsumers; @@ -1307,6 +1308,24 @@ public ManagedCursor getCursor() { return cursor; } + @Override + public void markDeletePositionMoveForward() { + // Event-driven mark-delete notification: update the topic-level delayed tracker view + Optional trackerOpt; + synchronized (this) { + trackerOpt = this.delayedDeliveryTracker; + } + trackerOpt.ifPresent(tracker -> { + if (tracker instanceof InMemoryTopicDelayedDeliveryTrackerView view) { + Position md = cursor.getMarkDeletedPosition(); + if (md != null) { + // Only update the cached mark-delete; avoid heavy operations in this callback + view.updateMarkDeletePosition(md); + } + } + }); + } + protected int getStickyKeyHash(Entry entry) { return StickyKeyConsumerSelector.STICKY_KEY_HASH_NOT_SET; } From 6784c434c9e435196c458cda960c52383088d06a Mon Sep 17 00:00:00 2001 From: Denovo1998 Date: Sat, 1 Nov 2025 18:58:40 +0800 Subject: [PATCH 07/16] feat[broker] Simplify InMemoryTopicDelayedDeliveryTrackerManager by removing deprecated constructor and methods for improved clarity --- ...MemoryTopicDelayedDeliveryTrackerManager.java | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java index f90e0bc6e594c..2648c35df501d 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java @@ -134,13 +134,6 @@ long getCutoffTime() { private final Runnable onEmptyCallback; - public InMemoryTopicDelayedDeliveryTrackerManager(Timer timer, long tickTimeMillis, - boolean isDelayedDeliveryDeliverAtTimeStrict, - long fixedDelayDetectionLookahead) { - this(timer, tickTimeMillis, Clock.systemUTC(), isDelayedDeliveryDeliverAtTimeStrict, - fixedDelayDetectionLookahead, null); - } - public InMemoryTopicDelayedDeliveryTrackerManager(Timer timer, long tickTimeMillis, Clock clock, boolean isDelayedDeliveryDeliverAtTimeStrict, long fixedDelayDetectionLookahead) { @@ -456,10 +449,6 @@ private void updateTimerLocked() { timeout = timer.newTimeout(this, calculatedDelayMillis, TimeUnit.MILLISECONDS); } - private void updateBufferMemoryEstimate() { - // No-op in incremental mode (kept for compatibility) - } - private long getNumberOfVisibleDelayedMessagesForSub(SubContext subContext) { // Simplified implementation - returns total count // Could be enhanced to count only messages visible to this subscription @@ -597,9 +586,4 @@ public void run(Timeout timeout) throws Exception { d.readMoreEntriesAsync(); } } - - private void refreshMarkDeletePosition(SubContext subContext) { - // Deprecated: mark-delete is now updated via event-driven callbacks from dispatcher. - // Intentionally no-op to keep API surface; will be removed in a later cleanup. - } } From 701ffbf0e2a84ec5dc8a6949ca666473b263c51e Mon Sep 17 00:00:00 2001 From: Denovo1998 Date: Sat, 1 Nov 2025 19:25:17 +0800 Subject: [PATCH 08/16] feat[broker] Add testing-friendly constructor and accessors to InMemoryDelayedDeliveryTrackerFactory for improved testability --- ...InMemoryDelayedDeliveryTrackerFactory.java | 25 ++ ...DelayedDeliveryTrackerFactoryUnitTest.java | 110 +++++ .../InMemoryTopicDeliveryTrackerTest.java | 406 ++++++++++++++++++ 3 files changed, 541 insertions(+) create mode 100644 pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTrackerFactoryUnitTest.java diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTrackerFactory.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTrackerFactory.java index 7bfab427488ff..58840832e1fd6 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTrackerFactory.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTrackerFactory.java @@ -46,6 +46,31 @@ public class InMemoryDelayedDeliveryTrackerFactory implements DelayedDeliveryTra // Cache of topic-level managers: topic name -> manager instance private final ConcurrentMap topicManagers = new ConcurrentHashMap<>(); + public InMemoryDelayedDeliveryTrackerFactory() { + + } + + // Testing-friendly constructor and accessors + @VisibleForTesting + InMemoryDelayedDeliveryTrackerFactory(Timer timer, long tickTimeMillis, + boolean isDelayedDeliveryDeliverAtTimeStrict, + long fixedDelayDetectionLookahead) { + this.timer = timer; + this.tickTimeMillis = tickTimeMillis; + this.isDelayedDeliveryDeliverAtTimeStrict = isDelayedDeliveryDeliverAtTimeStrict; + this.fixedDelayDetectionLookahead = fixedDelayDetectionLookahead; + } + + @VisibleForTesting + int getCachedManagersSize() { + return topicManagers.size(); + } + + @VisibleForTesting + boolean hasManagerForTopic(String topicName) { + return topicManagers.containsKey(topicName); + } + @Override public void initialize(PulsarService pulsarService) { ServiceConfiguration config = pulsarService.getConfig(); diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTrackerFactoryUnitTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTrackerFactoryUnitTest.java new file mode 100644 index 0000000000000..b478dd7737c97 --- /dev/null +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTrackerFactoryUnitTest.java @@ -0,0 +1,110 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pulsar.broker.delayed; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertTrue; +import io.netty.util.Timer; +import org.apache.pulsar.broker.service.Subscription; +import org.apache.pulsar.broker.service.persistent.AbstractPersistentDispatcherMultipleConsumers; +import org.apache.pulsar.broker.service.persistent.PersistentTopic; +import org.testng.annotations.Test; + +@Test(groups = "broker") +public class InMemoryDelayedDeliveryTrackerFactoryUnitTest { + + private static AbstractPersistentDispatcherMultipleConsumers mockDispatcher(String topicName, String subName) + throws Exception { + AbstractPersistentDispatcherMultipleConsumers dispatcher = + mock(AbstractPersistentDispatcherMultipleConsumers.class); + PersistentTopic topic = mock(PersistentTopic.class); + when(topic.getName()).thenReturn(topicName); + when(dispatcher.getTopic()).thenReturn(topic); + Subscription sub = mock(Subscription.class); + when(sub.getName()).thenReturn(subName); + when(dispatcher.getSubscription()).thenReturn(sub); + return dispatcher; + } + + @Test + public void testManagersSharedPerTopicAndIndependentAcrossTopics() throws Exception { + Timer timer = mock(Timer.class); + InMemoryDelayedDeliveryTrackerFactory f = + new InMemoryDelayedDeliveryTrackerFactory(timer, 100, true, 0); + + AbstractPersistentDispatcherMultipleConsumers dA1 = mockDispatcher("persistent://ns/topicA", "sub1"); + AbstractPersistentDispatcherMultipleConsumers dA2 = mockDispatcher("persistent://ns/topicA", "sub2"); + AbstractPersistentDispatcherMultipleConsumers dB1 = mockDispatcher("persistent://ns/topicB", "sub1"); + + DelayedDeliveryTracker vA1 = f.newTracker0(dA1); + DelayedDeliveryTracker vA2 = f.newTracker0(dA2); + DelayedDeliveryTracker vB1 = f.newTracker0(dB1); + + assertEquals(f.getCachedManagersSize(), 2); + assertTrue(f.hasManagerForTopic("persistent://ns/topicA")); + assertTrue(f.hasManagerForTopic("persistent://ns/topicB")); + + // Add an entry via A1 and verify A2 observes the same shared topic-level count, B is independent + org.testng.Assert.assertTrue(vA1.addMessage(1, 1, System.currentTimeMillis() + 60000)); + org.testng.Assert.assertEquals(vA2.getNumberOfDelayedMessages(), 1L); + org.testng.Assert.assertEquals(vB1.getNumberOfDelayedMessages(), 0L); + + vA1.close(); + vA2.close(); + vB1.close(); + } + + @Test + public void testOnEmptyCallbackRemovesManagerFromCache() throws Exception { + Timer timer = mock(Timer.class); + InMemoryDelayedDeliveryTrackerFactory f = + new InMemoryDelayedDeliveryTrackerFactory(timer, 100, true, 0); + + AbstractPersistentDispatcherMultipleConsumers dA1 = mockDispatcher("persistent://ns/topicA", "sub1"); + AbstractPersistentDispatcherMultipleConsumers dA2 = mockDispatcher("persistent://ns/topicA", "sub2"); + + DelayedDeliveryTracker vA1 = f.newTracker0(dA1); + DelayedDeliveryTracker vA2 = f.newTracker0(dA2); + assertEquals(f.getCachedManagersSize(), 1); + + // Close both -> manager should unregister and onEmptyCallback should remove from cache + vA1.close(); + vA2.close(); + assertEquals(f.getCachedManagersSize(), 0); + } + + @Test + public void testFactoryCloseClosesManagersAndStopsTimer() throws Exception { + Timer timer = mock(Timer.class); + InMemoryDelayedDeliveryTrackerFactory f = + new InMemoryDelayedDeliveryTrackerFactory(timer, 100, true, 0); + + AbstractPersistentDispatcherMultipleConsumers dA1 = mockDispatcher("persistent://ns/topicA", "sub1"); + AbstractPersistentDispatcherMultipleConsumers dB1 = mockDispatcher("persistent://ns/topicB", "sub1"); + f.newTracker0(dA1); + f.newTracker0(dB1); + assertEquals(f.getCachedManagersSize(), 2); + + f.close(); + assertEquals(f.getCachedManagersSize(), 0); + // Cannot verify timer.stop() since factory owns the timer; ensure no exception + } +} diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDeliveryTrackerTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDeliveryTrackerTest.java index 9b6b1d3481412..1c8f51ac26229 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDeliveryTrackerTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDeliveryTrackerTest.java @@ -31,10 +31,16 @@ import io.netty.util.Timer; import io.netty.util.TimerTask; import java.time.Clock; +import java.util.ArrayList; +import java.util.List; import java.util.NavigableMap; import java.util.NavigableSet; import java.util.TreeMap; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import org.apache.bookkeeper.mledger.ManagedCursor; import org.apache.bookkeeper.mledger.Position; @@ -338,4 +344,404 @@ public void testBufferMemoryUsageAndCleanup() throws Exception { assertEquals(manager.topicDelayedMessages(), 0); assertEquals(manager.topicBufferMemoryBytes(), 0); } + + @Test + public void testGetScheduledMessagesLimit() throws Exception { + TestEnv env = new TestEnv(); + InMemoryTopicDelayedDeliveryTrackerManager manager = + new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 1, env.clock, true, 0); + ManagedCursor cursor = mock(ManagedCursor.class); + AbstractPersistentDispatcherMultipleConsumers dispatcher = newDispatcher("sub", cursor); + DelayedDeliveryTracker view = manager.createOrGetView(dispatcher); + + env.time.set(1000); + for (int i = 0; i < 10; i++) { + assertTrue(view.addMessage(1, i, 1001)); + } + env.time.set(2000); + NavigableSet positions = view.getScheduledMessages(3); + assertEquals(positions.size(), 3); + + Position prev = null; + for (Position p : positions) { + if (prev != null) { + assertTrue(prev.compareTo(p) < 0); + } + prev = p; + } + + view.close(); + } + + @Test + public void testHasMessageAvailableIgnoresMarkDelete() throws Exception { + TestEnv env = new TestEnv(); + InMemoryTopicDelayedDeliveryTrackerManager manager = + new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 100, env.clock, true, 0); + ManagedCursor cursor = mock(ManagedCursor.class); + AbstractPersistentDispatcherMultipleConsumers dispatcher = newDispatcher("s", cursor); + InMemoryTopicDelayedDeliveryTrackerView view = + (InMemoryTopicDelayedDeliveryTrackerView) manager.createOrGetView(dispatcher); + + env.time.set(900); + assertTrue(view.addMessage(1, 1, 1000)); + env.time.set(1000); + view.updateMarkDeletePosition(PositionFactory.create(1, 1)); + assertTrue(view.hasMessageAvailable()); + assertTrue(view.getScheduledMessages(10).isEmpty()); + + view.close(); + } + + @Test + public void testCrossBucketDuplicatesDedupOnRead() throws Exception { + TestEnv env = new TestEnv(); + long tick = 256; + InMemoryTopicDelayedDeliveryTrackerManager manager = + new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, tick, env.clock, true, 0); + + ManagedCursor c1 = mock(ManagedCursor.class); + ManagedCursor c2 = mock(ManagedCursor.class); + AbstractPersistentDispatcherMultipleConsumers d1 = newDispatcher("s1", c1); + AbstractPersistentDispatcherMultipleConsumers d2 = newDispatcher("s2", c2); + DelayedDeliveryTracker v1 = manager.createOrGetView(d1); + DelayedDeliveryTracker v2 = manager.createOrGetView(d2); + + env.time.set(1000); + long deliverAt = 1023; + assertTrue(v1.addMessage(9, 9, deliverAt)); + long before = manager.topicBufferMemoryBytes(); + + v2.resetTickTime(32); + assertTrue(v2.addMessage(9, 9, deliverAt)); + + env.time.set(2000); + NavigableSet scheduled = v1.getScheduledMessages(10); + assertEquals(scheduled.size(), 1); + assertTrue(manager.topicDelayedMessages() >= 1); + assertTrue(manager.topicBufferMemoryBytes() > before); + + v1.close(); + v2.close(); + } + + @Test + public void testClearIsNoOp() throws Exception { + TestEnv env = new TestEnv(); + InMemoryTopicDelayedDeliveryTrackerManager manager = + new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 1, env.clock, true, 0); + ManagedCursor c = mock(ManagedCursor.class); + AbstractPersistentDispatcherMultipleConsumers d = newDispatcher("s", c); + DelayedDeliveryTracker v = manager.createOrGetView(d); + + env.time.set(0); + assertTrue(v.addMessage(1, 1, 10)); + long before = manager.topicDelayedMessages(); + v.clear().join(); + assertEquals(manager.topicDelayedMessages(), before); + v.close(); + } + + @Test + public void testMultiSubscriptionCloseDoesNotClear() throws Exception { + TestEnv env = new TestEnv(); + InMemoryTopicDelayedDeliveryTrackerManager manager = + new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 1, env.clock, true, 0); + + ManagedCursor c1 = mock(ManagedCursor.class); + ManagedCursor c2 = mock(ManagedCursor.class); + AbstractPersistentDispatcherMultipleConsumers d1 = newDispatcher("s1", c1); + AbstractPersistentDispatcherMultipleConsumers d2 = newDispatcher("s2", c2); + DelayedDeliveryTracker v1 = manager.createOrGetView(d1); + DelayedDeliveryTracker v2 = manager.createOrGetView(d2); + + env.time.set(0); + assertTrue(v1.addMessage(1, 1, 10)); + assertTrue(manager.topicDelayedMessages() > 0); + + v1.close(); + assertTrue(manager.topicDelayedMessages() > 0); + assertFalse(v2.getScheduledMessages(10).isEmpty()); + + v2.close(); + assertEquals(manager.topicDelayedMessages(), 0); + } + + @Test + public void testBoundaryInputsRejected() throws Exception { + TestEnv env = new TestEnv(); + ManagedCursor c = mock(ManagedCursor.class); + AbstractPersistentDispatcherMultipleConsumers d = newDispatcher("s", c); + + InMemoryTopicDelayedDeliveryTrackerManager mStrict = + new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 100, env.clock, true, 0); + DelayedDeliveryTracker vStrict = mStrict.createOrGetView(d); + env.time.set(1000); + assertFalse(vStrict.addMessage(1, 1, -1)); + assertFalse(vStrict.addMessage(1, 2, 1000)); + vStrict.close(); + + InMemoryTopicDelayedDeliveryTrackerManager mNonStrict = + new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 100, env.clock, false, 0); + DelayedDeliveryTracker vNon = mNonStrict.createOrGetView(d); + env.time.set(1000); + assertFalse(vNon.addMessage(1, 3, 1100)); + vNon.close(); + } + + private static void expectIllegalState(Runnable r) { + try { + r.run(); + org.testng.Assert.fail("Expected IllegalStateException"); + } catch (IllegalStateException expected) { + // ok + } + } + + @Test + public void testClosedViewThrowsOnOperations() throws Exception { + TestEnv env = new TestEnv(); + InMemoryTopicDelayedDeliveryTrackerManager manager = + new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 1, env.clock, true, 0); + ManagedCursor c = mock(ManagedCursor.class); + AbstractPersistentDispatcherMultipleConsumers d = newDispatcher("s", c); + InMemoryTopicDelayedDeliveryTrackerView v = + (InMemoryTopicDelayedDeliveryTrackerView) manager.createOrGetView(d); + v.close(); + + expectIllegalState(() -> v.addMessage(1, 1, 10)); + expectIllegalState(v::hasMessageAvailable); + expectIllegalState(() -> v.getScheduledMessages(1)); + expectIllegalState(v::clear); + } + + @Test + public void testRescheduleOnEarlierDeliverAt() throws Exception { + TestEnv env = new TestEnv(); + InMemoryTopicDelayedDeliveryTrackerManager manager = + new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 1, env.clock, true, 0); + ManagedCursor c = mock(ManagedCursor.class); + AbstractPersistentDispatcherMultipleConsumers d = newDispatcher("s", c); + DelayedDeliveryTracker v = manager.createOrGetView(d); + + env.time.set(0); + assertTrue(v.addMessage(1, 1, 10)); + assertEquals(env.tasks.firstKey().longValue(), 10L); + + assertTrue(v.addMessage(1, 2, 5)); + assertEquals(env.tasks.size(), 1); + assertEquals(env.tasks.firstKey().longValue(), 5L); + + v.close(); + } + + @Test + public void testEmptyIndexCancelsTimerOnClose() throws Exception { + TestEnv env = new TestEnv(); + InMemoryTopicDelayedDeliveryTrackerManager manager = + new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 100, env.clock, true, 0); + ManagedCursor c = mock(ManagedCursor.class); + AbstractPersistentDispatcherMultipleConsumers d = newDispatcher("s", c); + DelayedDeliveryTracker v = manager.createOrGetView(d); + + env.time.set(0); + assertTrue(v.addMessage(1, 1, 1000)); + assertTrue(env.tasks.size() >= 1); + v.close(); + assertTrue(env.tasks.isEmpty()); + } + + @Test + public void testMemoryGrowthAndPruneShrink() throws Exception { + TestEnv env = new TestEnv(); + InMemoryTopicDelayedDeliveryTrackerManager manager = + new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 10, env.clock, true, 0); + ManagedCursor c = mock(ManagedCursor.class); + AbstractPersistentDispatcherMultipleConsumers d = newDispatcher("s", c); + InMemoryTopicDelayedDeliveryTrackerView v = + (InMemoryTopicDelayedDeliveryTrackerView) manager.createOrGetView(d); + + env.time.set(0); + for (int i = 0; i < 50; i++) { + assertTrue(v.addMessage(1, i, 100)); + } + long memBefore = manager.topicBufferMemoryBytes(); + assertTrue(memBefore > 0); + + env.time.set(200); + v.updateMarkDeletePosition(PositionFactory.create(1, 25)); + v.getScheduledMessages(100); + // Wait for prune-by-time throttling window and trigger reads to allow prune to occur + long memAfter = manager.topicBufferMemoryBytes(); + long startWall = System.currentTimeMillis(); + while (memAfter >= memBefore && System.currentTimeMillis() - startWall < 2000) { + try { + Thread.sleep(60); + } catch (InterruptedException ignored) { + } + v.getScheduledMessages(1); + memAfter = manager.topicBufferMemoryBytes(); + } + assertTrue(memAfter < memBefore, "Memory should shrink after prune"); + + v.close(); + } + + @Test + public void testTimerCancelAndReschedule() throws Exception { + TestEnv env = new TestEnv(); + InMemoryTopicDelayedDeliveryTrackerManager manager = + new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 10, env.clock, true, 0); + ManagedCursor c = mock(ManagedCursor.class); + AbstractPersistentDispatcherMultipleConsumers d = newDispatcher("s", c); + DelayedDeliveryTracker v = manager.createOrGetView(d); + + env.time.set(0); + assertTrue(v.addMessage(1, 1, 100)); + long first = env.tasks.firstKey(); + assertTrue(first >= 10); + + assertTrue(v.addMessage(1, 2, 50)); + assertEquals(env.tasks.size(), 1); + + v.close(); + } + + @Test + public void testSortedAndDedupScheduled() throws Exception { + TestEnv env = new TestEnv(); + InMemoryTopicDelayedDeliveryTrackerManager manager = + new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 1, env.clock, true, 0); + ManagedCursor c = mock(ManagedCursor.class); + AbstractPersistentDispatcherMultipleConsumers d = newDispatcher("s", c); + DelayedDeliveryTracker v = manager.createOrGetView(d); + + env.time.set(0); + assertTrue(v.addMessage(2, 3, 10)); + assertTrue(v.addMessage(1, 5, 10)); + assertTrue(v.addMessage(1, 5, 10)); + env.time.set(100); + + NavigableSet scheduled = v.getScheduledMessages(10); + assertEquals(scheduled.size(), 2); + List list = new ArrayList<>(scheduled); + assertTrue(list.get(0).compareTo(list.get(1)) < 0); + + v.close(); + } + + @Test + public void testGlobalDelayedCountSemantics() throws Exception { + TestEnv env = new TestEnv(); + InMemoryTopicDelayedDeliveryTrackerManager manager = + new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 1, env.clock, true, 0); + ManagedCursor c = mock(ManagedCursor.class); + AbstractPersistentDispatcherMultipleConsumers d = newDispatcher("s", c); + InMemoryTopicDelayedDeliveryTrackerView v = + (InMemoryTopicDelayedDeliveryTrackerView) manager.createOrGetView(d); + + env.time.set(0); + assertTrue(v.addMessage(1, 1, 10)); + assertEquals(v.getNumberOfDelayedMessages(), 1L); + v.updateMarkDeletePosition(PositionFactory.create(1, 1)); + env.time.set(100); + assertTrue(v.getScheduledMessages(10).isEmpty()); + assertEquals(v.getNumberOfDelayedMessages(), 1L); + v.close(); + } + + @Test(enabled = false) + public void testExpiredDeliverAtShouldScheduleImmediately() throws Exception { + TestEnv env = new TestEnv(); + InMemoryTopicDelayedDeliveryTrackerManager manager = + new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 10, env.clock, true, 0); + ManagedCursor c = mock(ManagedCursor.class); + AbstractPersistentDispatcherMultipleConsumers d = newDispatcher("s", c); + DelayedDeliveryTracker v = manager.createOrGetView(d); + + env.time.set(1000); + assertTrue(v.addMessage(1, 1, 999)); + assertFalse(env.tasks.isEmpty()); + v.close(); + } + + @Test + public void testConcurrentAdditionsSameBucket() throws Exception { + TestEnv env = new TestEnv(); + InMemoryTopicDelayedDeliveryTrackerManager manager = + new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 1, env.clock, true, 0); + ManagedCursor c = mock(ManagedCursor.class); + AbstractPersistentDispatcherMultipleConsumers d = newDispatcher("s", c); + DelayedDeliveryTracker v = manager.createOrGetView(d); + + env.time.set(0); + int threads = 5; + int perThread = 50; + CountDownLatch start = new CountDownLatch(1); + ExecutorService es = Executors.newFixedThreadPool(threads); + for (int t = 0; t < threads; t++) { + final int base = t * perThread; + es.submit(() -> { + try { + start.await(); + for (int i = 0; i < perThread; i++) { + v.addMessage(1, base + i, 10); + } + } catch (InterruptedException ignored) { + } + }); + } + start.countDown(); + es.shutdown(); + es.awaitTermination(10, TimeUnit.SECONDS); + + env.time.set(100); + NavigableSet scheduled = v.getScheduledMessages(threads * perThread); + assertEquals(scheduled.size(), threads * perThread); + assertEquals(manager.topicDelayedMessages(), threads * perThread); + v.close(); + } + + @Test + public void testConcurrentAdditionsMultipleBucketsAndReads() throws Exception { + TestEnv env = new TestEnv(); + InMemoryTopicDelayedDeliveryTrackerManager manager = + new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 5, env.clock, true, 0); + ManagedCursor c = mock(ManagedCursor.class); + AbstractPersistentDispatcherMultipleConsumers d = newDispatcher("s", c); + InMemoryTopicDelayedDeliveryTrackerView v = + (InMemoryTopicDelayedDeliveryTrackerView) manager.createOrGetView(d); + + env.time.set(0); + ExecutorService es = Executors.newFixedThreadPool(2); + AtomicInteger added = new AtomicInteger(); + CountDownLatch start = new CountDownLatch(1); + es.submit(() -> { + try { + start.await(); + for (int i = 0; i < 200; i++) { + if (v.addMessage(1 + (i % 3), i, 10 + (i % 10))) { + added.incrementAndGet(); + } + } + } catch (InterruptedException ignored) { + } + }); + es.submit(() -> { + try { + start.await(); + env.time.set(1000); + for (int i = 0; i < 10; i++) { + v.getScheduledMessages(50); + } + } catch (InterruptedException ignored) { + } + }); + start.countDown(); + es.shutdown(); + es.awaitTermination(10, TimeUnit.SECONDS); + + assertTrue(manager.topicDelayedMessages() >= 0); + v.close(); + } } From c74a87912a1ce619e5ef6072ea8b863ed1bd7aae Mon Sep 17 00:00:00 2001 From: Denovo1998 Date: Sat, 1 Nov 2025 19:57:14 +0800 Subject: [PATCH 09/16] [feat][broker] Refactor timestamp handling and bucket logic in InMemoryTopicDelayedDeliveryTrackerManager for improved accuracy and performance --- ...oryTopicDelayedDeliveryTrackerManager.java | 113 ++++++++++------ .../InMemoryTopicDeliveryTrackerTest.java | 121 ++++++++++-------- 2 files changed, 138 insertions(+), 96 deletions(-) diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java index 2648c35df501d..8738d7a0b2632 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java @@ -86,12 +86,6 @@ public class InMemoryTopicDelayedDeliveryTrackerManager implements TopicDelayedD private volatile long highestDeliveryTimeTracked = 0; private volatile boolean messagesHaveFixedDelay = true; - // Timestamp precision for memory optimization - // TODO: Due to the dynamic adjustment of tickTimeMillis, the same message Position(ledgerId, entryId) - // may be bucketed into different timestamp buckets under different time precisions, - // causing "cross-bucket duplicate indexes" and thus duplicate memory occupancy and duplicate returns. - private volatile int timestampPrecisionBitCnt; - // Per-bucket locks (timestamp -> lock) for fine-grained concurrency private final ConcurrentHashMap bucketLocks = new ConcurrentHashMap<>(); @@ -149,7 +143,6 @@ public InMemoryTopicDelayedDeliveryTrackerManager(Timer timer, long tickTimeMill this.clock = clock; this.isDelayedDeliveryDeliverAtTimeStrict = isDelayedDeliveryDeliverAtTimeStrict; this.fixedDelayDetectionLookahead = fixedDelayDetectionLookahead; - this.timestampPrecisionBitCnt = calculateTimestampPrecisionBitCnt(tickTimeMillis); this.onEmptyCallback = onEmptyCallback; // Default prune throttle interval: clamp to [5ms, 50ms] using tickTimeMillis as hint // TODO: make configurable if needed @@ -157,17 +150,20 @@ public InMemoryTopicDelayedDeliveryTrackerManager(Timer timer, long tickTimeMill this.minPruneIntervalNanos = TimeUnit.MILLISECONDS.toNanos(pruneMs); } - private static int calculateTimestampPrecisionBitCnt(long tickTimeMillis) { - int bitCnt = 0; - while (tickTimeMillis > 0) { - tickTimeMillis >>= 1; - bitCnt++; + // We bucket messages by aligning the deliverAt timestamp to the start of the logical tick window: + // bucketStart = deliverAt - (deliverAt % tickTimeMillis) + // If tickTimeMillis changes over time, the same message may land in different buckets when re-added + // by another subscription. Read paths dedup via TreeSet and counts include duplicates by design. + private long bucketStart(long timestamp) { + long t = this.tickTimeMillis; + if (t <= 0) { + return timestamp; } - return bitCnt > 0 ? bitCnt - 1 : 0; - } - - private static long trimLowerBit(long timestamp, int bits) { - return timestamp & (-1L << bits); + long mod = timestamp % t; + if (mod == 0) { + return timestamp; + } + return timestamp - mod; } @Override @@ -219,8 +215,6 @@ public void onTickTimeUpdated(long newTickTimeMillis) { return; } this.tickTimeMillis = newTickTimeMillis; - // Update precision bits for new tick time (accept old/new buckets co-exist) - this.timestampPrecisionBitCnt = calculateTimestampPrecisionBitCnt(newTickTimeMillis); // Propagate to all subscriptions for (SubContext sc : subscriptionContexts.values()) { sc.tickTimeMillis = newTickTimeMillis; @@ -276,7 +270,7 @@ boolean addMessageForSub(SubContext subContext, long ledgerId, long entryId, lon return false; } - long timestamp = trimLowerBit(deliverAt, timestampPrecisionBitCnt); + long timestamp = bucketStart(deliverAt); ReentrantLock bLock = bucketLocks.computeIfAbsent(timestamp, k -> new ReentrantLock()); bLock.lock(); try { @@ -384,10 +378,9 @@ NavigableSet getScheduledMessagesForSub(SubContext subContext, int max } } - // Throttled prune: avoid heavy prune on every hot-path call - if (!positions.isEmpty()) { - maybePruneByTime(); - } + // Throttled prune: attempt prune even when result is empty (mark-delete might have filtered everything) + // Throttling ensures we don't pay the cost on every call + maybePruneByTime(); return positions; } @@ -432,16 +425,17 @@ private void updateTimerLocked() { return; } long nextDeliveryTime = first.getKey(); - if (nextDeliveryTime == currentTimeoutTarget) { + long now = clock.millis(); + if (timeout != null && nextDeliveryTime == currentTimeoutTarget && currentTimeoutTarget >= now) { return; } if (timeout != null) { timeout.cancel(); } - long now = clock.millis(); long delayMillis = nextDeliveryTime - now; if (delayMillis < 0) { - return; + // Bucket already in the past: schedule immediate to unblock readers + delayMillis = 0; } long remainingTickDelayMillis = lastTickRun + tickTimeMillis - now; long calculatedDelayMillis = Math.max(delayMillis, remainingTickDelayMillis); @@ -456,19 +450,17 @@ private long getNumberOfVisibleDelayedMessagesForSub(SubContext subContext) { } private void pruneByMinMarkDelete() { - // Find the minimum mark delete position across all subscriptions + // Find the minimum mark delete position across all subscriptions. + // If any subscription hasn't established a mark-delete yet, skip pruning to preserve global visibility. Position minMarkDelete = null; for (SubContext subContext : subscriptionContexts.values()) { Position markDelete = subContext.getMarkDeletePosition(); - if (markDelete != null) { - if (minMarkDelete == null || markDelete.compareTo(minMarkDelete) < 0) { - minMarkDelete = markDelete; - } + if (markDelete == null) { + return; // at least one subscription without mark-delete -> no pruning + } + if (minMarkDelete == null || markDelete.compareTo(minMarkDelete) < 0) { + minMarkDelete = markDelete; } - } - - if (minMarkDelete == null) { - return; } // No idempotency set to clean (Option A): rely on per-bitmap removal below @@ -562,10 +554,8 @@ public void run(Timeout timeout) throws Exception { // Use firstEntry() to avoid NoSuchElementException on concurrent empty map Map.Entry> first = delayedMessageMap.firstEntry(); if (first != null) { - long earliestTs = first.getKey(); for (SubContext subContext : subscriptionContexts.values()) { - long cutoff = subContext.getCutoffTime(); - if (earliestTs <= cutoff) { + if (hasVisibleMessageForSub(subContext)) { toTrigger.add(subContext.getDispatcher()); } } @@ -573,10 +563,8 @@ public void run(Timeout timeout) throws Exception { // If a significant portion of subscriptions are eligible, opportunistically prune (throttled) int subs = subscriptionContexts.size(); int eligible = toTrigger.size(); - // majority by default int threshold = Math.max(1, subs / 2); if (eligible >= threshold) { - // Not under timerLock or any bucket lock; prune uses per-bucket locks and is safe here maybePruneByTime(); } } @@ -586,4 +574,47 @@ public void run(Timeout timeout) throws Exception { d.readMoreEntriesAsync(); } } + + private boolean hasVisibleMessageForSub(SubContext subContext) { + long cutoffTime = subContext.getCutoffTime(); + Position markDelete = subContext.getMarkDeletePosition(); + List tsList = new ArrayList<>(delayedMessageMap.headMap(cutoffTime, true).keySet()); + for (Long ts : tsList) { + ReentrantLock bLock = bucketLocks.get(ts); + if (bLock == null) { + continue; + } + bLock.lock(); + try { + Long2ObjectRBTreeMap ledgerMap = delayedMessageMap.get(ts); + if (ledgerMap == null) { + continue; + } + for (Long2ObjectMap.Entry ledgerEntry : ledgerMap.long2ObjectEntrySet()) { + long ledgerId = ledgerEntry.getLongKey(); + if (markDelete != null && ledgerId < markDelete.getLedgerId()) { + continue; + } + Roaring64Bitmap entryIds = ledgerEntry.getValue(); + if (markDelete == null || ledgerId > markDelete.getLedgerId()) { + // at least one entry exists in this ledger bucket + if (!entryIds.isEmpty()) { + return true; + } + } else { + LongIterator it = entryIds.getLongIterator(); + while (it.hasNext()) { + long e = it.next(); + if (e > markDelete.getEntryId()) { + return true; + } + } + } + } + } finally { + bLock.unlock(); + } + } + return false; + } } diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDeliveryTrackerTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDeliveryTrackerTest.java index 1c8f51ac26229..37699506ef88d 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDeliveryTrackerTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDeliveryTrackerTest.java @@ -94,7 +94,7 @@ private static AbstractPersistentDispatcherMultipleConsumers newDispatcher(Strin return dispatcher; } - @Test + @Test(invocationCount = 10) public void testSingleSubscriptionBasicFlow() throws Exception { TestEnv env = new TestEnv(); long tickMs = 100; @@ -129,17 +129,25 @@ public void testSingleSubscriptionBasicFlow() throws Exception { assertEquals(view.getNumberOfDelayedMessages(), 3); // Mark-delete beyond the scheduled positions and prune - when(cursor.getMarkDeletedPosition()).thenReturn(PositionFactory.create(1L, 2L)); + ((InMemoryTopicDelayedDeliveryTrackerView) view) + .updateMarkDeletePosition(PositionFactory.create(1L, 2L)); // Trigger pruning by another get view.getScheduledMessages(10); - // Now only one entry remains in global index + // Now only one entry should remain in global index (prune is throttled -> wait up to 2s) + long start = System.currentTimeMillis(); + while (view.getNumberOfDelayedMessages() != 1 && System.currentTimeMillis() - start < 2000) { + try { + Thread.sleep(30); + } catch (InterruptedException ignored) {} + view.getScheduledMessages(1); + } assertEquals(view.getNumberOfDelayedMessages(), 1); // Cleanup view.close(); } - @Test + @Test(invocationCount = 10) public void testSharedIndexDedupAcrossSubscriptions() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -165,7 +173,7 @@ public void testSharedIndexDedupAcrossSubscriptions() throws Exception { v2.close(); } - @Test + @Test(invocationCount = 10) public void testTimerRunTriggersOnlyAvailableSubscriptions() throws Exception { TestEnv env = new TestEnv(); long tickMs = 100; @@ -190,8 +198,10 @@ public void testTimerRunTriggersOnlyAvailableSubscriptions() throws Exception { // Set time after cutoff and set sub-a mark-delete behind entries, sub-b beyond entries env.time.set(600); - when(c1.getMarkDeletedPosition()).thenReturn(PositionFactory.create(0L, 0L)); // visible - when(c2.getMarkDeletedPosition()).thenReturn(PositionFactory.create(1L, 5L)); // not visible + ((InMemoryTopicDelayedDeliveryTrackerView) v1) + .updateMarkDeletePosition(PositionFactory.create(0L, 0L)); // visible for sub-a + ((InMemoryTopicDelayedDeliveryTrackerView) v2) + .updateMarkDeletePosition(PositionFactory.create(1L, 5L)); // filtered for sub-b // Invoke manager timer task directly manager.run(mock(Timeout.class)); @@ -204,7 +214,7 @@ public void testTimerRunTriggersOnlyAvailableSubscriptions() throws Exception { v2.close(); } - @Test + @Test(invocationCount = 10) public void testPauseWithFixedDelays() throws Exception { TestEnv env = new TestEnv(); long lookahead = 5; @@ -230,7 +240,7 @@ public void testPauseWithFixedDelays() throws Exception { view.close(); } - @Test + @Test(invocationCount = 10) public void testDynamicTickTimeUpdateAffectsCutoff() throws Exception { TestEnv env = new TestEnv(); // non-strict mode: cutoff = now + tick @@ -254,7 +264,7 @@ public void testDynamicTickTimeUpdateAffectsCutoff() throws Exception { view.close(); } - @Test + @Test(invocationCount = 10) public void testMinMarkDeleteAcrossSubscriptions() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -287,14 +297,22 @@ public void testMinMarkDeleteAcrossSubscriptions() throws Exception { // Advance c1 mark-delete beyond (1,2) v1.updateMarkDeletePosition(PositionFactory.create(1L, 2L)); v1.getScheduledMessages(10); - // Now only (2,1) should remain + long start2 = System.currentTimeMillis(); + while (v1.getNumberOfDelayedMessages() != 1 && System.currentTimeMillis() - start2 < 2000) { + try { + Thread.sleep(30); + } catch (InterruptedException ignored) { + + } + v1.getScheduledMessages(1); + } assertEquals(v1.getNumberOfDelayedMessages(), 1); v1.close(); v2.close(); } - @Test + @Test(invocationCount = 10) public void testTimerSchedulingWindowAlignment() throws Exception { TestEnv env = new TestEnv(); long tickMs = 1000; @@ -317,15 +335,15 @@ public void testTimerSchedulingWindowAlignment() throws Exception { // If no recent tick run, deliverAt should determine env.tasks.clear(); env.time.set(20000); - // No run -> lastTickRun remains 10000; deliverAt=20005 < lastTickRun+tick(11000)? no, so schedule at deliverAt + // No run -> lastTickRun remains 10000; bucketStart(20005)=20000; schedule aligns to bucket start immediately assertTrue(view.addMessage(1, 2, 20005)); long scheduledAt2 = env.tasks.firstKey(); - assertEquals(scheduledAt2, 20005); + assertEquals(scheduledAt2, 20000); view.close(); } - @Test + @Test(invocationCount = 10) public void testBufferMemoryUsageAndCleanup() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -345,7 +363,7 @@ public void testBufferMemoryUsageAndCleanup() throws Exception { assertEquals(manager.topicBufferMemoryBytes(), 0); } - @Test + @Test(invocationCount = 10) public void testGetScheduledMessagesLimit() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -373,7 +391,7 @@ public void testGetScheduledMessagesLimit() throws Exception { view.close(); } - @Test + @Test(invocationCount = 10) public void testHasMessageAvailableIgnoresMarkDelete() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -393,7 +411,7 @@ public void testHasMessageAvailableIgnoresMarkDelete() throws Exception { view.close(); } - @Test + @Test(invocationCount = 10) public void testCrossBucketDuplicatesDedupOnRead() throws Exception { TestEnv env = new TestEnv(); long tick = 256; @@ -425,7 +443,7 @@ public void testCrossBucketDuplicatesDedupOnRead() throws Exception { v2.close(); } - @Test + @Test(invocationCount = 10) public void testClearIsNoOp() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -442,7 +460,7 @@ public void testClearIsNoOp() throws Exception { v.close(); } - @Test + @Test(invocationCount = 10) public void testMultiSubscriptionCloseDoesNotClear() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -461,13 +479,15 @@ public void testMultiSubscriptionCloseDoesNotClear() throws Exception { v1.close(); assertTrue(manager.topicDelayedMessages() > 0); + // Move time forward so remaining view can read + env.time.set(20); assertFalse(v2.getScheduledMessages(10).isEmpty()); v2.close(); assertEquals(manager.topicDelayedMessages(), 0); } - @Test + @Test(invocationCount = 10) public void testBoundaryInputsRejected() throws Exception { TestEnv env = new TestEnv(); ManagedCursor c = mock(ManagedCursor.class); @@ -498,7 +518,7 @@ private static void expectIllegalState(Runnable r) { } } - @Test + @Test(invocationCount = 10) public void testClosedViewThrowsOnOperations() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -515,7 +535,7 @@ public void testClosedViewThrowsOnOperations() throws Exception { expectIllegalState(v::clear); } - @Test + @Test(invocationCount = 10) public void testRescheduleOnEarlierDeliverAt() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -535,7 +555,7 @@ public void testRescheduleOnEarlierDeliverAt() throws Exception { v.close(); } - @Test + @Test(invocationCount = 10) public void testEmptyIndexCancelsTimerOnClose() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -551,7 +571,7 @@ public void testEmptyIndexCancelsTimerOnClose() throws Exception { assertTrue(env.tasks.isEmpty()); } - @Test + @Test(invocationCount = 10) public void testMemoryGrowthAndPruneShrink() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -587,7 +607,7 @@ public void testMemoryGrowthAndPruneShrink() throws Exception { v.close(); } - @Test + @Test(invocationCount = 10) public void testTimerCancelAndReschedule() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -607,7 +627,7 @@ public void testTimerCancelAndReschedule() throws Exception { v.close(); } - @Test + @Test(invocationCount = 10) public void testSortedAndDedupScheduled() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -630,42 +650,33 @@ public void testSortedAndDedupScheduled() throws Exception { v.close(); } - @Test + @Test(invocationCount = 10) public void testGlobalDelayedCountSemantics() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 1, env.clock, true, 0); - ManagedCursor c = mock(ManagedCursor.class); - AbstractPersistentDispatcherMultipleConsumers d = newDispatcher("s", c); - InMemoryTopicDelayedDeliveryTrackerView v = - (InMemoryTopicDelayedDeliveryTrackerView) manager.createOrGetView(d); + ManagedCursor c1 = mock(ManagedCursor.class); + ManagedCursor c2 = mock(ManagedCursor.class); + AbstractPersistentDispatcherMultipleConsumers d1 = newDispatcher("s1", c1); + AbstractPersistentDispatcherMultipleConsumers d2 = newDispatcher("s2", c2); + InMemoryTopicDelayedDeliveryTrackerView v1 = + (InMemoryTopicDelayedDeliveryTrackerView) manager.createOrGetView(d1); + InMemoryTopicDelayedDeliveryTrackerView v2 = + (InMemoryTopicDelayedDeliveryTrackerView) manager.createOrGetView(d2); env.time.set(0); - assertTrue(v.addMessage(1, 1, 10)); - assertEquals(v.getNumberOfDelayedMessages(), 1L); - v.updateMarkDeletePosition(PositionFactory.create(1, 1)); + assertTrue(v1.addMessage(1, 1, 10)); + assertEquals(v1.getNumberOfDelayedMessages(), 1L); + // Hide for sub-1 only; keep sub-2 without mark-delete so global min mark-delete doesn't prune + v1.updateMarkDeletePosition(PositionFactory.create(1, 1)); env.time.set(100); - assertTrue(v.getScheduledMessages(10).isEmpty()); - assertEquals(v.getNumberOfDelayedMessages(), 1L); - v.close(); - } - - @Test(enabled = false) - public void testExpiredDeliverAtShouldScheduleImmediately() throws Exception { - TestEnv env = new TestEnv(); - InMemoryTopicDelayedDeliveryTrackerManager manager = - new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 10, env.clock, true, 0); - ManagedCursor c = mock(ManagedCursor.class); - AbstractPersistentDispatcherMultipleConsumers d = newDispatcher("s", c); - DelayedDeliveryTracker v = manager.createOrGetView(d); - - env.time.set(1000); - assertTrue(v.addMessage(1, 1, 999)); - assertFalse(env.tasks.isEmpty()); - v.close(); + assertTrue(v1.getScheduledMessages(10).isEmpty()); + assertEquals(v1.getNumberOfDelayedMessages(), 1L); + v1.close(); + v2.close(); } - @Test + @Test(invocationCount = 10) public void testConcurrentAdditionsSameBucket() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -702,7 +713,7 @@ public void testConcurrentAdditionsSameBucket() throws Exception { v.close(); } - @Test + @Test(invocationCount = 10) public void testConcurrentAdditionsMultipleBucketsAndReads() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = From e576c658f4269fdd75ab4453c5e0286cb7ba5c83 Mon Sep 17 00:00:00 2001 From: Denovo1998 Date: Sat, 1 Nov 2025 20:07:42 +0800 Subject: [PATCH 10/16] [feat][broker] Remove deprecated methods and simplify test annotations in InMemoryTopicDelayedDeliveryTrackerManager for improved clarity and maintainability --- ...oryTopicDelayedDeliveryTrackerManager.java | 6 -- .../InMemoryTopicDeliveryTrackerTest.java | 59 +++++++++---------- 2 files changed, 29 insertions(+), 36 deletions(-) diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java index 8738d7a0b2632..02f3ccac27b6b 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java @@ -260,8 +260,6 @@ public void close() { bufferMemoryBytes.set(0); } - // Internal methods for subscription views - /** * Add a message to the global delayed message index. */ @@ -411,8 +409,6 @@ void updateMarkDeletePosition(SubContext subContext, Position position) { subContext.updateMarkDeletePosition(position); } - // Private helper methods - private void updateTimerLocked() { // Use firstEntry() to avoid NoSuchElementException on concurrent empty map Map.Entry> first = delayedMessageMap.firstEntry(); @@ -532,8 +528,6 @@ private void maybePruneByTime() { } } - // idempotency set removed per Option A - @Override public void run(Timeout timeout) throws Exception { if (timeout == null || timeout.isCancelled()) { diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDeliveryTrackerTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDeliveryTrackerTest.java index 37699506ef88d..39e1bf1f31370 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDeliveryTrackerTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDeliveryTrackerTest.java @@ -83,8 +83,7 @@ private static class TestEnv { } } - private static AbstractPersistentDispatcherMultipleConsumers newDispatcher(String subName, ManagedCursor cursor) - throws Exception { + private static AbstractPersistentDispatcherMultipleConsumers newDispatcher(String subName, ManagedCursor cursor) { AbstractPersistentDispatcherMultipleConsumers dispatcher = mock(AbstractPersistentDispatcherMultipleConsumers.class); Subscription subscription = mock(Subscription.class); @@ -94,7 +93,7 @@ private static AbstractPersistentDispatcherMultipleConsumers newDispatcher(Strin return dispatcher; } - @Test(invocationCount = 10) + @Test public void testSingleSubscriptionBasicFlow() throws Exception { TestEnv env = new TestEnv(); long tickMs = 100; @@ -147,7 +146,7 @@ public void testSingleSubscriptionBasicFlow() throws Exception { view.close(); } - @Test(invocationCount = 10) + @Test public void testSharedIndexDedupAcrossSubscriptions() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -173,7 +172,7 @@ public void testSharedIndexDedupAcrossSubscriptions() throws Exception { v2.close(); } - @Test(invocationCount = 10) + @Test public void testTimerRunTriggersOnlyAvailableSubscriptions() throws Exception { TestEnv env = new TestEnv(); long tickMs = 100; @@ -214,7 +213,7 @@ public void testTimerRunTriggersOnlyAvailableSubscriptions() throws Exception { v2.close(); } - @Test(invocationCount = 10) + @Test public void testPauseWithFixedDelays() throws Exception { TestEnv env = new TestEnv(); long lookahead = 5; @@ -229,7 +228,7 @@ public void testPauseWithFixedDelays() throws Exception { // Add strictly increasing deliverAt times (fixed delay scenario) env.time.set(0); for (int i = 1; i <= lookahead; i++) { - assertTrue(view.addMessage(i, i, i * 100)); + assertTrue(view.addMessage(i, i, i * 100L)); } assertTrue(view.shouldPauseAllDeliveries()); @@ -240,7 +239,7 @@ public void testPauseWithFixedDelays() throws Exception { view.close(); } - @Test(invocationCount = 10) + @Test public void testDynamicTickTimeUpdateAffectsCutoff() throws Exception { TestEnv env = new TestEnv(); // non-strict mode: cutoff = now + tick @@ -264,7 +263,7 @@ public void testDynamicTickTimeUpdateAffectsCutoff() throws Exception { view.close(); } - @Test(invocationCount = 10) + @Test public void testMinMarkDeleteAcrossSubscriptions() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -285,9 +284,9 @@ public void testMinMarkDeleteAcrossSubscriptions() throws Exception { assertTrue(v1.addMessage(2, 1, 100)); assertEquals(v1.getNumberOfDelayedMessages(), 3); - // c1 behind, c2 ahead - when(c1.getMarkDeletedPosition()).thenReturn(PositionFactory.create(0L, 0L)); - when(c2.getMarkDeletedPosition()).thenReturn(PositionFactory.create(10L, 10L)); + // c1 behind, c2 ahead (set via view so manager receives updates) + v1.updateMarkDeletePosition(PositionFactory.create(0L, 0L)); + v2.updateMarkDeletePosition(PositionFactory.create(10L, 10L)); env.time.set(200); // Trigger v2 read + prune attempt; min mark-delete still from c1 => no prune @@ -312,7 +311,7 @@ public void testMinMarkDeleteAcrossSubscriptions() throws Exception { v2.close(); } - @Test(invocationCount = 10) + @Test public void testTimerSchedulingWindowAlignment() throws Exception { TestEnv env = new TestEnv(); long tickMs = 1000; @@ -343,7 +342,7 @@ public void testTimerSchedulingWindowAlignment() throws Exception { view.close(); } - @Test(invocationCount = 10) + @Test public void testBufferMemoryUsageAndCleanup() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -363,7 +362,7 @@ public void testBufferMemoryUsageAndCleanup() throws Exception { assertEquals(manager.topicBufferMemoryBytes(), 0); } - @Test(invocationCount = 10) + @Test public void testGetScheduledMessagesLimit() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -391,7 +390,7 @@ public void testGetScheduledMessagesLimit() throws Exception { view.close(); } - @Test(invocationCount = 10) + @Test public void testHasMessageAvailableIgnoresMarkDelete() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -411,7 +410,7 @@ public void testHasMessageAvailableIgnoresMarkDelete() throws Exception { view.close(); } - @Test(invocationCount = 10) + @Test public void testCrossBucketDuplicatesDedupOnRead() throws Exception { TestEnv env = new TestEnv(); long tick = 256; @@ -443,7 +442,7 @@ public void testCrossBucketDuplicatesDedupOnRead() throws Exception { v2.close(); } - @Test(invocationCount = 10) + @Test public void testClearIsNoOp() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -460,7 +459,7 @@ public void testClearIsNoOp() throws Exception { v.close(); } - @Test(invocationCount = 10) + @Test public void testMultiSubscriptionCloseDoesNotClear() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -487,7 +486,7 @@ public void testMultiSubscriptionCloseDoesNotClear() throws Exception { assertEquals(manager.topicDelayedMessages(), 0); } - @Test(invocationCount = 10) + @Test public void testBoundaryInputsRejected() throws Exception { TestEnv env = new TestEnv(); ManagedCursor c = mock(ManagedCursor.class); @@ -518,7 +517,7 @@ private static void expectIllegalState(Runnable r) { } } - @Test(invocationCount = 10) + @Test public void testClosedViewThrowsOnOperations() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -535,7 +534,7 @@ public void testClosedViewThrowsOnOperations() throws Exception { expectIllegalState(v::clear); } - @Test(invocationCount = 10) + @Test public void testRescheduleOnEarlierDeliverAt() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -555,7 +554,7 @@ public void testRescheduleOnEarlierDeliverAt() throws Exception { v.close(); } - @Test(invocationCount = 10) + @Test public void testEmptyIndexCancelsTimerOnClose() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -566,12 +565,12 @@ public void testEmptyIndexCancelsTimerOnClose() throws Exception { env.time.set(0); assertTrue(v.addMessage(1, 1, 1000)); - assertTrue(env.tasks.size() >= 1); + assertFalse(env.tasks.isEmpty()); v.close(); assertTrue(env.tasks.isEmpty()); } - @Test(invocationCount = 10) + @Test public void testMemoryGrowthAndPruneShrink() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -607,7 +606,7 @@ public void testMemoryGrowthAndPruneShrink() throws Exception { v.close(); } - @Test(invocationCount = 10) + @Test public void testTimerCancelAndReschedule() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -627,7 +626,7 @@ public void testTimerCancelAndReschedule() throws Exception { v.close(); } - @Test(invocationCount = 10) + @Test public void testSortedAndDedupScheduled() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -650,7 +649,7 @@ public void testSortedAndDedupScheduled() throws Exception { v.close(); } - @Test(invocationCount = 10) + @Test public void testGlobalDelayedCountSemantics() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -676,7 +675,7 @@ public void testGlobalDelayedCountSemantics() throws Exception { v2.close(); } - @Test(invocationCount = 10) + @Test public void testConcurrentAdditionsSameBucket() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = @@ -713,7 +712,7 @@ public void testConcurrentAdditionsSameBucket() throws Exception { v.close(); } - @Test(invocationCount = 10) + @Test public void testConcurrentAdditionsMultipleBucketsAndReads() throws Exception { TestEnv env = new TestEnv(); InMemoryTopicDelayedDeliveryTrackerManager manager = From 938e4f6d5203d72b9d4c9030bccb7ed6cf5906b7 Mon Sep 17 00:00:00 2001 From: Denovo1998 Date: Sat, 1 Nov 2025 20:21:49 +0800 Subject: [PATCH 11/16] [feat][broker] Replace highestDeliveryTimeTracked with AtomicLong for thread-safe updates and improve concurrency handling in InMemoryTopicDelayedDeliveryTrackerManager --- .../InMemoryTopicDelayedDeliveryTrackerManager.java | 13 ++++++++----- .../InMemoryTopicDelayedDeliveryTrackerView.java | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java index 02f3ccac27b6b..c16c4f599255d 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java @@ -83,7 +83,7 @@ public class InMemoryTopicDelayedDeliveryTrackerManager implements TopicDelayedD private final long minPruneIntervalNanos; // Fixed-delay detection (parity with legacy behavior) - private volatile long highestDeliveryTimeTracked = 0; + private final AtomicLong highestDeliveryTimeTracked = new AtomicLong(0); private volatile boolean messagesHaveFixedDelay = true; // Per-bucket locks (timestamp -> lock) for fine-grained concurrency @@ -302,10 +302,13 @@ boolean addMessageForSub(SubContext subContext, long ledgerId, long entryId, lon } private void checkAndUpdateHighest(long deliverAt) { - if (deliverAt < (highestDeliveryTimeTracked - tickTimeMillis)) { - messagesHaveFixedDelay = false; - } - highestDeliveryTimeTracked = Math.max(highestDeliveryTimeTracked, deliverAt); + long current; + do { + current = highestDeliveryTimeTracked.get(); + if (deliverAt < (current - tickTimeMillis)) { + messagesHaveFixedDelay = false; + } + } while (deliverAt > current && !highestDeliveryTimeTracked.compareAndSet(current, deliverAt)); } /** diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerView.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerView.java index c7af60d50924b..072e5b2661a6b 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerView.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerView.java @@ -33,7 +33,7 @@ public class InMemoryTopicDelayedDeliveryTrackerView implements DelayedDeliveryT private final InMemoryTopicDelayedDeliveryTrackerManager manager; private final InMemoryTopicDelayedDeliveryTrackerManager.SubContext subContext; - private boolean closed = false; + private volatile boolean closed = false; public InMemoryTopicDelayedDeliveryTrackerView(InMemoryTopicDelayedDeliveryTrackerManager manager, InMemoryTopicDelayedDeliveryTrackerManager.SubContext subContext) { From e1f2c898104d0be21b01422c9a69341892c7d0db Mon Sep 17 00:00:00 2001 From: Denovo1998 Date: Sun, 2 Nov 2025 11:25:19 +0800 Subject: [PATCH 12/16] [feat][broker] Introduce InMemoryTopicDelayedDeliveryTrackerFactory for topic-level delayed message tracking and update related configurations --- conf/broker.conf | 2 + conf/standalone.conf | 2 + .../pulsar/broker/ServiceConfiguration.java | 6 +- ...InMemoryDelayedDeliveryTrackerFactory.java | 59 +------- ...oryTopicDelayedDeliveryTrackerFactory.java | 135 ++++++++++++++++++ .../DelayedDeliveryTrackerFactoryTest.java | 6 +- ...picDelayedDeliveryTrackerFactoryTest.java} | 17 ++- 7 files changed, 158 insertions(+), 69 deletions(-) create mode 100644 pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerFactory.java rename pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/{InMemoryDelayedDeliveryTrackerFactoryUnitTest.java => InMemoryTopicDelayedDeliveryTrackerFactoryTest.java} (89%) diff --git a/conf/broker.conf b/conf/broker.conf index 8fd0e18af3696..389842fd6b22e 100644 --- a/conf/broker.conf +++ b/conf/broker.conf @@ -654,6 +654,8 @@ delayedDeliveryEnabled=true # Class name of the factory that implements the delayed deliver tracker. # If value is "org.apache.pulsar.broker.delayed.BucketDelayedDeliveryTrackerFactory", # will create bucket based delayed message index tracker. +# If value is "org.apache.pulsar.broker.delayed.InMemoryTopicDelayedDeliveryTrackerFactory", +# will use topic-level in-memory delayed message index tracker (alias of the default in-memory implementation). delayedDeliveryTrackerFactoryClassName=org.apache.pulsar.broker.delayed.InMemoryDelayedDeliveryTrackerFactory # Control the tick time for when retrying on delayed delivery, diff --git a/conf/standalone.conf b/conf/standalone.conf index 708d4905b8ab3..48199a0f15e55 100644 --- a/conf/standalone.conf +++ b/conf/standalone.conf @@ -1422,6 +1422,8 @@ delayedDeliveryEnabled=true # Class name of the factory that implements the delayed deliver tracker. # If value is "org.apache.pulsar.broker.delayed.BucketDelayedDeliveryTrackerFactory", # will create bucket based delayed message index tracker. +# If value is "org.apache.pulsar.broker.delayed.InMemoryTopicDelayedDeliveryTrackerFactory", +# will use topic-level in-memory delayed message index tracker (alias of the default in-memory implementation). delayedDeliveryTrackerFactoryClassName=org.apache.pulsar.broker.delayed.InMemoryDelayedDeliveryTrackerFactory # Control the tick time for when retrying on delayed delivery, diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java index 30fef55ece3bd..9b3fd19ec7f4f 100644 --- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java +++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java @@ -370,7 +370,11 @@ public class ServiceConfiguration implements PulsarConfiguration { @FieldContext(category = CATEGORY_SERVER, doc = """ Class name of the factory that implements the delayed deliver tracker. If value is "org.apache.pulsar.broker.delayed.BucketDelayedDeliveryTrackerFactory", \ - will create bucket based delayed message index tracker. + will create bucket based delayed message index tracker.\n + If value is "org.apache.pulsar.broker.delayed.InMemoryTopicDelayedDeliveryTrackerFactory", \ + will create topic-level in-memory delayed message index tracker.\n + If value is "org.apache.pulsar.broker.delayed.InMemoryDelayedDeliveryTrackerFactory", \ + will create in-memory delayed delivery tracker (per existing implementation). """) private String delayedDeliveryTrackerFactoryClassName = "org.apache.pulsar.broker.delayed" + ".InMemoryDelayedDeliveryTrackerFactory"; diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTrackerFactory.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTrackerFactory.java index 58840832e1fd6..c8137d1ed917c 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTrackerFactory.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTrackerFactory.java @@ -22,9 +22,6 @@ import io.netty.util.HashedWheelTimer; import io.netty.util.Timer; import io.netty.util.concurrent.DefaultThreadFactory; -import java.time.Clock; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; import java.util.concurrent.TimeUnit; import org.apache.pulsar.broker.PulsarService; import org.apache.pulsar.broker.ServiceConfiguration; @@ -43,34 +40,6 @@ public class InMemoryDelayedDeliveryTrackerFactory implements DelayedDeliveryTra private long fixedDelayDetectionLookahead; - // Cache of topic-level managers: topic name -> manager instance - private final ConcurrentMap topicManagers = new ConcurrentHashMap<>(); - - public InMemoryDelayedDeliveryTrackerFactory() { - - } - - // Testing-friendly constructor and accessors - @VisibleForTesting - InMemoryDelayedDeliveryTrackerFactory(Timer timer, long tickTimeMillis, - boolean isDelayedDeliveryDeliverAtTimeStrict, - long fixedDelayDetectionLookahead) { - this.timer = timer; - this.tickTimeMillis = tickTimeMillis; - this.isDelayedDeliveryDeliverAtTimeStrict = isDelayedDeliveryDeliverAtTimeStrict; - this.fixedDelayDetectionLookahead = fixedDelayDetectionLookahead; - } - - @VisibleForTesting - int getCachedManagersSize() { - return topicManagers.size(); - } - - @VisibleForTesting - boolean hasManagerForTopic(String topicName) { - return topicManagers.containsKey(topicName); - } - @Override public void initialize(PulsarService pulsarService) { ServiceConfiguration config = pulsarService.getConfig(); @@ -97,35 +66,13 @@ public DelayedDeliveryTracker newTracker(AbstractPersistentDispatcherMultipleCon } @VisibleForTesting - DelayedDeliveryTracker newTracker0(AbstractPersistentDispatcherMultipleConsumers dispatcher) { - String topicName = dispatcher.getTopic().getName(); - - // Get or create topic-level manager for this topic with onEmpty callback to remove from cache - final TopicDelayedDeliveryTrackerManager[] holder = new TopicDelayedDeliveryTrackerManager[1]; - TopicDelayedDeliveryTrackerManager manager = topicManagers.computeIfAbsent(topicName, k -> { - InMemoryTopicDelayedDeliveryTrackerManager m = new InMemoryTopicDelayedDeliveryTrackerManager( - timer, tickTimeMillis, Clock.systemUTC(), isDelayedDeliveryDeliverAtTimeStrict, - fixedDelayDetectionLookahead, () -> topicManagers.remove(topicName, holder[0])); - holder[0] = m; - return m; - }); - - // Create a per-subscription view from the topic-level manager - return manager.createOrGetView(dispatcher); + InMemoryDelayedDeliveryTracker newTracker0(AbstractPersistentDispatcherMultipleConsumers dispatcher) { + return new InMemoryDelayedDeliveryTracker(dispatcher, timer, tickTimeMillis, + isDelayedDeliveryDeliverAtTimeStrict, fixedDelayDetectionLookahead); } @Override public void close() { - // Close all topic-level managers - for (TopicDelayedDeliveryTrackerManager manager : topicManagers.values()) { - try { - manager.close(); - } catch (Exception e) { - log.warn("Failed to close topic-level delayed delivery manager", e); - } - } - topicManagers.clear(); - if (timer != null) { timer.stop(); } diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerFactory.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerFactory.java new file mode 100644 index 0000000000000..c1229ff484e7c --- /dev/null +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerFactory.java @@ -0,0 +1,135 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pulsar.broker.delayed; + +import com.google.common.annotations.VisibleForTesting; +import io.netty.util.HashedWheelTimer; +import io.netty.util.Timer; +import io.netty.util.concurrent.DefaultThreadFactory; +import java.time.Clock; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.TimeUnit; +import org.apache.pulsar.broker.PulsarService; +import org.apache.pulsar.broker.ServiceConfiguration; +import org.apache.pulsar.broker.service.persistent.AbstractPersistentDispatcherMultipleConsumers; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class InMemoryTopicDelayedDeliveryTrackerFactory implements DelayedDeliveryTrackerFactory { + + private static final Logger log = LoggerFactory.getLogger(InMemoryTopicDelayedDeliveryTrackerFactory.class); + + private Timer timer; + + private long tickTimeMillis; + + private boolean isDelayedDeliveryDeliverAtTimeStrict; + + private long fixedDelayDetectionLookahead; + + // Cache of topic-level managers: topic name -> manager instance + private final ConcurrentMap topicManagers = new ConcurrentHashMap<>(); + + public InMemoryTopicDelayedDeliveryTrackerFactory() { + + } + + // Testing-friendly constructor and accessors + @VisibleForTesting + InMemoryTopicDelayedDeliveryTrackerFactory(Timer timer, long tickTimeMillis, + boolean isDelayedDeliveryDeliverAtTimeStrict, + long fixedDelayDetectionLookahead) { + this.timer = timer; + this.tickTimeMillis = tickTimeMillis; + this.isDelayedDeliveryDeliverAtTimeStrict = isDelayedDeliveryDeliverAtTimeStrict; + this.fixedDelayDetectionLookahead = fixedDelayDetectionLookahead; + } + + @VisibleForTesting + int getCachedManagersSize() { + return topicManagers.size(); + } + + @VisibleForTesting + boolean hasManagerForTopic(String topicName) { + return topicManagers.containsKey(topicName); + } + + @Override + public void initialize(PulsarService pulsarService) { + ServiceConfiguration config = pulsarService.getConfig(); + this.timer = new HashedWheelTimer(new DefaultThreadFactory("pulsar-delayed-delivery"), + config.getDelayedDeliveryTickTimeMillis(), TimeUnit.MILLISECONDS); + this.tickTimeMillis = config.getDelayedDeliveryTickTimeMillis(); + this.isDelayedDeliveryDeliverAtTimeStrict = config.isDelayedDeliveryDeliverAtTimeStrict(); + this.fixedDelayDetectionLookahead = config.getDelayedDeliveryFixedDelayDetectionLookahead(); + } + + @Override + public DelayedDeliveryTracker newTracker(AbstractPersistentDispatcherMultipleConsumers dispatcher) { + String topicName = dispatcher.getTopic().getName(); + String subscriptionName = dispatcher.getSubscription().getName(); + DelayedDeliveryTracker tracker = DelayedDeliveryTracker.DISABLE; + try { + tracker = newTracker0(dispatcher); + } catch (Exception e) { + // it should never go here + log.warn("Failed to create InMemoryDelayedDeliveryTracker, topic {}, subscription {}", + topicName, subscriptionName, e); + } + return tracker; + } + + @VisibleForTesting + DelayedDeliveryTracker newTracker0(AbstractPersistentDispatcherMultipleConsumers dispatcher) { + String topicName = dispatcher.getTopic().getName(); + + // Get or create topic-level manager for this topic with onEmpty callback to remove from cache + final TopicDelayedDeliveryTrackerManager[] holder = new TopicDelayedDeliveryTrackerManager[1]; + TopicDelayedDeliveryTrackerManager manager = topicManagers.computeIfAbsent(topicName, k -> { + InMemoryTopicDelayedDeliveryTrackerManager m = new InMemoryTopicDelayedDeliveryTrackerManager( + timer, tickTimeMillis, Clock.systemUTC(), isDelayedDeliveryDeliverAtTimeStrict, + fixedDelayDetectionLookahead, () -> topicManagers.remove(topicName, holder[0])); + holder[0] = m; + return m; + }); + + // Create a per-subscription view from the topic-level manager + return manager.createOrGetView(dispatcher); + } + + @Override + public void close() { + // Close all topic-level managers + for (TopicDelayedDeliveryTrackerManager manager : topicManagers.values()) { + try { + manager.close(); + } catch (Exception e) { + log.warn("Failed to close topic-level delayed delivery manager", e); + } + } + topicManagers.clear(); + + if (timer != null) { + timer.stop(); + } + } +} + diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/DelayedDeliveryTrackerFactoryTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/DelayedDeliveryTrackerFactoryTest.java index f01391c770d47..f43b5495fac25 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/DelayedDeliveryTrackerFactoryTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/DelayedDeliveryTrackerFactoryTest.java @@ -81,7 +81,7 @@ public void testFallbackToInMemoryTracker() throws Exception { // the factory should be fallback to InMemoryDelayedDeliveryTrackerFactory @Cleanup DelayedDeliveryTracker tracker = brokerService.getDelayedDeliveryTrackerFactory().newTracker(dispatcher); - Assert.assertTrue(tracker instanceof InMemoryTopicDelayedDeliveryTrackerView); + Assert.assertTrue(tracker instanceof InMemoryDelayedDeliveryTracker); DelayedDeliveryTrackerFactory fallbackFactory = brokerService.getFallbackDelayedDeliveryTrackerFactory(); Assert.assertTrue(fallbackFactory instanceof InMemoryDelayedDeliveryTrackerFactory); @@ -223,11 +223,11 @@ public void testPublishDelayMessagesAndCreateBucketDelayDeliveryTrackerFailed() }); Optional optional = reference.get(); - Assert.assertTrue(optional.get() instanceof InMemoryTopicDelayedDeliveryTrackerView); + Assert.assertTrue(optional.get() instanceof InMemoryDelayedDeliveryTracker); // Mock DelayedDeliveryTracker and Count the number of addMessage() calls AtomicInteger counter = new AtomicInteger(0); - InMemoryTopicDelayedDeliveryTrackerView tracker = (InMemoryTopicDelayedDeliveryTrackerView) optional.get(); + InMemoryDelayedDeliveryTracker tracker = (InMemoryDelayedDeliveryTracker) optional.get(); tracker = Mockito.spy(tracker); Mockito.doAnswer(inv -> { counter.incrementAndGet(); diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTrackerFactoryUnitTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerFactoryTest.java similarity index 89% rename from pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTrackerFactoryUnitTest.java rename to pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerFactoryTest.java index b478dd7737c97..018943ef53228 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryDelayedDeliveryTrackerFactoryUnitTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerFactoryTest.java @@ -29,10 +29,9 @@ import org.testng.annotations.Test; @Test(groups = "broker") -public class InMemoryDelayedDeliveryTrackerFactoryUnitTest { +public class InMemoryTopicDelayedDeliveryTrackerFactoryTest { - private static AbstractPersistentDispatcherMultipleConsumers mockDispatcher(String topicName, String subName) - throws Exception { + private static AbstractPersistentDispatcherMultipleConsumers mockDispatcher(String topicName, String subName) { AbstractPersistentDispatcherMultipleConsumers dispatcher = mock(AbstractPersistentDispatcherMultipleConsumers.class); PersistentTopic topic = mock(PersistentTopic.class); @@ -47,8 +46,8 @@ private static AbstractPersistentDispatcherMultipleConsumers mockDispatcher(Stri @Test public void testManagersSharedPerTopicAndIndependentAcrossTopics() throws Exception { Timer timer = mock(Timer.class); - InMemoryDelayedDeliveryTrackerFactory f = - new InMemoryDelayedDeliveryTrackerFactory(timer, 100, true, 0); + InMemoryTopicDelayedDeliveryTrackerFactory f = + new InMemoryTopicDelayedDeliveryTrackerFactory(timer, 100, true, 0); AbstractPersistentDispatcherMultipleConsumers dA1 = mockDispatcher("persistent://ns/topicA", "sub1"); AbstractPersistentDispatcherMultipleConsumers dA2 = mockDispatcher("persistent://ns/topicA", "sub2"); @@ -75,8 +74,8 @@ public void testManagersSharedPerTopicAndIndependentAcrossTopics() throws Except @Test public void testOnEmptyCallbackRemovesManagerFromCache() throws Exception { Timer timer = mock(Timer.class); - InMemoryDelayedDeliveryTrackerFactory f = - new InMemoryDelayedDeliveryTrackerFactory(timer, 100, true, 0); + InMemoryTopicDelayedDeliveryTrackerFactory f = + new InMemoryTopicDelayedDeliveryTrackerFactory(timer, 100, true, 0); AbstractPersistentDispatcherMultipleConsumers dA1 = mockDispatcher("persistent://ns/topicA", "sub1"); AbstractPersistentDispatcherMultipleConsumers dA2 = mockDispatcher("persistent://ns/topicA", "sub2"); @@ -94,8 +93,8 @@ public void testOnEmptyCallbackRemovesManagerFromCache() throws Exception { @Test public void testFactoryCloseClosesManagersAndStopsTimer() throws Exception { Timer timer = mock(Timer.class); - InMemoryDelayedDeliveryTrackerFactory f = - new InMemoryDelayedDeliveryTrackerFactory(timer, 100, true, 0); + InMemoryTopicDelayedDeliveryTrackerFactory f = + new InMemoryTopicDelayedDeliveryTrackerFactory(timer, 100, true, 0); AbstractPersistentDispatcherMultipleConsumers dA1 = mockDispatcher("persistent://ns/topicA", "sub1"); AbstractPersistentDispatcherMultipleConsumers dB1 = mockDispatcher("persistent://ns/topicB", "sub1"); From 32f21700bf0bdc1ab2a4c7724f56b0ef00a86634 Mon Sep 17 00:00:00 2001 From: Denovo1998 Date: Sun, 2 Nov 2025 12:13:35 +0800 Subject: [PATCH 13/16] [feat][broker] Add configuration options for in-memory topic-level delayed delivery tracker and rename view class for clarity --- conf/broker.conf | 18 ++ conf/standalone.conf | 18 ++ .../pulsar/broker/ServiceConfiguration.java | 22 +++ ... InMemoryTopicDelayedDeliveryTracker.java} | 13 +- ...oryTopicDelayedDeliveryTrackerFactory.java | 39 ++++- ...oryTopicDelayedDeliveryTrackerManager.java | 33 +++- .../TopicDelayedDeliveryTrackerManager.java | 10 +- ...PersistentDispatcherMultipleConsumers.java | 4 +- ...entDispatcherMultipleConsumersClassic.java | 4 +- .../InMemoryTopicDeliveryTrackerTest.java | 154 +++++++++--------- 10 files changed, 206 insertions(+), 109 deletions(-) rename pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/{InMemoryTopicDelayedDeliveryTrackerView.java => InMemoryTopicDelayedDeliveryTracker.java} (87%) diff --git a/conf/broker.conf b/conf/broker.conf index 389842fd6b22e..a995af650a8d1 100644 --- a/conf/broker.conf +++ b/conf/broker.conf @@ -671,6 +671,24 @@ delayedDeliveryTickTimeMillis=1000 # delayedDeliveryTickTimeMillis. isDelayedDeliveryDeliverAtTimeStrict=false +# Minimum interval (in milliseconds) between prune attempts within the in-memory topic-level delayed delivery tracker. +# (org.apache.pulsar.broker.delayed.InMemoryTopicDelayedDeliveryTrackerFactory) +# Set to a positive value to override the default adaptive interval based on delayedDeliveryTickTimeMillis. +# Set to 0 or a negative value to use the default adaptive interval. +delayedDeliveryPruneMinIntervalMillis=0 + +# The ratio [0.0, 1.0] of subscriptions that need to be eligible for delivery in order to trigger an opportunistic +# prune in the in-memory topic-level delayed delivery tracker. +# (org.apache.pulsar.broker.delayed.InMemoryTopicDelayedDeliveryTrackerFactory) +# For example, 0.5 means prune when at least half of the subscriptions are eligible. Default is 0.5. +delayedDeliveryPruneEligibleRatio=0.5 + +# Idle timeout (in milliseconds) for the topic-level in-memory delayed delivery tracker manager. +# (org.apache.pulsar.broker.delayed.InMemoryTopicDelayedDeliveryTrackerFactory) +# When the last subscription is unregistered, the manager will be removed from the factory cache after this idle +# timeout, provided no new subscriptions have been registered in the meantime. Set to 0 to remove immediately (default). +delayedDeliveryTopicManagerIdleMillis=0 + # The delayed message index bucket min index count. # When the index count of the current bucket is more than this value and all message indexes of current ledger # have already been added to the tracker we will seal the bucket. diff --git a/conf/standalone.conf b/conf/standalone.conf index 48199a0f15e55..e83777aad8511 100644 --- a/conf/standalone.conf +++ b/conf/standalone.conf @@ -1439,6 +1439,24 @@ delayedDeliveryTickTimeMillis=1000 # delayedDeliveryTickTimeMillis. isDelayedDeliveryDeliverAtTimeStrict=false +# Minimum interval (in milliseconds) between prune attempts within the in-memory topic-level delayed delivery tracker. +# (org.apache.pulsar.broker.delayed.InMemoryTopicDelayedDeliveryTrackerFactory) +# Set to a positive value to override the default adaptive interval based on delayedDeliveryTickTimeMillis. +# Set to 0 or a negative value to use the default adaptive interval. +delayedDeliveryPruneMinIntervalMillis=0 + +# The ratio [0.0, 1.0] of subscriptions that need to be eligible for delivery in order to trigger an opportunistic +# prune in the in-memory topic-level delayed delivery tracker. +# (org.apache.pulsar.broker.delayed.InMemoryTopicDelayedDeliveryTrackerFactory) +# For example, 0.5 means prune when at least half of the subscriptions are eligible. Default is 0.5. +delayedDeliveryPruneEligibleRatio=0.5 + +# Idle timeout (in milliseconds) for the topic-level in-memory delayed delivery tracker manager. +# (org.apache.pulsar.broker.delayed.InMemoryTopicDelayedDeliveryTrackerFactory) +# When the last subscription is unregistered, the manager will be removed from the factory cache after this idle +# timeout, provided no new subscriptions have been registered in the meantime. Set to 0 to remove immediately (default). +delayedDeliveryTopicManagerIdleMillis=0 + # The delayed message index bucket min index count. # When the index count of the current bucket is more than this value and all message indexes of current ledger # have already been added to the tracker we will seal the bucket. diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java index 9b3fd19ec7f4f..81335ff4975e0 100644 --- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java +++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java @@ -421,6 +421,28 @@ The delayed message index time step(in seconds) in per bucket snapshot segment, + "logic to handle fixed delays in messages in a different way.") private long delayedDeliveryFixedDelayDetectionLookahead = 50_000; + @FieldContext(category = CATEGORY_SERVER, doc = """ + Minimum interval (in milliseconds) between prune attempts within the in-memory topic-level delayed + delivery tracker. Set to a positive value to override the default adaptive interval based on + delayedDeliveryTickTimeMillis. Set to 0 or a negative value to use the default adaptive interval. + """) + private long delayedDeliveryPruneMinIntervalMillis = 0; + + @FieldContext(category = CATEGORY_SERVER, doc = """ + The ratio [0.0, 1.0] of subscriptions that need to be eligible for delivery in order to trigger an + opportunistic prune in the in-memory topic-level delayed delivery tracker. For example, 0.5 means prune + when at least half of the subscriptions are eligible. Default is 0.5. + """) + private double delayedDeliveryPruneEligibleRatio = 0.5; + + @FieldContext(category = CATEGORY_SERVER, doc = """ + Idle timeout (in milliseconds) for the topic-level in-memory delayed delivery tracker manager. When the + last subscription is unregistered, the manager will be removed from the factory cache after this idle + timeout, provided no new subscriptions have been registered in the meantime. Set to 0 to remove + immediately (default). + """) + private long delayedDeliveryTopicManagerIdleMillis = 0; + @FieldContext(category = CATEGORY_SERVER, doc = """ The max allowed delay for delayed delivery (in milliseconds). If the broker receives a message which \ exceeds this max delay, then it will return an error to the producer. \ diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerView.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTracker.java similarity index 87% rename from pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerView.java rename to pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTracker.java index 072e5b2661a6b..09f5bd8db8351 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerView.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTracker.java @@ -24,19 +24,18 @@ import org.apache.bookkeeper.mledger.Position; /** - * View object for a subscription that implements DelayedDeliveryTracker interface. - * This view forwards all operations to the topic-level manager while maintaining - * compatibility with existing dispatcher logic. + * Subscription-scoped tracker implementing {@link DelayedDeliveryTracker} by delegating all + * operations to the topic-level {@link InMemoryTopicDelayedDeliveryTrackerManager}. */ @Slf4j -public class InMemoryTopicDelayedDeliveryTrackerView implements DelayedDeliveryTracker { +public class InMemoryTopicDelayedDeliveryTracker implements DelayedDeliveryTracker { private final InMemoryTopicDelayedDeliveryTrackerManager manager; private final InMemoryTopicDelayedDeliveryTrackerManager.SubContext subContext; private volatile boolean closed = false; - public InMemoryTopicDelayedDeliveryTrackerView(InMemoryTopicDelayedDeliveryTrackerManager manager, - InMemoryTopicDelayedDeliveryTrackerManager.SubContext subContext) { + public InMemoryTopicDelayedDeliveryTracker(InMemoryTopicDelayedDeliveryTrackerManager manager, + InMemoryTopicDelayedDeliveryTrackerManager.SubContext subContext) { this.manager = manager; this.subContext = subContext; } @@ -117,4 +116,4 @@ private void checkClosed() { throw new IllegalStateException("DelayedDeliveryTracker is already closed"); } } -} \ No newline at end of file +} diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerFactory.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerFactory.java index c1229ff484e7c..b4ccf94dcfa5d 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerFactory.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerFactory.java @@ -44,14 +44,14 @@ public class InMemoryTopicDelayedDeliveryTrackerFactory implements DelayedDelive private long fixedDelayDetectionLookahead; + // New tuning knobs + private long pruneMinIntervalMillis; + private double pruneEligibleRatio; + private long topicManagerIdleMillis; + // Cache of topic-level managers: topic name -> manager instance private final ConcurrentMap topicManagers = new ConcurrentHashMap<>(); - public InMemoryTopicDelayedDeliveryTrackerFactory() { - - } - - // Testing-friendly constructor and accessors @VisibleForTesting InMemoryTopicDelayedDeliveryTrackerFactory(Timer timer, long tickTimeMillis, boolean isDelayedDeliveryDeliverAtTimeStrict, @@ -60,6 +60,9 @@ public InMemoryTopicDelayedDeliveryTrackerFactory() { this.tickTimeMillis = tickTimeMillis; this.isDelayedDeliveryDeliverAtTimeStrict = isDelayedDeliveryDeliverAtTimeStrict; this.fixedDelayDetectionLookahead = fixedDelayDetectionLookahead; + this.pruneMinIntervalMillis = 0; + this.pruneEligibleRatio = 0.5; + this.topicManagerIdleMillis = 0; } @VisibleForTesting @@ -80,6 +83,9 @@ public void initialize(PulsarService pulsarService) { this.tickTimeMillis = config.getDelayedDeliveryTickTimeMillis(); this.isDelayedDeliveryDeliverAtTimeStrict = config.isDelayedDeliveryDeliverAtTimeStrict(); this.fixedDelayDetectionLookahead = config.getDelayedDeliveryFixedDelayDetectionLookahead(); + this.pruneMinIntervalMillis = config.getDelayedDeliveryPruneMinIntervalMillis(); + this.pruneEligibleRatio = config.getDelayedDeliveryPruneEligibleRatio(); + this.topicManagerIdleMillis = config.getDelayedDeliveryTopicManagerIdleMillis(); } @Override @@ -106,13 +112,29 @@ DelayedDeliveryTracker newTracker0(AbstractPersistentDispatcherMultipleConsumers TopicDelayedDeliveryTrackerManager manager = topicManagers.computeIfAbsent(topicName, k -> { InMemoryTopicDelayedDeliveryTrackerManager m = new InMemoryTopicDelayedDeliveryTrackerManager( timer, tickTimeMillis, Clock.systemUTC(), isDelayedDeliveryDeliverAtTimeStrict, - fixedDelayDetectionLookahead, () -> topicManagers.remove(topicName, holder[0])); + fixedDelayDetectionLookahead, pruneMinIntervalMillis, pruneEligibleRatio, () -> { + if (topicManagerIdleMillis <= 0) { + topicManagers.remove(topicName, holder[0]); + } else { + timer.newTimeout(__ -> { + TopicDelayedDeliveryTrackerManager tm = holder[0]; + if (tm instanceof InMemoryTopicDelayedDeliveryTrackerManager) { + if (!((InMemoryTopicDelayedDeliveryTrackerManager) tm).hasActiveSubscriptions()) { + topicManagers.remove(topicName, tm); + } + } else { + // If the manager has been replaced or removed, ensure entry is cleaned up + topicManagers.remove(topicName, tm); + } + }, topicManagerIdleMillis, TimeUnit.MILLISECONDS); + } + }); holder[0] = m; return m; }); - // Create a per-subscription view from the topic-level manager - return manager.createOrGetView(dispatcher); + // Create a per-subscription tracker from the topic-level manager + return manager.createOrGetTracker(dispatcher); } @Override @@ -132,4 +154,3 @@ public void close() { } } } - diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java index c16c4f599255d..4fb274bf96486 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java @@ -82,6 +82,9 @@ public class InMemoryTopicDelayedDeliveryTrackerManager implements TopicDelayedD // Minimum interval between prunes private final long minPruneIntervalNanos; + // Ratio of eligible subscriptions required to opportunistically prune [0.0, 1.0] + private final double pruneEligibleRatio; + // Fixed-delay detection (parity with legacy behavior) private final AtomicLong highestDeliveryTimeTracked = new AtomicLong(0); private volatile boolean messagesHaveFixedDelay = true; @@ -131,12 +134,15 @@ long getCutoffTime() { public InMemoryTopicDelayedDeliveryTrackerManager(Timer timer, long tickTimeMillis, Clock clock, boolean isDelayedDeliveryDeliverAtTimeStrict, long fixedDelayDetectionLookahead) { - this(timer, tickTimeMillis, clock, isDelayedDeliveryDeliverAtTimeStrict, fixedDelayDetectionLookahead, null); + this(timer, tickTimeMillis, clock, isDelayedDeliveryDeliverAtTimeStrict, fixedDelayDetectionLookahead, + 0, 0.5, null); } public InMemoryTopicDelayedDeliveryTrackerManager(Timer timer, long tickTimeMillis, Clock clock, boolean isDelayedDeliveryDeliverAtTimeStrict, long fixedDelayDetectionLookahead, + long pruneMinIntervalMillis, + double pruneEligibleRatio, Runnable onEmptyCallback) { this.timer = timer; this.tickTimeMillis = tickTimeMillis; @@ -144,10 +150,16 @@ public InMemoryTopicDelayedDeliveryTrackerManager(Timer timer, long tickTimeMill this.isDelayedDeliveryDeliverAtTimeStrict = isDelayedDeliveryDeliverAtTimeStrict; this.fixedDelayDetectionLookahead = fixedDelayDetectionLookahead; this.onEmptyCallback = onEmptyCallback; - // Default prune throttle interval: clamp to [5ms, 50ms] using tickTimeMillis as hint - // TODO: make configurable if needed - long pruneMs = Math.max(5L, Math.min(50L, tickTimeMillis)); + // Prune throttle interval: use configured override if positive, otherwise adaptive clamp [5ms, 50ms] + long pruneMs = pruneMinIntervalMillis > 0 + ? pruneMinIntervalMillis + : Math.max(5L, Math.min(50L, tickTimeMillis)); this.minPruneIntervalNanos = TimeUnit.MILLISECONDS.toNanos(pruneMs); + // Prune eligible ratio: clamp into [0.0, 1.0] + if (Double.isNaN(pruneEligibleRatio)) { + pruneEligibleRatio = 0.5; + } + this.pruneEligibleRatio = Math.max(0.0, Math.min(1.0, pruneEligibleRatio)); } // We bucket messages by aligning the deliverAt timestamp to the start of the logical tick window: @@ -167,13 +179,13 @@ private long bucketStart(long timestamp) { } @Override - public DelayedDeliveryTracker createOrGetView(AbstractPersistentDispatcherMultipleConsumers dispatcher) { + public DelayedDeliveryTracker createOrGetTracker(AbstractPersistentDispatcherMultipleConsumers dispatcher) { String subscriptionName = dispatcher.getSubscription().getName(); SubContext subContext = subscriptionContexts.computeIfAbsent(subscriptionName, k -> new SubContext(dispatcher, tickTimeMillis, isDelayedDeliveryDeliverAtTimeStrict, fixedDelayDetectionLookahead, clock)); - return new InMemoryTopicDelayedDeliveryTrackerView(this, subContext); + return new InMemoryTopicDelayedDeliveryTracker(this, subContext); } @Override @@ -209,6 +221,13 @@ public void unregister(AbstractPersistentDispatcherMultipleConsumers dispatcher) } } + /** + * Whether there are active subscriptions registered with this manager. + */ + public boolean hasActiveSubscriptions() { + return !subscriptionContexts.isEmpty(); + } + @Override public void onTickTimeUpdated(long newTickTimeMillis) { if (this.tickTimeMillis == newTickTimeMillis) { @@ -560,7 +579,7 @@ public void run(Timeout timeout) throws Exception { // If a significant portion of subscriptions are eligible, opportunistically prune (throttled) int subs = subscriptionContexts.size(); int eligible = toTrigger.size(); - int threshold = Math.max(1, subs / 2); + int threshold = Math.max(1, (int) Math.ceil(subs * pruneEligibleRatio)); if (eligible >= threshold) { maybePruneByTime(); } diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/TopicDelayedDeliveryTrackerManager.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/TopicDelayedDeliveryTrackerManager.java index 707bcefc76a53..d87fff18697f2 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/TopicDelayedDeliveryTrackerManager.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/TopicDelayedDeliveryTrackerManager.java @@ -26,18 +26,18 @@ * allowing different implementations (in-memory, bucket-based) to share the same contract. *

* The manager maintains a single global delayed message index per topic that is shared by all - * subscriptions, and provides per-subscription "view" objects that implement DelayedDeliveryTracker + * subscriptions, and provides per-subscription tracker objects that implement DelayedDeliveryTracker * interface for compatibility with existing dispatcher logic. */ public interface TopicDelayedDeliveryTrackerManager extends AutoCloseable { /** - * Create or get a delayed delivery tracker view for the specified subscription. + * Create or get a delayed delivery tracker for the specified subscription. * * @param dispatcher the dispatcher instance for the subscription - * @return a DelayedDeliveryTracker view for the subscription + * @return a DelayedDeliveryTracker bound to the subscription */ - DelayedDeliveryTracker createOrGetView(AbstractPersistentDispatcherMultipleConsumers dispatcher); + DelayedDeliveryTracker createOrGetTracker(AbstractPersistentDispatcherMultipleConsumers dispatcher); /** * Unregister a subscription from the manager. @@ -71,4 +71,4 @@ public interface TopicDelayedDeliveryTrackerManager extends AutoCloseable { * Close the manager and release all resources. */ void close(); -} \ No newline at end of file +} diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java index 6bd8c9d2ad5a0..a4faaa7619709 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java @@ -53,7 +53,7 @@ import org.apache.pulsar.broker.delayed.DelayedDeliveryTracker; import org.apache.pulsar.broker.delayed.DelayedDeliveryTrackerFactory; import org.apache.pulsar.broker.delayed.InMemoryDelayedDeliveryTracker; -import org.apache.pulsar.broker.delayed.InMemoryTopicDelayedDeliveryTrackerView; +import org.apache.pulsar.broker.delayed.InMemoryTopicDelayedDeliveryTracker; import org.apache.pulsar.broker.delayed.bucket.BucketDelayedDeliveryTracker; import org.apache.pulsar.broker.loadbalance.extensions.data.BrokerLookupData; import org.apache.pulsar.broker.service.AbstractDispatcherMultipleConsumers; @@ -1485,7 +1485,7 @@ public void markDeletePositionMoveForward() { trackerOpt = this.delayedDeliveryTracker; } trackerOpt.ifPresent(tracker -> { - if (tracker instanceof InMemoryTopicDelayedDeliveryTrackerView view) { + if (tracker instanceof InMemoryTopicDelayedDeliveryTracker view) { Position md = cursor.getMarkDeletedPosition(); if (md != null) { // Only update the cached mark-delete; avoid heavy operations in this callback diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumersClassic.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumersClassic.java index c1e070d89266e..dc0e51a29211a 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumersClassic.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumersClassic.java @@ -50,7 +50,7 @@ import org.apache.pulsar.broker.delayed.DelayedDeliveryTracker; import org.apache.pulsar.broker.delayed.DelayedDeliveryTrackerFactory; import org.apache.pulsar.broker.delayed.InMemoryDelayedDeliveryTracker; -import org.apache.pulsar.broker.delayed.InMemoryTopicDelayedDeliveryTrackerView; +import org.apache.pulsar.broker.delayed.InMemoryTopicDelayedDeliveryTracker; import org.apache.pulsar.broker.delayed.bucket.BucketDelayedDeliveryTracker; import org.apache.pulsar.broker.loadbalance.extensions.data.BrokerLookupData; import org.apache.pulsar.broker.service.AbstractDispatcherMultipleConsumers; @@ -1316,7 +1316,7 @@ public void markDeletePositionMoveForward() { trackerOpt = this.delayedDeliveryTracker; } trackerOpt.ifPresent(tracker -> { - if (tracker instanceof InMemoryTopicDelayedDeliveryTrackerView view) { + if (tracker instanceof InMemoryTopicDelayedDeliveryTracker view) { Position md = cursor.getMarkDeletedPosition(); if (md != null) { // Only update the cached mark-delete; avoid heavy operations in this callback diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDeliveryTrackerTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDeliveryTrackerTest.java index 39e1bf1f31370..7fce4deebff87 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDeliveryTrackerTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDeliveryTrackerTest.java @@ -104,46 +104,46 @@ public void testSingleSubscriptionBasicFlow() throws Exception { ManagedCursor cursor = mock(ManagedCursor.class); AbstractPersistentDispatcherMultipleConsumers dispatcher = newDispatcher("sub-a", cursor); - DelayedDeliveryTracker view = manager.createOrGetView(dispatcher); + DelayedDeliveryTracker tracker = manager.createOrGetTracker(dispatcher); - assertFalse(view.hasMessageAvailable()); + assertFalse(tracker.hasMessageAvailable()); // Add 3 messages in the future env.time.set(1000); - assertTrue(view.addMessage(1, 1, 1200)); - assertTrue(view.addMessage(1, 2, 1300)); - assertTrue(view.addMessage(2, 1, 1400)); + assertTrue(tracker.addMessage(1, 1, 1200)); + assertTrue(tracker.addMessage(1, 2, 1300)); + assertTrue(tracker.addMessage(2, 1, 1400)); - assertFalse(view.hasMessageAvailable()); - assertEquals(view.getNumberOfDelayedMessages(), 3); + assertFalse(tracker.hasMessageAvailable()); + assertEquals(tracker.getNumberOfDelayedMessages(), 3); // Advance time so first 2 buckets are visible env.time.set(1350); - assertTrue(view.hasMessageAvailable()); - NavigableSet scheduled = view.getScheduledMessages(10); + assertTrue(tracker.hasMessageAvailable()); + NavigableSet scheduled = tracker.getScheduledMessages(10); // Should include both positions from first 2 buckets assertEquals(scheduled.size(), 2); // Global counter doesn't drop until mark-delete pruning - assertEquals(view.getNumberOfDelayedMessages(), 3); + assertEquals(tracker.getNumberOfDelayedMessages(), 3); // Mark-delete beyond the scheduled positions and prune - ((InMemoryTopicDelayedDeliveryTrackerView) view) + ((InMemoryTopicDelayedDeliveryTracker) tracker) .updateMarkDeletePosition(PositionFactory.create(1L, 2L)); // Trigger pruning by another get - view.getScheduledMessages(10); + tracker.getScheduledMessages(10); // Now only one entry should remain in global index (prune is throttled -> wait up to 2s) long start = System.currentTimeMillis(); - while (view.getNumberOfDelayedMessages() != 1 && System.currentTimeMillis() - start < 2000) { + while (tracker.getNumberOfDelayedMessages() != 1 && System.currentTimeMillis() - start < 2000) { try { Thread.sleep(30); } catch (InterruptedException ignored) {} - view.getScheduledMessages(1); + tracker.getScheduledMessages(1); } - assertEquals(view.getNumberOfDelayedMessages(), 1); + assertEquals(tracker.getNumberOfDelayedMessages(), 1); // Cleanup - view.close(); + tracker.close(); } @Test @@ -157,8 +157,8 @@ public void testSharedIndexDedupAcrossSubscriptions() throws Exception { AbstractPersistentDispatcherMultipleConsumers d1 = newDispatcher("sub-a", c1); AbstractPersistentDispatcherMultipleConsumers d2 = newDispatcher("sub-b", c2); - DelayedDeliveryTracker v1 = manager.createOrGetView(d1); - DelayedDeliveryTracker v2 = manager.createOrGetView(d2); + DelayedDeliveryTracker v1 = manager.createOrGetTracker(d1); + DelayedDeliveryTracker v2 = manager.createOrGetTracker(d2); env.time.set(1000); assertTrue(v1.addMessage(10, 20, 2000)); @@ -183,8 +183,8 @@ public void testTimerRunTriggersOnlyAvailableSubscriptions() throws Exception { ManagedCursor c2 = mock(ManagedCursor.class); AbstractPersistentDispatcherMultipleConsumers d1 = newDispatcher("sub-a", c1); AbstractPersistentDispatcherMultipleConsumers d2 = newDispatcher("sub-b", c2); - DelayedDeliveryTracker v1 = manager.createOrGetView(d1); - DelayedDeliveryTracker v2 = manager.createOrGetView(d2); + DelayedDeliveryTracker v1 = manager.createOrGetTracker(d1); + DelayedDeliveryTracker v2 = manager.createOrGetTracker(d2); env.time.set(0); // Add two buckets. Only sub-a will have messages available based on mark-delete @@ -197,9 +197,9 @@ public void testTimerRunTriggersOnlyAvailableSubscriptions() throws Exception { // Set time after cutoff and set sub-a mark-delete behind entries, sub-b beyond entries env.time.set(600); - ((InMemoryTopicDelayedDeliveryTrackerView) v1) + ((InMemoryTopicDelayedDeliveryTracker) v1) .updateMarkDeletePosition(PositionFactory.create(0L, 0L)); // visible for sub-a - ((InMemoryTopicDelayedDeliveryTrackerView) v2) + ((InMemoryTopicDelayedDeliveryTracker) v2) .updateMarkDeletePosition(PositionFactory.create(1L, 5L)); // filtered for sub-b // Invoke manager timer task directly @@ -222,21 +222,21 @@ public void testPauseWithFixedDelays() throws Exception { ManagedCursor cursor = mock(ManagedCursor.class); AbstractPersistentDispatcherMultipleConsumers dispatcher = newDispatcher("sub-a", cursor); - InMemoryTopicDelayedDeliveryTrackerView view = - (InMemoryTopicDelayedDeliveryTrackerView) manager.createOrGetView(dispatcher); + InMemoryTopicDelayedDeliveryTracker tracker = + (InMemoryTopicDelayedDeliveryTracker) manager.createOrGetTracker(dispatcher); // Add strictly increasing deliverAt times (fixed delay scenario) env.time.set(0); for (int i = 1; i <= lookahead; i++) { - assertTrue(view.addMessage(i, i, i * 100L)); + assertTrue(tracker.addMessage(i, i, i * 100L)); } - assertTrue(view.shouldPauseAllDeliveries()); + assertTrue(tracker.shouldPauseAllDeliveries()); // Move time forward to make messages available -> pause should be lifted env.time.set(lookahead * 100 + 1); - assertFalse(view.shouldPauseAllDeliveries()); + assertFalse(tracker.shouldPauseAllDeliveries()); - view.close(); + tracker.close(); } @Test @@ -248,19 +248,19 @@ public void testDynamicTickTimeUpdateAffectsCutoff() throws Exception { ManagedCursor cursor = mock(ManagedCursor.class); AbstractPersistentDispatcherMultipleConsumers dispatcher = newDispatcher("sub-a", cursor); - DelayedDeliveryTracker view = manager.createOrGetView(dispatcher); + DelayedDeliveryTracker tracker = manager.createOrGetTracker(dispatcher); env.time.set(1000); // deliverAt within current tick window -> rejected - assertFalse(view.addMessage(1, 1, 1050)); // cutoff=1100 - assertEquals(view.getNumberOfDelayedMessages(), 0); + assertFalse(tracker.addMessage(1, 1, 1050)); // cutoff=1100 + assertEquals(tracker.getNumberOfDelayedMessages(), 0); // shrink tick: cutoff reduces -> same deliverAt becomes accepted - view.resetTickTime(10); - assertTrue(view.addMessage(1, 1, 1050)); // cutoff=1010 - assertEquals(view.getNumberOfDelayedMessages(), 1); + tracker.resetTickTime(10); + assertTrue(tracker.addMessage(1, 1, 1050)); // cutoff=1010 + assertEquals(tracker.getNumberOfDelayedMessages(), 1); - view.close(); + tracker.close(); } @Test @@ -273,10 +273,10 @@ public void testMinMarkDeleteAcrossSubscriptions() throws Exception { ManagedCursor c2 = mock(ManagedCursor.class); AbstractPersistentDispatcherMultipleConsumers d1 = newDispatcher("sub-a", c1); AbstractPersistentDispatcherMultipleConsumers d2 = newDispatcher("sub-b", c2); - InMemoryTopicDelayedDeliveryTrackerView v1 = - (InMemoryTopicDelayedDeliveryTrackerView) manager.createOrGetView(d1); - InMemoryTopicDelayedDeliveryTrackerView v2 = - (InMemoryTopicDelayedDeliveryTrackerView) manager.createOrGetView(d2); + InMemoryTopicDelayedDeliveryTracker v1 = + (InMemoryTopicDelayedDeliveryTracker) manager.createOrGetTracker(d1); + InMemoryTopicDelayedDeliveryTracker v2 = + (InMemoryTopicDelayedDeliveryTracker) manager.createOrGetTracker(d2); env.time.set(0); assertTrue(v1.addMessage(1, 1, 100)); @@ -320,14 +320,14 @@ public void testTimerSchedulingWindowAlignment() throws Exception { ManagedCursor cursor = mock(ManagedCursor.class); AbstractPersistentDispatcherMultipleConsumers dispatcher = newDispatcher("sub-a", cursor); - DelayedDeliveryTracker view = manager.createOrGetView(dispatcher); + DelayedDeliveryTracker tracker = manager.createOrGetTracker(dispatcher); // Establish lastTickRun via a manual run at t=10000 env.time.set(10000); manager.run(mock(Timeout.class)); // Add with deliverAt=10001, but tick window alignment should schedule at >= 11000 - assertTrue(view.addMessage(1, 1, 10001)); + assertTrue(tracker.addMessage(1, 1, 10001)); long scheduledAt = env.tasks.firstKey(); assertTrue(scheduledAt >= 11000, "scheduledAt=" + scheduledAt); @@ -335,11 +335,11 @@ public void testTimerSchedulingWindowAlignment() throws Exception { env.tasks.clear(); env.time.set(20000); // No run -> lastTickRun remains 10000; bucketStart(20005)=20000; schedule aligns to bucket start immediately - assertTrue(view.addMessage(1, 2, 20005)); + assertTrue(tracker.addMessage(1, 2, 20005)); long scheduledAt2 = env.tasks.firstKey(); assertEquals(scheduledAt2, 20000); - view.close(); + tracker.close(); } @Test @@ -350,7 +350,7 @@ public void testBufferMemoryUsageAndCleanup() throws Exception { ManagedCursor c = mock(ManagedCursor.class); AbstractPersistentDispatcherMultipleConsumers d = newDispatcher("sub-a", c); - DelayedDeliveryTracker v = manager.createOrGetView(d); + DelayedDeliveryTracker v = manager.createOrGetTracker(d); env.time.set(0); assertTrue(v.addMessage(1, 1, 10)); @@ -369,14 +369,14 @@ public void testGetScheduledMessagesLimit() throws Exception { new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 1, env.clock, true, 0); ManagedCursor cursor = mock(ManagedCursor.class); AbstractPersistentDispatcherMultipleConsumers dispatcher = newDispatcher("sub", cursor); - DelayedDeliveryTracker view = manager.createOrGetView(dispatcher); + DelayedDeliveryTracker tracker = manager.createOrGetTracker(dispatcher); env.time.set(1000); for (int i = 0; i < 10; i++) { - assertTrue(view.addMessage(1, i, 1001)); + assertTrue(tracker.addMessage(1, i, 1001)); } env.time.set(2000); - NavigableSet positions = view.getScheduledMessages(3); + NavigableSet positions = tracker.getScheduledMessages(3); assertEquals(positions.size(), 3); Position prev = null; @@ -387,7 +387,7 @@ public void testGetScheduledMessagesLimit() throws Exception { prev = p; } - view.close(); + tracker.close(); } @Test @@ -397,17 +397,17 @@ public void testHasMessageAvailableIgnoresMarkDelete() throws Exception { new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 100, env.clock, true, 0); ManagedCursor cursor = mock(ManagedCursor.class); AbstractPersistentDispatcherMultipleConsumers dispatcher = newDispatcher("s", cursor); - InMemoryTopicDelayedDeliveryTrackerView view = - (InMemoryTopicDelayedDeliveryTrackerView) manager.createOrGetView(dispatcher); + InMemoryTopicDelayedDeliveryTracker tracker = + (InMemoryTopicDelayedDeliveryTracker) manager.createOrGetTracker(dispatcher); env.time.set(900); - assertTrue(view.addMessage(1, 1, 1000)); + assertTrue(tracker.addMessage(1, 1, 1000)); env.time.set(1000); - view.updateMarkDeletePosition(PositionFactory.create(1, 1)); - assertTrue(view.hasMessageAvailable()); - assertTrue(view.getScheduledMessages(10).isEmpty()); + tracker.updateMarkDeletePosition(PositionFactory.create(1, 1)); + assertTrue(tracker.hasMessageAvailable()); + assertTrue(tracker.getScheduledMessages(10).isEmpty()); - view.close(); + tracker.close(); } @Test @@ -421,8 +421,8 @@ public void testCrossBucketDuplicatesDedupOnRead() throws Exception { ManagedCursor c2 = mock(ManagedCursor.class); AbstractPersistentDispatcherMultipleConsumers d1 = newDispatcher("s1", c1); AbstractPersistentDispatcherMultipleConsumers d2 = newDispatcher("s2", c2); - DelayedDeliveryTracker v1 = manager.createOrGetView(d1); - DelayedDeliveryTracker v2 = manager.createOrGetView(d2); + DelayedDeliveryTracker v1 = manager.createOrGetTracker(d1); + DelayedDeliveryTracker v2 = manager.createOrGetTracker(d2); env.time.set(1000); long deliverAt = 1023; @@ -449,7 +449,7 @@ public void testClearIsNoOp() throws Exception { new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 1, env.clock, true, 0); ManagedCursor c = mock(ManagedCursor.class); AbstractPersistentDispatcherMultipleConsumers d = newDispatcher("s", c); - DelayedDeliveryTracker v = manager.createOrGetView(d); + DelayedDeliveryTracker v = manager.createOrGetTracker(d); env.time.set(0); assertTrue(v.addMessage(1, 1, 10)); @@ -469,8 +469,8 @@ public void testMultiSubscriptionCloseDoesNotClear() throws Exception { ManagedCursor c2 = mock(ManagedCursor.class); AbstractPersistentDispatcherMultipleConsumers d1 = newDispatcher("s1", c1); AbstractPersistentDispatcherMultipleConsumers d2 = newDispatcher("s2", c2); - DelayedDeliveryTracker v1 = manager.createOrGetView(d1); - DelayedDeliveryTracker v2 = manager.createOrGetView(d2); + DelayedDeliveryTracker v1 = manager.createOrGetTracker(d1); + DelayedDeliveryTracker v2 = manager.createOrGetTracker(d2); env.time.set(0); assertTrue(v1.addMessage(1, 1, 10)); @@ -494,7 +494,7 @@ public void testBoundaryInputsRejected() throws Exception { InMemoryTopicDelayedDeliveryTrackerManager mStrict = new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 100, env.clock, true, 0); - DelayedDeliveryTracker vStrict = mStrict.createOrGetView(d); + DelayedDeliveryTracker vStrict = mStrict.createOrGetTracker(d); env.time.set(1000); assertFalse(vStrict.addMessage(1, 1, -1)); assertFalse(vStrict.addMessage(1, 2, 1000)); @@ -502,7 +502,7 @@ public void testBoundaryInputsRejected() throws Exception { InMemoryTopicDelayedDeliveryTrackerManager mNonStrict = new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 100, env.clock, false, 0); - DelayedDeliveryTracker vNon = mNonStrict.createOrGetView(d); + DelayedDeliveryTracker vNon = mNonStrict.createOrGetTracker(d); env.time.set(1000); assertFalse(vNon.addMessage(1, 3, 1100)); vNon.close(); @@ -524,8 +524,8 @@ public void testClosedViewThrowsOnOperations() throws Exception { new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 1, env.clock, true, 0); ManagedCursor c = mock(ManagedCursor.class); AbstractPersistentDispatcherMultipleConsumers d = newDispatcher("s", c); - InMemoryTopicDelayedDeliveryTrackerView v = - (InMemoryTopicDelayedDeliveryTrackerView) manager.createOrGetView(d); + InMemoryTopicDelayedDeliveryTracker v = + (InMemoryTopicDelayedDeliveryTracker) manager.createOrGetTracker(d); v.close(); expectIllegalState(() -> v.addMessage(1, 1, 10)); @@ -541,7 +541,7 @@ public void testRescheduleOnEarlierDeliverAt() throws Exception { new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 1, env.clock, true, 0); ManagedCursor c = mock(ManagedCursor.class); AbstractPersistentDispatcherMultipleConsumers d = newDispatcher("s", c); - DelayedDeliveryTracker v = manager.createOrGetView(d); + DelayedDeliveryTracker v = manager.createOrGetTracker(d); env.time.set(0); assertTrue(v.addMessage(1, 1, 10)); @@ -561,7 +561,7 @@ public void testEmptyIndexCancelsTimerOnClose() throws Exception { new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 100, env.clock, true, 0); ManagedCursor c = mock(ManagedCursor.class); AbstractPersistentDispatcherMultipleConsumers d = newDispatcher("s", c); - DelayedDeliveryTracker v = manager.createOrGetView(d); + DelayedDeliveryTracker v = manager.createOrGetTracker(d); env.time.set(0); assertTrue(v.addMessage(1, 1, 1000)); @@ -577,8 +577,8 @@ public void testMemoryGrowthAndPruneShrink() throws Exception { new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 10, env.clock, true, 0); ManagedCursor c = mock(ManagedCursor.class); AbstractPersistentDispatcherMultipleConsumers d = newDispatcher("s", c); - InMemoryTopicDelayedDeliveryTrackerView v = - (InMemoryTopicDelayedDeliveryTrackerView) manager.createOrGetView(d); + InMemoryTopicDelayedDeliveryTracker v = + (InMemoryTopicDelayedDeliveryTracker) manager.createOrGetTracker(d); env.time.set(0); for (int i = 0; i < 50; i++) { @@ -613,7 +613,7 @@ public void testTimerCancelAndReschedule() throws Exception { new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 10, env.clock, true, 0); ManagedCursor c = mock(ManagedCursor.class); AbstractPersistentDispatcherMultipleConsumers d = newDispatcher("s", c); - DelayedDeliveryTracker v = manager.createOrGetView(d); + DelayedDeliveryTracker v = manager.createOrGetTracker(d); env.time.set(0); assertTrue(v.addMessage(1, 1, 100)); @@ -633,7 +633,7 @@ public void testSortedAndDedupScheduled() throws Exception { new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 1, env.clock, true, 0); ManagedCursor c = mock(ManagedCursor.class); AbstractPersistentDispatcherMultipleConsumers d = newDispatcher("s", c); - DelayedDeliveryTracker v = manager.createOrGetView(d); + DelayedDeliveryTracker v = manager.createOrGetTracker(d); env.time.set(0); assertTrue(v.addMessage(2, 3, 10)); @@ -658,10 +658,10 @@ public void testGlobalDelayedCountSemantics() throws Exception { ManagedCursor c2 = mock(ManagedCursor.class); AbstractPersistentDispatcherMultipleConsumers d1 = newDispatcher("s1", c1); AbstractPersistentDispatcherMultipleConsumers d2 = newDispatcher("s2", c2); - InMemoryTopicDelayedDeliveryTrackerView v1 = - (InMemoryTopicDelayedDeliveryTrackerView) manager.createOrGetView(d1); - InMemoryTopicDelayedDeliveryTrackerView v2 = - (InMemoryTopicDelayedDeliveryTrackerView) manager.createOrGetView(d2); + InMemoryTopicDelayedDeliveryTracker v1 = + (InMemoryTopicDelayedDeliveryTracker) manager.createOrGetTracker(d1); + InMemoryTopicDelayedDeliveryTracker v2 = + (InMemoryTopicDelayedDeliveryTracker) manager.createOrGetTracker(d2); env.time.set(0); assertTrue(v1.addMessage(1, 1, 10)); @@ -682,7 +682,7 @@ public void testConcurrentAdditionsSameBucket() throws Exception { new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 1, env.clock, true, 0); ManagedCursor c = mock(ManagedCursor.class); AbstractPersistentDispatcherMultipleConsumers d = newDispatcher("s", c); - DelayedDeliveryTracker v = manager.createOrGetView(d); + DelayedDeliveryTracker v = manager.createOrGetTracker(d); env.time.set(0); int threads = 5; @@ -719,8 +719,8 @@ public void testConcurrentAdditionsMultipleBucketsAndReads() throws Exception { new InMemoryTopicDelayedDeliveryTrackerManager(env.timer, 5, env.clock, true, 0); ManagedCursor c = mock(ManagedCursor.class); AbstractPersistentDispatcherMultipleConsumers d = newDispatcher("s", c); - InMemoryTopicDelayedDeliveryTrackerView v = - (InMemoryTopicDelayedDeliveryTrackerView) manager.createOrGetView(d); + InMemoryTopicDelayedDeliveryTracker v = + (InMemoryTopicDelayedDeliveryTracker) manager.createOrGetTracker(d); env.time.set(0); ExecutorService es = Executors.newFixedThreadPool(2); From 14fc81b15f106b7ef4de808dee8051d0fb89f902 Mon Sep 17 00:00:00 2001 From: Denovo1998 Date: Sun, 2 Nov 2025 12:37:34 +0800 Subject: [PATCH 14/16] [feat][broker] Replace manual wait loops with Awaitility for pruning tests in InMemoryTopicDelayedDeliveryTrackerTest --- ...oryTopicDelayedDeliveryTrackerManager.java | 4 +- .../InMemoryTopicDeliveryTrackerTest.java | 52 ++++++++----------- 2 files changed, 23 insertions(+), 33 deletions(-) diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java index 4fb274bf96486..fb3278ed044dd 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java @@ -481,8 +481,6 @@ private void pruneByMinMarkDelete() { } } - // No idempotency set to clean (Option A): rely on per-bitmap removal below - // Prune per bucket under bucket lock for (Long ts : new ArrayList<>(delayedMessageMap.keySet())) { ReentrantLock bLock = bucketLocks.get(ts); @@ -550,6 +548,8 @@ private void maybePruneByTime() { } } + // Note: pruning is throttled by minPruneIntervalNanos. Tests should use Awaitility + // to wait for prune to occur instead of relying on direct triggers here. @Override public void run(Timeout timeout) throws Exception { if (timeout == null || timeout.isCancelled()) { diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDeliveryTrackerTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDeliveryTrackerTest.java index 7fce4deebff87..2c3edbde28225 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDeliveryTrackerTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/delayed/InMemoryTopicDeliveryTrackerTest.java @@ -31,6 +31,7 @@ import io.netty.util.Timer; import io.netty.util.TimerTask; import java.time.Clock; +import java.time.Duration; import java.util.ArrayList; import java.util.List; import java.util.NavigableMap; @@ -47,6 +48,7 @@ import org.apache.bookkeeper.mledger.PositionFactory; import org.apache.pulsar.broker.service.Subscription; import org.apache.pulsar.broker.service.persistent.AbstractPersistentDispatcherMultipleConsumers; +import org.awaitility.Awaitility; import org.mockito.stubbing.Answer; import org.testng.annotations.Test; @@ -132,15 +134,12 @@ public void testSingleSubscriptionBasicFlow() throws Exception { .updateMarkDeletePosition(PositionFactory.create(1L, 2L)); // Trigger pruning by another get tracker.getScheduledMessages(10); - // Now only one entry should remain in global index (prune is throttled -> wait up to 2s) - long start = System.currentTimeMillis(); - while (tracker.getNumberOfDelayedMessages() != 1 && System.currentTimeMillis() - start < 2000) { - try { - Thread.sleep(30); - } catch (InterruptedException ignored) {} - tracker.getScheduledMessages(1); - } - assertEquals(tracker.getNumberOfDelayedMessages(), 1); + // Now only one entry should remain in global index; wait until prune catches up + Awaitility.await().atMost(Duration.ofSeconds(5)).pollInterval(Duration.ofMillis(30)) + .untilAsserted(() -> { + tracker.getScheduledMessages(1); + assertEquals(tracker.getNumberOfDelayedMessages(), 1); + }); // Cleanup tracker.close(); @@ -296,16 +295,12 @@ public void testMinMarkDeleteAcrossSubscriptions() throws Exception { // Advance c1 mark-delete beyond (1,2) v1.updateMarkDeletePosition(PositionFactory.create(1L, 2L)); v1.getScheduledMessages(10); - long start2 = System.currentTimeMillis(); - while (v1.getNumberOfDelayedMessages() != 1 && System.currentTimeMillis() - start2 < 2000) { - try { - Thread.sleep(30); - } catch (InterruptedException ignored) { - - } - v1.getScheduledMessages(1); - } - assertEquals(v1.getNumberOfDelayedMessages(), 1); + // Wait until prune catches up + Awaitility.await().atMost(Duration.ofSeconds(5)).pollInterval(Duration.ofMillis(30)) + .untilAsserted(() -> { + v1.getScheduledMessages(1); + assertEquals(v1.getNumberOfDelayedMessages(), 1); + }); v1.close(); v2.close(); @@ -590,18 +585,13 @@ public void testMemoryGrowthAndPruneShrink() throws Exception { env.time.set(200); v.updateMarkDeletePosition(PositionFactory.create(1, 25)); v.getScheduledMessages(100); - // Wait for prune-by-time throttling window and trigger reads to allow prune to occur - long memAfter = manager.topicBufferMemoryBytes(); - long startWall = System.currentTimeMillis(); - while (memAfter >= memBefore && System.currentTimeMillis() - startWall < 2000) { - try { - Thread.sleep(60); - } catch (InterruptedException ignored) { - } - v.getScheduledMessages(1); - memAfter = manager.topicBufferMemoryBytes(); - } - assertTrue(memAfter < memBefore, "Memory should shrink after prune"); + // Wait until memory shrinks due to prune + Awaitility.await().atMost(Duration.ofSeconds(5)).pollInterval(Duration.ofMillis(60)) + .untilAsserted(() -> { + v.getScheduledMessages(1); + long memAfter = manager.topicBufferMemoryBytes(); + assertTrue(memAfter < memBefore, "Memory should shrink after prune"); + }); v.close(); } From 23fbfb6d3ba06560762d60ed9433d5f927087b25 Mon Sep 17 00:00:00 2001 From: Denovo1998 Date: Sun, 2 Nov 2025 13:08:56 +0800 Subject: [PATCH 15/16] [feat][broker] Update documentation for InMemoryTopicDelayedDeliveryTrackerFactory and related configurations to clarify memory usage and implementation details --- conf/broker.conf | 3 ++- conf/standalone.conf | 3 ++- .../InMemoryTopicDelayedDeliveryTrackerFactory.java | 2 +- .../InMemoryTopicDelayedDeliveryTrackerManager.java | 12 ++++++++++++ 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/conf/broker.conf b/conf/broker.conf index a995af650a8d1..dc3b055682038 100644 --- a/conf/broker.conf +++ b/conf/broker.conf @@ -655,7 +655,8 @@ delayedDeliveryEnabled=true # If value is "org.apache.pulsar.broker.delayed.BucketDelayedDeliveryTrackerFactory", # will create bucket based delayed message index tracker. # If value is "org.apache.pulsar.broker.delayed.InMemoryTopicDelayedDeliveryTrackerFactory", -# will use topic-level in-memory delayed message index tracker (alias of the default in-memory implementation). +# will use a topic-level in-memory delayed message index tracker (a distinct implementation that shares +# the index across subscriptions to reduce memory usage in multi-subscription scenarios). delayedDeliveryTrackerFactoryClassName=org.apache.pulsar.broker.delayed.InMemoryDelayedDeliveryTrackerFactory # Control the tick time for when retrying on delayed delivery, diff --git a/conf/standalone.conf b/conf/standalone.conf index e83777aad8511..1e1f5c64d8825 100644 --- a/conf/standalone.conf +++ b/conf/standalone.conf @@ -1423,7 +1423,8 @@ delayedDeliveryEnabled=true # If value is "org.apache.pulsar.broker.delayed.BucketDelayedDeliveryTrackerFactory", # will create bucket based delayed message index tracker. # If value is "org.apache.pulsar.broker.delayed.InMemoryTopicDelayedDeliveryTrackerFactory", -# will use topic-level in-memory delayed message index tracker (alias of the default in-memory implementation). +# will use a topic-level in-memory delayed message index tracker (a distinct implementation that shares +# the index across subscriptions to reduce memory usage in multi-subscription scenarios). delayedDeliveryTrackerFactoryClassName=org.apache.pulsar.broker.delayed.InMemoryDelayedDeliveryTrackerFactory # Control the tick time for when retrying on delayed delivery, diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerFactory.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerFactory.java index b4ccf94dcfa5d..9b707e799fc3b 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerFactory.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerFactory.java @@ -97,7 +97,7 @@ public DelayedDeliveryTracker newTracker(AbstractPersistentDispatcherMultipleCon tracker = newTracker0(dispatcher); } catch (Exception e) { // it should never go here - log.warn("Failed to create InMemoryDelayedDeliveryTracker, topic {}, subscription {}", + log.warn("Failed to create InMemoryTopicDelayedDeliveryTracker, topic {}, subscription {}", topicName, subscriptionName, e); } return tracker; diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java index fb3278ed044dd..1b3f954298b2d 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/InMemoryTopicDelayedDeliveryTrackerManager.java @@ -108,6 +108,18 @@ static class SubContext { private final Clock clock; private volatile Position markDeletePosition; + /** + * Constructs a new SubContext for a subscription. + * + * @param dispatcher the dispatcher associated with the subscription + * @param tickTimeMillis the tick interval in milliseconds for delayed delivery checks + * @param isDelayedDeliveryDeliverAtTimeStrict if true, delayed messages are delivered strictly at their + * scheduled time; if false, messages may be delivered in the next + * tick window + * @param fixedDelayDetectionLookahead the lookahead window (in milliseconds) used for + * detecting fixed-delay messages + * @param clock the clock instance used for time calculations + */ SubContext(AbstractPersistentDispatcherMultipleConsumers dispatcher, long tickTimeMillis, boolean isDelayedDeliveryDeliverAtTimeStrict, long fixedDelayDetectionLookahead, Clock clock) { From 4aada3991834655733db4a6b0f48ee12977e6504 Mon Sep 17 00:00:00 2001 From: Denovo1998 Date: Sun, 2 Nov 2025 16:55:29 +0800 Subject: [PATCH 16/16] feat[broker] Centralize mark-delete propagation to topic-level delayed tracker across dispatcher implementations --- ...PersistentDispatcherMultipleConsumers.java | 28 +++++++++++++++++++ ...PersistentDispatcherMultipleConsumers.java | 22 +++++---------- ...entDispatcherMultipleConsumersClassic.java | 22 +++++---------- ...tStickyKeyDispatcherMultipleConsumers.java | 2 ++ ...KeyDispatcherMultipleConsumersClassic.java | 3 ++ 5 files changed, 47 insertions(+), 30 deletions(-) diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/AbstractPersistentDispatcherMultipleConsumers.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/AbstractPersistentDispatcherMultipleConsumers.java index 79d365b9fee21..c3ebc159c22db 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/AbstractPersistentDispatcherMultipleConsumers.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/AbstractPersistentDispatcherMultipleConsumers.java @@ -19,9 +19,13 @@ package org.apache.pulsar.broker.service.persistent; import java.util.Map; +import java.util.Optional; import org.apache.bookkeeper.mledger.AsyncCallbacks; import org.apache.bookkeeper.mledger.ManagedCursor; +import org.apache.bookkeeper.mledger.Position; import org.apache.pulsar.broker.ServiceConfiguration; +import org.apache.pulsar.broker.delayed.DelayedDeliveryTracker; +import org.apache.pulsar.broker.delayed.InMemoryTopicDelayedDeliveryTracker; import org.apache.pulsar.broker.service.AbstractDispatcherMultipleConsumers; import org.apache.pulsar.broker.service.Dispatcher; import org.apache.pulsar.broker.service.Subscription; @@ -64,4 +68,28 @@ public AbstractPersistentDispatcherMultipleConsumers(Subscription subscription, public abstract Map getBucketDelayedIndexStats(); public abstract boolean isClassic(); + + /** + * Expose the optional DelayedDeliveryTracker from implementations. + */ + protected abstract Optional getDelayedDeliveryTrackerOptional(); + + /** + * Propagate mark-delete progress to topic-level in-memory delayed tracker (when enabled). + * Centralizes repeated logic across dispatcher implementations. + */ + protected final void propagateMarkDeleteToTopicDelayedTracker() { + Optional trackerOpt; + synchronized (this) { + trackerOpt = getDelayedDeliveryTrackerOptional(); + } + trackerOpt.ifPresent(tracker -> { + if (tracker instanceof InMemoryTopicDelayedDeliveryTracker view) { + Position md = getCursor().getMarkDeletedPosition(); + if (md != null) { + view.updateMarkDeletePosition(md); + } + } + }); + } } diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java index a4faaa7619709..b38eac99af32e 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java @@ -53,7 +53,6 @@ import org.apache.pulsar.broker.delayed.DelayedDeliveryTracker; import org.apache.pulsar.broker.delayed.DelayedDeliveryTrackerFactory; import org.apache.pulsar.broker.delayed.InMemoryDelayedDeliveryTracker; -import org.apache.pulsar.broker.delayed.InMemoryTopicDelayedDeliveryTracker; import org.apache.pulsar.broker.delayed.bucket.BucketDelayedDeliveryTracker; import org.apache.pulsar.broker.loadbalance.extensions.data.BrokerLookupData; import org.apache.pulsar.broker.service.AbstractDispatcherMultipleConsumers; @@ -1477,22 +1476,15 @@ public ManagedCursor getCursor() { return cursor; } + @Override + protected Optional getDelayedDeliveryTrackerOptional() { + return delayedDeliveryTracker; + } + @Override public void markDeletePositionMoveForward() { - // Event-driven mark-delete notification: update the topic-level delayed tracker view - Optional trackerOpt; - synchronized (this) { - trackerOpt = this.delayedDeliveryTracker; - } - trackerOpt.ifPresent(tracker -> { - if (tracker instanceof InMemoryTopicDelayedDeliveryTracker view) { - Position md = cursor.getMarkDeletedPosition(); - if (md != null) { - // Only update the cached mark-delete; avoid heavy operations in this callback - view.updateMarkDeletePosition(md); - } - } - }); + // Centralized propagation to topic-level tracker + propagateMarkDeleteToTopicDelayedTracker(); } protected int getStickyKeyHash(Entry entry) { diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumersClassic.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumersClassic.java index dc0e51a29211a..2656074cd674b 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumersClassic.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumersClassic.java @@ -50,7 +50,6 @@ import org.apache.pulsar.broker.delayed.DelayedDeliveryTracker; import org.apache.pulsar.broker.delayed.DelayedDeliveryTrackerFactory; import org.apache.pulsar.broker.delayed.InMemoryDelayedDeliveryTracker; -import org.apache.pulsar.broker.delayed.InMemoryTopicDelayedDeliveryTracker; import org.apache.pulsar.broker.delayed.bucket.BucketDelayedDeliveryTracker; import org.apache.pulsar.broker.loadbalance.extensions.data.BrokerLookupData; import org.apache.pulsar.broker.service.AbstractDispatcherMultipleConsumers; @@ -1308,22 +1307,15 @@ public ManagedCursor getCursor() { return cursor; } + @Override + protected Optional getDelayedDeliveryTrackerOptional() { + return delayedDeliveryTracker; + } + @Override public void markDeletePositionMoveForward() { - // Event-driven mark-delete notification: update the topic-level delayed tracker view - Optional trackerOpt; - synchronized (this) { - trackerOpt = this.delayedDeliveryTracker; - } - trackerOpt.ifPresent(tracker -> { - if (tracker instanceof InMemoryTopicDelayedDeliveryTracker view) { - Position md = cursor.getMarkDeletedPosition(); - if (md != null) { - // Only update the cached mark-delete; avoid heavy operations in this callback - view.updateMarkDeletePosition(md); - } - } - }); + // Centralized propagation to topic-level tracker + propagateMarkDeleteToTopicDelayedTracker(); } protected int getStickyKeyHash(Entry entry) { diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentStickyKeyDispatcherMultipleConsumers.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentStickyKeyDispatcherMultipleConsumers.java index c2afc35c619e9..5d24d4078b6cf 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentStickyKeyDispatcherMultipleConsumers.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentStickyKeyDispatcherMultipleConsumers.java @@ -625,6 +625,8 @@ public void markDeletePositionMoveForward() { // reschedule a read with a backoff after moving the mark-delete position forward since there might have // been consumers that were blocked by hash and couldn't make progress reScheduleReadWithKeySharedUnblockingInterval(); + // Propagate mark-delete progress to topic-level tracker view + propagateMarkDeleteToTopicDelayedTracker(); } /** diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentStickyKeyDispatcherMultipleConsumersClassic.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentStickyKeyDispatcherMultipleConsumersClassic.java index 1f29cb7362685..dfdfc937d8b5e 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentStickyKeyDispatcherMultipleConsumersClassic.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentStickyKeyDispatcherMultipleConsumersClassic.java @@ -492,6 +492,9 @@ && removeConsumersFromRecentJoinedConsumers()) { // can finally read more messages. It's safe to call readMoreEntries() multiple times. readMoreEntries(); } + + // Propagate mark-delete progress to topic-level tracker view + propagateMarkDeleteToTopicDelayedTracker(); } }); }