-
Notifications
You must be signed in to change notification settings - Fork 546
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Return RetriableRequestException for Netty Max Active Stream error #1001
Conversation
r2-core/src/main/java/com/linkedin/r2/transport/http/common/HttpBridge.java
Outdated
Show resolved
Hide resolved
{ | ||
StreamException streamException = (StreamException) responseError; | ||
response = TransportResponseImpl.error( | ||
new RetriableRequestException("Failed to get response from server for URI " + uri, streamException), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we avoid duplicating the exception message?
RemoteInvocationException wrapException(String message, Throwable exception) {
if (shouldReturnRetriableRequestException(exception)) {
return new RetriableRequestException(message, exception);
} else {
return new RemoteInvocationException(message, exception);
}
}
r2-core/src/main/java/com/linkedin/r2/transport/http/common/HttpBridge.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This change adds logic in
HttpBridge
to returnRetriableRequestException
rather thanRemoteInvocationException
when StreamException from Netty is returned with cause ofMaximum active streams violated for this endpoint
. This allows the downstream client -RetryClient
to retry the request with different hosts when stream hits the maximum of a given host.wc-test passed