Merge pull request #32167 from vespa-engine/hmusum/require-compressio…

…n-types-in-request Hmusum/require compression types in request
vespa-engine · Aug 19, 2024 · 62c1280 · 62c1280
2 parents 2b97309 + 2ffdabd
commit 62c1280
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 38 deletions.
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
@@ -27,7 +27,6 @@
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.time.Duration;
-import java.time.Instant;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
@@ -39,7 +38,6 @@
 import static com.yahoo.vespa.config.server.filedistribution.FileDistributionUtil.getOtherConfigServersInCluster;
 import static com.yahoo.vespa.config.server.filedistribution.FileServer.FileApiErrorCodes.NOT_FOUND;
 import static com.yahoo.vespa.config.server.filedistribution.FileServer.FileApiErrorCodes.OK;
-import static com.yahoo.vespa.config.server.filedistribution.FileServer.FileApiErrorCodes.TIMEOUT;
 import static com.yahoo.vespa.config.server.filedistribution.FileServer.FileApiErrorCodes.TRANSFER_FAILED;
 import static com.yahoo.vespa.filedistribution.FileReferenceData.CompressionType;
 import static com.yahoo.vespa.filedistribution.FileReferenceData.CompressionType.gzip;
@@ -160,12 +158,12 @@ private FileReferenceData fileReferenceData(FileReference reference,
     public void serveFile(FileReference fileReference,
                           boolean downloadFromOtherSourceIfNotFound,
                           Set<CompressionType> acceptedCompressionTypes,
-                          Request request, Receiver receiver) {
+                          Request request,
+                          Receiver receiver) {
         log.log(Level.FINE, () -> "Received request for file reference '" + fileReference + "' from " + request.target());
-        Instant deadline = Instant.now().plus(timeout);
         String client = request.target().toString();
         executor.execute(() -> {
-            var result = serveFileInternal(fileReference, downloadFromOtherSourceIfNotFound, client, receiver, deadline, acceptedCompressionTypes);
+            var result = serveFileInternal(fileReference, downloadFromOtherSourceIfNotFound, client, receiver, acceptedCompressionTypes);
             request.returnValues()
                    .add(new Int32Value(result.getCode()))
                    .add(new StringValue(result.getDescription()));
@@ -177,13 +175,7 @@ private FileApiErrorCodes serveFileInternal(FileReference fileReference,
                                                 boolean downloadFromOtherSourceIfNotFound,
                                                 String client,
                                                 Receiver receiver,
-                                                Instant deadline,
                                                 Set<CompressionType> acceptedCompressionTypes) {
-        if (Instant.now().isAfter(deadline)) {
-            log.log(Level.INFO, () -> "Deadline exceeded for request for file reference '" + fileReference + "' from " + client);
-            return TIMEOUT;
-        }
-
         try {
             var fileReferenceDownload = new FileReferenceDownload(fileReference, client, downloadFromOtherSourceIfNotFound);
             var file = getFileDownloadIfNeeded(fileReferenceDownload);
@@ -250,10 +242,9 @@ private static List<CompressionType> compressionTypesAsList(List<String> compres
     }
 
     private static ConnectionPool createConnectionPool(List<String> configServers, Supervisor supervisor) {
-        ConfigSourceSet configSourceSet = new ConfigSourceSet(configServers);
-        if (configServers.size() == 0) return FileDownloader.emptyConnectionPool();
+        if (configServers.isEmpty()) return FileDownloader.emptyConnectionPool();
 
-        return new FileDistributionConnectionPool(configSourceSet, supervisor);
+        return new FileDistributionConnectionPool(new ConfigSourceSet(configServers), supervisor);
     }
 
 }
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
@@ -51,7 +51,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
-import java.util.Set;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.CompletionService;
 import java.util.concurrent.ConcurrentHashMap;
@@ -499,10 +498,7 @@ static Request createMetaRequest(FileReferenceData fileData) {
             request.parameters().add(new StringValue(fileData.filename()));
             request.parameters().add(new StringValue(fileData.type().name()));
             request.parameters().add(new Int64Value(fileData.size()));
-            // Only add parameter if not gzip, this is default and old clients will not handle the extra parameter
-            // TODO Always add parameter in Vespa 9
-            if (fileData.compressionType() != CompressionType.gzip)
-                request.parameters().add(new StringValue(fileData.compressionType().name()));
+            request.parameters().add(new StringValue(fileData.compressionType().name()));
             return request;
         }
 
@@ -555,18 +551,12 @@ private void serveFile(Request request) {
         request.detach();
         rpcAuthorizer.authorizeFileRequest(request)
                 .thenRun(() -> { // okay to do in authorizer thread as serveFile is async
-                    FileServer.Receiver receiver = new ChunkedFileReceiver(request.target());
-
                     FileReference reference = new FileReference(request.parameters().get(0).asString());
                     boolean downloadFromOtherSourceIfNotFound = request.parameters().get(1).asInt32() == 0;
-                    Set<FileReferenceData.CompressionType> acceptedCompressionTypes = Set.of(CompressionType.gzip);
-                    // Newer clients specify accepted compression types in request
-                    // TODO Require acceptedCompressionTypes parameter in Vespa 9
-                    if (request.parameters().size() > 2)
-                        acceptedCompressionTypes = Arrays.stream(request.parameters().get(2).asStringArray())
-                                                         .map(CompressionType::valueOf)
-                                                         .collect(Collectors.toSet());
-
+                    var acceptedCompressionTypes = Arrays.stream(request.parameters().get(2).asStringArray())
+                            .map(CompressionType::valueOf)
+                            .collect(Collectors.toSet());
+                    var receiver = new ChunkedFileReceiver(request.target());
                     fileServer.serveFile(reference, downloadFromOtherSourceIfNotFound, acceptedCompressionTypes, request, receiver);
                 });
     }

diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcServerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcServerTest.java
@@ -40,7 +40,6 @@
 
 import static com.yahoo.vespa.config.server.rpc.RpcServer.ChunkedFileReceiver.createMetaRequest;
 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.Type.compressed;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -106,19 +105,12 @@ public void testEmptySentinelConfigWhenAppDeletedOnHostedVespa() throws IOExcept
     public void testFileReceiverMetaRequest() throws IOException {
         File file = temporaryFolder.newFile();
         Request request = createMetaRequest(new LazyFileReferenceData(new FileReference("foo"), "fileA", compressed, file, gzip));
-        assertEquals(4, request.parameters().size());
-        assertEquals("foo", request.parameters().get(0).asString());
-        assertEquals("fileA", request.parameters().get(1).asString());
-        assertEquals("compressed", request.parameters().get(2).asString());
-        assertEquals(0, request.parameters().get(3).asInt64());
-
-        request = createMetaRequest(new LazyFileReferenceData(new FileReference("foo"), "fileA", compressed, file, lz4));
         assertEquals(5, request.parameters().size());
         assertEquals("foo", request.parameters().get(0).asString());
         assertEquals("fileA", request.parameters().get(1).asString());
         assertEquals("compressed", request.parameters().get(2).asString());
         assertEquals(0, request.parameters().get(3).asInt64());
-        assertEquals("lz4", request.parameters().get(4).asString());
+        assertEquals("gzip", request.parameters().get(4).asString());
     }
 
     private JRTClientConfigRequest createSimpleRequest() {