ConcurrencyLimitServerInterceptor ignores Limiter for one-way streams

[Paul-Folbrecht-zz]

The ConcurrencyLimitServerInterceptor simply passes thru, bypassing its Limiter, any calls that don’t use bi-directional streaming:

@Override public <ReqT, RespT> Listener interceptCall(final ServerCall<ReqT, RespT> call, final Metadata headers, final ServerCallHandler<ReqT, RespT> next)

{ if (!call.getMethodDescriptor().getType().serverSendsOneMessage() || !call.getMethodDescriptor().getType().clientSendsOneMessage()) { return next.startCall(call, headers); }

So, in systems that make use of only unary or one-direction streaming, which one would think are quite common, the Limiter, and essentially the entire subsystem, do nothing.

Am I correct in presuming that the assumption here is that any call that doesn’t do bidirectional streaming will be “fast” and has no need for limiting? Any plans to change or config this behavior?

Thanks.

[elandau]

It's actually the other way around. Only UNARY calls go through the limiter. This could could potentially be re-written to if (call.getMethodDescriptor.getType().equals(MethodType.UNARY) to be less confusing.

[Paul-Folbrecht-zz]

Thanks for the fast reply. Could you explain the reasoning behind this, and, any plans to broaden the options?

Of course, I can always write a custom Interceptor.

[elandau]

It's really impossible to know the lifecycle of a streaming API. In the simplest case it would just be chunking of responses. But requests could actually have an indefinite duration in which case limiting concurrency here wouldn't make much sense. I suppose we could make it configurable whether streaming APIs should be considered for concurrency limiting.

[Paul-Folbrecht-zz]

Agreed on the generalities. We implemented our API using streaming calls, but all are short-lived.

If you are able to make that simple change soon, please let me know, or perhaps I could do it and give you a PR.