Skip to content
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

Feign clients default ConnectTimeout & ReadTimeout aren't overriden by Ribbon configuration #696

Closed
codependent opened this issue Dec 9, 2015 · 6 comments
Assignees
Milestone

Comments

@codependent
Copy link

Given a Feign client with the following Ribbon configuration:

images-microservice:
  ribbon:
    ConnectTimeout: 250
    ReadTimeout: 1000
    OkToRetryOnAllOperations: true
    MaxAutoRetriesNextServer: 2
    MaxAutoRetries: 2

One would expect that ConnectTimeout and ReadTimeout are 250 and 1000. However they never override Feign's defaults.

In org.springframework.cloud.netflix.feign.ribbon.FeignLoadBalancer.execute()

    Request.Options options;
    if (configOverride != null) {
        options = new Request.Options(
                configOverride.get(CommonClientConfigKey.ConnectTimeout,
                        this.connectTimeout),
                (configOverride.get(CommonClientConfigKey.ReadTimeout,
                        this.readTimeout)));
    }

this.connectTimeout is correct (250), but what actually is gotten from configOverride.get() is 10000. This value comes from Feign.Options' default value:

public static class Options {

    private final int connectTimeoutMillis;
    private final int readTimeoutMillis;
    ...
    public Options() {
      this(10 * 1000, 60 * 1000);
    }

Shouldn't FeignLoadBalancer.execute() prioritize Ribbon's configuration over the default Feign's Request.Options? If not, how are we supposed to override it?

@spencergibb
Copy link
Member

I don't see how Feign.Options can override those values in FeignLoadBalancer. https://github.com/spring-cloud/spring-cloud-netflix/blob/master/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancer.java#L66-L73

I haven't verified the problem, just that we don't invoke the empty Feign.Options.

@codependent
Copy link
Author

Hi spencergibb, let me explain it further. The sequence is as follows:

  1. feign.SynchronousMethodHandler.executeAndDecode(RequestTemplate template) is called. And here we have response = client.execute(request, options); where options is a Request.Options object with connectTimeout = 10000 and readTimeout = 60000, that is to say, Request.Options' default values.

  2. client.execute is LoadBalancerFeignClient.execute(Request request, Request.Options options), where the next code is executed:

    return lbClient(clientName).executeWithLoadBalancer(ribbonRequest,
                    new FeignOptionsClientConfig(options)).toResponse();
    

    As we see, Request.Options options is wrapped in a FeignOptionsClientConfig object.

  3. The next step is FeignLoadBalancer.executeWithLoadBalancer(final S request, final IClientConfig requestConfig). which eventually calls FeingLoadBalancer.execute(RibbonRequest request, IClientConfig configOverride):

@Override
    public RibbonResponse execute(RibbonRequest request, IClientConfig configOverride)
            throws IOException {
        Request.Options options;
        if (configOverride != null) {
            options = new Request.Options(
                    configOverride.get(CommonClientConfigKey.ConnectTimeout,
                            this.connectTimeout),
                    (configOverride.get(CommonClientConfigKey.ReadTimeout,
                            this.readTimeout)));
        }

Notice that IClientConfig requestConfig/configOverride is the object that wrapped the default Request.Options in step 2. Therefore, when
options = new Request.Options( configOverride.get(CommonClientConfigKey.ConnectTimeout, this.connectTimeout),
is executed, configOverride already has ConnectTimeout and ReadTimeout values (10000 and 60000) loaded from the former Request.Options, so this.connectTimeout (250) is never applied.

I hope I managed to explain myself...

@spencergibb
Copy link
Member

Yes, very well explained. A workaround is to create a @Bean of type Request.Options and match that to the feign client using feign configurations. I was able to reproduce.

@pmvilaca
Copy link

pmvilaca commented Mar 8, 2016

👍

Just to give a +1 to this issue as I was also debugging the same issue yesterday and the workaround force the clients to re-write the interface that can be provided by the servers in a separated jar

@PedroAlvarado
Copy link
Contributor

+1 just ran into this issue as well. As suggested by @spencergibb here is the workaround I'm using.

    @Bean
    Request.Options requestOptions(ConfigurableEnvironment env){
        int ribbonReadTimeout = env.getProperty("ribbon.ReadTimeout", int.class, 6000);
        int ribbonConnectionTimeout = env.getProperty("ribbon.ConnectTimeout", int.class, 3000);

        return new Request.Options(ribbonConnectionTimeout, ribbonReadTimeout);
    }

@spencergibb spencergibb self-assigned this Mar 21, 2016
@spencergibb spencergibb added this to the 1.1.0.RC1 milestone Mar 21, 2016
@asarkar
Copy link
Contributor

asarkar commented May 6, 2017

@codependent What's the images-microservice in your question, is the the value in @FeignClient(name = images-microservice)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants