liangyepianzhou commented on code in PR #25502:
URL: https://github.com/apache/pulsar/pull/25502#discussion_r3063437578


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/PublishRateLimiterImpl.java:
##########


Review Comment:
   Based on the latest version of the code, this change appears to have a 
correctness issue. In the path where rate limiting is disabled, the value 
passed to the following method will never be null:
   
   
`org.apache.pulsar.broker.service.PublishRateLimiter#update(org.apache.pulsar.common.policies.data.PublishRate)`
   
   Otherwise, the following method would throw an NPE:
   
   `org.apache.pulsar.broker.service.AbstractTopic#updatePublishRateLimiter`.
   
   Here it should fall back to the broker-level configuration, where the 
publishRate is never null.



##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/PublishRateLimiterImpl.java:
##########


Review Comment:
   ```suggestion
           if (maxPublishRate != null) {
           if (PublishRate.normalize(maxPublishRate) != null) {
   ```



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to