Merge pull request #20258 from vespa-engine/hmusum/move-logging-to-callers

Move logging in getFile() to callers

5 files changed, 9 insertions, 12 deletions

diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileDistributionRpcServer.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileDistributionRpcServer.java
index dfbd605ab50..d748ebb46e5 100644
--- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileDistributionRpcServer.java
+++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileDistributionRpcServer.java
@@ -11,6 +11,7 @@ import com.yahoo.jrt.StringValue;
 import com.yahoo.jrt.Supervisor;
 import com.yahoo.net.HostName;
 import com.yahoo.vespa.filedistribution.FileDownloader;
+import com.yahoo.vespa.filedistribution.FileReferenceDownload;
 
 import java.io.File;
 import java.util.Map;
@@ -102,7 +103,7 @@ class FileDistributionRpcServer {
     private void downloadFile(Request req) {
         FileReference fileReference = new FileReference(req.parameters().get(0).asString());
         log.log(Level.FINE, () -> "getFile() called for file reference '" + fileReference.value() + "'");
-        Optional<File> file = downloader.getFile(fileReference, HostName.getLocalhost());
+        Optional<File> file = downloader.getFile(new FileReferenceDownload(fileReference, HostName.getLocalhost()));
         if (file.isPresent()) {
             new RequestTracker().trackRequest(file.get().getParentFile());
             req.returnValues().add(new StringValue(file.get().getAbsolutePath()));
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 02fad2357c3..79337f3d32b 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
@@ -194,7 +194,10 @@ public class FileServer {
             // Create new FileReferenceDownload with downloadFromOtherSourceIfNotFound set to false
             // to avoid config servers requesting a file reference perpetually, e.g. for a file that does not exist anymore
             FileReferenceDownload newDownload = new FileReferenceDownload(fileReference, false, fileReferenceDownload.client());
-            return downloader.getFile(newDownload).isPresent();
+            boolean fileExists = downloader.getFile(newDownload).isPresent();
+            if ( ! fileExists)
+                log.log(Level.WARNING, "Failed downloading '" + fileReferenceDownload + "'");
+            return fileExists;
         } else {
             log.log(Level.FINE, "File not found, will not download from another source, since request came from another config server");
             return false;
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java
index 9cc475a56a0..d8295373207 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java
@@ -82,8 +82,8 @@ public class ApplicationPackageMaintainer extends ConfigServerMaintainer {
                                                                                    this.getClass().getSimpleName());
                         if (fileDownloader.getFile(download).isEmpty()) {
                             failures++;
-                            log.warning("Failed to download application package (" + appFileReference + ")" +
-                                                " for " + applicationId + " (session " + sessionId + ")");
+                            log.info("Failed downloading application package (" + appFileReference + ")" +
+                                             " for " + applicationId + " (session " + sessionId + ")");
                             continue;
                         }
                     }
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 b2efd35e41e..1821c8971e7 100644
--- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDownloader.java
+++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDownloader.java
@@ -6,7 +6,6 @@ import com.yahoo.jrt.Supervisor;
 import com.yahoo.vespa.config.Connection;
 import com.yahoo.vespa.config.ConnectionPool;
 import com.yahoo.vespa.defaults.Defaults;
-import com.yahoo.yolean.Exceptions;
 
 import java.io.File;
 import java.time.Duration;
@@ -69,16 +68,10 @@ public class FileDownloader implements AutoCloseable {
                                                                    downloadDirectory);
     }
 
-    public Optional<File> getFile(FileReference fileReference, String client) {
-        return getFile(new FileReferenceDownload(fileReference, client));
-    }
-
     public Optional<File> getFile(FileReferenceDownload fileReferenceDownload) {
         try {
             return getFutureFile(fileReferenceDownload).get(timeout.toMillis(), TimeUnit.MILLISECONDS);
         } catch (InterruptedException | ExecutionException | TimeoutException e) {
-            log.log(Level.WARNING, "Failed downloading '" + fileReferenceDownload +
-                                   "', removing from download queue: " + Exceptions.toMessageString(e));
             fileReferenceDownloader.failedDownloading(fileReferenceDownload.fileReference());
             return Optional.empty();
         }
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 460a1ee593a..3655285efbe 100644
--- a/filedistribution/src/test/java/com/yahoo/vespa/filedistribution/FileDownloaderTest.java
+++ b/filedistribution/src/test/java/com/yahoo/vespa/filedistribution/FileDownloaderTest.java
@@ -297,7 +297,7 @@ public class FileDownloaderTest {
     }
 
     private Optional<File> getFile(FileReference fileReference) {
-        return fileDownloader.getFile(fileReference, "test");
+        return fileDownloader.getFile(new FileReferenceDownload(fileReference, "test"));
     }
 
     private static class MockConnection implements ConnectionPool, com.yahoo.vespa.config.Connection {