diff options
Diffstat (limited to 'filedistribution/src')
3 files changed, 17 insertions, 22 deletions
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 24b3fcac3e3..9b9867326d7 100644 --- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDownloader.java +++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDownloader.java @@ -60,16 +60,13 @@ public class FileDownloader { private Future<Optional<File>> getFutureFile(FileReferenceDownload fileReferenceDownload) { FileReference fileReference = fileReferenceDownload.fileReference(); Objects.requireNonNull(fileReference, "file reference cannot be null"); - log.log(LogLevel.DEBUG, () -> "Checking if file reference '" + fileReference.value() + "' exists in '" + - downloadDirectory.getAbsolutePath() + "' "); + Optional<File> file = getFileFromFileSystem(fileReference, downloadDirectory); if (file.isPresent()) { SettableFuture<Optional<File>> future = SettableFuture.create(); future.set(file); return future; } else { - log.log(LogLevel.DEBUG, () -> "File reference '" + fileReference.value() + "' not found in " + - downloadDirectory.getAbsolutePath() + ", starting download"); return download(fileReferenceDownload); } } @@ -86,6 +83,7 @@ public class FileDownloader { return downloadDirectory; } + // Files are moved atomically, so if file reference exists and is accessible we can use it private Optional<File> getFileFromFileSystem(FileReference fileReference, File directory) { File[] files = new File(directory, fileReference.value()).listFiles(); if (directory.exists() && directory.isDirectory() && files != null && files.length > 0) { @@ -111,27 +109,18 @@ public class FileDownloader { } } - public boolean downloadIfNeeded(FileReferenceDownload fileReferenceDownload) { - if (!alreadyDownloaded(fileReferenceDownload.fileReference())) { - download(fileReferenceDownload); - return true; - } else { - log.log(LogLevel.DEBUG, () -> "Download not needed, " + fileReferenceDownload.fileReference() + " already downloaded" ); - return false; - } + /** Start a download, don't wait for result */ + public void downloadIfNeeded(FileReferenceDownload fileReferenceDownload) { + FileReference fileReference = fileReferenceDownload.fileReference(); + if (alreadyDownloaded(fileReference) || fileReferenceDownloader.isDownloading(fileReference)) return; + + queueForDownload(fileReferenceDownload); } private synchronized Future<Optional<File>> download(FileReferenceDownload fileReferenceDownload) { FileReference fileReference = fileReferenceDownload.fileReference(); Future<Optional<File>> inProgress = fileReferenceDownloader.addDownloadListener(fileReference, () -> getFile(fileReferenceDownload)); - if (inProgress != null) { - log.log(LogLevel.DEBUG, () -> "Already downloading '" + fileReference.value() + "'"); - return inProgress; - } - - Future<Optional<File>> future = queueForDownload(fileReferenceDownload); - log.log(LogLevel.DEBUG, () -> "Queued '" + fileReference.value() + "' for download with timeout " + timeout); - return future; + return (inProgress == null) ? queueForDownload(fileReferenceDownload) : inProgress; } private Future<Optional<File>> queueForDownload(FileReferenceDownload fileReferenceDownload) { diff --git a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceDownloader.java b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceDownloader.java index 8a0212211e1..ae0040cc7dd 100644 --- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceDownloader.java +++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceDownloader.java @@ -40,7 +40,9 @@ public class FileReferenceDownloader { Executors.newFixedThreadPool(Math.max(8, Runtime.getRuntime().availableProcessors()), new DaemonThreadFactory("filereference downloader")); private final ConnectionPool connectionPool; + /* Ongoing downloads */ private final Map<FileReference, FileReferenceDownload> downloads = new LinkedHashMap<>(); + /* Status for ongoing and finished downloads */ private final Map<FileReference, Double> downloadStatus = new HashMap<>(); // between 0 and 1 private final Duration downloadTimeout; private final Duration sleepBetweenRetries; 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 d1d12cb07b7..92f5cd51d87 100644 --- a/filedistribution/src/test/java/com/yahoo/vespa/filedistribution/FileDownloaderTest.java +++ b/filedistribution/src/test/java/com/yahoo/vespa/filedistribution/FileDownloaderTest.java @@ -197,11 +197,15 @@ public class FileDownloaderTest { FileDownloader fileDownloader = new FileDownloader(connectionPool, downloadDir, tempDir, timeout, sleepBetweenRetries); FileReference foo = new FileReference("foo"); // Should download since we do not have the file on disk - assertTrue(fileDownloader.downloadIfNeeded(new FileReferenceDownload(foo))); + fileDownloader.downloadIfNeeded(new FileReferenceDownload(foo)); + assertTrue(fileDownloader.fileReferenceDownloader().isDownloading(foo)); + assertFalse(fileDownloader.getFile(foo).isPresent()); // Receive files to simulate download receiveFile(); // Should not download, since file has already been downloaded - assertFalse(fileDownloader.downloadIfNeeded(new FileReferenceDownload(foo))); + fileDownloader.downloadIfNeeded(new FileReferenceDownload(foo)); + // and file should be available + assertTrue(fileDownloader.getFile(foo).isPresent()); } @Test |