Thank you for your kind reply Oleg. Based on the Apache Http Client site, it seems that HttpClient 5.0 is still in beta. What is a good way for me and/or my company to contribute towards moving the project forward? I'm sure this is written down somewhere, but I haven't found it.
Thank you, Elijah On Tue, Feb 12, 2019 at 1:42 AM Oleg Kalnichevski <ol...@apache.org> wrote: > > 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 > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org For additional commands, e-mail: dev-h...@hc.apache.org
