dubbo-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From beiwei30 (GitHub) <git...@apache.org>
Subject [GitHub] [incubator-dubbo] beiwei30 commented on pull request #3035: [DUBBO-3023]: problem in ActiveLimitFilter
Date Fri, 21 Dec 2018 05:37:32 GMT
If we add max>0 in the condition there it will cause the issue because in finally block
we always reducing the endCount irrespective of max value. 

e.g. 
1)max value is 0 so in RpcStatus.java's begin count we will not increment the counter but
in ActiveLimitFilter's invoke's finally block we will be always reducing so the final value
for single invocation will landed up -1 active count.

I think not  including max>0 and adding 

```java
} finally {      
    if (max > 0) {
                RpcStatus.endCount(url, methodName, System.currentTimeMillis() - begin, isSuccess);
                synchronized (count) {
                    count.notifyAll();
                   }
               }
         }
```

In ActiveLimitFilter should also work, but I beleive you can rewrite this into more clear
way then we suggest.


I think we might have another issue too (not because of your changes), in a scenario if max>0
(in RpcStatus.java) get satisfied (or lets max>0 condition is not even present in code
as earlier version) and the `methodStatus.active.incrementAndGet() > max` return `false`
then code of else block will be executing and again it will increment the count. So for one
call we are eventually incrementing counter twice. Do you feel this might be also a another
problem in future?Correct me if my understanding not correct.

[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3035 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org


Mime
View raw message