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.
This commit is contained in:
Tom Manville 2014-02-26 13:28:20 -08:00 committed by Andrew Gaul
parent 08992c7a3d
commit c368bae28c
2 changed files with 75 additions and 2 deletions

View File

@ -20,11 +20,14 @@ import static org.jclouds.http.HttpUtils.closeClientButKeepContentStream;
import static org.jclouds.http.HttpUtils.releasePayload; import static org.jclouds.http.HttpUtils.releasePayload;
import javax.annotation.Resource; import javax.annotation.Resource;
import javax.inject.Named;
import org.jclouds.Constants;
import org.jclouds.domain.Credentials; import org.jclouds.domain.Credentials;
import org.jclouds.http.HttpCommand; import org.jclouds.http.HttpCommand;
import org.jclouds.http.HttpResponse; import org.jclouds.http.HttpResponse;
import org.jclouds.http.HttpRetryHandler; import org.jclouds.http.HttpRetryHandler;
import org.jclouds.http.handlers.BackoffLimitedRetryHandler;
import org.jclouds.logging.Logger; import org.jclouds.logging.Logger;
import org.jclouds.openstack.keystone.v1_1.domain.Auth; import org.jclouds.openstack.keystone.v1_1.domain.Auth;
import org.jclouds.openstack.reference.AuthHeaders; import org.jclouds.openstack.reference.AuthHeaders;
@ -42,14 +45,22 @@ import com.google.inject.Singleton;
*/ */
@Singleton @Singleton
public class RetryOnRenew implements HttpRetryHandler { public class RetryOnRenew implements HttpRetryHandler {
@Inject(optional = true)
@Named(Constants.PROPERTY_MAX_RETRIES)
private int retryCountLimit = 5;
@Resource @Resource
protected Logger logger = Logger.NULL; protected Logger logger = Logger.NULL;
private final LoadingCache<Credentials, Auth> authenticationResponseCache; private final LoadingCache<Credentials, Auth> authenticationResponseCache;
private final BackoffLimitedRetryHandler backoffHandler;
@Inject @Inject
protected RetryOnRenew(LoadingCache<Credentials, Auth> authenticationResponseCache) { protected RetryOnRenew(LoadingCache<Credentials, Auth> authenticationResponseCache,
BackoffLimitedRetryHandler backoffHandler) {
this.authenticationResponseCache = authenticationResponseCache; this.authenticationResponseCache = authenticationResponseCache;
this.backoffHandler = backoffHandler;
} }
@Override @Override
@ -74,6 +85,8 @@ public class RetryOnRenew implements HttpRetryHandler {
} }
} }
break; break;
case 408:
return backoffHandler.shouldRetryRequest(command, response);
} }
return retry; return retry;

View File

@ -27,6 +27,7 @@ import org.jclouds.domain.Credentials;
import org.jclouds.http.HttpCommand; import org.jclouds.http.HttpCommand;
import org.jclouds.http.HttpRequest; import org.jclouds.http.HttpRequest;
import org.jclouds.http.HttpResponse; import org.jclouds.http.HttpResponse;
import org.jclouds.http.handlers.BackoffLimitedRetryHandler;
import org.jclouds.io.Payloads; import org.jclouds.io.Payloads;
import org.jclouds.openstack.keystone.v1_1.domain.Auth; import org.jclouds.openstack.keystone.v1_1.domain.Auth;
import org.testng.annotations.Test; import org.testng.annotations.Test;
@ -47,6 +48,7 @@ public class RetryOnRenewTest {
HttpResponse response = createMock(HttpResponse.class); HttpResponse response = createMock(HttpResponse.class);
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
LoadingCache<Credentials, Auth> cache = createMock(LoadingCache.class); LoadingCache<Credentials, Auth> cache = createMock(LoadingCache.class);
BackoffLimitedRetryHandler backoffHandler = createMock(BackoffLimitedRetryHandler.class);
expect(command.getCurrentRequest()).andReturn(request); expect(command.getCurrentRequest()).andReturn(request);
@ -59,13 +61,71 @@ public class RetryOnRenewTest {
replay(command); replay(command);
replay(response); replay(response);
replay(cache); replay(cache);
replay(backoffHandler);
RetryOnRenew retry = new RetryOnRenew(cache); RetryOnRenew retry = new RetryOnRenew(cache, backoffHandler);
assertTrue(retry.shouldRetryRequest(command, response)); assertTrue(retry.shouldRetryRequest(command, response));
verify(command); verify(command);
verify(response); verify(response);
verify(cache); 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<Credentials, Auth> 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<Credentials, Auth> 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);
} }
} }