From 88af4ae0b13faba664f9d5d5c77d37a6b6f09498 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Wed, 1 Feb 2023 13:20:10 +0100 Subject: Stop using file distribution feature flags, not needed anymore --- .../FileDistributionAndUrlDownload.java | 21 ++------------ .../config/server/filedistribution/FileServer.java | 33 ++++------------------ .../maintenance/ApplicationPackageMaintainer.java | 17 ++--------- .../server/filedistribution/FileServerTest.java | 9 ++---- .../vespa/config/server/rpc/MockRpcServer.java | 2 +- .../yahoo/vespa/config/server/rpc/RpcTester.java | 2 +- .../vespa/filedistribution/FileDownloader.java | 17 ++++------- .../filedistribution/FileReferenceDownloader.java | 15 +++++----- .../vespa/filedistribution/FileDownloaderTest.java | 18 +----------- 9 files changed, 29 insertions(+), 105 deletions(-) diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileDistributionAndUrlDownload.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileDistributionAndUrlDownload.java index 3107f9029d8..9e6a54d8e2c 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileDistributionAndUrlDownload.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileDistributionAndUrlDownload.java @@ -6,14 +6,6 @@ import com.yahoo.jrt.Supervisor; import com.yahoo.vespa.filedistribution.FileDistributionConnectionPool; import com.yahoo.vespa.filedistribution.FileDownloader; import java.time.Duration; -import java.util.Arrays; -import java.util.Set; -import java.util.stream.Collectors; - -import static com.yahoo.vespa.filedistribution.FileReferenceData.CompressionType; -import static com.yahoo.vespa.filedistribution.FileReferenceData.CompressionType.gzip; -import static com.yahoo.vespa.filedistribution.FileReferenceData.CompressionType.lz4; -import static com.yahoo.vespa.filedistribution.FileReferenceData.CompressionType.zstd; /** * Keeps track of file distribution and url download rpc servers. @@ -41,18 +33,9 @@ public class FileDistributionAndUrlDownload { private FileDownloader createDownloader(Supervisor supervisor, ConfigSourceSet source) { return new FileDownloader(new FileDistributionConnectionPool(source, supervisor), supervisor, - Duration.ofMinutes(5), - acceptedCompressionTypes()); + Duration.ofMinutes(5)); } - private Set acceptedCompressionTypes() { - Set acceptedCompressionTypes = Set.of(gzip, lz4, zstd); - String env = System.getenv("VESPA_FILE_DISTRIBUTION_ACCEPTED_COMPRESSION_TYPES"); - if (env != null && ! env.isEmpty()) { - String[] types = env.split(","); - acceptedCompressionTypes = Arrays.stream(types).map(CompressionType::valueOf).collect(Collectors.toSet()); - } - return acceptedCompressionTypes; - } + } 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 92e1101e398..57d57d16d2f 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 @@ -20,8 +20,6 @@ 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; import java.io.IOException; @@ -29,14 +27,12 @@ import java.nio.file.Files; import java.nio.file.Path; import java.time.Duration; import java.time.Instant; -import java.util.LinkedHashSet; import java.util.List; import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.stream.Collectors; import static com.yahoo.vespa.config.server.filedistribution.FileDistributionUtil.getOtherConfigServersInCluster; import static com.yahoo.vespa.filedistribution.FileReferenceData.CompressionType; @@ -50,6 +46,7 @@ public class FileServer { // Set this low, to make sure we don't wait for a long time trying to download file private static final Duration timeout = Duration.ofSeconds(10); + private static final List compressionTypesToServe = compressionTypesAsList(List.of("zstd", "lz4", "gzip")); // In preferred order private final FileDirectory fileDirectory; private final ExecutorService executor; @@ -89,16 +86,8 @@ public class FileServer { @SuppressWarnings("WeakerAccess") // Created by dependency injection @Inject - public FileServer(ConfigserverConfig configserverConfig, FlagSource flagSource, FileDirectory fileDirectory) { - this(createFileDownloader(getOtherConfigServersInCluster(configserverConfig), - compressionTypes(Flags.FILE_DISTRIBUTION_ACCEPTED_COMPRESSION_TYPES.bindTo(flagSource).value())), - compressionTypesAsList(Flags.FILE_DISTRIBUTION_COMPRESSION_TYPES_TO_SERVE.bindTo(flagSource).value()), - fileDirectory); - } - - // For testing only - public FileServer(FileDirectory fileDirectory) { - this(createFileDownloader(List.of(), Set.of(gzip)), List.of(gzip), fileDirectory); + public FileServer(ConfigserverConfig configserverConfig, FileDirectory fileDirectory) { + this(createFileDownloader(getOtherConfigServersInCluster(configserverConfig)), compressionTypesToServe, fileDirectory); } FileServer(FileDownloader fileDownloader, List compressionTypes, FileDirectory fileDirectory) { @@ -237,21 +226,9 @@ public class FileServer { executor.shutdown(); } - private static FileDownloader createFileDownloader(List configServers, Set acceptedCompressionTypes) { + private static FileDownloader createFileDownloader(List configServers) { Supervisor supervisor = new Supervisor(new Transport("filedistribution-pool")).setDropEmptyBuffers(true); - - return new FileDownloader(configServers.isEmpty() - ? FileDownloader.emptyConnectionPool() - : createConnectionPool(configServers, supervisor), - supervisor, - timeout, - acceptedCompressionTypes); - } - - private static LinkedHashSet compressionTypes(List compressionTypes) { - return compressionTypes.stream() - .map(CompressionType::valueOf) - .collect(Collectors.toCollection(LinkedHashSet::new)); + return new FileDownloader(createConnectionPool(configServers, supervisor), supervisor, timeout); } private static List compressionTypesAsList(List compressionTypes) { 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 346a462fe3e..2948b82dd96 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 @@ -17,17 +17,13 @@ import com.yahoo.vespa.filedistribution.FileDistributionConnectionPool; import com.yahoo.vespa.filedistribution.FileDownloader; import com.yahoo.vespa.filedistribution.FileReferenceDownload; import com.yahoo.vespa.flags.FlagSource; -import com.yahoo.vespa.flags.Flags; import java.io.File; import java.time.Duration; import java.util.List; import java.util.Optional; -import java.util.Set; import java.util.logging.Logger; -import java.util.stream.Collectors; import static com.yahoo.vespa.config.server.filedistribution.FileDistributionUtil.fileReferenceExistsOnDisk; -import static com.yahoo.vespa.filedistribution.FileReferenceData.CompressionType; /** * Verifies that all active sessions has an application package on local disk. @@ -54,10 +50,7 @@ public class ApplicationPackageMaintainer extends ConfigServerMaintainer { super(applicationRepository, curator, flagSource, applicationRepository.clock(), interval, false); this.applicationRepository = applicationRepository; this.downloadDirectory = new File(Defaults.getDefaults().underVespaHome(applicationRepository.configserverConfig().fileReferencesDir())); - this.fileDownloader = createFileDownloader(otherConfigServersInCluster, - downloadDirectory, - supervisor, - Flags.FILE_DISTRIBUTION_ACCEPTED_COMPRESSION_TYPES.bindTo(flagSource).value()); + this.fileDownloader = createFileDownloader(otherConfigServersInCluster, downloadDirectory, supervisor); } @Override @@ -100,14 +93,10 @@ public class ApplicationPackageMaintainer extends ConfigServerMaintainer { private static FileDownloader createFileDownloader(List otherConfigServersInCluster, File downloadDirectory, - Supervisor supervisor, - List flagValues) { + Supervisor supervisor) { ConfigSourceSet configSourceSet = new ConfigSourceSet(otherConfigServersInCluster); ConnectionPool connectionPool = new FileDistributionConnectionPool(configSourceSet, supervisor); - Set acceptedCompressionTypes = flagValues.stream() - .map(CompressionType::valueOf) - .collect(Collectors.toSet()); - return new FileDownloader(connectionPool, supervisor, downloadDirectory, Duration.ofSeconds(300), acceptedCompressionTypes); + return new FileDownloader(connectionPool, supervisor, downloadDirectory, Duration.ofSeconds(300)); } @Override 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 3c9ea238479..49458acd60b 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 @@ -11,7 +11,6 @@ import com.yahoo.vespa.filedistribution.FileDownloader; import com.yahoo.vespa.filedistribution.FileReferenceCompressor; 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; @@ -140,10 +139,7 @@ public class FileServerTest { private FileServer createFileServer(ConfigserverConfig.Builder configBuilder) throws IOException { File fileReferencesDir = temporaryFolder.newFolder(); configBuilder.fileReferencesDir(fileReferencesDir.getAbsolutePath()); - InMemoryFlagSource flagSource = new InMemoryFlagSource(); - return new FileServer(new ConfigserverConfig(configBuilder), - flagSource, - new FileDirectory(fileReferencesDir)); + return new FileServer(new ConfigserverConfig(configBuilder), new FileDirectory(fileReferencesDir)); } private static class FileReceiver implements FileServer.Receiver { @@ -168,8 +164,7 @@ public class FileServerTest { new Supervisor(new Transport("mock")).setDropEmptyBuffers(true), downloadDirectory, Duration.ofMillis(100), - Duration.ofMillis(100), - Set.of(gzip)); + Duration.ofMillis(100)); } } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRpcServer.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRpcServer.java index 0bf6012a668..c5e307f62f0 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRpcServer.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRpcServer.java @@ -37,7 +37,7 @@ public class MockRpcServer extends RpcServer { null, Metrics.createTestMetrics(), new HostRegistry(), - new FileServer(new FileDirectory(tempDir)), + new FileServer(createConfig(port), new FileDirectory(tempDir)), new NoopRpcAuthorizer(), new RpcRequestHandlerProvider()); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcTester.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcTester.java index e0b7909f032..b29edd480ad 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcTester.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcTester.java @@ -117,7 +117,7 @@ public class RpcTester implements AutoCloseable { flagSource)), Metrics.createTestMetrics(), hostRegistry, - new FileServer(new FileDirectory(temporaryFolder.newFolder())), + new FileServer(configserverConfig, new FileDirectory(temporaryFolder.newFolder())), new NoopRpcAuthorizer(), new RpcRequestHandlerProvider()); rpcServer.setUpGetConfigHandlers(); 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 63ae8faacfe..68f969df3af 100644 --- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDownloader.java +++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileDownloader.java @@ -10,7 +10,6 @@ import java.io.File; import java.time.Duration; import java.util.Map; import java.util.Optional; -import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; @@ -19,8 +18,6 @@ import java.util.concurrent.TimeoutException; import java.util.logging.Level; import java.util.logging.Logger; -import static com.yahoo.vespa.filedistribution.FileReferenceData.CompressionType; - /** * Handles downloads of files (file references only for now) * @@ -40,20 +37,19 @@ public class FileDownloader implements AutoCloseable { private final FileReferenceDownloader fileReferenceDownloader; private final Downloads downloads = new Downloads(); - public FileDownloader(ConnectionPool connectionPool, Supervisor supervisor, Duration timeout, Set acceptedCompressionTypes) { - this(connectionPool, supervisor, defaultDownloadDirectory, timeout, defaultSleepBetweenRetries, acceptedCompressionTypes); + public FileDownloader(ConnectionPool connectionPool, Supervisor supervisor, Duration timeout) { + this(connectionPool, supervisor, defaultDownloadDirectory, timeout, defaultSleepBetweenRetries); } - public FileDownloader(ConnectionPool connectionPool, Supervisor supervisor, File downloadDirectory, Duration timeout, Set acceptedCompressionTypes) { - this(connectionPool, supervisor, downloadDirectory, timeout, defaultSleepBetweenRetries, acceptedCompressionTypes); + public FileDownloader(ConnectionPool connectionPool, Supervisor supervisor, File downloadDirectory, Duration timeout) { + this(connectionPool, supervisor, downloadDirectory, timeout, defaultSleepBetweenRetries); } public FileDownloader(ConnectionPool connectionPool, Supervisor supervisor, File downloadDirectory, Duration timeout, - Duration sleepBetweenRetries, - Set acceptedCompressionTypes) { + Duration sleepBetweenRetries) { this.connectionPool = connectionPool; this.supervisor = supervisor; this.downloadDirectory = downloadDirectory; @@ -64,8 +60,7 @@ public class FileDownloader implements AutoCloseable { downloads, timeout, sleepBetweenRetries, - downloadDirectory, - acceptedCompressionTypes); + downloadDirectory); if (forceDownload) log.log(Level.INFO, "Force download of file references (download even if file reference exists on disk)"); } 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 f14de0b9807..0966561ce5d 100644 --- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceDownloader.java +++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceDownloader.java @@ -23,6 +23,9 @@ import java.util.logging.Level; import java.util.logging.Logger; import static com.yahoo.vespa.filedistribution.FileReferenceData.CompressionType; +import static com.yahoo.vespa.filedistribution.FileReferenceData.CompressionType.gzip; +import static com.yahoo.vespa.filedistribution.FileReferenceData.CompressionType.lz4; +import static com.yahoo.vespa.filedistribution.FileReferenceData.CompressionType.zstd; /** * Downloads file reference from config server and keeps track of files being downloaded @@ -31,7 +34,8 @@ import static com.yahoo.vespa.filedistribution.FileReferenceData.CompressionType */ public class FileReferenceDownloader { - private final static Logger log = Logger.getLogger(FileReferenceDownloader.class.getName()); + private static final Logger log = Logger.getLogger(FileReferenceDownloader.class.getName()); + private static final Set defaultAcceptedCompressionTypes = Set.of(gzip, lz4, zstd); private final ExecutorService downloadExecutor = Executors.newFixedThreadPool(Math.max(8, Runtime.getRuntime().availableProcessors()), @@ -42,14 +46,12 @@ public class FileReferenceDownloader { private final Duration sleepBetweenRetries; private final Duration rpcTimeout; private final File downloadDirectory; - private final Set acceptedCompressionTypes; FileReferenceDownloader(ConnectionPool connectionPool, Downloads downloads, Duration timeout, Duration sleepBetweenRetries, - File downloadDirectory, - Set acceptedCompressionTypes) { + File downloadDirectory) { this.connectionPool = connectionPool; this.downloads = downloads; this.downloadTimeout = timeout; @@ -57,7 +59,6 @@ public class FileReferenceDownloader { this.downloadDirectory = downloadDirectory; String timeoutString = System.getenv("VESPA_CONFIGPROXY_FILEDOWNLOAD_RPC_TIMEOUT"); this.rpcTimeout = Duration.ofSeconds(timeoutString == null ? 30 : Integer.parseInt(timeoutString)); - this.acceptedCompressionTypes = requireNonEmpty(acceptedCompressionTypes); } private void waitUntilDownloadStarted(FileReferenceDownload fileReferenceDownload) { @@ -139,8 +140,8 @@ public class FileReferenceDownloader { Request request = new Request("filedistribution.serveFile"); request.parameters().add(new StringValue(fileReferenceDownload.fileReference().value())); request.parameters().add(new Int32Value(fileReferenceDownload.downloadFromOtherSourceIfNotFound() ? 0 : 1)); - String[] temp = new String[acceptedCompressionTypes.size()]; - acceptedCompressionTypes.stream().map(Enum::name).toList().toArray(temp); + String[] temp = new String[defaultAcceptedCompressionTypes.size()]; + defaultAcceptedCompressionTypes.stream().map(Enum::name).toList().toArray(temp); request.parameters().add(new StringArray(temp)); return request; } 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 ffef06e6367..d593a783064 100644 --- a/filedistribution/src/test/java/com/yahoo/vespa/filedistribution/FileDownloaderTest.java +++ b/filedistribution/src/test/java/com/yahoo/vespa/filedistribution/FileDownloaderTest.java @@ -25,7 +25,6 @@ import java.nio.file.Path; import java.time.Duration; import java.util.Arrays; import java.util.Optional; -import java.util.Set; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -41,7 +40,6 @@ import static org.junit.Assert.fail; public class FileDownloaderTest { private static final Duration sleepBetweenRetries = Duration.ofMillis(10); - private static final Set acceptedCompressionTypes = Set.of(gzip); private MockConnection connection; private FileDownloader fileDownloader; @@ -265,16 +263,6 @@ public class FileDownloaderTest { assertEquals("content", IOUtils.readFile(downloadedFile)); } - @Test - public void testCompressionTypes() { - try { - createDownloader(connection, Duration.ofSeconds(1), Set.of()); - fail("expected to fail when set is empty"); - } catch (IllegalArgumentException e) { - // ignore - } - } - private void writeFileReference(File dir, String fileReferenceString, String fileName) throws IOException { File fileReferenceDir = new File(dir, fileReferenceString); fileReferenceDir.mkdir(); @@ -314,11 +302,7 @@ public class FileDownloaderTest { } private FileDownloader createDownloader(MockConnection connection, Duration timeout) { - return createDownloader(connection, timeout, acceptedCompressionTypes); - } - - private FileDownloader createDownloader(MockConnection connection, Duration timeout, Set acceptedCompressionTypes) { - return new FileDownloader(connection, supervisor, downloadDir, timeout, sleepBetweenRetries, acceptedCompressionTypes); + return new FileDownloader(connection, supervisor, downloadDir, timeout, sleepBetweenRetries); } private static class MockConnection implements ConnectionPool, com.yahoo.vespa.config.Connection { -- cgit v1.2.3