Fix for Issue 37: Added configurable retry handler to impose maximum limits and a back-off delay algorithm to retries in response to server 5xx errors

git-svn-id: http://jclouds.googlecode.com/svn/trunk@850 3d8758e0-26b5-11de-8745-db77d3ebf521

aws/s3/core/src/main/java/org/jclouds/aws/s3/S3ContextFactory.java

@@ -34,6 +34,7 @@ import static org.jclouds.command.pool.PoolConstants.PROPERTY_POOL_REQUEST_INVOK
 import static org.jclouds.http.HttpConstants.PROPERTY_HTTP_ADDRESS;
 import static org.jclouds.http.HttpConstants.PROPERTY_HTTP_PORT;
 import static org.jclouds.http.HttpConstants.PROPERTY_HTTP_SECURE;
+import static org.jclouds.http.HttpConstants.PROPERTY_HTTP_MAX_RETRIES;
 
 import java.util.List;
 import java.util.Properties;
@@ -79,6 +80,7 @@ public class S3ContextFactory {
       DEFAULT_PROPERTIES.setProperty(PROPERTY_HTTP_ADDRESS, "s3.amazonaws.com");
       DEFAULT_PROPERTIES.setProperty(PROPERTY_HTTP_PORT, "443");
       DEFAULT_PROPERTIES.setProperty(PROPERTY_HTTP_SECURE, "true");
+      DEFAULT_PROPERTIES.setProperty(PROPERTY_HTTP_MAX_RETRIES, "5");
       DEFAULT_PROPERTIES.setProperty(PROPERTY_POOL_MAX_CONNECTION_REUSE, "75");
       DEFAULT_PROPERTIES.setProperty(PROPERTY_POOL_MAX_SESSION_FAILURES, "2");
       DEFAULT_PROPERTIES.setProperty(PROPERTY_POOL_REQUEST_INVOKER_THREADS, "1");

aws/s3/core/src/main/java/org/jclouds/aws/s3/config/LiveS3ConnectionModule.java

@@ -35,9 +35,12 @@ import org.jclouds.aws.s3.internal.LiveS3Connection;
 import org.jclouds.http.HttpConstants;
 import org.jclouds.http.HttpRequestFilter;
 import org.jclouds.http.HttpResponseHandler;
+import org.jclouds.http.HttpRetryHandler;
 import org.jclouds.http.annotation.ClientErrorHandler;
 import org.jclouds.http.annotation.RedirectHandler;
+import org.jclouds.http.annotation.RetryHandler;
 import org.jclouds.http.annotation.ServerErrorHandler;
+import org.jclouds.http.handlers.BackoffLimitedRetryHandler;
 import org.jclouds.http.handlers.CloseContentAndSetExceptionHandler;
 import org.jclouds.logging.Logger;
 
@@ -77,6 +80,8 @@ public class LiveS3ConnectionModule extends AbstractModule {
                ParseAWSErrorFromXmlContent.class).in(Scopes.SINGLETON);
       bind(HttpResponseHandler.class).annotatedWith(ServerErrorHandler.class).to(
                ParseAWSErrorFromXmlContent.class).in(Scopes.SINGLETON);
+      bind(HttpRetryHandler.class).annotatedWith(RetryHandler.class).to(
+               BackoffLimitedRetryHandler.class).in(Scopes.SINGLETON);
       requestInjection(this);
       logger.info("S3 Context = %1$s://%2$s:%3$s", (isSecure ? "https" : "http"), address, port);
    }

aws/s3/core/src/test/java/org/jclouds/aws/s3/config/S3ContextModuleTest.java

@@ -23,22 +23,25 @@
  */
 package org.jclouds.aws.s3.config;
 
+import static org.testng.Assert.assertEquals;
+
+import org.jclouds.aws.s3.handlers.ParseAWSErrorFromXmlContent;
+import org.jclouds.aws.s3.reference.S3Constants;
+import org.jclouds.http.HttpResponseHandler;
+import org.jclouds.http.HttpRetryHandler;
+import org.jclouds.http.annotation.ClientErrorHandler;
+import org.jclouds.http.annotation.RedirectHandler;
+import org.jclouds.http.annotation.RetryHandler;
+import org.jclouds.http.annotation.ServerErrorHandler;
+import org.jclouds.http.config.JavaUrlHttpFutureCommandClientModule;
+import org.jclouds.http.handlers.BackoffLimitedRetryHandler;
+import org.jclouds.http.handlers.CloseContentAndSetExceptionHandler;
+import org.testng.annotations.Test;
+
 import com.google.inject.Guice;
 import com.google.inject.Inject;
 import com.google.inject.Injector;
 import com.google.inject.name.Names;
-import org.jclouds.aws.s3.handlers.ParseAWSErrorFromXmlContent;
-import org.jclouds.aws.s3.reference.S3Constants;
-import org.jclouds.http.HttpResponseHandler;
-import org.jclouds.http.annotation.ClientErrorHandler;
-import org.jclouds.http.annotation.RedirectHandler;
-import org.jclouds.http.annotation.ServerErrorHandler;
-import org.jclouds.http.config.JavaUrlHttpFutureCommandClientModule;
-import org.jclouds.http.handlers.CloseContentAndSetExceptionHandler;
-import static org.testng.Assert.assertEquals;
-import org.testng.annotations.AfterMethod;
-import org.testng.annotations.BeforeMethod;
-import org.testng.annotations.Test;
 
 /**
  * @author Adrian Cole
@@ -58,6 +61,7 @@ public class S3ContextModuleTest {
                      "localhost");
             bindConstant().annotatedWith(Names.named(S3Constants.PROPERTY_HTTP_PORT)).to("1000");
             bindConstant().annotatedWith(Names.named(S3Constants.PROPERTY_HTTP_SECURE)).to("false");
+            bindConstant().annotatedWith(Names.named(S3Constants.PROPERTY_HTTP_MAX_RETRIES)).to("5");
             super.configure();
          }
       }, new JavaUrlHttpFutureCommandClientModule());
@@ -99,4 +103,16 @@ public class S3ContextModuleTest {
       assertEquals(error.errorHandler.getClass(), CloseContentAndSetExceptionHandler.class);
    }
 
+   private static class RetryHandlerTest {
+      @Inject
+      @RetryHandler
+      HttpRetryHandler retryHandler;
+   }
+
+   @Test
+   void testRetryHandler() {
+      RetryHandlerTest handler = createInjector().getInstance(RetryHandlerTest.class);
+      assertEquals(handler.retryHandler.getClass(), BackoffLimitedRetryHandler.class);
+   }
+
 }

core/src/main/java/org/jclouds/http/HttpConstants.java

@@ -32,4 +32,5 @@ public interface HttpConstants extends HttpHeaders, ContentTypes {
     public static final String PROPERTY_HTTP_SECURE = "jclouds.http.secure";
     public static final String PROPERTY_HTTP_PORT = "jclouds.http.port";
     public static final String PROPERTY_HTTP_ADDRESS = "jclouds.http.address";
+    public static final String PROPERTY_HTTP_MAX_RETRIES = "jclouds.http.max-retries";
 }

core/src/main/java/org/jclouds/http/internal/BaseHttpFutureCommandClient.java

@@ -27,7 +27,9 @@ import com.google.inject.Inject;
 import org.jclouds.http.*;
 import org.jclouds.http.annotation.ClientErrorHandler;
 import org.jclouds.http.annotation.RedirectHandler;
+import org.jclouds.http.annotation.RetryHandler;
 import org.jclouds.http.annotation.ServerErrorHandler;
+import org.jclouds.http.handlers.BackoffLimitedRetryHandler;
 import org.jclouds.http.handlers.CloseContentAndSetExceptionHandler;
 import org.jclouds.logging.Logger;
 
@@ -55,21 +57,15 @@ public abstract class BaseHttpFutureCommandClient implements HttpFutureCommandCl
     @Inject(optional = true)
     protected HttpResponseHandler serverErrorHandler = new CloseContentAndSetExceptionHandler();
     
+    @RetryHandler
+    @Inject(optional = true)
+    protected HttpRetryHandler httpRetryHandler = new BackoffLimitedRetryHandler(5);   
+
     @Inject
     public BaseHttpFutureCommandClient(URL target) {
         this.target = target;
     }
 
-    protected boolean isRetryable(HttpFutureCommand<?> command,
-                                  HttpResponse response) {
-        int code = response.getStatusCode();
-        if (command.getRequest().isReplayable() && code >= 500) {
-            logger.debug("resubmitting command: %1$s", command);
-            return true;
-        }
-        return false;
-    }
-
     protected void handleResponse(HttpFutureCommand<?> command, HttpResponse response) {
         int code = response.getStatusCode();
         if (code >= 500) {

core/src/main/java/org/jclouds/http/internal/JavaUrlHttpFutureCommandClient.java

@@ -71,7 +71,7 @@ public class JavaUrlHttpFutureCommandClient extends BaseHttpFutureCommandClient
 				connection);
 		response = getResponse(connection);
 		logger.trace("%1$s - received response %2$s", target, response);
-		if (isRetryable(command, response))
+		if (response.getStatusCode() >= 500 && httpRetryHandler.retryRequest(command, response))
 		    continue;
 		break;
 	    }

extensions/gae/src/main/java/org/jclouds/gae/URLFetchServiceClient.java

@@ -75,7 +75,8 @@ public class URLFetchServiceClient extends BaseHttpFutureCommandClient {
                                     target, gaeResponse.getResponseCode(),
                                     headersAsString(gaeResponse.getHeaders()));
                 response = convert(gaeResponse);
-                if (isRetryable(command, response))
+                int statusCode = response.getStatusCode();
+                if (statusCode >= 500 && httpRetryHandler.retryRequest(command, response))
                     continue;
                 break;
             }

extensions/httpnio/src/main/java/org/jclouds/http/httpnio/pool/HttpNioFutureCommandExecutionHandler.java

@@ -23,7 +23,12 @@
  */
 package org.jclouds.http.httpnio.pool;
 
-import com.google.inject.Inject;
+import java.io.IOException;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.ExecutorService;
+
+import javax.annotation.Resource;
+
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpEntityEnclosingRequest;
 import org.apache.http.HttpResponse;
@@ -33,17 +38,17 @@ import org.apache.http.protocol.HttpContext;
 import org.jclouds.http.HttpFutureCommand;
 import org.jclouds.http.HttpRequest;
 import org.jclouds.http.HttpResponseHandler;
+import org.jclouds.http.HttpRetryHandler;
 import org.jclouds.http.annotation.ClientErrorHandler;
 import org.jclouds.http.annotation.RedirectHandler;
+import org.jclouds.http.annotation.RetryHandler;
 import org.jclouds.http.annotation.ServerErrorHandler;
+import org.jclouds.http.handlers.BackoffLimitedRetryHandler;
 import org.jclouds.http.handlers.CloseContentAndSetExceptionHandler;
 import org.jclouds.http.httpnio.util.HttpNioUtils;
 import org.jclouds.logging.Logger;
 
-import javax.annotation.Resource;
+import com.google.inject.Inject;
-import java.io.IOException;
-import java.util.concurrent.BlockingQueue;
-import java.util.concurrent.ExecutorService;
 
 /**
  * // TODO: Adrian: Document this!
@@ -70,6 +75,10 @@ public class HttpNioFutureCommandExecutionHandler implements
     @Inject(optional = true)
     private HttpResponseHandler serverErrorHandler = new CloseContentAndSetExceptionHandler();
 
+    @RetryHandler
+    @Inject(optional = true)
+    protected HttpRetryHandler httpRetryHandler = new BackoffLimitedRetryHandler(5);   
+
     public interface ConsumingNHttpEntityFactory {
         public ConsumingNHttpEntity create(HttpEntity httpEntity);
     }
@@ -115,11 +124,17 @@ public class HttpNioFutureCommandExecutionHandler implements
 
                 int code = response.getStatusCode();
                 if (code >= 500) {
-                    if (isRetryable(command)) {
+                   boolean retryRequest = false;
-                        commandQueue.add(command);
+                   try {
-                    } else {
+                	   retryRequest = httpRetryHandler.retryRequest(command, response);
-                        this.serverErrorHandler.handle(command, response);
+                   } catch (InterruptedException ie) {
-                    }
+                      // TODO: Add interrupt exception to command and abort? 
+                   }
+                   if (retryRequest) {
+                      commandQueue.add(command);
+                   } else {
+                      this.serverErrorHandler.handle(command, response);
+                   }
                 } else if (code >= 400 && code < 500) {
                     this.clientErrorHandler.handle(command, response);
                 } else if (code >= 300 && code < 400) {
@@ -136,14 +151,6 @@ public class HttpNioFutureCommandExecutionHandler implements
         }
     }
 
-    protected boolean isRetryable(HttpFutureCommand<?> command) {
-        if (command.getRequest().isReplayable()) {
-            logger.debug("resubmitting command: %1$s", command);
-            return true;
-        }
-        return false;
-    }
-
     protected void releaseConnectionToPool(
             HttpNioFutureCommandConnectionHandle handle) {
         try {