Merge pull request #2939 from mapfish/backport/2842-to-master

[Backport master] Fix reload
mapfish · May 8, 2023 · c3e7c31 · c3e7c31
2 parents af75b0f + 152aa46
commit c3e7c31
Show file tree

Hide file tree

Showing 7 changed files with 64 additions and 27 deletions.
diff --git a/Makefile b/Makefile
@@ -3,7 +3,7 @@ export DOCKER_BUILDKIT = 1
 
 .PHONY: build
 build:
-	# Required and not nesseerly exists
+	# Required and not necessarily exists
 	touch CI.asc
 
 	docker build $(GIT_HEAD_ARG) --target=builder --tag=mapfish_print_builder .

diff --git a/core/build.gradle b/core/build.gradle
@@ -132,7 +132,6 @@ dependencies {
             "org.springframework:spring-jdbc:5.3.27",
             "org.springframework:spring-tx:5.3.27",
             "org.springframework:spring-test:5.3.27",
-            "org.springframework.retry:spring-retry:1.3.4",
     )
     metrics(
             "io.dropwizard.metrics:metrics-core:4.2.18",

diff --git a/core/src/main/java/org/mapfish/print/http/HttpRequestFetcher.java b/core/src/main/java/org/mapfish/print/http/HttpRequestFetcher.java
@@ -14,20 +14,21 @@
 import org.springframework.http.client.ClientHttpRequest;
 import org.springframework.http.client.ClientHttpResponse;
 import org.springframework.util.StreamUtils;
-import org.springframework.retry.annotation.Retryable;
-import org.springframework.retry.annotation.Backoff;
 
 import java.io.File;
 import java.io.FileInputStream;
-import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.URI;
+import java.nio.file.Files;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ForkJoinPool;
 import java.util.concurrent.ForkJoinTask;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
 
+import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 /**
@@ -41,10 +42,11 @@ public final class HttpRequestFetcher {
     private static final Logger LOGGER = LoggerFactory.getLogger(HttpRequestFetcher.class);
 
     private final File temporaryDirectory;
-
     private final MetricRegistry registry;
     private final Processor.ExecutionContext context;
     private final ForkJoinPool requestForkJoinPool;
+    private final int maxNumberFetchRetry;
+    private final int fetchRetryIntervalMillis;
 
     /**
      * Constructor.
@@ -57,11 +59,14 @@ public final class HttpRequestFetcher {
     public HttpRequestFetcher(
             final File temporaryDirectory, final MetricRegistry registry,
             final Processor.ExecutionContext context,
-            final ForkJoinPool requestForkJoinPool) {
+            final ForkJoinPool requestForkJoinPool, final int maxNumberFetchRetry,
+            final int fetchRetryIntervalMillis) {
         this.temporaryDirectory = temporaryDirectory;
         this.registry = registry;
         this.context = context;
         this.requestForkJoinPool = requestForkJoinPool;
+        this.maxNumberFetchRetry = maxNumberFetchRetry;
+        this.fetchRetryIntervalMillis = fetchRetryIntervalMillis;
     }
 
     private CachedClientHttpRequest add(final CachedClientHttpRequest request) {
@@ -94,12 +99,13 @@ private CachedClientHttpResponse(final ClientHttpResponse originalResponse) thro
             this.statusText = originalResponse.getStatusText();
             this.cachedFile =
                     File.createTempFile("cacheduri", null, HttpRequestFetcher.this.temporaryDirectory);
-            try (OutputStream os = new FileOutputStream(this.cachedFile)) {
+            try (OutputStream os = Files.newOutputStream(this.cachedFile.toPath())) {
                 IOUtils.copy(originalResponse.getBody(), os);
             }
         }
 
         @Override
+        @Nonnull
         public InputStream getBody() throws IOException {
             if (this.body == null) {
                 this.body = new FileInputStream(this.cachedFile);
@@ -108,6 +114,7 @@ public InputStream getBody() throws IOException {
         }
 
         @Override
+        @Nonnull
         public HttpHeaders getHeaders() {
             return this.headers;
         }
@@ -118,6 +125,7 @@ public int getRawStatusCode() {
         }
 
         @Override
+        @Nonnull
         public String getStatusText() {
             return this.statusText;
         }
@@ -154,28 +162,33 @@ public HttpMethod getMethod() {
         }
 
         @Override
+        @Nonnull
         public String getMethodValue() {
             final HttpMethod method = this.originalRequest.getMethod();
             return method != null ? method.name() : "";
         }
 
         @Override
+        @Nonnull
         public URI getURI() {
             return this.originalRequest.getURI();
         }
 
         @Override
+        @Nonnull
         public HttpHeaders getHeaders() {
             return this.originalRequest.getHeaders();
         }
 
         @Override
+        @Nonnull
         public OutputStream getBody() {
             //body should be written before creating this object
             throw new UnsupportedOperationException();
         }
 
         @Override
+        @Nonnull
         public ClientHttpResponse execute() {
             assert this.future != null;
             final Timer.Context timerWait =
@@ -193,14 +206,29 @@ public ClientHttpResponse execute() {
             return result;
         }
 
-        @Retryable(value = IOException.class, maxAttemptsExpression = "${httpfetch.retry.maxAttempts}",
-               backoff = @Backoff(delayExpression = "${httpfetch.retry.backoffDelay}"))
-        private ClientHttpResponse fetch() throws IOException {
-            LOGGER.debug("Fetching URI resource {}", this.originalRequest.getURI());
-            ClientHttpResponse originalResponse = this.originalRequest.execute();
-            context.stopIfCanceled();
-            return new CachedClientHttpResponse(originalResponse);
+        /**
+         * Retry implemented manually.
+         * @see <a href="https://github.com/spring-projects/spring-retry/issues/148">Spring issue 148</a>
+         */
+        private ClientHttpResponse fetchWithRetry() throws IOException, InterruptedException {
+            AtomicInteger counter = new AtomicInteger();
+            do {
+                try {
+                    LOGGER.debug("Fetching URI resource {}", this.originalRequest.getURI());
+                    ClientHttpResponse originalResponse = this.originalRequest.execute();
+                    context.stopIfCanceled();
+                    return new CachedClientHttpResponse(originalResponse);
+                } catch (final IOException e) {
+                    if (counter.incrementAndGet() < HttpRequestFetcher.this.maxNumberFetchRetry) {
+                        TimeUnit.MILLISECONDS.sleep(HttpRequestFetcher.this.fetchRetryIntervalMillis);
+                    } else {
+                        LOGGER.debug("Fetching failed after {}", this.originalRequest.getURI());
+                        throw e;
+                    }
+                }
+            } while (true);
         }
+
         @Override
         public Void call() throws Exception {
             return context.mdcContextEx(() -> {
@@ -210,16 +238,18 @@ public Void call() throws Exception {
                 final Timer.Context timerDownload =
                         HttpRequestFetcher.this.registry.timer(baseMetricName).time();
                 try {
-                    this.response = this.fetch();
+                    this.response = this.fetchWithRetry();
                 } catch (IOException e) {
                     LOGGER.error("Request failed {}", this.originalRequest.getURI(), e);
                     this.response = new AbstractClientHttpResponse() {
                         @Override
+                        @Nonnull
                         public HttpHeaders getHeaders() {
                             return new HttpHeaders();
                         }
 
                         @Override
+                        @Nonnull
                         public InputStream getBody() {
                             return StreamUtils.emptyInput();
                         }
@@ -230,6 +260,7 @@ public int getRawStatusCode() {
                         }
 
                         @Override
+                        @Nonnull
                         public String getStatusText() {
                             return e.getMessage();
                         }

diff --git a/core/src/main/java/org/mapfish/print/processor/map/CreateMapProcessor.java b/core/src/main/java/org/mapfish/print/processor/map/CreateMapProcessor.java
@@ -56,6 +56,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Value;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 
@@ -113,6 +114,12 @@ public final class CreateMapProcessor
     @Resource(name = "requestForkJoinPool")
     private ForkJoinPool requestForkJoinPool;
 
+    @Value("${httpRequest.fetchRetry.maxNumber}")
+    private int httpRequestMaxNumberFetchRetry;
+
+    @Value("${httpRequest.fetchRetry.intervalMillis}")
+    private int httpRequestFetchRetryIntervalMillis;
+
     /**
      * Constructor.
      */
@@ -389,7 +396,9 @@ private List<URI> createLayerGraphics(
         final List<URI> graphics = new ArrayList<>(layers.size());
 
         HttpRequestFetcher cache = new HttpRequestFetcher(printDirectory, this.metricRegistry,
-                                                          context, this.requestForkJoinPool);
+                                                          context, this.requestForkJoinPool,
+                                                          this.httpRequestMaxNumberFetchRetry,
+                                                          this.httpRequestFetchRetryIntervalMillis);
 
         //prepare layers for rendering
         for (final MapLayer layer: layers) {

diff --git a/core/src/main/resources/mapfish-cli-spring-application-context.xml b/core/src/main/resources/mapfish-cli-spring-application-context.xml
@@ -2,6 +2,7 @@
 
 <beans xmlns="http://www.springframework.org/schema/beans" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-3.0.xsd">
+
     <import resource="mapfish-spring-application-context.xml" />
     <bean id="servletContext" class="org.mapfish.print.cli.CliServletContext"/>
     <bean id="main" class="org.mapfish.print.cli.Main"/>

diff --git a/core/src/main/resources/mapfish-spring.properties b/core/src/main/resources/mapfish-spring.properties
@@ -47,6 +47,8 @@ db.schema=public
 # The default print-apps location
 printapps.location=servlet:///print-apps
 
-# Configure the retry mechanism for HTTP requests
-httpfetch.retry.maxAttempts=2
-httpfetch.retry.backoffDelay=100
+# Maximum number of times the same request can be executed if the response was not obtained
+httpRequest.fetchRetry.maxNumber=3
+
+# Number of milliseconds between 2 executions of the same request
+httpRequest.fetchRetry.intervalMillis=100
diff --git a/docs/src/main/resources/templates/docker.html b/docs/src/main/resources/templates/docker.html
@@ -124,17 +124,12 @@ <h4 id="multi">
 
 <p>Example:</p>
 
-<p>
-  <code>
-    <pre>
+<pre><code>
 docker run --name=mapfish-print-test --publish=8080:8080
 --env=TOMCAT_LOG_TYPE=json
 --env=CATALINA_OPTS="-Ddb.name=mydb -Ddb.host=myserver -Ddb.username=myuser -Ddb.password=mypwd -Ddb.port=5432"
 mydockerhub/mapfish-print:latest
-        </pre
-    >
-  </code>
-</p>
+</code></pre>
 
 <p>
   The necessary tables are created automatically (print_accountings, print_job_results, print_job_statuses).