====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

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to