Remove support for maxRetryTimeout from low-level REST client (#38085)

We have had various reports of problems caused by the maxRetryTimeout
setting in the low-level REST client. Such setting was initially added
in the attempts to not have requests go through retries if the request
already took longer than the provided timeout.

The implementation was problematic though as such timeout would also
expire in the first request attempt (see #31834), would leave the
request executing after expiration causing memory leaks (see #33342),
and would not take into account the http client internal queuing (see #25951).

Given all these issues, it seems that this custom timeout mechanism 
gives little benefits while causing a lot of harm. We should rather rely 
on connect and socket timeout exposed by the underlying http client 
and accept that a request can overall take longer than the configured 
timeout, which is the case even with a single retry anyways.

This commit removes the `maxRetryTimeout` setting and all of its usages.
This commit is contained in:
Luca Cavanna 2019-02-06 08:43:47 +01:00 committed by GitHub
parent 9492dcea42
commit a7046e001c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
23 changed files with 511 additions and 771 deletions

View file

@ -18,8 +18,7 @@ https://hc.apache.org/httpcomponents-client-ga/httpclient/apidocs/org/apache/htt
as an argument and has the same return type. The request config builder can
be modified and then returned. In the following example we increase the
connect timeout (defaults to 1 second) and the socket timeout (defaults to 30
seconds). Also we adjust the max retry timeout accordingly (defaults to 30
seconds too).
seconds).
["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------

View file

@ -180,15 +180,6 @@ include-tagged::{doc-tests}/RestClientDocumentation.java[rest-client-init-defaul
<1> Set the default headers that need to be sent with each request, to
prevent having to specify them with each single request
["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------
include-tagged::{doc-tests}/RestClientDocumentation.java[rest-client-init-max-retry-timeout]
--------------------------------------------------
<1> Set the timeout that should be honoured in case multiple attempts are made
for the same request. The default value is 30 seconds, same as the default
socket timeout. In case the socket timeout is customized, the maximum retry
timeout should be adjusted accordingly
["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------
include-tagged::{doc-tests}/RestClientDocumentation.java[rest-client-init-failure-listener]