You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2020/08/20 15:45:59 UTC

[GitHub] [pulsar] hangc0276 opened a new pull request #7863: support message dispatch rate on topic level

hangc0276 opened a new pull request #7863:
URL: https://github.com/apache/pulsar/pull/7863


   ### Motivation
   Support message dispatch rate on topic level.
   Based on the system topic function.
   
   ### Modifications
   Support set message dispatch rate on topic level.
   Support get message dispatch rate on topic level.
   Support remove message dispatch rate on topic level.
   
   ### Verifying this change
   This change added tests and can be verified as follows:
   - test set topic message dispatch rate
   - test remove topic message dispatch rate
   - test get topic message dispatch rate
   - test disabled topic message dispatch rate
   
   ### Does this pull request potentially affect one of the following parts:
   If yes was chosen, please highlight the changes
   
   - Dependencies (does it add or upgrade a dependency): (no)
   - The public API: (no)
   - The schema: (don't know)
   - The default values of configurations: (yes / no)
   - The wire protocol: (yes / no)
   - The rest endpoints: (yes / no)
   - The admin cli options: (yes)
   - Anything that affects deployment: (no)
   
   ### Documentation
   Does this pull request introduce a new feature? (yes)
   If yes, how is the feature documented? (docs / JavaDocs)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] hangc0276 commented on pull request #7863: support message dispatch rate on topic level

Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #7863:
URL: https://github.com/apache/pulsar/pull/7863#issuecomment-682498392


   /pulsarbot run-failure-checks


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] hangc0276 commented on pull request #7863: support message dispatch rate on topic level

Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #7863:
URL: https://github.com/apache/pulsar/pull/7863#issuecomment-679460580


   /pulsarbot run-failure-checks


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] sijie commented on pull request #7863: support message dispatch rate on topic level

Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #7863:
URL: https://github.com/apache/pulsar/pull/7863#issuecomment-679392924


   /pulsarbot run-failure-checks


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] hangc0276 commented on pull request #7863: support message dispatch rate on topic level

Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #7863:
URL: https://github.com/apache/pulsar/pull/7863#issuecomment-682424091


   /pulsarbot run-failure-checks


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] hangc0276 commented on a change in pull request #7863: support message dispatch rate on topic level

Posted by GitBox <gi...@apache.org>.
hangc0276 commented on a change in pull request #7863:
URL: https://github.com/apache/pulsar/pull/7863#discussion_r475166741



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/SystemTopicBasedTopicPoliciesService.java
##########
@@ -329,5 +337,15 @@ Boolean getPoliciesCacheInit(NamespaceName namespaceName) {
         return policyCacheInitMap.get(namespaceName);
     }
 
+    @Override
+    public void registerListener(TopicName topicName, TopicPolicyListener<TopicPolicies> listener) {
+        listeners.computeIfAbsent(topicName, k -> Lists.newCopyOnWriteArrayList()).add(listener);
+    }
+
+    @Override
+    public void unregisterListener(TopicName topicName, TopicPolicyListener<TopicPolicies> listener) {
+        listeners.computeIfAbsent(topicName, k -> Lists.newCopyOnWriteArrayList());

Review comment:
       It should remove the specific listener,  and i have updated the code, please take a look again. Thanks




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui merged pull request #7863: support message dispatch rate on topic level

Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #7863:
URL: https://github.com/apache/pulsar/pull/7863


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui commented on pull request #7863: support message dispatch rate on topic level

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #7863:
URL: https://github.com/apache/pulsar/pull/7863#issuecomment-678000133


   @MarvinCai @jianyun8023 Please help review this PR, thanks.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] hangc0276 commented on a change in pull request #7863: support message dispatch rate on topic level

Posted by GitBox <gi...@apache.org>.
hangc0276 commented on a change in pull request #7863:
URL: https://github.com/apache/pulsar/pull/7863#discussion_r475168873



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -3056,4 +3057,53 @@ protected void internalGetLastMessageId(AsyncResponse asyncResponse, boolean aut
         });
 
     }
+
+    protected void internalGetDispatchRate(AsyncResponse asyncResponse) {

Review comment:
       I have updated the implement interface, please take a look again. Thanks




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] MarvinCai commented on a change in pull request #7863: support message dispatch rate on topic level

Posted by GitBox <gi...@apache.org>.
MarvinCai commented on a change in pull request #7863:
URL: https://github.com/apache/pulsar/pull/7863#discussion_r475149695



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
##########
@@ -2343,6 +2347,34 @@ public int getMaxUnackedMessagesOnSubscription() {
         return maxUnackedMessagesOnSubscription;
     }
 
+    @Override
+    public void onUpdate(TopicPolicies policies) {
+        if (log.isDebugEnabled()) {
+            log.debug("[{}] update topic policy: {}", topic, policies);
+        }
+
+        initializeTopicDispatchRateLimiterIfNeeded(Optional.ofNullable(policies));
+        if (this.dispatchRateLimiter.isPresent() && policies != null
+                && policies.getDispatchRate() != null) {
+            dispatchRateLimiter.ifPresent(dispatchRateLimiter ->
+                dispatchRateLimiter.updateDispatchRate(policies.getDispatchRate()));
+        }
+    }
+
+    private void initializeTopicDispatchRateLimiterIfNeeded(Optional<TopicPolicies> policies) {

Review comment:
       minor: probably updateXXIfNeeded would be better 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] hangc0276 commented on pull request #7863: support message dispatch rate on topic level

Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #7863:
URL: https://github.com/apache/pulsar/pull/7863#issuecomment-677976494


   /pulsarbot run-failure-checks


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] hangc0276 commented on pull request #7863: support message dispatch rate on topic level

Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #7863:
URL: https://github.com/apache/pulsar/pull/7863#issuecomment-678622169


   /pulsarbot run-failure-checks


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] hangc0276 commented on pull request #7863: support message dispatch rate on topic level

Posted by GitBox <gi...@apache.org>.
hangc0276 commented on pull request #7863:
URL: https://github.com/apache/pulsar/pull/7863#issuecomment-678847989


   /pulsarbot run-failure-checks


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] MarvinCai commented on a change in pull request #7863: support message dispatch rate on topic level

Posted by GitBox <gi...@apache.org>.
MarvinCai commented on a change in pull request #7863:
URL: https://github.com/apache/pulsar/pull/7863#discussion_r475149304



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/SystemTopicBasedTopicPoliciesService.java
##########
@@ -329,5 +337,15 @@ Boolean getPoliciesCacheInit(NamespaceName namespaceName) {
         return policyCacheInitMap.get(namespaceName);
     }
 
+    @Override
+    public void registerListener(TopicName topicName, TopicPolicyListener<TopicPolicies> listener) {
+        listeners.computeIfAbsent(topicName, k -> Lists.newCopyOnWriteArrayList()).add(listener);
+    }
+
+    @Override
+    public void unregisterListener(TopicName topicName, TopicPolicyListener<TopicPolicies> listener) {
+        listeners.computeIfAbsent(topicName, k -> Lists.newCopyOnWriteArrayList());

Review comment:
       wouldn't this remove all listeners for a given topic?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] jianyun8023 commented on a change in pull request #7863: support message dispatch rate on topic level

Posted by GitBox <gi...@apache.org>.
jianyun8023 commented on a change in pull request #7863:
URL: https://github.com/apache/pulsar/pull/7863#discussion_r475082742



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
##########
@@ -3056,4 +3057,53 @@ protected void internalGetLastMessageId(AsyncResponse asyncResponse, boolean aut
         });
 
     }
+
+    protected void internalGetDispatchRate(AsyncResponse asyncResponse) {

Review comment:
       It is recommended to return CompletableFuture instead of passing in the parameter AsyncResponse




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org