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 2021/12/29 01:28:36 UTC

[GitHub] [pulsar] yuruguo opened a new pull request #13543: [Broker] Other old values are still valid when updating resource group

yuruguo opened a new pull request #13543:
URL: https://github.com/apache/pulsar/pull/13543


   ### Motivation
   Currently, the remaining old values will be invalid when we update some values of the resource group, it is as follows:
   **step 1**
   ```
   bin/pulsar-admin resourcegroups create --msg-publish-rate 100 --byte-publish-rate 1024 --msg-dispatch-rate 200 --byte-dispatch-rate 2048 rg-test-01
   ```
   | option | value |
   | ------ | ------ |
   | msg-publish-rate | 100 |
   | byte-publish-rate | 1024 |
   | msg-dispatch-rate | 200 |
   | byte-dispatch-rate | 2048 |
   
   **step 2**
   ```
   bin/pulsar-admin resourcegroups update --msg-publish-rate 150 rg-test-01
   ```
   | option | value |
   | ------ | ------ |
   | msg-publish-rate | 150 |
   | byte-publish-rate | -1 |
   | msg-dispatch-rate | -1 |
   | byte-dispatch-rate | -1 |
   
   The purpose of this PR is to ensure other old values are still valid when updating resource group.
   **NOTE**:  #12242 and #12251  are similar to this PR.
   
   ### Documentation
   - [x] `no-need-doc` 
   
   


-- 
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



[GitHub] [pulsar] yuruguo removed a comment on pull request #13543: [Broker] Other old values are still valid when updating resource group

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #13543:
URL: https://github.com/apache/pulsar/pull/13543#issuecomment-1008023305


   /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.

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

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



[GitHub] [pulsar] codelipenghui commented on pull request #13543: [Broker] Other old values are still valid when updating resource group

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


   @Jason918 Plesae 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.

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

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



[GitHub] [pulsar] yuruguo commented on pull request #13543: [Broker] Other old values are still valid when updating resource group

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


   @codelipenghui @hangc0276 PTAL, thx!


-- 
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



[GitHub] [pulsar] yuruguo commented on pull request #13543: [Broker] Other old values are still valid when updating resource group

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


   @codelipenghui ptal thx


-- 
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



[GitHub] [pulsar] codelipenghui commented on a change in pull request #13543: [Broker] Other old values are still valid when updating resource group

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



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/ResourceGroupsBase.java
##########
@@ -63,10 +63,18 @@ protected void internalUpdateResourceGroup(String rgName, ResourceGroup rgConfig
             /*
              * assuming read-modify-write
              */
-            resourceGroup.setPublishRateInMsgs(rgConfig.getPublishRateInMsgs());
-            resourceGroup.setPublishRateInBytes(rgConfig.getPublishRateInBytes());
-            resourceGroup.setDispatchRateInMsgs(rgConfig.getDispatchRateInMsgs());
-            resourceGroup.setDispatchRateInBytes(rgConfig.getDispatchRateInBytes());
+            if (rgConfig.getPublishRateInMsgs() != -1) {

Review comment:
       Yes, this was why I am considering this part. If the existing resource group is 
   
   ```java
   private int publishRateInMsgs = 10; 
   private long publishRateInBytes = 10; 
   private int dispatchRateInMsgs = 10; 
   private long dispatchRateInBytes = 10;
   ```
   
   And want to remove the limit for `dispatchRateInMsgs`, the new resource group should be:
   
   ```java
   private int publishRateInMsgs = 10; 
   private long publishRateInBytes = 10; 
   private int dispatchRateInMsgs = -1; 
   private long dispatchRateInBytes = 10;
   ```
   
   But we can't apply `dispatchRateInMsgs = -1` in your fix right?




-- 
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



[GitHub] [pulsar] yuruguo commented on pull request #13543: [Broker] Other old values are still valid when updating resource group

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


   @merlimat @freeznet PTAL, thx!


-- 
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



[GitHub] [pulsar] Jason918 commented on a change in pull request #13543: [Broker] Other old values are still valid when updating resource group

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



##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdResourceGroups.java
##########
@@ -55,23 +55,23 @@ void run() throws PulsarAdminException {
         @Parameter(names = { "--msg-publish-rate",
                 "-mp" }, description = "message-publish-rate "
                 + "(default -1 will be overwrite if not passed)", required = false)

Review comment:
       > (default -1 will be overwrite if not passed)
   
   Do we need to update this description ?  




-- 
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



[GitHub] [pulsar] yuruguo commented on a change in pull request #13543: [Broker] Other old values are still valid when updating resource group

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



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/ResourceGroupsBase.java
##########
@@ -63,10 +63,18 @@ protected void internalUpdateResourceGroup(String rgName, ResourceGroup rgConfig
             /*
              * assuming read-modify-write
              */
-            resourceGroup.setPublishRateInMsgs(rgConfig.getPublishRateInMsgs());
-            resourceGroup.setPublishRateInBytes(rgConfig.getPublishRateInBytes());
-            resourceGroup.setDispatchRateInMsgs(rgConfig.getDispatchRateInMsgs());
-            resourceGroup.setDispatchRateInBytes(rgConfig.getDispatchRateInBytes());
+            if (rgConfig.getPublishRateInMsgs() != -1) {

Review comment:
       Good point, you are right!
   `-1` is an occupied value, we can‘t judge whether `-1` is the default value or  user-specified value, so maybe we should remove the `-1` and judge by `null`, redefine `ResourceGroup`
   ```
    public class ResourceGroup { 
        private Integer publishRateInMsgs; 
        private Long publishRateInBytes; 
        private Integer dispatchRateInMsgs; 
        private Long dispatchRateInBytes; 
    } 
   ```




-- 
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



[GitHub] [pulsar] yuruguo removed a comment on pull request #13543: [Broker] Other old values are still valid when updating resource group

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #13543:
URL: https://github.com/apache/pulsar/pull/13543#issuecomment-1008161127


   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.

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

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



[GitHub] [pulsar] codelipenghui commented on a change in pull request #13543: [Broker] Other old values are still valid when updating resource group

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



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/ResourceGroupsBase.java
##########
@@ -63,10 +63,18 @@ protected void internalUpdateResourceGroup(String rgName, ResourceGroup rgConfig
             /*
              * assuming read-modify-write
              */
-            resourceGroup.setPublishRateInMsgs(rgConfig.getPublishRateInMsgs());
-            resourceGroup.setPublishRateInBytes(rgConfig.getPublishRateInBytes());
-            resourceGroup.setDispatchRateInMsgs(rgConfig.getDispatchRateInMsgs());
-            resourceGroup.setDispatchRateInBytes(rgConfig.getDispatchRateInBytes());
+            if (rgConfig.getPublishRateInMsgs() != -1) {

Review comment:
       What does `-1` means here? do not have a limit for publish rate? or `0` means no limit for publish rate?




-- 
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



[GitHub] [pulsar] yuruguo commented on a change in pull request #13543: [Broker] Other old values are still valid when updating resource group

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



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/ResourceGroupsBase.java
##########
@@ -63,10 +63,18 @@ protected void internalUpdateResourceGroup(String rgName, ResourceGroup rgConfig
             /*
              * assuming read-modify-write
              */
-            resourceGroup.setPublishRateInMsgs(rgConfig.getPublishRateInMsgs());
-            resourceGroup.setPublishRateInBytes(rgConfig.getPublishRateInBytes());
-            resourceGroup.setDispatchRateInMsgs(rgConfig.getDispatchRateInMsgs());
-            resourceGroup.setDispatchRateInBytes(rgConfig.getDispatchRateInBytes());
+            if (rgConfig.getPublishRateInMsgs() != -1) {

Review comment:
       The default value of `rate` is `- 1`
   https://github.com/apache/pulsar/blob/ef8120f0ae16d829036292dc0b50640bc515ae98/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdResourceGroups.java#L87-L106
   https://github.com/apache/pulsar/blob/ef8120f0ae16d829036292dc0b50640bc515ae98/pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/PublishRate.java#L28-L37
   which means not take effect or not limit.




-- 
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



[GitHub] [pulsar] yuruguo commented on pull request #13543: [Broker] Other old values are still valid when updating resource group

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


   /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.

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

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



[GitHub] [pulsar] yuruguo commented on pull request #13543: [Broker] Other old values are still valid when updating resource group

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


   /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.

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

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



[GitHub] [pulsar] yuruguo commented on pull request #13543: [Broker] Other old values are still valid when updating resource group

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


   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.

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

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



[GitHub] [pulsar] yuruguo removed a comment on pull request #13543: [Broker] Other old values are still valid when updating resource group

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #13543:
URL: https://github.com/apache/pulsar/pull/13543#issuecomment-1013118689


   @codelipenghui @hangc0276 PTAL, thx!


-- 
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



[GitHub] [pulsar] yuruguo removed a comment on pull request #13543: [Broker] Other old values are still valid when updating resource group

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #13543:
URL: https://github.com/apache/pulsar/pull/13543#issuecomment-1016249857


   /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.

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

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



[GitHub] [pulsar] codelipenghui merged pull request #13543: [Broker] Other old values are still valid when updating resource group

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


   


-- 
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



[GitHub] [pulsar] yuruguo commented on a change in pull request #13543: [Broker] Other old values are still valid when updating resource group

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



##########
File path: pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdResourceGroups.java
##########
@@ -55,23 +55,23 @@ void run() throws PulsarAdminException {
         @Parameter(names = { "--msg-publish-rate",
                 "-mp" }, description = "message-publish-rate "
                 + "(default -1 will be overwrite if not passed)", required = false)

Review comment:
       [Create](https://github.com/apache/pulsar/blob/master/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdResourceGroups.java#L51) command not need update but [Update](https://github.com/apache/pulsar/blob/master/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdResourceGroups.java#L90) command need.




-- 
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



[GitHub] [pulsar] yuruguo commented on pull request #13543: [Broker] Other old values are still valid when updating resource group

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


   @eolivelli @codelipenghui @hangc0276 @315157973 ptal


-- 
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



[GitHub] [pulsar] yuruguo removed a comment on pull request #13543: [Broker] Other old values are still valid when updating resource group

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #13543:
URL: https://github.com/apache/pulsar/pull/13543#issuecomment-1011652705


   @codelipenghui ptal thx


-- 
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



[GitHub] [pulsar] yuruguo removed a comment on pull request #13543: [Broker] Other old values are still valid when updating resource group

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #13543:
URL: https://github.com/apache/pulsar/pull/13543#issuecomment-1008123897


   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.

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

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



[GitHub] [pulsar] yuruguo commented on pull request #13543: [Broker] Other old values are still valid when updating resource group

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


   /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.

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

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



[GitHub] [pulsar] yuruguo commented on pull request #13543: [Broker] Other old values are still valid when updating resource group

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


   /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.

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

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



[GitHub] [pulsar] yuruguo commented on pull request #13543: [Broker] Other old values are still valid when updating resource group

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


   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.

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

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



[GitHub] [pulsar] yuruguo commented on a change in pull request #13543: [Broker] Other old values are still valid when updating resource group

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



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/ResourceGroupsBase.java
##########
@@ -63,10 +63,18 @@ protected void internalUpdateResourceGroup(String rgName, ResourceGroup rgConfig
             /*
              * assuming read-modify-write
              */
-            resourceGroup.setPublishRateInMsgs(rgConfig.getPublishRateInMsgs());
-            resourceGroup.setPublishRateInBytes(rgConfig.getPublishRateInBytes());
-            resourceGroup.setDispatchRateInMsgs(rgConfig.getDispatchRateInMsgs());
-            resourceGroup.setDispatchRateInBytes(rgConfig.getDispatchRateInBytes());
+            if (rgConfig.getPublishRateInMsgs() != -1) {

Review comment:
       Good point, you are right!
   `-1` is an occupied value, we can‘t judge whether `-1` is the default value or  user-specified value, so maybe we should remove the `-1` and judge by `null`, redefine `ResourceGroup`
   ```
    public class ResourceGroup { 
        private int publishRateInMsgs = -1; 
        private long publishRateInBytes = -1; 
        private int dispatchRateInMsgs = -1; 
        private long dispatchRateInBytes = -1; 
    } 
   ```




-- 
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



[GitHub] [pulsar] yuruguo commented on a change in pull request #13543: [Broker] Other old values are still valid when updating resource group

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



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/ResourceGroupsBase.java
##########
@@ -63,10 +63,18 @@ protected void internalUpdateResourceGroup(String rgName, ResourceGroup rgConfig
             /*
              * assuming read-modify-write
              */
-            resourceGroup.setPublishRateInMsgs(rgConfig.getPublishRateInMsgs());
-            resourceGroup.setPublishRateInBytes(rgConfig.getPublishRateInBytes());
-            resourceGroup.setDispatchRateInMsgs(rgConfig.getDispatchRateInMsgs());
-            resourceGroup.setDispatchRateInBytes(rgConfig.getDispatchRateInBytes());
+            if (rgConfig.getPublishRateInMsgs() != -1) {

Review comment:
       The default value of `rate` is `- 1`
   https://github.com/apache/pulsar/blob/ef8120f0ae16d829036292dc0b50640bc515ae98/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdResourceGroups.java#L87-L106
   https://github.com/apache/pulsar/blob/1f102814b96db723592b2fdfb2bbf288d9b75fb9/pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/ResourceGroup.java#L24-L29
   which means not take effect or not limit.




-- 
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



[GitHub] [pulsar] yuruguo removed a comment on pull request #13543: [Broker] Other old values are still valid when updating resource group

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #13543:
URL: https://github.com/apache/pulsar/pull/13543#issuecomment-1002419938


   @eolivelli @codelipenghui @hangc0276 @315157973 ptal


-- 
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



[GitHub] [pulsar] eolivelli commented on pull request #13543: [Broker] Other old values are still valid when updating resource group

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


   Please note that this is kind of behavior change and we cannot cherry pick this to other released branches.
   
   I also marked this as to be noted in the release notes.
   
   Can you please advertise this change on dev@ 
   This way more folks will be notified 


-- 
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



[GitHub] [pulsar] yuruguo commented on pull request #13543: [Broker] Other old values are still valid when updating resource group

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


   /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.

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

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



[GitHub] [pulsar] yuruguo removed a comment on pull request #13543: [Broker] Other old values are still valid when updating resource group

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #13543:
URL: https://github.com/apache/pulsar/pull/13543#issuecomment-1016271866


   /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.

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

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



[GitHub] [pulsar] yuruguo commented on pull request #13543: [Broker] Other old values are still valid when updating resource group

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


   > Please note that this is kind of behavior change and we cannot cherry pick this to other released branches.
   > 
   > I also marked this as to be noted in the release notes.
   > 
   > Can you please advertise this change on dev@ This way more folks will be notified
   
   I have sent a notification email to @dev


-- 
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



[GitHub] [pulsar] yuruguo commented on a change in pull request #13543: [Broker] Other old values are still valid when updating resource group

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



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/ResourceGroupsBase.java
##########
@@ -63,10 +63,18 @@ protected void internalUpdateResourceGroup(String rgName, ResourceGroup rgConfig
             /*
              * assuming read-modify-write
              */
-            resourceGroup.setPublishRateInMsgs(rgConfig.getPublishRateInMsgs());
-            resourceGroup.setPublishRateInBytes(rgConfig.getPublishRateInBytes());
-            resourceGroup.setDispatchRateInMsgs(rgConfig.getDispatchRateInMsgs());
-            resourceGroup.setDispatchRateInBytes(rgConfig.getDispatchRateInBytes());
+            if (rgConfig.getPublishRateInMsgs() != -1) {

Review comment:
       @codelipenghui ptal again, thx!




-- 
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



[GitHub] [pulsar] yuruguo removed a comment on pull request #13543: [Broker] Other old values are still valid when updating resource group

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #13543:
URL: https://github.com/apache/pulsar/pull/13543#issuecomment-1006580645


   @merlimat @freeznet PTAL, thx!


-- 
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



[GitHub] [pulsar] yuruguo removed a comment on pull request #13543: [Broker] Other old values are still valid when updating resource group

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #13543:
URL: https://github.com/apache/pulsar/pull/13543#issuecomment-1008161231


   /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.

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

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



[GitHub] [pulsar] yuruguo commented on a change in pull request #13543: [Broker] Other old values are still valid when updating resource group

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



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/ResourceGroupsBase.java
##########
@@ -63,10 +63,18 @@ protected void internalUpdateResourceGroup(String rgName, ResourceGroup rgConfig
             /*
              * assuming read-modify-write
              */
-            resourceGroup.setPublishRateInMsgs(rgConfig.getPublishRateInMsgs());
-            resourceGroup.setPublishRateInBytes(rgConfig.getPublishRateInBytes());
-            resourceGroup.setDispatchRateInMsgs(rgConfig.getDispatchRateInMsgs());
-            resourceGroup.setDispatchRateInBytes(rgConfig.getDispatchRateInBytes());
+            if (rgConfig.getPublishRateInMsgs() != -1) {

Review comment:
       @codelipenghui ptal again, thx!




-- 
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



[GitHub] [pulsar] yuruguo removed a comment on pull request #13543: [Broker] Other old values are still valid when updating resource group

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #13543:
URL: https://github.com/apache/pulsar/pull/13543#issuecomment-1008161167


   /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.

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

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