Only download file if needed (if it does not exist on disk already)

4 files changed, 36 insertions, 13 deletions

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 2f5fc043f97..d82a806b79e 100644
--- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDistributionRpcServer.java
+++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDistributionRpcServer.java
@@ -105,7 +105,7 @@ public class FileDistributionRpcServer {
     public final void setFileReferencesToDownload(Request req) {
         Arrays.stream(req.parameters().get(0).asStringArray())
                 .map(FileReference::new)
-                .forEach(fileReference -> downloader.download(new FileReferenceDownload(fileReference)));
+                .forEach(fileReference -> downloader.downloadIfNeeded(new FileReferenceDownload(fileReference)));
         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 9aa49f57896..ad08a4467f5 100644
--- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDownloader.java
+++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDownloader.java
@@ -106,7 +106,26 @@ public class FileDownloader {
         return Optional.empty();
     }
 
-    public synchronized Future<Optional<File>> download(FileReferenceDownload fileReferenceDownload) {
+    private boolean alreadyDownloaded(FileReference fileReference) {
+        try {
+            if (getFileFromFileSystem(fileReference, downloadDirectory).isPresent())
+                return true;
+        } catch (RuntimeException e) {
+            /* ignored */
+        }
+        return false;
+    }
+
+    public boolean downloadIfNeeded(FileReferenceDownload fileReferenceDownload) {
+        if (!alreadyDownloaded(fileReferenceDownload.fileReference())) {
+            download(fileReferenceDownload);
+            return true;
+        } else {
+            return false;
+        }
+    }
+
+    synchronized Future<Optional<File>> download(FileReferenceDownload fileReferenceDownload) {
         FileReference fileReference = fileReferenceDownload.fileReference();
         Future<Optional<File>> inProgress = fileReferenceDownloader.addDownloadListener(fileReference, () -> getFile(fileReferenceDownload));
         if (inProgress != null) {
diff --git a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReceiver.java b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReceiver.java
index 26a1cad2220..b9714cf8126 100644
--- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReceiver.java
+++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReceiver.java
@@ -3,6 +3,7 @@
 package com.yahoo.vespa.filedistribution;
 
 import com.yahoo.config.FileReference;
+import com.yahoo.io.IOUtils;
 import com.yahoo.jrt.Int32Value;
 import com.yahoo.jrt.Method;
 import com.yahoo.jrt.Request;
@@ -191,11 +192,16 @@ public class FileReceiver {
             log.log(LogLevel.DEBUG, () -> "File moved from " + tempFile.getAbsolutePath()+ " to " + destination.getAbsolutePath());
         } catch (FileAlreadyExistsException e) {
             // Don't fail if it already exists (we might get the file from several config servers when retrying, servers are down etc.
-            // so it might be written already). Delete temp file in that case, to avoid filling the disk.
+            // so it might be written already). Delete temp file/dir in that case, to avoid filling the disk.
             log.log(LogLevel.DEBUG, () -> "File '" + destination.getAbsolutePath() + "' already exists, continuing: " + e.getMessage());
             try {
-                Files.delete(tempFile.toPath());
-            } catch (IOException ioe) { /* ignore failure */}
+                if (tempFile.isDirectory())
+                    IOUtils.recursiveDeleteDir(tempFile);
+                else
+                    Files.delete(tempFile.toPath());
+            } catch (IOException ioe) {
+                log.log(LogLevel.WARNING, "Failed deleting file/dir " + tempFile);
+            }
         } catch (IOException e) {
             String message = "Failed moving file '" + tempFile.getAbsolutePath() + "' to '" + destination.getAbsolutePath() + "'";
             log.log(LogLevel.ERROR, message, e);
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 0ba30b5c6f7..2a40d27831c 100644
--- a/filedistribution/src/test/java/com/yahoo/vespa/filedistribution/FileDownloaderTest.java
+++ b/filedistribution/src/test/java/com/yahoo/vespa/filedistribution/FileDownloaderTest.java
@@ -186,18 +186,16 @@ public class FileDownloaderTest {
     public void setFilesToDownload() throws IOException {
         Duration timeout = Duration.ofMillis(200);
         Duration sleepBetweenRetries = Duration.ofMillis(200);
-        File downloadDir = Files.createTempDirectory("filedistribution").toFile();
         MockConnection connectionPool = new MockConnection();
         connectionPool.setResponseHandler(new MockConnection.WaitResponseHandler(timeout.plus(Duration.ofMillis(1000))));
         FileDownloader fileDownloader = new FileDownloader(connectionPool, downloadDir, tempDir, timeout, sleepBetweenRetries);
         FileReference foo = new FileReference("foo");
-        FileReference bar = new FileReference("bar");
-        fileDownloader.download(new FileReferenceDownload(foo));
-        fileDownloader.download(new FileReferenceDownload(bar));
-
-        // Verify download status
-        assertDownloadStatus(fileDownloader, foo, 0.0);
-        assertDownloadStatus(fileDownloader, bar, 0.0);
+        // Should download since we do not have the file on disk
+        assertTrue(fileDownloader.downloadIfNeeded(new FileReferenceDownload(foo)));
+        // Receive files to simulate download
+        receiveFile();
+        // Should not download, since file has already been downloaded
+        assertFalse(fileDownloader.downloadIfNeeded(new FileReferenceDownload(foo)));
     }
 
     @Test