From 2a66e8e2410e7ef0f54e9cf2f11e7b73810f9d82 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Thu, 22 Aug 2024 11:00:22 +0200 Subject: [PATCH 1/2] Move FileApiErrorCodes into filedistribution module --- .../server/filedistribution/FileServer.java | 37 +++++------------- .../filedistribution/FileApiErrorCodes.java | 39 +++++++++++++++++++ .../FileReferenceDownloader.java | 9 +++-- 3 files changed, 54 insertions(+), 31 deletions(-) create mode 100644 filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileApiErrorCodes.java 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 88fff35fc7bf..683604adfd1e 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 @@ -12,6 +12,7 @@ import com.yahoo.jrt.Supervisor; import com.yahoo.jrt.Transport; import com.yahoo.vespa.config.ConnectionPool; +import com.yahoo.vespa.filedistribution.FileApiErrorCodes; import com.yahoo.vespa.filedistribution.FileDistributionConnectionPool; import com.yahoo.vespa.filedistribution.FileDownloader; import com.yahoo.vespa.filedistribution.FileReferenceCompressor; @@ -36,11 +37,13 @@ import java.util.logging.Logger; 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.TRANSFER_FAILED; +import static com.yahoo.vespa.filedistribution.FileApiErrorCodes.NOT_FOUND; +import static com.yahoo.vespa.filedistribution.FileApiErrorCodes.OK; +import static com.yahoo.vespa.filedistribution.FileApiErrorCodes.TRANSFER_FAILED; 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; import static com.yahoo.vespa.filedistribution.FileReferenceData.Type; import static com.yahoo.vespa.filedistribution.FileReferenceData.Type.compressed; import static com.yahoo.yolean.Exceptions.uncheck; @@ -51,7 +54,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 static final List compressionTypesToServe = List.of(zstd, lz4, gzip); // In preferred order private static final String tempFilereferencedataPrefix = "filereferencedata"; private static final Path tempFilereferencedataDir = Paths.get(System.getProperty("java.io.tmpdir")); @@ -60,22 +63,6 @@ public class FileServer { private final FileDownloader downloader; private final List compressionTypes; // compression types to use, in preferred order - // TODO: Move to filedistribution module, so that it can be used by both clients and servers - enum FileApiErrorCodes { - OK(0, "OK"), - NOT_FOUND(1, "File reference not found"), - TIMEOUT(2, "Timeout"), - TRANSFER_FAILED(3, "Failed transferring file"); - private final int code; - private final String description; - FileApiErrorCodes(int code, String description) { - this.code = code; - this.description = description; - } - int getCode() { return code; } - String getDescription() { return description; } - } - public static class ReplayStatus { private final int code; private final String description; @@ -165,8 +152,8 @@ public void serveFile(FileReference fileReference, executor.execute(() -> { var result = serveFileInternal(fileReference, downloadFromOtherSourceIfNotFound, client, receiver, acceptedCompressionTypes); request.returnValues() - .add(new Int32Value(result.getCode())) - .add(new StringValue(result.getDescription())); + .add(new Int32Value(result.code())) + .add(new StringValue(result.description())); request.returnRequest(); }); } @@ -235,12 +222,6 @@ private static FileDownloader createFileDownloader(List configServers) { return new FileDownloader(createConnectionPool(configServers, supervisor), supervisor, timeout); } - private static List compressionTypesAsList(List compressionTypes) { - return compressionTypes.stream() - .map(CompressionType::valueOf) - .toList(); - } - private static ConnectionPool createConnectionPool(List configServers, Supervisor supervisor) { if (configServers.isEmpty()) return FileDownloader.emptyConnectionPool(); diff --git a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileApiErrorCodes.java b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileApiErrorCodes.java new file mode 100644 index 000000000000..76deefa0c266 --- /dev/null +++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileApiErrorCodes.java @@ -0,0 +1,39 @@ +// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.filedistribution; + +/** + * Error codes used in RPC responses for file distribution + * + * @author hmusum + */ +public enum FileApiErrorCodes { + + OK(0, "OK"), + NOT_FOUND(1, "File reference not found"), + TIMEOUT(2, "Timeout"), + TRANSFER_FAILED(3, "Failed transferring file"); + private final int code; + private final String description; + + FileApiErrorCodes(int code, String description) { + this.code = code; + this.description = description; + } + + static FileApiErrorCodes get(int code) { + for (FileApiErrorCodes error : FileApiErrorCodes.values()) { + if (error.code() == code) { + return error; + } + } + return null; + } + + public int code() { return code; } + + public String description() { return description; } + + @Override + public String toString() { return code + ": " + description; } + +} 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 00b62390ef1a..14ca38a52ebd 100644 --- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceDownloader.java +++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceDownloader.java @@ -151,18 +151,21 @@ private boolean startDownloadRpc(FileReferenceDownload fileReferenceDownload, in Level logLevel = (retryCount > 3 ? Level.INFO : Level.FINE); FileReference fileReference = fileReferenceDownload.fileReference(); + String address = connection.getAddress(); if (validateResponse(request)) { log.log(Level.FINE, () -> "Request callback, OK. Req: " + request + "\nSpec: " + connection); int errorCode = request.returnValues().get(0).asInt32(); + if (errorCode == 0) { - log.log(Level.FINE, () -> "Found " + fileReference + " available at " + connection.getAddress()); + log.log(Level.FINE, () -> "Found " + fileReference + " available at " + address); return true; } else { - log.log(logLevel, fileReference + " not found or timed out (error code " + errorCode + ") at " + connection.getAddress()); + var error = FileApiErrorCodes.get(errorCode); + log.log(logLevel, "Downloading " + fileReference + " from " + address + " failed (" + error + ")"); return false; } } else { - log.log(logLevel, "Downloading " + fileReference + " from " + connection.getAddress() + " failed:" + + log.log(logLevel, "Downloading " + fileReference + " from " + address + " failed:" + " error code " + request.errorCode() + " (" + request.errorMessage() + ")." + " (retry " + retryCount + ", rpc timeout " + rpcTimeout + ")"); return false; From 931e0222d98f6bc6412e48081011fa8844240958 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Thu, 22 Aug 2024 12:46:10 +0200 Subject: [PATCH 2/2] Remove unused exception --- .../yahoo/vespa/config/ConfigPayloadApplier.java | 2 -- .../FileReferenceDoesNotExistException.java | 16 ---------------- fileacquirer/abi-spec.json | 1 - .../fileacquirer/FileAcquirer.java | 3 --- .../fileacquirer/FileAcquirerImpl.java | 2 -- .../fileacquirer/MockFileAcquirer.java | 11 ----------- .../src/test/java/MockFileAcquirerTest.java | 6 ------ 7 files changed, 41 deletions(-) delete mode 100644 config/src/main/java/com/yahoo/vespa/config/FileReferenceDoesNotExistException.java diff --git a/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadApplier.java b/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadApplier.java index 06eb84cb03de..f12796d73f2e 100644 --- a/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadApplier.java +++ b/config/src/main/java/com/yahoo/vespa/config/ConfigPayloadApplier.java @@ -60,8 +60,6 @@ public void applyPayload(ConfigPayload payload) { stack.push(new NamedBuilder(rootBuilder)); try { handleValue(payload.getSlime().get()); - } catch (FileReferenceDoesNotExistException e) { - throw e; } catch (Exception e) { throw new RuntimeException("Not able to create config builder for payload '" + payload.toString() + "'", e); } diff --git a/config/src/main/java/com/yahoo/vespa/config/FileReferenceDoesNotExistException.java b/config/src/main/java/com/yahoo/vespa/config/FileReferenceDoesNotExistException.java deleted file mode 100644 index c26dcaccc2e5..000000000000 --- a/config/src/main/java/com/yahoo/vespa/config/FileReferenceDoesNotExistException.java +++ /dev/null @@ -1,16 +0,0 @@ -// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config; - -/** - * @author Tony Vaagenes - */ -public class FileReferenceDoesNotExistException extends RuntimeException { - - public final String fileReference; - - public FileReferenceDoesNotExistException(String fileReference) { - super("Could not retrieve file with file reference '" + fileReference + "'"); - this.fileReference = fileReference; - } - -} diff --git a/fileacquirer/abi-spec.json b/fileacquirer/abi-spec.json index caec5f8629c2..d633e419fb42 100644 --- a/fileacquirer/abi-spec.json +++ b/fileacquirer/abi-spec.json @@ -39,7 +39,6 @@ "public static com.yahoo.filedistribution.fileacquirer.FileAcquirer returnFile(java.io.File)", "public static com.yahoo.filedistribution.fileacquirer.FileAcquirer returnFiles(java.util.Map)", "public static com.yahoo.filedistribution.fileacquirer.FileAcquirer throwTimeoutException()", - "public static com.yahoo.filedistribution.fileacquirer.FileAcquirer throwFileReferenceDoesNotExistException()", "public void shutdown()" ], "fields" : [ ] diff --git a/fileacquirer/src/main/java/com/yahoo/filedistribution/fileacquirer/FileAcquirer.java b/fileacquirer/src/main/java/com/yahoo/filedistribution/fileacquirer/FileAcquirer.java index 3fe81aff5d25..e93822465a00 100644 --- a/fileacquirer/src/main/java/com/yahoo/filedistribution/fileacquirer/FileAcquirer.java +++ b/fileacquirer/src/main/java/com/yahoo/filedistribution/fileacquirer/FileAcquirer.java @@ -2,7 +2,6 @@ package com.yahoo.filedistribution.fileacquirer; import com.yahoo.config.FileReference; -import com.yahoo.vespa.config.FileReferenceDoesNotExistException; import java.io.File; import java.util.concurrent.TimeUnit; @@ -22,8 +21,6 @@ public interface FileAcquirer { * config system. * * @throws TimeoutException if the file or directory could not be retrieved in time. - * @throws FileReferenceDoesNotExistException if the file is no - * longer available(due to reloading of config). */ File waitFor(FileReference fileReference, long timeout, TimeUnit timeUnit) throws InterruptedException; diff --git a/fileacquirer/src/main/java/com/yahoo/filedistribution/fileacquirer/FileAcquirerImpl.java b/fileacquirer/src/main/java/com/yahoo/filedistribution/fileacquirer/FileAcquirerImpl.java index e28a9e9c5d22..ca0b5c33e859 100644 --- a/fileacquirer/src/main/java/com/yahoo/filedistribution/fileacquirer/FileAcquirerImpl.java +++ b/fileacquirer/src/main/java/com/yahoo/filedistribution/fileacquirer/FileAcquirerImpl.java @@ -8,7 +8,6 @@ import com.yahoo.jrt.Supervisor; import com.yahoo.jrt.Target; import com.yahoo.jrt.Transport; -import com.yahoo.vespa.config.FileReferenceDoesNotExistException; import java.io.File; import java.time.Duration; @@ -133,7 +132,6 @@ public void shutdown() { * config system. * * @throws TimeoutException if the file or directory could not be retrieved in time. - * @throws FileReferenceDoesNotExistException if the file is no longer available (due to reloading of config). */ public File waitFor(FileReference fileReference, long timeout, TimeUnit timeUnit) throws InterruptedException { Timer timer = new Timer(timeout, timeUnit); diff --git a/fileacquirer/src/main/java/com/yahoo/filedistribution/fileacquirer/MockFileAcquirer.java b/fileacquirer/src/main/java/com/yahoo/filedistribution/fileacquirer/MockFileAcquirer.java index fdccaa9824e3..79eb8eef463e 100644 --- a/fileacquirer/src/main/java/com/yahoo/filedistribution/fileacquirer/MockFileAcquirer.java +++ b/fileacquirer/src/main/java/com/yahoo/filedistribution/fileacquirer/MockFileAcquirer.java @@ -2,7 +2,6 @@ package com.yahoo.filedistribution.fileacquirer; import com.yahoo.config.FileReference; -import com.yahoo.vespa.config.FileReferenceDoesNotExistException; import java.io.File; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -43,16 +42,6 @@ public File waitFor(FileReference fileReference, long timeout, TimeUnit timeUnit }; } - /** Creates a FileAcquirer that throws FileReferenceDoesNotExistException **/ - public static FileAcquirer throwFileReferenceDoesNotExistException() { - return new MockFileAcquirer() { - @Override - public File waitFor(FileReference fileReference, long timeout, TimeUnit timeUnit) { - throw new FileReferenceDoesNotExistException(null); - } - }; - } - @Override public void shutdown() {} diff --git a/fileacquirer/src/test/java/MockFileAcquirerTest.java b/fileacquirer/src/test/java/MockFileAcquirerTest.java index f7f56f79a2e9..85c4e7e4a6e4 100644 --- a/fileacquirer/src/test/java/MockFileAcquirerTest.java +++ b/fileacquirer/src/test/java/MockFileAcquirerTest.java @@ -4,7 +4,6 @@ import com.yahoo.filedistribution.fileacquirer.FileAcquirer; import com.yahoo.filedistribution.fileacquirer.MockFileAcquirer; import com.yahoo.filedistribution.fileacquirer.TimeoutException; -import com.yahoo.vespa.config.FileReferenceDoesNotExistException; import org.junit.Test; import java.io.File; import java.lang.reflect.Constructor; @@ -46,11 +45,6 @@ public void testThrowTimeoutException() throws Exception { waitFor(MockFileAcquirer.throwTimeoutException()); } - @Test(expected = FileReferenceDoesNotExistException.class) - public void testThrowFileReferenceDoesNotExistException() throws Exception { - waitFor(MockFileAcquirer.throwFileReferenceDoesNotExistException()); - } - private File waitFor(FileAcquirer fileAcquirer) throws InterruptedException { return waitFor(fileAcquirer, null); }