RequestSpecificRetryHandler retries same server for SERVER_THROTTLED

[brenuart]

Netflix's RequestSpecificRetryHandler allows to retry on same server for SERVER_THROTTLED error.

When the okToRetryOnAllErrors property is true, the retry handler allows to retry on the same server unconditionally without taking into account the actual error. This is useless when the server responded with SERVER_THROTTLED.

It looks like the if statement at https://github.com/Netflix/ribbon/blob/master/ribbon-core/src/main/java/com/netflix/client/RequestSpecificRetryHandler.java#L58-L65 should be considered first before the other conditions.

I doubt Netflix is willing to change this behavior but we could override it in SpringCloud. What do you think ?

[holy12345]

Hi @brenuart My understanding of spring cloud not only can set the number of retries the same server but also can set the number of retries of different servers. You mean that we should not unconditionally retry the same server, but according to the specific situation to decide whether to continue to retry the same server?(For example, according to an exception to decide whether to retry the same server)

[brenuart]

Ribbon can indeed be configured to retry the same server a couple of times before trying the next one. If the server replies with SERVER_THROTTLED, there is no point in retrying the same server - whatever the number of retries that is allowed... With the default configuration, you will get the following scenario:

• (first call) server1: SERVER_THROTTLED
• (retry same) server1: SERVER_THROTTLED
• (retry) server2: OK

The second call is useless...

[holy12345]

@brenuart Thank you for your reply, I would like to ask whether to continue retrying the same server when an exception occurs. Can this choice be up to the user? (such as provide a property to allow users to configure to decide which the situation does not need to retry on the same server).Instead of just for SERVER_THROTTLED situation

[ryanjbaxter]

Is there a "server throttled" status code? I cant seem to find any information on it, besides its use in Ribbon.

[brenuart]

It is indeed a Netflix/Ribbon specific feature that is hardcoded in their RestClient. The client returns ClientException.ErrorType.SERVER_THROTTLED when it gets a 503 (Service Unavailable) from the downstream server.

When okToRetryOnAllErrors is true, the current implementation of RequestSpecificRetryHandler is such that the same server will be retried on 503 - which is obviously pointless. See the code below for more insight:

    @Override
    public boolean isRetriableException(Throwable e, boolean sameServer) {
        if (okToRetryOnAllErrors) {
            return true;
        } 
        else if (e instanceof ClientException) {
            ClientException ce = (ClientException) e;
            if (ce.getErrorType() == ClientException.ErrorType.SERVER_THROTTLED) {
                return !sameServer;
            } else {
                return false;
            }
        } 
        else  {
            return okToRetryOnConnectErrors && isConnectionException(e);
        }
    }

We used to override that implementation to check the SERVER_THROTTLED case first before considering the okToRetryOnAllErrors flag.

[ryanjbaxter]

So this is only when using the Ribbon rest client then, correct?

[brenuart]

AFAIK yes. I noticed ApacheHttp and OkHttp configurations seem to make use of RequestSpecificRetryHandler too... but I have not investigated any further.

[ryanjbaxter]

Feel free to submit a PR for this. If not it will be a low priority item for us since the Netflix Rest Client is deprecated.

[brenuart]

I understand it is becoming low priority since the move to Spring Retry. However some of us are currently stuck on using Netflix's RestClient until they are able to move to more recent clients. It is actually our case for the moment mainly because of the following reasons:

• we rely on several modifications we made on some Ribbon aspects during the last two years. We are slowly migrating to Spring Retry...
• we also rely on Netflix metrics for monitoring.

I'll be more than happy to submit the PR and help people in the same situation as us. If you don't mind, we'll keep posting (so-called) issues if we believe it can help others, even if RestClient is deprecated. Is that ok for you?

[ryanjbaxter]

That is perfectly fine. We are always willing to accept any kind of contribution.