[GitHub] liubao68 commented on a change in pull request #770: [SCB-651] Fixes bug of qps rate limit value

[GitBox <gi...@apache.org>]

liubao68 commented on a change in pull request #770: [SCB-651] Fixes bug of qps rate limit value
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/770#discussion_r195332212
 
 

 ##########
 File path: handlers/handler-flowcontrol-qps/src/main/java/org/apache/servicecomb/qps/QpsController.java
 ##########
 @@ -57,6 +60,11 @@ public void setQpsLimit(Integer qpsLimit) {
   public boolean isLimitNewRequest() {
     long newCount = requestCount.incrementAndGet();
     long msNow = System.currentTimeMillis();
+    if (firstCycle) {
 
 Review comment:
   Is this solution the same as set lastRequestCount = 1?
   In concurrent scenario, flow control is not work very precisely as the configured rate. In practice, we can not assume a concrete rate and will set the limit a bit higher than the expected value. 
   So I think this modification is not quit necessary. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services