Merge pull request #4428 from vespa-engine/hmusum/start-file-distribution-downloads-early

Notify config proxy about starting downloads while preparing

9 files changed, 79 insertions, 31 deletions

diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/FileDistribution.java b/config-model-api/src/main/java/com/yahoo/config/model/api/FileDistribution.java
index 990bce539ba..3f1c0046a85 100644
--- a/config-model-api/src/main/java/com/yahoo/config/model/api/FileDistribution.java
+++ b/config-model-api/src/main/java/com/yahoo/config/model/api/FileDistribution.java
@@ -11,12 +11,21 @@ import java.util.Set;
 /**
  * Interface for models towards filedistribution.
  *
- * @author lulf
- * @since 5.1
+ * @author Ulf Lilleengen
  */
 public interface FileDistribution {
 
     void sendDeployedFiles(String hostName, Set<FileReference> fileReferences);
+
+    /**
+     * Notifies client which file references to download. Used to start downloading early (while
+     * preparing application package).
+     *
+     * @param hostName       host which should be notified about file references to download
+     * @param fileReferences set of file references to start downloading
+     */
+    void startDownload(String hostName, Set<FileReference> fileReferences);
+
     void reloadDeployFileDistributor();
 
     void removeDeploymentsThatHaveDifferentApplicationId(Collection<String> targetHostnames);
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributor.java b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributor.java
index f6cc9203d00..8fcece3aa80 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributor.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributor.java
@@ -9,13 +9,10 @@ import com.yahoo.vespa.model.Host;
 import java.util.*;
 import java.util.stream.Collectors;
 
-import static java.util.Arrays.asList;
-
-
 /**
  * Responsible for directing distribution of files to hosts.
  *
- * @author tonytv
+ * @author Tony Vaagenes
  */
 public class FileDistributor {
 
@@ -99,6 +96,7 @@ public class FileDistributor {
         for (Host host : getTargetHosts()) {
             if ( ! host.getHostName().equals(fileSourceHost)) {
                 dbHandler.sendDeployedFiles(host.getHostName(), filesToSendToHost(host));
+                dbHandler.startDownload(host.getHostName(), filesToSendToHost(host));
             }
         }
         dbHandler.sendDeployedFiles(fileSourceHost, allFilesToSend());
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/CombinedLegacyDistribution.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/CombinedLegacyDistribution.java
index 588f2d1d63f..f0e64a936a5 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/CombinedLegacyDistribution.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/CombinedLegacyDistribution.java
@@ -3,22 +3,43 @@ package com.yahoo.vespa.config.server.filedistribution;
 
 import com.yahoo.config.FileReference;
 import com.yahoo.config.model.api.FileDistribution;
+import com.yahoo.jrt.Request;
+import com.yahoo.jrt.Spec;
+import com.yahoo.jrt.StringArray;
+import com.yahoo.jrt.Supervisor;
+import com.yahoo.jrt.Target;
+import com.yahoo.jrt.Transport;
+import com.yahoo.log.LogLevel;
 
 import java.util.Collection;
 import java.util.Set;
+import java.util.logging.Logger;
 
+/**
+ * @author baldersheim
+ */
 public class CombinedLegacyDistribution implements FileDistribution {
+    private final static Logger log = Logger.getLogger(CombinedLegacyDistribution.class.getName());
+
+    private final Supervisor supervisor = new Supervisor(new Transport());
     private final FileDistribution legacy;
 
     CombinedLegacyDistribution(FileDBHandler legacy) {
         this.legacy = legacy;
     }
+
     @Override
     public void sendDeployedFiles(String hostName, Set<FileReference> fileReferences) {
         legacy.sendDeployedFiles(hostName, fileReferences);
     }
 
     @Override
+    public void startDownload(String hostName, Set<FileReference> fileReferences) {
+        // TODO: Not active for now
+        // startDownloadingFileReferences(hostName, fileReferences);
+    }
+
+    @Override
     public void reloadDeployFileDistributor() {
         legacy.reloadDeployFileDistributor();
     }
@@ -27,4 +48,18 @@ public class CombinedLegacyDistribution implements FileDistribution {
     public void removeDeploymentsThatHaveDifferentApplicationId(Collection<String> targetHostnames) {
         legacy.removeDeploymentsThatHaveDifferentApplicationId(targetHostnames);
     }
+
+    // Notifies config proxy which file references it should start downloading. It's OK if the call does not succeed,
+    // as downloading will then start synchronously when a service requests a file reference instead
+    private void startDownloadingFileReferences(String hostName, Set<FileReference> fileReferences) {
+        Target target = supervisor.connect(new Spec(hostName, 19090));
+        double timeout = 0.1;
+        Request request = new Request("filedistribution.setFileReferencesToDownload");
+        request.parameters().add(new StringArray(fileReferences.stream().map(FileReference::value).toArray(String[]::new)));
+        log.log(LogLevel.INFO, "Executing " + request.methodName() + " against " + target.toString());
+        target.invokeSync(request, timeout);
+        if (request.isError()) {
+            log.log(LogLevel.INFO, request.methodName() + " failed: " + request.errorCode() + " (" + request.errorMessage() + ")");
+        }
+    }
 }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDBHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDBHandler.java
index f0ce6104496..9b3f4c39a45 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDBHandler.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDBHandler.java
@@ -10,9 +10,8 @@ import java.util.*;
 /**
  * Implements invoker of filedistribution using manager with JNI.
  *
- * @author tonytv
- * @author lulf
- * @since 5.1.14
+ * @author Tony Vaagenes
+ * @author Ulf Lilleengen
  */
 public class FileDBHandler implements FileDistribution {
     private final FileDistributionManager manager;
@@ -31,6 +30,11 @@ public class FileDBHandler implements FileDistribution {
     }
 
     @Override
+    public void startDownload(String hostName, Set<FileReference> fileReferences) {
+        throw new UnsupportedOperationException("Not valid for this Filedistribution implementation");
+    }
+
+    @Override
     public void removeDeploymentsThatHaveDifferentApplicationId(Collection<String> targetHostnames) {
         manager.removeDeploymentsThatHaveDifferentApplicationId(targetHostnames);
     }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileServer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileServer.java
index 81f5e62016a..a9c56cf99d0 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileServer.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileServer.java
@@ -6,12 +6,12 @@ import com.yahoo.cloud.config.ConfigserverConfig;
 import com.yahoo.config.FileReference;
 import com.yahoo.config.model.api.FileDistribution;
 import com.yahoo.config.subscription.ConfigSourceSet;
-import com.yahoo.io.IOUtils;
 import com.yahoo.jrt.Int32Value;
 import com.yahoo.jrt.Request;
 import com.yahoo.jrt.StringValue;
 import com.yahoo.jrt.Supervisor;
 import com.yahoo.jrt.Transport;
+import com.yahoo.log.LogLevel;
 import com.yahoo.net.HostName;
 import com.yahoo.vespa.config.Connection;
 import com.yahoo.vespa.config.ConnectionPool;
@@ -108,8 +108,7 @@ public class FileServer {
 
     private void serveFile(FileReference reference, Receiver target) {
         File file = root.getFile(reference);
-        // TODO remove once verified in system tests.
-        log.info("Start serving reference '" + reference.value() + "' with file '" + file.getAbsolutePath() + "'");
+        log.log(LogLevel.DEBUG, "Start serving reference '" + reference.value() + "' with file '" + file.getAbsolutePath() + "'");
         boolean success = false;
         String errorDescription = "OK";
         FileReferenceData fileData = FileReferenceDataBlob.empty(reference, file.getName());
@@ -122,8 +121,7 @@ public class FileServer {
         }
 
         target.receive(fileData, new ReplayStatus(success ? 0 : 1, success ? "OK" : errorDescription));
-        // TODO remove once verified in system tests.
-        log.info("Done serving reference '" + reference.toString() + "' with file '" + file.getAbsolutePath() + "'");
+        log.log(LogLevel.DEBUG, "Done serving reference '" + reference.toString() + "' with file '" + file.getAbsolutePath() + "'");
     }
 
     private FileReferenceData readFileReferenceData(FileReference reference) throws IOException {
@@ -143,8 +141,7 @@ public class FileServer {
     private void serveFile(String fileReference, Request request, Receiver receiver) {
         FileApiErrorCodes result;
         try {
-            // TODO remove once verified in system tests.
-            log.info("Received request for reference '" + fileReference + "'");
+            log.log(LogLevel.DEBUG, "Received request for reference '" + fileReference + "'");
             result = hasFile(fileReference)
                     ? FileApiErrorCodes.OK
                     : FileApiErrorCodes.NOT_FOUND;
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/MockFileDBHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/MockFileDBHandler.java
index eb23d38e23e..caf64cca4d0 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/MockFileDBHandler.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/MockFileDBHandler.java
@@ -8,8 +8,7 @@ import java.util.Collection;
 import java.util.Set;
 
 /**
- * @author lulf
- * @since 5.1
+ * @author Ulf Lilleengen
  */
 public class MockFileDBHandler implements FileDistribution {
     public int sendDeployedFilesCalled = 0;
@@ -22,6 +21,9 @@ public class MockFileDBHandler implements FileDistribution {
     }
 
     @Override
+    public void startDownload(String hostName, Set<FileReference> fileReferences) { /* not implemented */ }
+
+    @Override
     public void reloadDeployFileDistributor() {
         reloadDeployFileDistributorCalled++;
     }
diff --git a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDistributionRpcServer.java b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDistributionRpcServer.java
index d09cf17b9e3..2e58455bc39 100644
--- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDistributionRpcServer.java
+++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDistributionRpcServer.java
@@ -118,7 +118,7 @@ public class FileDistributionRpcServer {
         List<FileReference> fileReferences = Stream.of(fileReferenceStrings)
                 .map(FileReference::new)
                 .collect(Collectors.toList());
-        downloader.queueForDownload(fileReferences);
+        downloader.queueForAsyncDownload(fileReferences);
 
         req.returnValues().add(new Int32Value(0));
     }
diff --git a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDownloader.java b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDownloader.java
index 6610893d8ae..05bcaacb107 100644
--- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDownloader.java
+++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDownloader.java
@@ -67,12 +67,19 @@ public class FileDownloader {
         } else {
             log.log(LogLevel.INFO, "File reference '" + fileReference.value() + "' not found in " +
                     directory.getAbsolutePath() + ", starting download");
-            return queueForDownload(fileReference, timeout);
+            return queueForAsyncDownload(fileReference, timeout);
         }
     }
 
-    public void queueForDownload(List<FileReference> fileReferences) {
-        fileReferences.forEach(this::queueForDownload);
+    // Start downloading, but there is no Future used get file being downloaded
+    public void queueForAsyncDownload(List<FileReference> fileReferences) {
+        fileReferences.forEach(fileReference -> {
+            if (fileReferenceDownloader.isDownloading(fileReference)) {
+                log.log(LogLevel.DEBUG, "Already downloading '" + fileReference.value() + "'");
+            } else {
+                queueForAsyncDownload(fileReference).cancel(false);
+            }
+        });
     }
 
     void receiveFile(FileReferenceData fileReferenceData) {
@@ -107,24 +114,20 @@ public class FileDownloader {
         return Optional.empty();
     }
 
-    private synchronized Future<Optional<File>> queueForDownload(FileReference fileReference, Duration timeout) {
+    private synchronized Future<Optional<File>> queueForAsyncDownload(FileReference fileReference, Duration timeout) {
         Future<Optional<File>> inProgress = fileReferenceDownloader.addDownloadListener(fileReference, () -> getFile(fileReference));
         if (inProgress != null) {
             log.log(LogLevel.DEBUG, "Already downloading '" + fileReference.value() + "'");
             return inProgress;
         }
 
-        Future<Optional<File>> future = queueForDownload(fileReference);
+        Future<Optional<File>> future = queueForAsyncDownload(fileReference);
         log.log(LogLevel.INFO, "Queued '" + fileReference.value() + "' for download with timeout " + timeout);
         return future;
     }
 
-    // We don't care about the future in this call
-    private Future<Optional<File>> queueForDownload(FileReference fileReference) {
-        return queueForDownload(new FileReferenceDownload(fileReference, SettableFuture.create()));
-    }
-
-    private Future<Optional<File>> queueForDownload(FileReferenceDownload fileReferenceDownload) {
+    private Future<Optional<File>> queueForAsyncDownload(FileReference fileReference) {
+        FileReferenceDownload fileReferenceDownload = new FileReferenceDownload(fileReference, SettableFuture.create());
         fileReferenceDownloader.addToDownloadQueue(fileReferenceDownload);
         return fileReferenceDownload.future();
     }
diff --git a/filedistribution/src/test/java/com/yahoo/vespa/filedistribution/FileDownloaderTest.java b/filedistribution/src/test/java/com/yahoo/vespa/filedistribution/FileDownloaderTest.java
index 60478550084..d2da020539a 100644
--- a/filedistribution/src/test/java/com/yahoo/vespa/filedistribution/FileDownloaderTest.java
+++ b/filedistribution/src/test/java/com/yahoo/vespa/filedistribution/FileDownloaderTest.java
@@ -189,7 +189,7 @@ public class FileDownloaderTest {
         FileReference foo = new FileReference("foo");
         FileReference bar = new FileReference("bar");
         List<FileReference> fileReferences = Arrays.asList(foo, bar);
-        fileDownloader.queueForDownload(fileReferences);
+        fileDownloader.queueForAsyncDownload(fileReferences);
 
         // Verify download status
         assertDownloadStatus(fileDownloader, foo, 0.0);