Skip to content

Commit f3818ec

Browse files
committed
[improve][broker] Load topic policies on non-persistent topic load and gate the policy replay (#26134)
(cherry picked from commit 0629dc5)
1 parent 8840524 commit f3818ec

11 files changed

Lines changed: 451 additions & 102 deletions

File tree

pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1729,6 +1729,15 @@ The max allowed delay for delayed delivery (in milliseconds). If the broker rece
17291729
+ "please enable the system topic first.")
17301730
private boolean topicLevelPoliciesEnabled = true;
17311731

1732+
@FieldContext(
1733+
category = CATEGORY_SERVER,
1734+
doc = "When enabled, all registered topic-policy listeners in a namespace are re-notified with the current"
1735+
+ " topic policies after the namespace's topic-policy cache finishes its initial load. Topics load"
1736+
+ " and apply their own policies when they are loaded, so this broadcast is normally redundant; it"
1737+
+ " is only needed for custom plugins that register TopicPolicyListeners and depend on it for"
1738+
+ " backwards compatibility. Disabled by default.")
1739+
private boolean topicPolicyListenerReplayEnabled = false;
1740+
17321741
@FieldContext(
17331742
category = CATEGORY_SERVER,
17341743
doc = """

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractTopic.java

Lines changed: 99 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import java.util.Collection;
3232
import java.util.Collections;
3333
import java.util.EnumSet;
34+
import java.util.HashMap;
3435
import java.util.List;
3536
import java.util.Map;
3637
import java.util.Objects;
@@ -48,6 +49,7 @@
4849
import java.util.concurrent.atomic.AtomicLongFieldUpdater;
4950
import java.util.concurrent.atomic.LongAdder;
5051
import java.util.concurrent.locks.ReentrantReadWriteLock;
52+
import java.util.function.Function;
5153
import java.util.function.ToLongFunction;
5254
import lombok.Getter;
5355
import lombok.Setter;
@@ -58,6 +60,7 @@
5860
import org.apache.commons.lang3.tuple.Pair;
5961
import org.apache.pulsar.broker.PulsarService;
6062
import org.apache.pulsar.broker.ServiceConfiguration;
63+
import org.apache.pulsar.broker.loadbalance.extensions.ExtensibleLoadManagerImpl;
6164
import org.apache.pulsar.broker.resourcegroup.ResourceGroup;
6265
import org.apache.pulsar.broker.resourcegroup.ResourceGroupPublishLimiter;
6366
import org.apache.pulsar.broker.service.BrokerServiceException.ConsumerBusyException;
@@ -114,6 +117,10 @@ public abstract class AbstractTopic implements Topic, TopicPolicyListener {
114117

115118
protected final BrokerService brokerService;
116119

120+
// Wraps this topic as a TopicPolicyListener so topic-policy updates received while the initial policy is still
121+
// loading are buffered and applied in order once initTopicPolicy() completes initialization.
122+
protected final TopicPolicyListenerWrapper topicPolicyListener = new TopicPolicyListenerWrapper(this);
123+
117124
// Prefix for replication cursors
118125
protected final String replicatorPrefix;
119126

@@ -173,7 +180,17 @@ public abstract class AbstractTopic implements Topic, TopicPolicyListener {
173180
AtomicLongFieldUpdater.newUpdater(AbstractTopic.class, "usageCount");
174181
private volatile long usageCount = 0;
175182

176-
private Map<String/*subscription*/, SubscriptionPolicies> subscriptionPolicies = Collections.emptyMap();
183+
// Effective per-subscription policies, merged from the local and global topic policies with local precedence.
184+
// Unlike the PolicyHierarchyValue-backed fields, subscriptionPolicies is a plain map. It is kept as the merge of
185+
// the two scopes below (rather than assigned directly) because the local-before-global initialization order
186+
// (see TopicPolicyListenerWrapper) would otherwise let the global map -- empty by default in TopicPolicies --
187+
// overwrite and clear the local per-subscription policies that were applied just before it.
188+
// subscriptionPolicies is volatile because it is read on dispatch threads (getSubscriptionDispatchRate) while it
189+
// is updated on the policy-update thread. localSubscriptionPolicies/globalSubscriptionPolicies are only ever read
190+
// and written on the (single) policy-update thread, so they don't need to be volatile.
191+
private volatile Map<String/*subscription*/, SubscriptionPolicies> subscriptionPolicies = Collections.emptyMap();
192+
private Map<String/*subscription*/, SubscriptionPolicies> localSubscriptionPolicies = Collections.emptyMap();
193+
private Map<String/*subscription*/, SubscriptionPolicies> globalSubscriptionPolicies = Collections.emptyMap();
177194

178195
protected final LongAdder msgOutFromRemovedSubscriptions = new LongAdder();
179196
protected final LongAdder bytesOutFromRemovedSubscriptions = new LongAdder();
@@ -310,11 +327,36 @@ protected void updateTopicPolicy(TopicPolicies data) {
310327
topicPolicies.getEntryFilters().updateTopicValue(data.getEntryFilters(), isGlobalPolicies);
311328
topicPolicies.getDispatcherPauseOnAckStatePersistentEnabled()
312329
.updateTopicValue(data.getDispatcherPauseOnAckStatePersistentEnabled(), isGlobalPolicies);
313-
this.subscriptionPolicies = data.getSubscriptionPolicies();
330+
331+
// Merge instead of assigning directly: keep the local and global per-subscription policies separately and
332+
// recompute the effective map with local precedence, so applying the (default-empty) global map does not
333+
// clear the local per-subscription policies during the local-before-global initialization.
334+
if (isGlobalPolicies) {
335+
globalSubscriptionPolicies = data.getSubscriptionPolicies();
336+
} else {
337+
localSubscriptionPolicies = data.getSubscriptionPolicies();
338+
}
339+
subscriptionPolicies = mergeSubscriptionPolicies(globalSubscriptionPolicies, localSubscriptionPolicies);
314340

315341
updateEntryFilters();
316342
}
317343

344+
// Merges the global and local per-subscription policies with local precedence: a subscription present in the
345+
// local policies keeps its local value; otherwise the global value (if any) is used.
346+
private static Map<String, SubscriptionPolicies> mergeSubscriptionPolicies(
347+
Map<String, SubscriptionPolicies> globalSubscriptionPolicies,
348+
Map<String, SubscriptionPolicies> localSubscriptionPolicies) {
349+
if (globalSubscriptionPolicies.isEmpty()) {
350+
return localSubscriptionPolicies;
351+
}
352+
if (localSubscriptionPolicies.isEmpty()) {
353+
return globalSubscriptionPolicies;
354+
}
355+
Map<String, SubscriptionPolicies> merged = new HashMap<>(globalSubscriptionPolicies);
356+
merged.putAll(localSubscriptionPolicies);
357+
return merged;
358+
}
359+
318360
protected void updateTopicPolicyByNamespacePolicy(Policies namespacePolicies) {
319361
if (log.isDebugEnabled()) {
320362
log.debug("[{}]updateTopicPolicyByNamespacePolicy,data={}", topic, namespacePolicies);
@@ -550,19 +592,68 @@ protected boolean isProducersExceeded(boolean isRemote) {
550592
}
551593

552594
protected TopicPolicyListener getTopicPolicyListener() {
553-
return this;
554-
}
555-
556-
protected void registerTopicPolicyListener() {
557-
brokerService.getPulsar().getTopicPoliciesService()
558-
.registerListenerAsync(TopicName.getPartitionedTopicName(topic), getTopicPolicyListener());
595+
return topicPolicyListener;
559596
}
560597

561598
protected void unregisterTopicPolicyListener() {
562599
brokerService.getPulsar().getTopicPoliciesService()
563600
.unregisterListener(TopicName.getPartitionedTopicName(topic), getTopicPolicyListener());
564601
}
565602

603+
/**
604+
* Registers the topic-policy listener and applies the topic's initial policies (global and local) to this topic.
605+
* Shared by {@link org.apache.pulsar.broker.service.persistent.PersistentTopic} and
606+
* {@link org.apache.pulsar.broker.service.nonpersistent.NonPersistentTopic} so both load their own policies on
607+
* topic load, which removes the need to broadcast every topic's policy when a namespace's policy cache finishes
608+
* loading (see {@code topicPolicyListenerReplayEnabled}).
609+
*
610+
* <p>Each call re-initializes the listener wrapper and, whatever the outcome, always completes its initialization
611+
* afterwards, so the wrapper never stays in the buffering phase (dropping updates) even if policy loading fails.
612+
* This makes the method safe to run again (e.g. a future retry); runs are expected to be serialized.
613+
*/
614+
protected CompletableFuture<Void> initTopicPolicy() {
615+
final var topicPoliciesService = brokerService.getPulsar().getTopicPoliciesService();
616+
final var partitionedTopicName = TopicName.getPartitionedTopicName(topic);
617+
618+
// Begin a fresh initialization phase: updates are buffered until initialization completes below. This resets
619+
// any previous phase so the method can be run again.
620+
topicPolicyListener.startInitialization();
621+
CompletableFuture<Void> initTopicPolicyFuture =
622+
topicPoliciesService.registerListenerAsync(partitionedTopicName, topicPolicyListener)
623+
.thenCompose(registered -> {
624+
if (!registered) {
625+
return CompletableFuture.completedFuture(null);
626+
}
627+
if (ExtensibleLoadManagerImpl.isInternalTopic(topic)) {
628+
// Internal topics don't load topic-level policies
629+
return CompletableFuture.completedFuture(null);
630+
}
631+
// future for fetching global topic policies
632+
CompletableFuture<Optional<TopicPolicies>> globalPoliciesFuture =
633+
topicPoliciesService.getTopicPoliciesAsync(partitionedTopicName,
634+
TopicPoliciesService.GetType.GLOBAL_ONLY);
635+
// future for fetching local topic policies
636+
CompletableFuture<Optional<TopicPolicies>> localPoliciesFuture =
637+
topicPoliciesService.getTopicPoliciesAsync(partitionedTopicName,
638+
TopicPoliciesService.GetType.LOCAL_ONLY);
639+
return globalPoliciesFuture.thenCombine(localPoliciesFuture, (global, local) -> {
640+
// finally update the topic policies with the latest value or loaded value
641+
return CompletableFuture.runAsync(() ->
642+
topicPolicyListener.completeInitialization(global.orElse(null),
643+
local.orElse(null)),
644+
getPoliciesNotifyThread());
645+
}).thenCompose(Function.identity());
646+
});
647+
// Whatever the outcome -- success, failure, or the listener not being registered -- make sure the wrapper
648+
// leaves the initialization (buffering) phase, so it forwards any buffered value plus all future live updates
649+
// instead of dropping them. This is a no-op when the loaded policies were already applied above. Return the
650+
// whenComplete stage (not initTopicPolicyFuture) so the returned future completes only after this has run, and
651+
// whenComplete's pass-through semantics carry the original success or failure to the caller's initialize().
652+
return initTopicPolicyFuture.whenCompleteAsync((v, ex) -> {
653+
topicPolicyListener.completeInitializationUnlessAlreadyCompleted();
654+
}, getPoliciesNotifyThread());
655+
}
656+
566657
protected boolean isSameAddressProducersExceeded(Producer producer) {
567658
if (isSystemTopic() || producer.isRemote()) {
568659
return false;

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/SystemTopicBasedTopicPoliciesService.java

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -864,18 +864,43 @@ private void initPolicesCache(SystemTopicClient.Reader<PulsarEvent> reader, Comp
864864
log.debug("[{}] Reach the end of the system topic.", reader.getSystemTopic().getTopicName());
865865
}
866866

867-
// replay policy message
868-
List<CompletableFuture<Void>> notifyFutures = new ArrayList<>();
869-
for (Map.Entry<TopicName, TopicPolicies> entry : policiesCache.entrySet()) {
870-
TopicName topicName = entry.getKey();
871-
TopicPolicies policies = entry.getValue();
872-
notifyFutures.add(notifyListenersForTopicAsync(topicName, policies));
867+
// Optionally re-notify the topic-policy listeners for this namespace with the cached policies.
868+
// Topics load their own policies on load (AbstractTopic#initTopicPolicy), so this replay is only
869+
// needed for custom plugins that register TopicPolicyListeners and rely on the broadcast; it is
870+
// off by default (topicPolicyListenerReplayEnabled).
871+
if (pulsarService.getConfiguration().isTopicPolicyListenerReplayEnabled()) {
872+
NamespaceName namespaceObject = reader.getSystemTopic().getTopicName().getNamespaceObject();
873+
FutureUtil.completeAfter(future, replayTopicPolicyListeners(namespaceObject));
874+
} else {
875+
future.complete(null);
873876
}
874-
FutureUtil.completeAfter(future, FutureUtil.waitForAll(notifyFutures));
875877
}
876878
});
877879
}
878880

881+
/**
882+
* Re-notifies the registered topic-policy listeners for every topic in {@code namespace} with the currently
883+
* cached policies (both local and global). Topics apply their own policies on load, so this is only needed for
884+
* custom plugins that register {@link TopicPolicyListener}s and rely on the broadcast when a namespace's policy
885+
* cache finishes loading (see {@code topicPolicyListenerReplayEnabled}).
886+
*/
887+
@VisibleForTesting
888+
CompletableFuture<Void> replayTopicPolicyListeners(NamespaceName namespace) {
889+
List<CompletableFuture<Void>> notifyFutures = new ArrayList<>();
890+
addNamespacePolicyNotifications(policiesCache, namespace, notifyFutures);
891+
addNamespacePolicyNotifications(globalPoliciesCache, namespace, notifyFutures);
892+
return FutureUtil.waitForAll(notifyFutures);
893+
}
894+
895+
private void addNamespacePolicyNotifications(Map<TopicName, TopicPolicies> cache, NamespaceName namespace,
896+
List<CompletableFuture<Void>> notifyFutures) {
897+
for (Map.Entry<TopicName, TopicPolicies> entry : cache.entrySet()) {
898+
if (Objects.equals(entry.getKey().getNamespaceObject(), namespace)) {
899+
notifyFutures.add(notifyListenersForTopicAsync(entry.getKey(), entry.getValue()));
900+
}
901+
}
902+
}
903+
879904
// Full teardown of a namespace's topic-policies state: removes and closes the reader, the message-handler
880905
// tracker, the cached policies and the init future. Used when the whole namespace is unloaded.
881906
@VisibleForTesting

0 commit comments

Comments
 (0)