Skip to content

Commit

Permalink
[Fix][broker]Fix topic dispatch rate limiter not init on broker-level (
Browse files Browse the repository at this point in the history
  • Loading branch information
AnonHxy authored and weimob-wuxuanqi committed Jul 14, 2022
1 parent dd4d06d commit 80faa8c
Show file tree
Hide file tree
Showing 4 changed files with 154 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -894,6 +894,9 @@ public void resetTopicPublishCountAndEnableReadIfRequired() {
}
}

public void updateDispatchRateLimiter() {
}

@Override
public void resetBrokerPublishCountAndEnableReadIfRequired(boolean doneBrokerReset) {
// topic rate not exceeded, and completed broker limiter reset.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2414,9 +2414,7 @@ private void updateTopicMessageDispatchRate() {
forEachTopic(topic -> {
if (topic instanceof AbstractTopic) {
((AbstractTopic) topic).updateBrokerDispatchRate();
}
if (topic.getDispatchRateLimiter().isPresent()) {
topic.getDispatchRateLimiter().get().updateDispatchRate();
((AbstractTopic) topic).updateDispatchRateLimiter();
}
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2357,6 +2357,12 @@ public CompletableFuture<Void> onLocalPoliciesUpdate() {
return checkPersistencePolicies();
}

@Override
public void updateDispatchRateLimiter() {
initializeDispatchRateLimiterIfNeeded();
dispatchRateLimiter.ifPresent(DispatchRateLimiter::updateDispatchRate);
}

@Override
public CompletableFuture<Void> onPoliciesUpdate(Policies data) {
if (log.isDebugEnabled()) {
Expand All @@ -2374,7 +2380,7 @@ public CompletableFuture<Void> onPoliciesUpdate(Policies data) {

isAllowAutoUpdateSchema = data.is_allow_auto_update_schema;

initializeDispatchRateLimiterIfNeeded();
updateDispatchRateLimiter();

updateSubscribeRateLimiter();

Expand All @@ -2393,8 +2399,6 @@ public CompletableFuture<Void> onPoliciesUpdate(Policies data) {
CompletableFuture<Void> replicationFuture = checkReplicationAndRetryOnFailure();
CompletableFuture<Void> dedupFuture = checkDeduplicationStatus();
CompletableFuture<Void> persistentPoliciesFuture = checkPersistencePolicies();
// update rate-limiter if policies updated
dispatchRateLimiter.ifPresent(DispatchRateLimiter::updateDispatchRate);
return CompletableFuture.allOf(replicationFuture, dedupFuture, persistentPoliciesFuture,
preCreateSubscriptionForCompactionIfNeeded());
});
Expand Down Expand Up @@ -3013,10 +3017,7 @@ public void onUpdate(TopicPolicies policies) {
}
updateTopicPolicy(policies);

initializeTopicDispatchRateLimiterIfNeeded(policies);

dispatchRateLimiter.ifPresent(DispatchRateLimiter::updateDispatchRate);

updateDispatchRateLimiter();
updateSubscriptionsDispatcherRateLimiter().thenRun(() -> {
updatePublishDispatcher();
updateSubscribeRateLimiter();
Expand Down Expand Up @@ -3054,14 +3055,6 @@ private CompletableFuture<Void> updateSubscriptionsDispatcherRateLimiter() {
return FutureUtil.waitForAll(subscriptionCheckFutures);
}

private void initializeTopicDispatchRateLimiterIfNeeded(TopicPolicies policies) {
synchronized (dispatchRateLimiterLock) {
if (!dispatchRateLimiter.isPresent() && policies.getDispatchRate() != null) {
this.dispatchRateLimiter = Optional.of(new DispatchRateLimiter(this, Type.TOPIC));
}
}
}

protected CompletableFuture<Void> initTopicPolicy() {
if (brokerService.pulsar().getConfig().isSystemTopicEnabled()
&& brokerService.pulsar().getConfig().isTopicLevelPoliciesEnabled()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
/**
* 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.service;

import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertTrue;
import lombok.Cleanup;
import org.apache.pulsar.broker.service.persistent.PersistentTopic;
import org.apache.pulsar.client.api.Producer;
import org.apache.pulsar.common.policies.data.DispatchRate;
import org.awaitility.Awaitility;
import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;

@Test(groups = "broker")
public class TopicDispatchRateLimiterTest extends BrokerTestBase {
@BeforeMethod
@Override
protected void setup() throws Exception {
conf.setDispatchThrottlingRatePerTopicInMsg(0);
conf.setDispatchThrottlingRatePerTopicInByte(0L);
super.baseSetup();
}

@AfterMethod(alwaysRun = true)
@Override
protected void cleanup() throws Exception {
super.internalCleanup();
}

@Test
public void testTopicDispatchRateLimiterPerTopicInMsgOnlyBrokerLevel() throws Exception {
final String topicName = "persistent://" + newTopicName();

@Cleanup
Producer<byte[]> producer = pulsarClient.newProducer().topic(topicName).create();
PersistentTopic topic = (PersistentTopic) pulsar.getBrokerService().getOrCreateTopic(topicName).get();
assertNotNull(topic);
assertTrue(topic.getDispatchRateLimiter().isEmpty());

admin.brokers().updateDynamicConfiguration("dispatchThrottlingRatePerTopicInMsg", "100");
Awaitility.await().untilAsserted(() ->
assertEquals(pulsar.getConfig().getDispatchThrottlingRatePerTopicInMsg(), 100));

Awaitility.await().untilAsserted(() ->
assertTrue(topic.getDispatchRateLimiter().isPresent()));
assertEquals(topic.getDispatchRateLimiter().get().getAvailableDispatchRateLimitOnMsg(), 100L);
}

@Test
public void testTopicDispatchRateLimiterPerTopicInByteOnlyBrokerLevel() throws Exception {
final String topicName = "persistent://" + newTopicName();

@Cleanup
Producer<byte[]> producer = pulsarClient.newProducer().topic(topicName).create();
PersistentTopic topic = (PersistentTopic) pulsar.getBrokerService().getOrCreateTopic(topicName).get();
assertNotNull(topic);
assertTrue(topic.getDispatchRateLimiter().isEmpty());

admin.brokers().updateDynamicConfiguration("dispatchThrottlingRatePerTopicInByte", "1000");
Awaitility.await().untilAsserted(() ->
assertEquals(pulsar.getConfig().getDispatchThrottlingRatePerTopicInByte(), 1000L));

Awaitility.await().untilAsserted(() ->
assertTrue(topic.getDispatchRateLimiter().isPresent()));
assertEquals(topic.getDispatchRateLimiter().get().getAvailableDispatchRateLimitOnByte(), 1000L);
}

@Test
public void testTopicDispatchRateLimiterOnlyNamespaceLevel() throws Exception {
final String topicName = "persistent://" + newTopicName();

@Cleanup
Producer<byte[]> producer = pulsarClient.newProducer().topic(topicName).create();
PersistentTopic topic = (PersistentTopic) pulsar.getBrokerService().getOrCreateTopic(topicName).get();
assertNotNull(topic);
assertTrue(topic.getDispatchRateLimiter().isEmpty());

DispatchRate dispatchRate = DispatchRate
.builder()
.dispatchThrottlingRateInMsg(100)
.dispatchThrottlingRateInByte(1000L)
.build();
admin.namespaces().setDispatchRate("prop/ns-abc", dispatchRate);

Awaitility.await().untilAsserted(() -> {
assertNotNull(admin.namespaces().getDispatchRate("prop/ns-abc"));
assertEquals(admin.namespaces().getDispatchRate("prop/ns-abc").getDispatchThrottlingRateInMsg(), 100);
assertEquals(admin.namespaces().getDispatchRate("prop/ns-abc").getDispatchThrottlingRateInByte(), 1000L);
});

Awaitility.await().untilAsserted(() -> assertTrue(topic.getDispatchRateLimiter().isPresent()));
assertEquals(topic.getDispatchRateLimiter().get().getAvailableDispatchRateLimitOnMsg(), 100);
assertEquals(topic.getDispatchRateLimiter().get().getAvailableDispatchRateLimitOnByte(), 1000L);
}

@Test
public void testTopicDispatchRateLimiterOnlyTopicLevel() throws Exception {
final String topicName = "persistent://" + newTopicName();

@Cleanup
Producer<byte[]> producer = pulsarClient.newProducer().topic(topicName).create();
PersistentTopic topic = (PersistentTopic) pulsar.getBrokerService().getOrCreateTopic(topicName).get();
assertNotNull(topic);
assertTrue(topic.getDispatchRateLimiter().isEmpty());

DispatchRate dispatchRate = DispatchRate
.builder()
.dispatchThrottlingRateInMsg(100)
.dispatchThrottlingRateInByte(1000L)
.build();
admin.topicPolicies().setDispatchRate(topicName, dispatchRate);

Awaitility.await().untilAsserted(() -> {
assertNotNull(admin.topicPolicies().getDispatchRate(topicName));
assertEquals(admin.topicPolicies().getDispatchRate(topicName).getDispatchThrottlingRateInMsg(), 100);
assertEquals(admin.topicPolicies().getDispatchRate(topicName).getDispatchThrottlingRateInByte(), 1000L);
});

Awaitility.await().untilAsserted(() -> assertTrue(topic.getDispatchRateLimiter().isPresent()));
assertEquals(topic.getDispatchRateLimiter().get().getAvailableDispatchRateLimitOnMsg(), 100);
assertEquals(topic.getDispatchRateLimiter().get().getAvailableDispatchRateLimitOnByte(), 1000L);
}
}

0 comments on commit 80faa8c

Please sign in to comment.