diff options
5 files changed, 37 insertions, 14 deletions
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/RpcServer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/RpcServer.java index 7844fa14b16..d612a0fb8f2 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/RpcServer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/RpcServer.java @@ -537,7 +537,7 @@ public class RpcServer implements Runnable, ReloadListener, TenantListener { String[] fileReferenceStrings = req.parameters().get(0).asStringArray(); Stream.of(fileReferenceStrings) .map(FileReference::new) - .forEach(fileReference -> downloader.download( + .forEach(fileReference -> downloader.downloadIfNeeded( new FileReferenceDownload(fileReference, false /* downloadFromOtherSourceIfNotFound */))); req.returnValues().add(new Int32Value(0)); } 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 |