Merge pull request #28569 from vespa-engine/hmusum/simplify-waiting-for-url

Simplify waiting for url when getting config

3 files changed, 19 insertions, 35 deletions

diff --git a/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadApplier.java b/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadApplier.java
index 0dbc40a246c..c5a6e8a35c6 100644
--- a/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadApplier.java
+++ b/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadApplier.java
@@ -16,6 +16,7 @@ import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.lang.reflect.ParameterizedType;
 import java.nio.file.Path;
+import java.time.Duration;
 import java.util.ArrayDeque;
 import java.util.Deque;
 import java.util.HashMap;
@@ -244,7 +245,7 @@ public class ConfigPayloadApplier<T extends ConfigInstance.Builder> {
 
     private UrlReference resolveUrl(String url) {
         if ( ! isClientside()) return new UrlReference(url);
-        File file = urlDownloader.waitFor(new UrlReference(url), 60 * 60);
+        File file = urlDownloader.waitFor(new UrlReference(url), Duration.ofMinutes(60));
         return new UrlReference(file.getAbsolutePath());
     }
 
diff --git a/config/src/main/java/com/yahoo/vespa/config/UrlDownloader.java b/config/src/main/java/com/yahoo/vespa/config/UrlDownloader.java
index 62308315c95..976fe5573ec 100644
--- a/config/src/main/java/com/yahoo/vespa/config/UrlDownloader.java
+++ b/config/src/main/java/com/yahoo/vespa/config/UrlDownloader.java
@@ -15,7 +15,6 @@ import java.time.Duration;
 import java.util.logging.Logger;
 
 import static java.util.logging.Level.FINE;
-import static java.util.logging.Level.INFO;
 
 /**
  * @author lesters
@@ -67,41 +66,24 @@ public class UrlDownloader {
         return target != null && target.isValid();
     }
 
-    private boolean temporaryError(Request req) {
-        return false;  // Currently, none of the errors are considered temporary
-    }
-
-    public File waitFor(UrlReference urlReference, long timeout) {
-        long start = System.currentTimeMillis() / 1000;
+   public File waitFor(UrlReference urlReference, Duration timeout) {
         if (! isValid())
             connect();
-        long timeLeft = timeout;
-        do {
-            Request request = new Request("url.waitFor");
-            request.parameters().add(new StringValue(urlReference.value()));
-
-            double rpcTimeout = Math.min(timeLeft, 60 * 60.0);
-            log.log(FINE, () -> "InvokeSync waitFor " + urlReference + " with " + rpcTimeout + " seconds timeout");
-            target.invokeSync(request, rpcTimeout);
-
-            if (request.checkReturnTypes("s")) {
-                return new File(request.returnValues().get(0).asString());
-            } else if (!request.isError()) {
-                throw new RuntimeException("Invalid response: " + request.returnValues());
-            } else if (temporaryError(request)) {
-                log.log(INFO, "Retrying waitFor for " + urlReference + ": " + request.errorCode() + " -- " + request.errorMessage());
-                try {
-                    Thread.sleep(1000);
-                } catch (InterruptedException e) {
-                    throw new RuntimeException("Interrupted sleep between retries of waitFor", e);
-                }
-            } else {
-                 throw new RuntimeException("Wait for " + urlReference + " failed: " + request.errorMessage() + " (" + request.errorCode() + ")");
-            }
-            timeLeft = start + timeout - System.currentTimeMillis() / 1000;
-        } while (timeLeft > 0);
 
-        throw new RuntimeException("Timed out waiting for " + urlReference + " after " + timeout);
+       Request request = new Request("url.waitFor");
+       request.parameters().add(new StringValue(urlReference.value()));
+
+       double rpcTimeout = timeout.toSeconds();
+       log.log(FINE, () -> "InvokeSync waitFor " + urlReference + " with " + rpcTimeout + " seconds timeout");
+       target.invokeSync(request, rpcTimeout);
+
+       if (request.checkReturnTypes("s")) {
+           return new File(request.returnValues().get(0).asString());
+       } else if (! request.isError()) {
+           throw new RuntimeException("Invalid response: " + request.returnValues());
+       } else {
+           throw new RuntimeException("Wait for " + urlReference + " failed: " + request.errorMessage() + " (" + request.errorCode() + ")");
+       }
     }
 
 }
diff --git a/config/src/test/java/com/yahoo/vespa/config/ConfigPayloadApplierTest.java b/config/src/test/java/com/yahoo/vespa/config/ConfigPayloadApplierTest.java
index a982949e2fc..2c849c81b7f 100644
--- a/config/src/test/java/com/yahoo/vespa/config/ConfigPayloadApplierTest.java
+++ b/config/src/test/java/com/yahoo/vespa/config/ConfigPayloadApplierTest.java
@@ -9,6 +9,7 @@ import org.junit.Test;
 
 import java.io.File;
 import java.nio.file.Path;
+import java.time.Duration;
 import java.util.Optional;
 
 import static org.junit.Assert.assertEquals;
@@ -90,7 +91,7 @@ public class ConfigPayloadApplierTest {
     private static class MockDownloader extends UrlDownloader {
 
         @Override
-        public File waitFor(UrlReference urlReference, long timeout) {
+        public File waitFor(UrlReference urlReference, Duration timeout) {
             return new File("resolvedUrl", urlReference.value());
         }