You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "poorbarcode (via GitHub)" <gi...@apache.org> on 2023/12/26 20:11:47 UTC

[PR] [improve] [broker] Do not check consumer permission when updating subscription dispatch throttling [pulsar]

poorbarcode opened a new pull request, #21805:
URL: https://github.com/apache/pulsar/pull/21805

   ### Motivation
   
   When updating subscription dispatch throttling, Pulsar checks the consumer permission first, I think this check is unnecessary, so remove this check.
   
   https://github.com/apache/pulsar/blob/master/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java#L3762-L3774
   
   ```java
   subscriptions.forEach((subName, sub) -> {
       List<CompletableFuture<Void>> consumerCheckFutures = new ArrayList<>(sub.getConsumers().size());
       sub.getConsumers().forEach(consumer -> consumerCheckFutures.add(consumer.checkPermissionsAsync()));
       subscriptionCheckFutures.add(FutureUtil.waitForAll(consumerCheckFutures).thenRun(() -> {
           Dispatcher dispatcher = sub.getDispatcher();
           if (dispatcher != null) {
               dispatcher.updateRateLimiter();
           }
       }));
   });
   ```
   
   ### Modifications
   
   Remove the unnecessary check when updating subscription dispatch throttling
   
   ### Documentation
   
   <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
   
   - [ ] `doc` <!-- Your PR contains doc changes. -->
   - [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
   - [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
   - [ ] `doc-complete` <!-- Docs have been already added -->
   
   ### Matching PR in forked repository
   
   PR in forked repository: x


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

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

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

Re: [PR] [improve] [broker] Do not check consumer permission when updating subscription dispatch throttling [pulsar]

Posted by "Technoboy- (via GitHub)" <gi...@apache.org>.
Technoboy- commented on PR #21805:
URL: https://github.com/apache/pulsar/pull/21805#issuecomment-1869873247

   If we remove this, we should also remove `producer.checkPermissions`
   https://github.com/apache/pulsar/blob/6169e3da9569b6f109546c44a1babf41da6c527e/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java#L3078-L3081


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

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

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

Re: [PR] [improve] [broker] Do not check consumer permission when updating subscription dispatch throttling [pulsar]

Posted by "poorbarcode (via GitHub)" <gi...@apache.org>.
poorbarcode closed pull request #21805: [improve] [broker] Do not check consumer permission when updating subscription dispatch throttling
URL: https://github.com/apache/pulsar/pull/21805


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

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

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