Ian,
The approach you mentioned, I am certain it would works. I was thinking
another approach
if (!RpcStatus.beginCount(url, methodName, max) && max > 0) {
}
...
finally {
//already existing endCount call.
}
Second approach I am mentioning because of simplification. Do let me know
your thoughts on this.
On Wed, Dec 19, 2018 at 6:39 PM Ian Luo <[email protected]> wrote:
> Hi Khan,
>
> Thanks for pointing out this issue. I guess we could change logic to:
>
> if (max > 0 && !RpcStatus.beginCount(url, methodName, max)) {
> ...
> } else {
> RpcStatus.beginCount(url, methodName);
> }
>
> And we should do the same logic in ExecuteLimitFilter. Let me know your
> opinion on this.
>
> Thanks,
> -Ian.
>
>
> On Wed, Dec 19, 2018 at 7:42 PM Imteyaz Khan <[email protected]>
> wrote:
>
> > Hi All,
> > I was trying to write UT for ActiveLimitFilter for RuntimeException
> > scenarion and below is my UT for the same
> >
> > ActiveLimitFilterTest.java
> >
> > @Test()
> > public void testInvokeRuntimeExceptionWithActiveCountMatch() {
> > URL url =
> >
> URL.valueOf("test://test:11/test?accesslog=true&group=dubbo&version=1.1&actives=0");
> > Invoker<ActiveLimitFilterTest> invoker = new
> > RuntimeExceptionInvoker(url);
> > Invocation invocation = new MockInvocation();
> > RpcStatus count = RpcStatus.getStatus(invoker.getUrl(),
> > invocation.getMethodName());
> > int beforeExceptionActiveCount = count.getActive();
> > try {
> > activeLimitFilter.invoke(invoker, invocation);
> > } catch (RuntimeException ex) {
> > int afterExceptionActiveCount = count.getActive();
> > assertEquals("After exception active count should be same"
> > , beforeExceptionActiveCount, afterExceptionActiveCount);
> > }
> > }
> >
> >
> > Where I am expecting RpcStatus active count before call and after invoke
> > should be same, irrespective of exceptional handling by ActiveLimitFilter
> > (e.g. in this case it should be 0). UT showing me that after encountering
> > exception it is not same, on my further investigation I found that
> >
> >
> > - If there is no *actives (ACTIVE_KEY) *is set or if its value is less
> > then *1* then it is always returning -1 (in UT active count), which
> > means there is more number of call can be possible then it is
> > allowed(this
> > is my interpretation , correct me if this is wrong). e.g. if within a
> > minute 10 call are allowed and if we encounter 5 *RuntimeException
> *then
> > in total we could landed up allowing 15 invoke. I am suspecting this
> is
> > because *max *being *0* we don't increment active count of RpcStatus
> > and then decrease it in finally block where we have not even have
> > increase
> > the count, because *max* is before the count increment
> >
> > if (max > 0 && !RpcStatus.beginCount(url, methodName,
> max))
> >
> > As a reference I am copying the current master branch code of
> > ActiveLimitFilter.java. Would request to correct my understanding if it
> is
> > wrong, and if you feel my observation is correct then I would can raise a
> > PR for fixing this issue.
> > *Note: UT is in my local I have not checked in into any branch.
> >
> > if (max > 0 && !RpcStatus.beginCount(url, methodName, max)) {
> > long timeout =
> > invoker.getUrl().getMethodParameter(invocation.getMethodName(),
> > Constants.TIMEOUT_KEY, 0);
> > long start = System.currentTimeMillis();
> > long remain = timeout;
> > synchronized (count) {
> > while (!RpcStatus.beginCount(url, methodName, max)) {
> > try {
> > count.wait(remain);
> > } catch (InterruptedException e) {
> > // ignore
> > }
> > long elapsed = System.currentTimeMillis() - start;
> > remain = timeout - elapsed;
> > if (remain <= 0) {
> > throw new RpcException("Waiting concurrent
> > invoke timeout in client-side for service: "
> > + invoker.getInterface().getName() +
> > ", method: "
> > + invocation.getMethodName() + ",
> > elapsed: " + elapsed
> > + ", timeout: " + timeout + ".
> > concurrent invokes: " + count.getActive()
> > + ". max concurrent invoke limit: " +
> max);
> > }
> > }
> > }
> > }
> >
> > boolean isSuccess = true;
> > long begin = System.currentTimeMillis();
> > try {
> > return invoker.invoke(invocation);
> > } catch (RuntimeException t) {
> > isSuccess = false;
> > throw t;
> > } finally {
> > RpcStatus.endCount(url, methodName,
> > System.currentTimeMillis() - begin, isSuccess);
> > if (max > 0) {
> > synchronized (count) {
> > count.notifyAll();
> > }
> > }
> > }
> >
>
