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

Connection/Read Timeouts with Load Balanced Rest Template #1873

Open
tkvangorder opened this issue Apr 19, 2017 · 10 comments
Open

Connection/Read Timeouts with Load Balanced Rest Template #1873

tkvangorder opened this issue Apr 19, 2017 · 10 comments

Comments

@tkvangorder
Copy link

We are migrating our load balanced RestTemplates from using the legacy, netflix http client to the new RestTemplate + Spring Retry.

One thing that surprised us was that the ribbon setting for connection timeouts are NOT applied when using the rest template. This actually seems like an obvious fact once you think about it, but it means that timeouts for the load balanced rest templates really need to be configured differently then how you configure those same timeouts in zuul (and likely Feign).

The connection/read timeouts for a load balanced rest template are NOT configured via the ribbon properties. Instead, you must set your timeouts when constructing the rest template via setConnectionTimeout and setReadTimeout in the restTemplateBuilder.

We ended up normalizing our code such that it just injects those same ribbon timeouts settings into our template:

	@Value("${ribbon.ConnectionTimeout:1000}")
	int connectionTimeout;

	@Value("${ribbon.ReadTimeout:20000}")
	int readTimeout;

	@Bean
	@LoadBalanced
	public RestTemplate buildRestTemplateRibbon(RestTemplateBuilder restTemplateBuilder) {
		return restTemplateBuilder.messageConverters(new BuildHttpMessageConverters().getConverters())
			.uriTemplateHandler(new BuildUriTemplateHandler())
			.setConnectTimeout(connectionTimeout)
			.setReadTimeout(readTimeout)
			.errorHandler(new ServiceClientErrorHandler()).build();
	}

It would be nice if that were the default behavior in spring cloud, as then the timeouts are set the same across zuul, feign, and the load balanced rest template.

Or alternatively, it might be good to make a note in the documentation: If you are using the load balanced rest template with retry, you need to explicitly set your read/connection timeouts when constructing the template.

@spencergibb
Copy link
Member

I don't think most people were using the legacy RestClient + RestTemplate. I think documentation is the way to go especially since we're not in the business of creating RestTemplate objects.

@tkvangorder
Copy link
Author

Just to clarify, we are no longer using the legacy RestClient, we are moving to the new and improved retry logic in Camden.SR6.

I just wanted to highlight that with the new spring retry logic in Camden.SR6 and Dalston:

The connection timeouts for Zuul and Feign still rely on the ribbon.ConnectionTimeout and ribbon.ReadTimeout. However, the Load Balanced rest templates do not use those two settings.

This becomes important when doing retries because connection timeout and read timeout exceptions are "retryable". If you fail to set the read timeouts on your rest template, your rest template's retries will work differently than your zuul/Feign timeouts.

@ryanjbaxter
Copy link
Contributor

I think we understand what you are saying. I think an argument for the opposite can also be made, when using a RestTemplate you may expect that it would be configured using the normal RestTemplate configuration. In other words, Ribbon configuration would not have any effect.

Documentation is definitely warranted.

@tkvangorder
Copy link
Author

Makes sense to me, appreciate the quick response.

@ryanjbaxter
Copy link
Contributor

NP, we appreciate the feedback!

@DaveBender
Copy link

If Documentation is definitely warranted, what would it say?

I'm somewhat lost on how to find/set and understand the different socket timing issues. Documentation would be helpful.

@tkvangorder
Copy link
Author

tkvangorder commented May 31, 2017

@DaveBender

If you are using Zuul or the Feign client you can use the

ribbon.ConnectionTimeout
ribbon.ReadTimeout

If you are using a RestTemplate with the @LoadBalanced annotations, you need to set your socket timeouts via the RestTemplateBuilder :

	@Bean
	@LoadBalanced
	public RestTemplate restTemplateRibbon(RestTemplateBuilder restTemplateBuilder) {
		return restTemplateBuilder.
			.setConnectTimeout(connectionTimeout)
			.setReadTimeout(readTimeout)
			.build();

If you end up wrapping your RestTemplate calls within a Hystrix circuit breaker, you will want to set the hystrix timeout HIGHER than the RestTemplate's timeouts:

Hystrix Timeout > (connect timeout + read timeout) * (Number of ribbon retries)

A cautionary tale: If you create a RestTemplate via its default constructor (not using a builder to set socket timeouts), it can potentially wait indefinitely because timeouts are NOT defined.

@geowalrus4gh
Copy link

geowalrus4gh commented Nov 6, 2017

@ryanjbaxter
We need to set connection timeout and read time out specific to each load balancing client(microservice). So in this case, if we configure connect timeout and read timeout direclty to RestTemplate, it would contribute to all clients right ? How to configure timeouts per client(microservice) ?

@ryanjbaxter
Copy link
Contributor

I think in that case you would have to create a RestTemplate object per service and not one global RestTemplate used everywhere.

@tkvangorder
Copy link
Author

@geowalrus4gh We are doing what Ryan suggested, we create different RestTemplates for our various third-party clients that have different timeout requirements. One thing we do to control this process is we configure a RestTemplateBuilder with some sensible defaults:

	@Bean
	public RestTemplateBuilder restTemplateBuilder() {
		RestTemplateBuilder builder = new RestTemplateBuilder();
		HttpMessageConverters converters = this.messageConverters.getIfUnique();
		if (converters != null) {
			builder = builder.messageConverters(converters.getConverters());
		}
		List<RestTemplateCustomizer> customizers = this.restTemplateCustomizers
				.getIfAvailable();
		if (!CollectionUtils.isEmpty(customizers)) {
			customizers = new ArrayList<RestTemplateCustomizer>(customizers);
			AnnotationAwareOrderComparator.sort(customizers);
			builder = builder.customizers(customizers);
		}
		
		builder = builder.setConnectTimeout(connectionTimeout);
		builder = builder.setReadTimeout(readTimeout);
		return builder;
	}

And then when we need to override those defaults, we do so in a PostConstruct method:

	@Autowired
	private RestTemplateBuilder restTemplateBuilder;
	protected RestTemplate restTemplate;



	@PostConstruct
	public void init() {
		restTemplate = restTemplateBuilder
			.setConnectTimeout(overrideConnectionTimeout)
			.setReadTimeout(overrideReadTimeout)
			.messageConverters(new StringHttpMessageConverter())
			.build();
	}

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

No branches or pull requests

5 participants