Merge branch 'master' into arnej/add-feature-flag-to-ignore-stack-sizes

7 files changed, 54 insertions, 33 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDistributionImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDistributionImpl.java
index a49e2ec76bb..605f5924e68 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDistributionImpl.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDistributionImpl.java
@@ -41,8 +41,8 @@ public class FileDistributionImpl implements FileDistribution, RequestWaiter {
         return fileReferencesDir;
     }
 
-    // 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
+    // Notifies client which file references it should start downloading. It's OK if the call does not succeed,
+    // as this is just a hint to the client to start downloading. Currently the only client is the config server
     private void startDownloadingFileReferences(String hostName, int port, Set<FileReference> fileReferences) {
         Target target = supervisor.connect(new Spec(hostName, port));
         Request request = new Request("filedistribution.setFileReferencesToDownload");
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 f34beae9c46..f4801c5a7ea 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
@@ -16,11 +16,14 @@ import com.yahoo.vespa.config.JRTConnectionPool;
 import com.yahoo.vespa.defaults.Defaults;
 import com.yahoo.vespa.filedistribution.CompressedFileReference;
 import com.yahoo.vespa.filedistribution.EmptyFileReferenceData;
+import com.yahoo.vespa.filedistribution.FileDistributionConnectionPool;
 import com.yahoo.vespa.filedistribution.FileDownloader;
 import com.yahoo.vespa.filedistribution.FileReferenceData;
 import com.yahoo.vespa.filedistribution.FileReferenceDownload;
 import com.yahoo.vespa.filedistribution.LazyFileReferenceData;
 import com.yahoo.vespa.filedistribution.LazyTemporaryStorageFileReferenceData;
+import com.yahoo.vespa.flags.FlagSource;
+import com.yahoo.vespa.flags.Flags;
 import com.yahoo.yolean.Exceptions;
 
 import java.io.File;
@@ -30,6 +33,7 @@ import java.nio.file.Path;
 import java.util.List;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
+import java.util.concurrent.ThreadPoolExecutor;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -73,14 +77,15 @@ public class FileServer {
 
     @SuppressWarnings("WeakerAccess") // Created by dependency injection
     @Inject
-    public FileServer(ConfigserverConfig configserverConfig) {
+    public FileServer(ConfigserverConfig configserverConfig, FlagSource flagSource) {
         this(new File(Defaults.getDefaults().underVespaHome(configserverConfig.fileReferencesDir())),
-             createFileDownloader(getOtherConfigServersInCluster(configserverConfig)));
+             createFileDownloader(getOtherConfigServersInCluster(configserverConfig),
+             Flags.USE_FILE_DISTRIBUTION_CONNECTION_POOL.bindTo(flagSource).value()));
     }
 
     // For testing only
     public FileServer(File rootDir) {
-        this(rootDir, createFileDownloader(List.of()));
+        this(rootDir, createFileDownloader(List.of(), true));
     }
 
     public FileServer(File rootDir, FileDownloader fileDownloader) {
@@ -94,10 +99,6 @@ public class FileServer {
         return hasFile(new FileReference(fileReference));
     }
 
-    FileDirectory getRootDir() {
-        return root;
-    }
-
     private boolean hasFile(FileReference reference) {
         try {
             return root.getFile(reference).exists();
@@ -107,6 +108,8 @@ public class FileServer {
         return false;
     }
 
+    FileDirectory getRootDir() { return root; }
+
     void startFileServing(String fileName, Receiver target) {
         FileReference reference = new FileReference(fileName);
         File file = root.getFile(reference);
@@ -153,6 +156,8 @@ public class FileServer {
     }
 
     public void serveFile(String fileReference, boolean downloadFromOtherSourceIfNotFound, Request request, Receiver receiver) {
+        if (executor instanceof ThreadPoolExecutor)
+            log.log(Level.FINE, () -> "Active threads is now " + ((ThreadPoolExecutor) executor).getActiveCount());
         executor.execute(() -> serveFileInternal(fileReference, downloadFromOtherSourceIfNotFound, request, receiver));
     }
 
@@ -191,32 +196,34 @@ public class FileServer {
             FileReferenceDownload newDownload = new FileReferenceDownload(fileReference, false, fileReferenceDownload.client());
             return downloader.getFile(newDownload).isPresent();
         } else {
-            log.log(Level.FINE, "File not found, will not download from another source since request came from another config server");
+            log.log(Level.FINE, "File not found, will not download from another source, since request came from another config server");
             return false;
         }
     }
 
-    public FileDownloader downloader() {
-        return downloader;
-    }
+    public FileDownloader downloader() { return downloader; }
 
     public void close() {
         downloader.close();
         executor.shutdown();
     }
 
-    private static FileDownloader createFileDownloader(List<String> configServers) {
+    private static FileDownloader createFileDownloader(List<String> configServers, boolean useFileDistributionConnectionPool) {
         Supervisor supervisor = new Supervisor(new Transport("filedistribution-pool")).setDropEmptyBuffers(true);
         return new FileDownloader(configServers.isEmpty()
                                           ? FileDownloader.emptyConnectionPool()
-                                          : getConnectionPool(configServers, supervisor),
+                                          : createConnectionPool(configServers, supervisor, useFileDistributionConnectionPool),
                                   supervisor);
     }
 
-    private static ConnectionPool getConnectionPool(List<String> configServers, Supervisor supervisor) {
-        return configServers.size() > 0
-                ? new JRTConnectionPool(new ConfigSourceSet(configServers), supervisor)
-                : FileDownloader.emptyConnectionPool();
+    private static ConnectionPool createConnectionPool(List<String> configServers, Supervisor supervisor, boolean useFileDistributionConnectionPool) {
+        ConfigSourceSet configSourceSet = new ConfigSourceSet(configServers);
+
+        if (configServers.size() == 0) return FileDownloader.emptyConnectionPool();
+
+        return useFileDistributionConnectionPool
+                ? new FileDistributionConnectionPool(configSourceSet, supervisor)
+                : new JRTConnectionPool(configSourceSet, supervisor);
     }
 
 }
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 d4d4a7fa7d3..08c300220df 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
@@ -16,7 +16,9 @@ import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.defaults.Defaults;
 import com.yahoo.vespa.filedistribution.FileDownloader;
 import com.yahoo.vespa.filedistribution.FileReferenceDownload;
+import com.yahoo.vespa.filedistribution.FileDistributionConnectionPool;
 import com.yahoo.vespa.flags.FlagSource;
+import com.yahoo.vespa.flags.Flags;
 
 import java.io.File;
 import java.time.Duration;
@@ -40,6 +42,8 @@ public class ApplicationPackageMaintainer extends ConfigServerMaintainer {
     private final File downloadDirectory;
     private final ConfigserverConfig configserverConfig;
     private final Supervisor supervisor;
+    private final boolean useFileDistributionConnectionPool;
+
 
     ApplicationPackageMaintainer(ApplicationRepository applicationRepository,
                                  Curator curator,
@@ -49,7 +53,8 @@ public class ApplicationPackageMaintainer extends ConfigServerMaintainer {
         this.applicationRepository = applicationRepository;
         this.configserverConfig = applicationRepository.configserverConfig();
         this.supervisor = new Supervisor(new Transport("filedistribution-pool")).setDropEmptyBuffers(true);
-        downloadDirectory = new File(Defaults.getDefaults().underVespaHome(configserverConfig.fileReferencesDir()));
+        this.downloadDirectory = new File(Defaults.getDefaults().underVespaHome(configserverConfig.fileReferencesDir()));
+        this.useFileDistributionConnectionPool = Flags.USE_FILE_DISTRIBUTION_CONNECTION_POOL.bindTo(flagSource).value();
     }
 
     @Override
@@ -61,25 +66,24 @@ public class ApplicationPackageMaintainer extends ConfigServerMaintainer {
 
         try (var fileDownloader = createFileDownloader()) {
             for (var applicationId : applicationRepository.listApplications()) {
-                log.fine(() -> "Verifying application package for " + applicationId);
+                log.finest(() -> "Verifying application package for " + applicationId);
                 Session session = applicationRepository.getActiveSession(applicationId);
                 if (session == null) continue;  // App might be deleted after call to listApplications() or not activated yet (bootstrap phase)
 
-                FileReference applicationPackage = session.getApplicationPackageReference();
-                long sessionId = session.getSessionId();
-                log.fine(() -> "Verifying application package file reference " + applicationPackage + " for session " + sessionId);
-
-                if (applicationPackage != null) {
+                FileReference appFileReference = session.getApplicationPackageReference();
+                if (appFileReference != null) {
+                    long sessionId = session.getSessionId();
                     attempts++;
-                    if (! fileReferenceExistsOnDisk(downloadDirectory, applicationPackage)) {
-                        log.fine(() -> "Downloading missing application package for application " + applicationId + " (session " + sessionId + ")");
+                    if (! fileReferenceExistsOnDisk(downloadDirectory, appFileReference)) {
+                        log.fine(() -> "Downloading application package for " + applicationId + " (session " + sessionId + ")");
 
-                        FileReferenceDownload download = new FileReferenceDownload(applicationPackage,
+                        FileReferenceDownload download = new FileReferenceDownload(appFileReference,
                                                                                    false,
                                                                                    this.getClass().getSimpleName());
                         if (fileDownloader.getFile(download).isEmpty()) {
                             failures++;
-                            log.warning("Failed to download application package for application " + applicationId + " (session " + sessionId + ")");
+                            log.warning("Failed to download application package (" + appFileReference + ")" +
+                                                " for " + applicationId + " (session " + sessionId + ")");
                             continue;
                         }
                     }
@@ -91,7 +95,10 @@ public class ApplicationPackageMaintainer extends ConfigServerMaintainer {
     }
 
     private FileDownloader createFileDownloader() {
-        return new FileDownloader(new JRTConnectionPool(new ConfigSourceSet(getOtherConfigServersInCluster(configserverConfig)), supervisor),
+        ConfigSourceSet configSourceSet = new ConfigSourceSet(getOtherConfigServersInCluster(configserverConfig));
+        return new FileDownloader(useFileDistributionConnectionPool
+                                          ? new FileDistributionConnectionPool(configSourceSet, supervisor)
+                                          : new JRTConnectionPool(configSourceSet, supervisor),
                                   supervisor,
                                   downloadDirectory);
     }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java
index 5a0a9b1d796..ce592c3282a 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java
@@ -242,7 +242,8 @@ public class SessionPreparer {
             try {
                 this.preprocessedApplicationPackage = applicationPackage.preprocess(properties.zone(), logger);
             } catch (IOException | RuntimeException e) {
-                throw new IllegalArgumentException("Error preprocessing application package for " + applicationId, e);
+                throw new IllegalArgumentException("Error preprocessing application package for " + applicationId +
+                                                           ", session " + sessionZooKeeperClient.sessionId(), e);
             }
             checkTimeout("preprocess");
         }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClient.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClient.java
index 11b2881bc34..0b34bd95f8e 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClient.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClient.java
@@ -66,6 +66,7 @@ public class SessionZooKeeperClient {
 
     private final Curator curator;
     private final TenantName tenantName;
+    private final long sessionId;
     private final Path sessionPath;
     private final Path sessionStatusPath;
     private final String serverId;  // hostname
@@ -75,6 +76,7 @@ public class SessionZooKeeperClient {
     public SessionZooKeeperClient(Curator curator, TenantName tenantName, long sessionId, String serverId, AddFileInterface fileManager, int maxNodeSize) {
         this.curator = curator;
         this.tenantName = tenantName;
+        this.sessionId = sessionId;
         this.sessionPath = getSessionPath(tenantName, sessionId);
         this.serverId = serverId;
         this.sessionStatusPath = sessionPath.append(ZKApplication.SESSIONSTATE_ZK_SUBPATH);
@@ -106,6 +108,8 @@ public class SessionZooKeeperClient {
         }
     }
 
+    public long sessionId() { return sessionId; }
+
     public CompletionWaiter createActiveWaiter() { return createCompletionWaiter(ACTIVE_BARRIER); }
 
     CompletionWaiter createPrepareWaiter() { return createCompletionWaiter(PREPARE_BARRIER); }
diff --git a/configserver/src/main/sh/start-configserver b/configserver/src/main/sh/start-configserver
index 317af4b2fea..efee86be29f 100755
--- a/configserver/src/main/sh/start-configserver
+++ b/configserver/src/main/sh/start-configserver
@@ -177,6 +177,7 @@ vespa-run-as-vespa-user vespa-runserver -s configserver -r 30 -p $pidfile -- \
     --add-opens=java.base/java.nio=ALL-UNNAMED \
     --add-opens=java.base/jdk.internal.loader=ALL-UNNAMED \
     --add-opens=java.base/sun.security.ssl=ALL-UNNAMED  \
+    --add-opens=java.base/sun.security.util=ALL-UNNAMED  \
 	-Djava.io.tmpdir=${VESPA_HOME}/tmp \
 	-Djava.library.path=${VESPA_HOME}/lib64 \
 	-Djava.awt.headless=true \
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileServerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileServerTest.java
index 29ec11bad26..f85ca37a351 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileServerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileServerTest.java
@@ -10,6 +10,7 @@ import com.yahoo.net.HostName;
 import com.yahoo.vespa.filedistribution.FileDownloader;
 import com.yahoo.vespa.filedistribution.FileReferenceData;
 import com.yahoo.vespa.filedistribution.FileReferenceDownload;
+import com.yahoo.vespa.flags.InMemoryFlagSource;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -117,7 +118,7 @@ public class FileServerTest {
     private FileServer createFileServer(ConfigserverConfig.Builder configBuilder) throws IOException {
         File fileReferencesDir = temporaryFolder.newFolder();
         configBuilder.fileReferencesDir(fileReferencesDir.getAbsolutePath());
-        return new FileServer(new ConfigserverConfig(configBuilder));
+        return new FileServer(new ConfigserverConfig(configBuilder), new InMemoryFlagSource());
     }
 
     private static class FileReceiver implements FileServer.Receiver {