From c368bae28c841c34252cdafe2d42145c65ead2f7 Mon Sep 17 00:00:00 2001 From: Tom Manville Date: Wed, 26 Feb 2014 13:28:20 -0800 Subject: [PATCH] JCLOUDS-342: Retry on HTTP 408 for swift To repro issue 342, the following steps were taken: - Spawn 500 parallel / 2000 total putBlobs to cloudfiles-uk - issue a SIGSTOP - wait 60 seconds - issue a SIGCONT Without this patch, there are several hundred 408s. With this patch, these are retried and complete successfully. --- .../keystone/v1_1/handlers/RetryOnRenew.java | 15 ++++- .../v1_1/handlers/RetryOnRenewTest.java | 62 ++++++++++++++++++- 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/common/openstack/src/main/java/org/jclouds/openstack/keystone/v1_1/handlers/RetryOnRenew.java b/common/openstack/src/main/java/org/jclouds/openstack/keystone/v1_1/handlers/RetryOnRenew.java index fe8309dd28..a9446fe1aa 100644 --- a/common/openstack/src/main/java/org/jclouds/openstack/keystone/v1_1/handlers/RetryOnRenew.java +++ b/common/openstack/src/main/java/org/jclouds/openstack/keystone/v1_1/handlers/RetryOnRenew.java @@ -20,11 +20,14 @@ import static org.jclouds.http.HttpUtils.closeClientButKeepContentStream; import static org.jclouds.http.HttpUtils.releasePayload; import javax.annotation.Resource; +import javax.inject.Named; +import org.jclouds.Constants; import org.jclouds.domain.Credentials; import org.jclouds.http.HttpCommand; import org.jclouds.http.HttpResponse; import org.jclouds.http.HttpRetryHandler; +import org.jclouds.http.handlers.BackoffLimitedRetryHandler; import org.jclouds.logging.Logger; import org.jclouds.openstack.keystone.v1_1.domain.Auth; import org.jclouds.openstack.reference.AuthHeaders; @@ -42,14 +45,22 @@ import com.google.inject.Singleton; */ @Singleton public class RetryOnRenew implements HttpRetryHandler { + @Inject(optional = true) + @Named(Constants.PROPERTY_MAX_RETRIES) + private int retryCountLimit = 5; + @Resource protected Logger logger = Logger.NULL; private final LoadingCache authenticationResponseCache; + private final BackoffLimitedRetryHandler backoffHandler; + @Inject - protected RetryOnRenew(LoadingCache authenticationResponseCache) { + protected RetryOnRenew(LoadingCache authenticationResponseCache, + BackoffLimitedRetryHandler backoffHandler) { this.authenticationResponseCache = authenticationResponseCache; + this.backoffHandler = backoffHandler; } @Override @@ -74,6 +85,8 @@ public class RetryOnRenew implements HttpRetryHandler { } } break; + case 408: + return backoffHandler.shouldRetryRequest(command, response); } return retry; diff --git a/common/openstack/src/test/java/org/jclouds/openstack/keystone/v1_1/handlers/RetryOnRenewTest.java b/common/openstack/src/test/java/org/jclouds/openstack/keystone/v1_1/handlers/RetryOnRenewTest.java index 59b4500651..b68c8f17ee 100644 --- a/common/openstack/src/test/java/org/jclouds/openstack/keystone/v1_1/handlers/RetryOnRenewTest.java +++ b/common/openstack/src/test/java/org/jclouds/openstack/keystone/v1_1/handlers/RetryOnRenewTest.java @@ -27,6 +27,7 @@ import org.jclouds.domain.Credentials; import org.jclouds.http.HttpCommand; import org.jclouds.http.HttpRequest; import org.jclouds.http.HttpResponse; +import org.jclouds.http.handlers.BackoffLimitedRetryHandler; import org.jclouds.io.Payloads; import org.jclouds.openstack.keystone.v1_1.domain.Auth; import org.testng.annotations.Test; @@ -47,6 +48,7 @@ public class RetryOnRenewTest { HttpResponse response = createMock(HttpResponse.class); @SuppressWarnings("unchecked") LoadingCache cache = createMock(LoadingCache.class); + BackoffLimitedRetryHandler backoffHandler = createMock(BackoffLimitedRetryHandler.class); expect(command.getCurrentRequest()).andReturn(request); @@ -59,13 +61,71 @@ public class RetryOnRenewTest { replay(command); replay(response); replay(cache); + replay(backoffHandler); - RetryOnRenew retry = new RetryOnRenew(cache); + RetryOnRenew retry = new RetryOnRenew(cache, backoffHandler); assertTrue(retry.shouldRetryRequest(command, response)); verify(command); verify(response); verify(cache); + verify(backoffHandler); + } + + @Test + public void test408ShouldRetry() { + HttpCommand command = createMock(HttpCommand.class); + HttpRequest request = createMock(HttpRequest.class); + HttpResponse response = createMock(HttpResponse.class); + @SuppressWarnings("unchecked") + LoadingCache cache = createMock(LoadingCache.class); + BackoffLimitedRetryHandler backoffHandler = createMock(BackoffLimitedRetryHandler.class); + + expect(response.getPayload()).andReturn(Payloads.newStringPayload( + "The server has waited too long for the request to be sent by the client.")).times(2); + expect(backoffHandler.shouldRetryRequest(command, response)).andReturn(true).once(); + expect(response.getStatusCode()).andReturn(408).once(); + + replay(command); + replay(response); + replay(cache); + replay(backoffHandler); + + RetryOnRenew retry = new RetryOnRenew(cache, backoffHandler); + + assertTrue(retry.shouldRetryRequest(command, response)); + + verify(command); + verify(response); + verify(cache); + verify(backoffHandler); + } + + @Test + public void test404ShouldNotRetry() { + HttpCommand command = createMock(HttpCommand.class); + HttpRequest request = createMock(HttpRequest.class); + HttpResponse response = createMock(HttpResponse.class); + @SuppressWarnings("unchecked") + LoadingCache cache = createMock(LoadingCache.class); + BackoffLimitedRetryHandler backoffHandler = createMock(BackoffLimitedRetryHandler.class); + + expect(response.getPayload()).andReturn(Payloads.newStringPayload("")).times(2); + expect(response.getStatusCode()).andReturn(404).once(); + + replay(command); + replay(response); + replay(cache); + replay(backoffHandler); + + RetryOnRenew retry = new RetryOnRenew(cache, backoffHandler); + + assertTrue(!retry.shouldRetryRequest(command, response)); + + verify(command); + verify(response); + verify(cache); + verify(backoffHandler); } }