MLE-23230 Better name for retry interceptor

And added comments to explain what the existing app-level retry support is doing vs what this for-now-undocumented interceptor will be doing.

Author: rjrudin

marklogic-client-api-functionaltests/src/test/java/com/marklogic/client/functionaltest/ConnectedRESTQA.java

@@ -17,7 +17,7 @@
 import com.marklogic.client.admin.ServerConfigurationManager;
 import com.marklogic.client.extra.okhttpclient.OkHttpClientConfigurator;
 import com.marklogic.client.impl.SSLUtil;
-import com.marklogic.client.impl.okhttp.RetryInterceptor;
+import com.marklogic.client.impl.okhttp.RetryIOExceptionInterceptor;
 import com.marklogic.client.io.DocumentMetadataHandle;
 import com.marklogic.client.io.DocumentMetadataHandle.Capability;
 import com.marklogic.client.query.QueryManager;
@@ -50,7 +50,7 @@ public abstract class ConnectedRESTQA {
 	static {
 		DatabaseClientFactory.removeConfigurators();
 		DatabaseClientFactory.addConfigurator((OkHttpClientConfigurator) client ->
-			client.addInterceptor(new RetryInterceptor(3, 1000, 2, 8000)));
+			client.addInterceptor(new RetryIOExceptionInterceptor(3, 1000, 2, 8000)));
 	}
 
 	private static Properties testProperties = null;

marklogic-client-api/src/main/java/com/marklogic/client/impl/OkHttpServices.java

@@ -99,15 +99,19 @@ public class OkHttpServices implements RESTServices {
 
 	private boolean released = false;
 
+	/**
+	 * The next 4 fields implement an application-level retry that only works for certain HTTP status codes. It will not
+	 * attempt a retry on any IOException or any type of connection failure. Sadly, the logic that uses these fields is
+	 * in several places and is slightly different in each place. It's also not possible to implement this logic in an
+	 * OkHttp interceptor as the logic needs access to details that are not available to an interceptor.
+	 */
 	private final Random randRetry = new Random();
-
 	private int maxDelay = DEFAULT_MAX_DELAY;
 	private int minRetry = DEFAULT_MIN_RETRY;
+	private final Set<Integer> retryStatus = new HashSet<>();
 
 	private boolean checkFirstRequest = true;
 
-	private final Set<Integer> retryStatus = new HashSet<>();
-
 	static protected class ThreadState {
 		boolean isFirstRequest;
 
@@ -510,9 +514,6 @@ private Response sendRequestWithRetry(
 		long startTime = System.currentTimeMillis();
 		int nextDelay = 0;
 		int retry = 0;
-		/*
-		 * This loop is for retrying the request if the service is unavailable
-		 */
 		for (; retry < minRetry || (System.currentTimeMillis() - startTime) < maxDelay; retry++) {
 			if (nextDelay > 0) {
 				try {
@@ -542,10 +543,10 @@ private Response sendRequestWithRetry(
 				break;
 			}
 			/*
-			 * This code will be executed whenever the service is unavailable.
-			 * When the service becomes unavailable, we close the Response
-			 * we got and retry it to try and get a new Response
-			 */
+				 * This code will be executed whenever the service is unavailable.
+				 * When the service becomes unavailable, we close the Response
+				 * we got and retry it to try and get a new Response
+				 */
 			closeResponse(response);
 			/*
 			 * There are scenarios where we don't want to retry and we just want to
@@ -572,9 +573,9 @@ private Response sendRequestWithRetry(
 					" seconds after " + retry + " retries");
 		}
 		/*
-		 * Once we break from the retry loop, we just return the Response
-		 * back to the caller in order to proceed with the flow
-		 */
+			 * Once we break from the retry loop, we just return the Response
+			 * back to the caller in order to proceed with the flow
+			 */
 		return response;
 	}
 
@@ -1199,6 +1200,7 @@ private TemporalDescriptor putPostDocumentImpl(RequestLogger reqlog, String meth
 				}
 			}
 
+			// This would be the "doFunction"
 			Object value = handleBase.sendContent();
 			if (value == null) {
 				throw new IllegalArgumentException(
@@ -5442,6 +5444,12 @@ public void writeTo(BufferedSink sink) throws IOException {
 				throw new IllegalStateException("Cannot write object of type: " + obj.getClass());
 			}
 		}
+
+		@Override
+		public boolean isOneShot() {
+			// Added in 8.0.0 to work with the retry interceptor so it knows whether the body can be retried or not.
+			return obj instanceof InputStream;
+		}
 	}
 
 	// API First Changes

marklogic-client-api/src/main/java/com/marklogic/client/impl/StreamingOutputImpl.java

@@ -45,4 +45,10 @@ public void writeTo(BufferedSink sink) throws IOException {
 
     handle.write(out);
   }
+
+	@Override
+	public boolean isOneShot() {
+		// Added in 8.0.0; streaming output cannot be retried as the stream is consumed on first write.
+		return true;
+	}
 }

marklogic-client-api/src/main/java/com/marklogic/client/impl/okhttp/RetryInterceptor.java renamed to marklogic-client-api/src/main/java/com/marklogic/client/impl/okhttp/RetryIOExceptionInterceptor.java

@@ -14,19 +14,22 @@
 import java.net.UnknownHostException;
 
 /**
- * OkHttp interceptor that retries requests on certain connection failures,
- * which can be helpful when MarkLogic is temporarily unavailable during restarts.
+ * Experimental interceptor added in 8.0.0 for retrying requests that fail due to connection issues. These issues are
+ * not handled by the application-level retry support in OkHttpServices, which only handles retries based on certain
+ * HTTP status codes. The main limitation of this approach is that it cannot retry a request that has a one-shot body,
+ * such as a streaming body. But for requests that don't have one-shot bodies, this interceptor can be helpful for
+ * retrying requests that fail due to temporary network issues or MarkLogic restarts.
  */
-public class RetryInterceptor implements Interceptor {
+public class RetryIOExceptionInterceptor implements Interceptor {
 
-	private final static Logger logger = org.slf4j.LoggerFactory.getLogger(RetryInterceptor.class);
+	private final static Logger logger = org.slf4j.LoggerFactory.getLogger(RetryIOExceptionInterceptor.class);
 
 	private final int maxRetries;
 	private final long initialDelayMs;
 	private final double backoffMultiplier;
 	private final long maxDelayMs;
 
-	public RetryInterceptor(int maxRetries, long initialDelayMs, double backoffMultiplier, long maxDelayMs) {
+	public RetryIOExceptionInterceptor(int maxRetries, long initialDelayMs, double backoffMultiplier, long maxDelayMs) {
 		this.maxRetries = maxRetries;
 		this.initialDelayMs = initialDelayMs;
 		this.backoffMultiplier = backoffMultiplier;
@@ -37,11 +40,18 @@ public RetryInterceptor(int maxRetries, long initialDelayMs, double backoffMulti
 	public Response intercept(Chain chain) throws IOException {
 		Request request = chain.request();
 
+		// isOneShot is overridden by some of our subclasses of RequestBody to return true when the body is some
+		// kind of stream.
+		final boolean requestCannotBeRetried = request.body() != null && request.body().isOneShot();
+		if (requestCannotBeRetried) {
+			return chain.proceed(request);
+		}
+
 		for (int attempt = 0; attempt <= maxRetries; attempt++) {
 			try {
 				return chain.proceed(request);
 			} catch (IOException e) {
-				if (attempt == maxRetries || !isRetryableException(e)) {
+				if (attempt == maxRetries || !isRetryableIOException(e)) {
 					logger.warn("Not retryable: {}; {}", e.getClass(), e.getMessage());
 					throw e;
 				}
@@ -58,7 +68,7 @@ public Response intercept(Chain chain) throws IOException {
 		throw new IllegalStateException("Unexpected end of retry loop");
 	}
 
-	private boolean isRetryableException(IOException e) {
+	private boolean isRetryableIOException(IOException e) {
 		return e instanceof ConnectException ||
 			e instanceof SocketTimeoutException ||
 			e instanceof UnknownHostException ||

marklogic-client-api/src/test/java/com/marklogic/client/test/Common.java

@@ -9,11 +9,10 @@
 import com.marklogic.client.DatabaseClientBuilder;
 import com.marklogic.client.DatabaseClientFactory;
 import com.marklogic.client.extra.okhttpclient.OkHttpClientConfigurator;
-import com.marklogic.client.impl.okhttp.RetryInterceptor;
+import com.marklogic.client.impl.okhttp.RetryIOExceptionInterceptor;
 import com.marklogic.client.io.DocumentMetadataHandle;
 import com.marklogic.mgmt.ManageClient;
 import com.marklogic.mgmt.ManageConfig;
-import okhttp3.OkHttpClient;
 import org.springframework.util.FileCopyUtils;
 import org.w3c.dom.DOMException;
 import org.w3c.dom.Document;
@@ -35,7 +34,7 @@ public class Common {
 	static {
 		DatabaseClientFactory.removeConfigurators();
 		DatabaseClientFactory.addConfigurator((OkHttpClientConfigurator) client ->
-			client.addInterceptor(new RetryInterceptor(3, 1000, 2, 8000)));
+			client.addInterceptor(new RetryIOExceptionInterceptor(3, 1000, 2, 8000)));
 	}
 
 	final public static String USER = "rest-writer";