On Mon, 2019-02-11 at 20:02 -0800, Elijah Zupancic wrote: > ====Summary==== > > There is no provision for handling the rfc2616 Retry-After header in > the ServiceUnavailableRetryStrategy or ConnectionBackoffStrategy > interfaces. This makes implementing a wait based on a response after > the server returns a 503 difficult. > > ====Long Form==== > > Hi Folks, > > I've been a user of HTTP Client for years and I want to thank all of > the developers for their hard work. This is a high quality project > and > I'm sure it requires a lot of work handling community engagement. > > I'm using HTTP Client to interface with an open source object store > called Manta (https://github.com/joyent/manta) via the Java SDK (also > open source: https://github.com/joyent/java-manta). Some users of > this > object store are sending large amounts of data into the object store > and pushing it to its limits. As a result the object store will > periodically return 503 Service Unavailable errors with a Retry-After > header that provides a hint to the client regarding how many seconds > to wait until it retries. > > When examining the > org.apache.http.client.ServiceUnavailableRetryStrategy, I see that > the > interface contract doesn't make a provision for getting at the > Retry-After header (see: > https://tools.ietf.org/html/rfc2616#section-14.37) because the > getRetryInterval() method doesn't take a HttpResponse parameter. > > As I understand the design, implementations of the > ServiceUnavailableRetryStrategy interface need to be thread-safe, so > storing the retry interface as parsed from the HTTP response header > when the retryRequest call is invoked, isn't really viable. I could > imagine a complex system using ThreadLocal instances to pull this > off, > but that is far from ideal. > > Looking at > org.apache.http.impl.execchain.ServiceUnavailableRetryExec:88: > > if (this.retryStrategy.retryRequest(response, c, context) > && RequestEntityProxy.isRepeatable(request)) { > response.close(); > final long nextInterval = this.retryStrategy.getRetryInterval(); > if (nextInterval > 0) { > try { > this.log.trace("Wait for " + nextInterval); > Thread.sleep(nextInterval); > } catch (final InterruptedException e) { > Thread.currentThread().interrupt(); > throw new InterruptedIOException(); > } > } > > One alternative approach, I considered was doing the Thread.sleep() > call within the retryRequest() call, but I see that every response is > filtered through this code path when a > ServiceUnavailableRetryStrategy > is assigned and this feels like an abuse of the API contract. > Additionally, RequestEntityProxy.isRepeatable() is scoped within the > default package scope, so I can't reuse that code path to see if the > request is repeatable and I would have to resort to reflection to > accomplish the same thing. > > I also took a look at the > org.apache.http.client.ConnectionBackoffStrategy class, but it makes > no provision for the total time to backoff but rather adjusts the > size > of the connection pool (at least that is my understanding). > > I've been wracking my brain trying to come up with a solution that > will preserve API compatibility, but honestly it is hard to do > without > introducing a wart into the design. > > One of the less bad options, I considered was adding a method to the > ServiceUnavailableRetryStrategy interface: > > /** > * @param response the response from the target server > * @return The interval between the subsequent auto-retries. > */ > long getRetryInterval(HttpResponse response); > > And modify the previously mentioned block as so: > > if (this.retryStrategy.retryRequest(response, c, context) > && RequestEntityProxy.isRepeatable(request)) { > response.close(); > final long nextInterval = this.retryStrategy.getRetryInterval() > > 0 ? > this.retryStrategy.getRetryInterval() : > this.retryStrategy.getRetryInterval(response); > if (nextInterval > 0) { > try { > this.log.trace("Wait for " + nextInterval); > Thread.sleep(nextInterval); > } catch (final InterruptedException e) { > Thread.currentThread().interrupt(); > throw new InterruptedIOException(); > } > } > > This implementation would require careful documentation in the > interface. > > If anyone else has alternative suggestions or feedback, I'm eager and > willing to contribute to the project. > > Thank you, > Elijah >
Hi Elijah Support for `Retry-After` was added to HttpClient 5.0 some while ago. https://github.com/ok2c/httpclient/commit/d5c520a8e1231ae60142158781f5c9bf978fd8e9 There is no elegant way of back-porting it to HttpClient 4.5 other then deprecating ServiceUnavailableRetryStrategy interface and replacing it with some other interface. It would probably be better to switch to HttpClient 5.0 instead. Oleg --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org For additional commands, e-mail: dev-h...@hc.apache.org
