diff options
25 files changed, 353 insertions, 269 deletions
diff --git a/client/go/internal/cli/cmd/cert.go b/client/go/internal/cli/cmd/cert.go index f7320e37626..7fbb357d1db 100644 --- a/client/go/internal/cli/cmd/cert.go +++ b/client/go/internal/cli/cmd/cert.go @@ -156,9 +156,16 @@ func doCertAdd(cli *CLI, overwriteCertificate bool, args []string) error { } func maybeCopyCertificate(force, ignoreZip bool, cli *CLI, target vespa.Target, pkg vespa.ApplicationPackage) error { - if pkg.IsZip() && !ignoreZip { - hint := "Try running 'mvn clean', then 'vespa auth cert add' and finally 'mvn package'" - return errHint(fmt.Errorf("cannot add certificate to compressed application package: %s", pkg.Path), hint) + if pkg.IsZip() { + if ignoreZip { + cli.printWarning("Cannot verify existence of "+color.CyanString("security/clients.pem")+" since "+pkg.Path+" is compressed", + "Deployment to Vespa Cloud requires certificate in application package", + "See https://cloud.vespa.ai/en/security/guide") + return nil + } else { + hint := "Try running 'mvn clean', then 'vespa auth cert add' and finally 'mvn package'" + return errHint(fmt.Errorf("cannot add certificate to compressed application package: %s", pkg.Path), hint) + } } if force { return copyCertificate(cli, target, pkg) diff --git a/client/go/internal/vespa/application.go b/client/go/internal/vespa/application.go index b31dde54d67..b6b5b9427b3 100644 --- a/client/go/internal/vespa/application.go +++ b/client/go/internal/vespa/application.go @@ -216,17 +216,28 @@ func copyFile(src *zip.File, dst string) error { // FindApplicationPackage finds the path to an application package from the zip file or directory zipOrDir. If // requirePackaging is true, the application package is required to be packaged with mvn package. +// +// Package to use is preferred in this order: +// 1. Given path, if it's a zip +// 2. target/application +// 3. target/application.zip +// 4. src/main/application +// 5. Given path, if it contains services.xml func FindApplicationPackage(zipOrDir string, requirePackaging bool) (ApplicationPackage, error) { if isZip(zipOrDir) { return ApplicationPackage{Path: zipOrDir}, nil } - if util.PathExists(filepath.Join(zipOrDir, "pom.xml")) { - zip := filepath.Join(zipOrDir, "target", "application.zip") - if util.PathExists(zip) { + // Prefer uncompressed application because this allows us to add security/clients.pem to the package on-demand + if path := filepath.Join(zipOrDir, "target", "application"); util.PathExists(path) { + return ApplicationPackage{Path: path}, nil + } + appZip := filepath.Join(zipOrDir, "target", "application.zip") + if util.PathExists(filepath.Join(zipOrDir, "pom.xml")) || util.PathExists(appZip) { + if util.PathExists(appZip) { if testZip := filepath.Join(zipOrDir, "target", "application-test.zip"); util.PathExists(testZip) { - return ApplicationPackage{Path: zip, TestPath: testZip}, nil + return ApplicationPackage{Path: appZip, TestPath: testZip}, nil } - return ApplicationPackage{Path: zip}, nil + return ApplicationPackage{Path: appZip}, nil } if requirePackaging { return ApplicationPackage{}, errors.New("found pom.xml, but target/application.zip does not exist: run 'mvn package' first") diff --git a/client/go/internal/vespa/deploy_test.go b/client/go/internal/vespa/deploy_test.go index 39a9f2bcdf2..c68ad750f1a 100644 --- a/client/go/internal/vespa/deploy_test.go +++ b/client/go/internal/vespa/deploy_test.go @@ -131,6 +131,11 @@ func TestFindApplicationPackage(t *testing.T) { existingFile: filepath.Join(dir, "services.xml"), }) assertFindApplicationPackage(t, dir, pkgFixture{ + expectedPath: dir, + expectedTestPath: dir, + existingFiles: []string{filepath.Join(dir, "services.xml"), filepath.Join(dir, "tests", "foo.json")}, + }) + assertFindApplicationPackage(t, dir, pkgFixture{ expectedPath: filepath.Join(dir, "src", "main", "application"), existingFile: filepath.Join(dir, "src", "main", "application") + string(os.PathSeparator), }) @@ -149,11 +154,17 @@ func TestFindApplicationPackage(t *testing.T) { existingFiles: []string{filepath.Join(dir, "pom.xml"), filepath.Join(dir, "target", "application.zip")}, requirePackaging: true, }) - dir2 := t.TempDir() - assertFindApplicationPackage(t, dir2, pkgFixture{ - expectedPath: dir2, - expectedTestPath: dir2, - existingFiles: []string{filepath.Join(dir2, "services.xml"), filepath.Join(dir2, "tests", "foo.json")}, + assertFindApplicationPackage(t, dir, pkgFixture{ + expectedPath: filepath.Join(dir, "target", "application.zip"), + existingFiles: []string{filepath.Join(dir, "target", "application.zip")}, + }) + assertFindApplicationPackage(t, dir, pkgFixture{ + expectedPath: filepath.Join(dir, "target", "application"), + existingFiles: []string{filepath.Join(dir, "target", "application"), filepath.Join(dir, "target", "application.zip")}, + }) + zip := filepath.Join(dir, "myapp.zip") + assertFindApplicationPackage(t, zip, pkgFixture{ + expectedPath: zip, }) } diff --git a/config-model/src/main/java/com/yahoo/schema/Schema.java b/config-model/src/main/java/com/yahoo/schema/Schema.java index 93bec4975a6..36730a502ea 100644 --- a/config-model/src/main/java/com/yahoo/schema/Schema.java +++ b/config-model/src/main/java/com/yahoo/schema/Schema.java @@ -319,16 +319,12 @@ public class Schema implements ImmutableSchema { return null; } - /** - * @return true if the document has been added. - */ + /** Returns true if the document has been added. */ public boolean hasDocument() { return documentType != null; } - /** - * @return The document in this search. - */ + /** Returns the document in this search. */ @Override public SDDocumentType getDocument() { return documentType; @@ -384,7 +380,7 @@ public class Schema implements ImmutableSchema { } /** - * Returns a field defined in one of the documents of this search definition. + * Returns a field defined in one of the documents of this schema. * This does not include the extra fields defined outside the document * (those accessible through the getExtraField() method). * diff --git a/config-model/src/main/java/com/yahoo/schema/document/ImmutableSDField.java b/config-model/src/main/java/com/yahoo/schema/document/ImmutableSDField.java index 2d826e164b7..4c7e7eb28f4 100644 --- a/config-model/src/main/java/com/yahoo/schema/document/ImmutableSDField.java +++ b/config-model/src/main/java/com/yahoo/schema/document/ImmutableSDField.java @@ -101,4 +101,5 @@ public interface ImmutableSDField { boolean existsIndex(String name); SummaryField getSummaryField(String name); boolean hasIndex(); + } diff --git a/config-model/src/main/java/com/yahoo/schema/processing/IndexingInputs.java b/config-model/src/main/java/com/yahoo/schema/processing/IndexingInputs.java index 88e84d5289f..985ec8653c7 100644 --- a/config-model/src/main/java/com/yahoo/schema/processing/IndexingInputs.java +++ b/config-model/src/main/java/com/yahoo/schema/processing/IndexingInputs.java @@ -96,11 +96,11 @@ public class IndexingInputs extends Processor { @Override protected void doVisit(Expression exp) { if ( ! (exp instanceof InputExpression)) return; - String inputField = ((InputExpression)exp).getFieldName(); - if (schema.getField(inputField).hasFullIndexingDocprocRights()) return; - - fail(schema, field, "Indexing script refers to field '" + inputField + "' which does not exist " + - "in document type '" + schema.getDocument().getName() + "', and is not a mutable attribute."); + var referencedFieldName = ((InputExpression)exp).getFieldName(); + var referencedField = schema.getField(referencedFieldName); + if (referencedField == null || ! referencedField.hasFullIndexingDocprocRights()) + fail(schema, field, "Indexing script refers to field '" + referencedFieldName + + "' which is neither a field in " + schema.getDocument() + " nor a mutable attribute"); } } } diff --git a/config-model/src/test/java/com/yahoo/schema/processing/IndexingInputsTestCase.java b/config-model/src/test/java/com/yahoo/schema/processing/IndexingInputsTestCase.java index 893ee3b1ea4..d420623f233 100644 --- a/config-model/src/test/java/com/yahoo/schema/processing/IndexingInputsTestCase.java +++ b/config-model/src/test/java/com/yahoo/schema/processing/IndexingInputsTestCase.java @@ -17,29 +17,29 @@ public class IndexingInputsTestCase { void requireThatExtraFieldInputExtraFieldThrows() throws IOException, ParseException { assertBuildFails("src/test/examples/indexing_extra_field_input_extra_field.sd", "For schema 'indexing_extra_field_input_extra_field', field 'bar': Indexing script refers " + - "to field 'bar' which does not exist in document type " + - "'indexing_extra_field_input_extra_field', and is not a mutable attribute."); + "to field 'bar' which is neither a field in document type " + + "'indexing_extra_field_input_extra_field' nor a mutable attribute"); } @Test void requireThatExtraFieldInputImplicitThrows() throws IOException, ParseException { assertBuildFails("src/test/examples/indexing_extra_field_input_implicit.sd", "For schema 'indexing_extra_field_input_implicit', field 'foo': Indexing script refers to " + - "field 'foo' which does not exist in document type 'indexing_extra_field_input_implicit', and is not a mutable attribute."); + "field 'foo' which is neither a field in document type 'indexing_extra_field_input_implicit' nor a mutable attribute"); } @Test void requireThatExtraFieldInputNullThrows() throws IOException, ParseException { assertBuildFails("src/test/examples/indexing_extra_field_input_null.sd", "For schema 'indexing_extra_field_input_null', field 'foo': Indexing script refers to field " + - "'foo' which does not exist in document type 'indexing_extra_field_input_null', and is not a mutable attribute."); + "'foo' which is neither a field in document type 'indexing_extra_field_input_null' nor a mutable attribute"); } @Test void requireThatExtraFieldInputSelfThrows() throws IOException, ParseException { assertBuildFails("src/test/examples/indexing_extra_field_input_self.sd", "For schema 'indexing_extra_field_input_self', field 'foo': Indexing script refers to field " + - "'foo' which does not exist in document type 'indexing_extra_field_input_self', and is not a mutable attribute."); + "'foo' which is neither a field in document type 'indexing_extra_field_input_self' nor a mutable attribute"); } } diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileReferencesAndDownloadsMaintainer.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileReferencesAndDownloadsMaintainer.java index 627a15aab65..eab1368a2a1 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileReferencesAndDownloadsMaintainer.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileReferencesAndDownloadsMaintainer.java @@ -3,24 +3,29 @@ package com.yahoo.vespa.config.proxy.filedistribution; import com.yahoo.concurrent.DaemonThreadFactory; import com.yahoo.io.IOUtils; +import com.yahoo.vespa.config.util.ConfigUtils; import com.yahoo.vespa.filedistribution.FileDownloader; + import java.io.File; import java.io.IOException; import java.io.UncheckedIOException; +import java.nio.file.Path; import java.nio.file.attribute.BasicFileAttributes; import java.time.Duration; import java.time.Instant; -import java.util.Arrays; +import java.util.Comparator; import java.util.HashSet; +import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.stream.Collectors; import static java.nio.file.Files.readAttributes; +import static java.util.logging.Level.INFO; /** * Deletes file references and url downloads that have not been used for some time. @@ -35,27 +40,43 @@ class FileReferencesAndDownloadsMaintainer implements Runnable { private static final File defaultUrlDownloadDir = UrlDownloadRpcServer.downloadDir; private static final File defaultFileReferencesDownloadDir = FileDownloader.defaultDownloadDirectory; private static final Duration defaultDurationToKeepFiles = Duration.ofDays(30); + private static final int defaultOutdatedFilesToKeep = 20; private static final Duration interval = Duration.ofMinutes(1); - private final ScheduledExecutorService executor = - new ScheduledThreadPoolExecutor(1, new DaemonThreadFactory("file references and downloads cleanup")); + private final Optional<ScheduledExecutorService> executor; private final File urlDownloadDir; private final File fileReferencesDownloadDir; private final Duration durationToKeepFiles; + private final int outDatedFilesToKeep; FileReferencesAndDownloadsMaintainer() { - this(defaultFileReferencesDownloadDir, defaultUrlDownloadDir, keepFileReferencesDuration()); + this(defaultFileReferencesDownloadDir, defaultUrlDownloadDir, keepFileReferencesDuration(), + outDatedFilesToKeep(), configServers()); } - FileReferencesAndDownloadsMaintainer(File fileReferencesDownloadDir, File urlDownloadDir, Duration durationToKeepFiles) { + FileReferencesAndDownloadsMaintainer(File fileReferencesDownloadDir, + File urlDownloadDir, + Duration durationToKeepFiles, + int outdatedFilesToKeep, + List<String> configServers) { this.fileReferencesDownloadDir = fileReferencesDownloadDir; this.urlDownloadDir = urlDownloadDir; this.durationToKeepFiles = durationToKeepFiles; - executor.scheduleAtFixedRate(this, interval.toSeconds(), interval.toSeconds(), TimeUnit.SECONDS); + this.outDatedFilesToKeep = outdatedFilesToKeep; + // Do not run on config servers + if (configServers.contains(ConfigUtils.getCanonicalHostName())) { + log.log(INFO, "Not running maintainer, since this is on a config server host"); + executor = Optional.empty(); + } else { + executor = Optional.of(new ScheduledThreadPoolExecutor(1, new DaemonThreadFactory("file references and downloads cleanup"))); + executor.get().scheduleAtFixedRate(this, interval.toSeconds(), interval.toSeconds(), TimeUnit.SECONDS); + } } @Override public void run() { + if (executor.isEmpty()) return; + try { deleteUnusedFiles(fileReferencesDownloadDir); deleteUnusedFiles(urlDownloadDir); @@ -65,42 +86,62 @@ class FileReferencesAndDownloadsMaintainer implements Runnable { } public void close() { - executor.shutdownNow(); - try { - if ( ! executor.awaitTermination(10, TimeUnit.SECONDS)) - throw new RuntimeException("Unable to shutdown " + executor + " before timeout"); - } catch (InterruptedException e) { - throw new RuntimeException(e); - } + executor.ifPresent(ex -> { + ex.shutdownNow(); + try { + if (! ex.awaitTermination(10, TimeUnit.SECONDS)) + throw new RuntimeException("Unable to shutdown " + executor + " before timeout"); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + }); } private void deleteUnusedFiles(File directory) { - Instant deleteNotUsedSinceInstant = Instant.now().minus(durationToKeepFiles); - Set<String> filesOnDisk = new HashSet<>(); + File[] files = directory.listFiles(); - if (files != null) - filesOnDisk.addAll(Arrays.stream(files).map(File::getName).collect(Collectors.toSet())); - log.log(Level.FINE, () -> "Files on disk (in " + directory + "): " + filesOnDisk); + if (files == null) return; + + List<File> filesToDelete = filesThatCanBeDeleted(files); + filesToDelete.forEach(fileReference -> { + if (IOUtils.recursiveDeleteDir(fileReference)) + log.log(Level.FINE, "Deleted " + fileReference.getAbsolutePath()); + else + log.log(Level.WARNING, "Could not delete " + fileReference.getAbsolutePath()); + }); + } - Set<String> filesToDelete = filesOnDisk + private List<File> filesThatCanBeDeleted(File[] files) { + Instant deleteNotUsedSinceInstant = Instant.now().minus(durationToKeepFiles); + + Set<File> filesOnDisk = new HashSet<>(List.of(files)); + log.log(Level.FINE, () -> "Files on disk: " + filesOnDisk); + int deleteCount = Math.max(0, filesOnDisk.size() - outDatedFilesToKeep); + var canBeDeleted = filesOnDisk .stream() - .filter(fileReference -> isFileLastModifiedBefore(new File(directory, fileReference), deleteNotUsedSinceInstant)) - .collect(Collectors.toSet()); - if (filesToDelete.size() > 0) { - log.log(Level.INFO, "Files that can be deleted in " + directory + " (not used since " + deleteNotUsedSinceInstant + "): " + filesToDelete); - filesToDelete.forEach(fileReference -> { - File file = new File(directory, fileReference); - if (!IOUtils.recursiveDeleteDir(file)) - log.log(Level.WARNING, "Could not delete " + file.getAbsolutePath()); - }); - } + .peek(file -> log.log(Level.FINE, () -> file + ":" + fileLastModifiedTime(file.toPath()))) + .filter(fileReference -> isFileLastModifiedBefore(fileReference, deleteNotUsedSinceInstant)) + .sorted(Comparator.comparing(fileReference -> fileLastModifiedTime(fileReference.toPath()))) + .toList(); + + // Make sure we keep some files + canBeDeleted = canBeDeleted.subList(0, Math.min(canBeDeleted.size(), deleteCount)); + if (canBeDeleted.size() > 0) + log.log(INFO, "Files that can be deleted (not accessed since " + deleteNotUsedSinceInstant + + ", will also keep " + outDatedFilesToKeep + + " no matter when last accessed): " + canBeDeleted); + + return canBeDeleted; } private boolean isFileLastModifiedBefore(File fileReference, Instant instant) { - BasicFileAttributes fileAttributes; + return fileLastModifiedTime(fileReference.toPath()).isBefore(instant); + } + + private static Instant fileLastModifiedTime(Path fileReference) { try { - fileAttributes = readAttributes(fileReference.toPath(), BasicFileAttributes.class); - return fileAttributes.lastModifiedTime().toInstant().isBefore(instant); + BasicFileAttributes fileAttributes = readAttributes(fileReference, BasicFileAttributes.class); + return fileAttributes.lastModifiedTime().toInstant(); } catch (IOException e) { throw new UncheckedIOException(e); } @@ -114,4 +155,21 @@ class FileReferencesAndDownloadsMaintainer implements Runnable { return defaultDurationToKeepFiles; } + private static int outDatedFilesToKeep() { + String env = System.getenv("VESPA_KEEP_FILE_REFERENCES_COUNT"); + if (env != null && !env.isEmpty()) + return Integer.parseInt(env); + else + return defaultOutdatedFilesToKeep; + } + + private static List<String> configServers() { + String env = System.getenv("VESPA_CONFIGSERVERS"); + if (env == null || env.isEmpty()) + return List.of(ConfigUtils.getCanonicalHostName()); + else { + return List.of(env.split(",")); + } + } + } diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/filedistribution/FileReferencesAndDownloadsMaintainerTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/filedistribution/FileReferencesAndDownloadsMaintainerTest.java index fad021c0119..c41305b4dc8 100644 --- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/filedistribution/FileReferencesAndDownloadsMaintainerTest.java +++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/filedistribution/FileReferencesAndDownloadsMaintainerTest.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.config.proxy.filedistribution; import com.yahoo.io.IOUtils; +import com.yahoo.vespa.config.util.ConfigUtils; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -10,6 +11,9 @@ import java.io.File; import java.io.IOException; import java.time.Duration; import java.time.Instant; +import java.util.ArrayList; +import java.util.List; +import java.util.stream.IntStream; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -19,9 +23,12 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; */ public class FileReferencesAndDownloadsMaintainerTest { + private static final Duration keepDuration = Duration.ofMinutes(1); + private static final int outDatedFilesToKeep = 9; + private File cachedFileReferences; private File cachedDownloads; - private FileReferencesAndDownloadsMaintainer cachedFilesMaintainer; + private FileReferencesAndDownloadsMaintainer maintainer; @TempDir public File tempFolder; @@ -30,22 +37,70 @@ public class FileReferencesAndDownloadsMaintainerTest { public void setup() throws IOException { cachedFileReferences = newFolder(tempFolder, "cachedFileReferences"); cachedDownloads = newFolder(tempFolder, "cachedDownloads"); - cachedFilesMaintainer = new FileReferencesAndDownloadsMaintainer(cachedFileReferences, cachedDownloads, Duration.ofMinutes(1)); } @Test - void require_old_files_to_be_deleted() throws IOException { + void require_old_files_to_be_deleted() { + maintainer = new FileReferencesAndDownloadsMaintainer(cachedFileReferences, cachedDownloads, keepDuration, outDatedFilesToKeep, + List.of("host1")); runMaintainerAndAssertFiles(0, 0); - File fileReference = writeFile(cachedFileReferences, "fileReference"); - File download = writeFile(cachedDownloads, "download"); - runMaintainerAndAssertFiles(1, 1); + var fileReferences = writeFiles(20); + var downloads = writeDownloads(21); + runMaintainerAndAssertFiles(20, 21); + + updateLastModifiedTimestamp(0, 5, fileReferences, downloads); + runMaintainerAndAssertFiles(15, 16); - updateLastModifiedTimeStamp(fileReference, Instant.now().minus(Duration.ofMinutes(10))); - runMaintainerAndAssertFiles(0, 1); + updateLastModifiedTimestamp(6, 20, fileReferences, downloads); + // Should keep at least outDatedFilesToKeep file references and downloads even if there are more that are old + runMaintainerAndAssertFiles(outDatedFilesToKeep, outDatedFilesToKeep); + } - updateLastModifiedTimeStamp(download, Instant.now().minus(Duration.ofMinutes(10))); + @Test + void require_no_files_deleted_when_running_on_config_server_host() { + maintainer = new FileReferencesAndDownloadsMaintainer(cachedFileReferences, cachedDownloads, keepDuration, + outDatedFilesToKeep, List.of(ConfigUtils.getCanonicalHostName())); runMaintainerAndAssertFiles(0, 0); + + var fileReferences = writeFiles(10); + var downloads = writeDownloads(10); + runMaintainerAndAssertFiles(10, 10); + + updateLastModifiedTimestamp(0, 10, fileReferences, downloads); + runMaintainerAndAssertFiles(10, 10); + } + + private void updateLastModifiedTimestamp(int startInclusive, int endExclusive, List<File> fileReferences, List<File> downloads) { + IntStream.range(startInclusive, endExclusive).forEach(i -> { + Instant instant = Instant.now().minus(keepDuration.plus(Duration.ofMinutes(1)).minus(Duration.ofSeconds(i))); + updateLastModifiedTimeStamp(fileReferences.get(i), instant); + updateLastModifiedTimeStamp(downloads.get(i), instant); + }); + } + + private List<File> writeFiles(int count) { + List<File> files = new ArrayList<>(); + IntStream.range(0, count).forEach(i -> { + try { + files.add(writeFile(cachedFileReferences, "fileReference" + i)); + } catch (IOException e) { + throw new RuntimeException(e); + } + }); + return files; + } + + private List<File> writeDownloads(int count) { + List<File> files = new ArrayList<>(); + IntStream.range(0, count).forEach(i -> { + try { + files.add(writeFile(cachedDownloads, "download" + i)); + } catch (IOException e) { + throw new RuntimeException(e); + } + }); + return files; } private void updateLastModifiedTimeStamp(File file, Instant instant) { @@ -55,7 +110,7 @@ public class FileReferencesAndDownloadsMaintainerTest { } private void runMaintainerAndAssertFiles(int fileReferenceCount, int downloadCount) { - cachedFilesMaintainer.run(); + maintainer.run(); File[] fileReferences = cachedFileReferences.listFiles(); assertNotNull(fileReferences); assertEquals(fileReferenceCount, fileReferences.length); 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 57d57d16d2f..dcd2720ae3e 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,7 +12,6 @@ import com.yahoo.jrt.StringValue; import com.yahoo.jrt.Supervisor; import com.yahoo.jrt.Transport; import com.yahoo.vespa.config.ConnectionPool; -import com.yahoo.vespa.filedistribution.EmptyFileReferenceData; import com.yahoo.vespa.filedistribution.FileDistributionConnectionPool; import com.yahoo.vespa.filedistribution.FileDownloader; import com.yahoo.vespa.filedistribution.FileReferenceCompressor; @@ -20,9 +19,9 @@ 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.yolean.Exceptions; import java.io.File; import java.io.IOException; +import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; import java.time.Duration; @@ -35,10 +34,15 @@ import java.util.logging.Level; 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.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; import static com.yahoo.vespa.filedistribution.FileReferenceData.Type; import static com.yahoo.vespa.filedistribution.FileReferenceData.Type.compressed; +import static com.yahoo.yolean.Exceptions.uncheck; public class FileServer { @@ -54,10 +58,11 @@ public class FileServer { private final List<CompressionType> compressionTypes; // compression types to use, in preferred order // TODO: Move to filedistribution module, so that it can be used by both clients and servers - private enum FileApiErrorCodes { + enum FileApiErrorCodes { OK(0, "OK"), NOT_FOUND(1, "File reference not found"), - TIMEOUT(2, "Timeout"); + TIMEOUT(2, "Timeout"), + TRANSFER_FAILED(3, "Failed transferring file"); private final int code; private final String description; FileApiErrorCodes(int code, String description) { @@ -114,29 +119,24 @@ public class FileServer { FileDirectory getRootDir() { return fileDirectory; } void startFileServing(FileReference reference, Receiver target, Set<CompressionType> acceptedCompressionTypes) { - if ( ! fileDirectory.getFile(reference).exists()) return; + File file = fileDirectory.getFile(reference); + if ( ! file.exists()) return; - File file = this.fileDirectory.getFile(reference); - log.log(Level.FINE, () -> "Start serving " + reference + " with file '" + file.getAbsolutePath() + "'"); - FileReferenceData fileData = EmptyFileReferenceData.empty(reference, file.getName()); - try { - fileData = readFileReferenceData(reference, acceptedCompressionTypes); + try (FileReferenceData fileData = fileReferenceData(reference, acceptedCompressionTypes, file)) { + log.log(Level.FINE, () -> "Start serving " + reference.value() + " with file '" + file.getAbsolutePath() + "'"); target.receive(fileData, new ReplayStatus(0, "OK")); log.log(Level.FINE, () -> "Done serving " + reference.value() + " with file '" + file.getAbsolutePath() + "'"); - } catch (IOException e) { - String errorDescription = "For" + reference.value() + ": failed reading file '" + file.getAbsolutePath() + "'"; - log.warning(errorDescription + " for sending to '" + target.toString() + "'. " + e.getMessage()); - target.receive(fileData, new ReplayStatus(1, errorDescription)); + } catch (IOException ioe) { + throw new UncheckedIOException("For " + reference.value() + ": failed reading file '" + file.getAbsolutePath() + "'" + + " for sending to '" + target.toString() + "'. ", ioe); } catch (Exception e) { - log.log(Level.WARNING, "Failed serving " + reference + ": " + Exceptions.toMessageString(e)); - } finally { - fileData.close(); + throw new RuntimeException("Failed serving " + reference.value() + " to '" + target + "': ", e); } } - private FileReferenceData readFileReferenceData(FileReference reference, Set<CompressionType> acceptedCompressionTypes) throws IOException { - File file = this.fileDirectory.getFile(reference); - + private FileReferenceData fileReferenceData(FileReference reference, + Set<CompressionType> acceptedCompressionTypes, + File file) throws IOException { if (file.isDirectory()) { Path tempFile = Files.createTempFile("filereferencedata", reference.value()); CompressionType compressionType = chooseCompressionType(acceptedCompressionTypes); @@ -172,20 +172,21 @@ public class FileServer { Set<CompressionType> acceptedCompressionTypes) { if (Instant.now().isAfter(deadline)) { log.log(Level.INFO, () -> "Deadline exceeded for request for file reference '" + fileReference + "' from " + client); - return FileApiErrorCodes.TIMEOUT; + return TIMEOUT; } - boolean fileExists; try { var fileReferenceDownload = new FileReferenceDownload(fileReference, client, downloadFromOtherSourceIfNotFound); - fileExists = hasFileDownloadIfNeeded(fileReferenceDownload); - if (fileExists) startFileServing(fileReference, receiver, acceptedCompressionTypes); - } catch (IllegalArgumentException e) { - fileExists = false; + boolean fileExists = hasFileDownloadIfNeeded(fileReferenceDownload); + if ( ! fileExists) return NOT_FOUND; + + startFileServing(fileReference, receiver, acceptedCompressionTypes); + } catch (Exception e) { log.warning("Failed serving file reference '" + fileReference + "', request from " + client + " failed with: " + e.getMessage()); + return TRANSFER_FAILED; } - return (fileExists ? FileApiErrorCodes.OK : FileApiErrorCodes.NOT_FOUND); + return OK; } /* Choose the first compression type (list is in preferred order) that matches an accepted compression type, or fail */ 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 index eee7d6ec63d..d26a22284c0 100644 --- 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 @@ -518,7 +518,7 @@ public class RpcServer implements Runnable, ConfigActivationListener, TenantList request.parameters().add(new StringValue(fileData.filename())); request.parameters().add(new StringValue(fileData.type().name())); request.parameters().add(new Int64Value(fileData.size())); - // Only add paramter if not gzip, this is default and old clients will not handle the extra parameter + // Only add parameter if not gzip, this is default and old clients will not handle the extra parameter if (fileData.compressionType() != CompressionType.gzip) request.parameters().add(new StringValue(fileData.compressionType().name())); return request; @@ -532,7 +532,7 @@ public class RpcServer implements Runnable, ConfigActivationListener, TenantList request.parameters().add(new DataValue(buf)); invokeRpcIfValidConnection(request); if (request.isError()) { - throw new IllegalArgumentException("Failed delivering reference '" + ref.value() + "' to " + + throw new IllegalArgumentException("Failed delivering part of reference '" + ref.value() + "' to " + target.toString() + " with error: '" + request.errorMessage() + "'."); } else { if (request.returnValues().get(0).asInt32() != 0) { @@ -550,7 +550,8 @@ public class RpcServer implements Runnable, ConfigActivationListener, TenantList request.parameters().add(new StringValue(status.getDescription())); invokeRpcIfValidConnection(request); if (request.isError()) { - throw new IllegalArgumentException("Failed delivering reference '" + fileData.fileReference().value() + "' with file '" + fileData.filename() + "' to " + + throw new IllegalArgumentException("Failed delivering eof for reference '" + fileData.fileReference().value() + + "' with file '" + fileData.filename() + "' to " + target.toString() + " with error: '" + request.errorMessage() + "'."); } else { if (request.returnValues().get(0).asInt32() != 0) { 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 49458acd60b..c17b68c6d12 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 @@ -29,6 +29,7 @@ import static com.yahoo.vespa.filedistribution.FileReferenceData.CompressionType import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; public class FileServerTest { @@ -130,6 +131,25 @@ public class FileServerTest { assertEquals(1, fileServer.downloader().connectionPool().getSize()); } + @Test + public void requireThatErrorsAreHandled() throws IOException, ExecutionException, InterruptedException { + File dir = getFileServerRootDir(); + IOUtils.writeFile(dir + "/12y/f1", "dummy-data", true); + CompletableFuture<byte []> content = new CompletableFuture<>(); + FailingFileReceiver fileReceiver = new FailingFileReceiver(content); + + // Should fail the first time, see FailingFileReceiver + try { + fileServer.startFileServing(new FileReference("12y"), fileReceiver, Set.of(gzip)); + fail("Should have failed"); + } catch (RuntimeException e) { + // expected + } + + fileServer.startFileServing(new FileReference("12y"), fileReceiver, Set.of(gzip)); + assertEquals(new String(content.get()), "dummy-data"); + } + private void writeFile(String dir) throws IOException { File rootDir = getFileServerRootDir(); IOUtils.createDirectory(rootDir + "/" + dir); @@ -153,6 +173,23 @@ public class FileServerTest { } } + private static class FailingFileReceiver implements FileServer.Receiver { + final CompletableFuture<byte []> content; + int counter = 0; + FailingFileReceiver(CompletableFuture<byte []> content) { + this.content = content; + } + @Override + public void receive(FileReferenceData fileData, FileServer.ReplayStatus status) { + counter++; + if (counter <= 1) + throw new RuntimeException("Failed to receive file"); + else { + this.content.complete(fileData.content().array()); + } + } + } + private File getFileServerRootDir() { return fileServer.getRootDir().getRoot(); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/OsController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/OsController.java index c426c27418d..58c3b4da5e4 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/OsController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/OsController.java @@ -11,6 +11,7 @@ import com.yahoo.vespa.hosted.controller.versions.OsVersionStatus; import com.yahoo.vespa.hosted.controller.versions.OsVersionTarget; import java.time.Instant; +import java.util.Comparator; import java.util.HashSet; import java.util.Map; import java.util.Objects; @@ -161,9 +162,12 @@ public record OsController(Controller controller) { /** Remove certifications for non-existent OS versions */ public void removeStaleCertifications(OsVersionStatus currentStatus) { try (Mutex lock = curator().lockCertifiedOsVersions()) { - Set<OsVersion> knownVersions = currentStatus.versions().keySet(); + Optional<OsVersion> minKnownVersion = currentStatus.versions().keySet().stream() + .filter(v -> !v.version().isEmpty()) + .min(Comparator.naturalOrder()); + if (minKnownVersion.isEmpty()) return; Set<CertifiedOsVersion> certifiedVersions = new HashSet<>(readCertified()); - if (certifiedVersions.removeIf(cv -> !knownVersions.contains(cv.osVersion()))) { + if (certifiedVersions.removeIf(cv -> cv.osVersion().version().isBefore(minKnownVersion.get().version()))) { curator().writeCertifiedOsVersions(certifiedVersions); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdaterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdaterTest.java index af535abce26..6f4052bf0ef 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdaterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsVersionStatusUpdaterTest.java @@ -68,12 +68,14 @@ public class OsVersionStatusUpdaterTest { .filter(osVersion -> !osVersion.version().isEmpty()) .collect(Collectors.toSet()); List<OsVersion> versionsToCertify = new ArrayList<>(knownVersions); - versionsToCertify.addAll(List.of(new OsVersion(Version.fromString("95.0.1"), cloud), - new OsVersion(Version.fromString("98.0.2"), cloud))); + OsVersion futureVersion = new OsVersion(Version.fromString("98.0.2"), cloud); // Keep future version + versionsToCertify.addAll(List.of(new OsVersion(Version.fromString("3.11"), cloud), + futureVersion)); for (OsVersion version : versionsToCertify) { tester.controller().os().certify(version.version(), version.cloud(), Version.fromString("1.2.3")); } - assertEquals(knownVersions.size() + 2, certifiedOsVersions(tester).size()); + knownVersions.add(futureVersion); + assertEquals(knownVersions.size() + 1, certifiedOsVersions(tester).size()); statusUpdater.maintain(); assertEquals(knownVersions, certifiedOsVersions(tester)); } diff --git a/eval/src/vespa/eval/eval/inline_operation.h b/eval/src/vespa/eval/eval/inline_operation.h index 9b862b59e37..910fa9cffaa 100644 --- a/eval/src/vespa/eval/eval/inline_operation.h +++ b/eval/src/vespa/eval/eval/inline_operation.h @@ -4,6 +4,7 @@ #include "operation.h" #include <vespa/vespalib/util/typify.h> +#include <cblas.h> #include <cmath> namespace vespalib::eval::operation { @@ -148,4 +149,31 @@ void apply_op2_vec_vec(D *dst, const A *a, const B *b, size_t n, OP2 &&f) { //----------------------------------------------------------------------------- +template <typename LCT, typename RCT> +struct DotProduct { + static double apply(const LCT * lhs, const RCT * rhs, size_t count) { + double result = 0.0; + for (size_t i = 0; i < count; ++i) { + result += lhs[i] * rhs[i]; + } + return result; + } +}; + +template <> +struct DotProduct<float,float> { + static float apply(const float * lhs, const float * rhs, size_t count) { + return cblas_sdot(count, lhs, 1, rhs, 1); + } +}; + +template <> +struct DotProduct<double,double> { + static double apply(const double * lhs, const double * rhs, size_t count) { + return cblas_ddot(count, lhs, 1, rhs, 1); + } +}; + +//----------------------------------------------------------------------------- + } diff --git a/eval/src/vespa/eval/instruction/best_similarity_function.cpp b/eval/src/vespa/eval/instruction/best_similarity_function.cpp index 964f27a4564..415a08d0d93 100644 --- a/eval/src/vespa/eval/instruction/best_similarity_function.cpp +++ b/eval/src/vespa/eval/instruction/best_similarity_function.cpp @@ -1,10 +1,9 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "best_similarity_function.h" -#include <vespa/eval/eval/operation.h> +#include <vespa/eval/eval/inline_operation.h> #include <vespa/eval/eval/value.h> #include <vespa/vespalib/util/binary_hamming_distance.h> -#include <cblas.h> namespace vespalib::eval { @@ -22,7 +21,7 @@ struct BestSimParam { struct UseDotProduct { static float calc(const float *pri, const float *sec, size_t size) { - return cblas_sdot(size, pri, 1, sec, 1); + return DotProduct<float,float>::apply(pri, sec, size); } }; diff --git a/eval/src/vespa/eval/instruction/dense_dot_product_function.cpp b/eval/src/vespa/eval/instruction/dense_dot_product_function.cpp index a2048707685..de9e029f377 100644 --- a/eval/src/vespa/eval/instruction/dense_dot_product_function.cpp +++ b/eval/src/vespa/eval/instruction/dense_dot_product_function.cpp @@ -1,9 +1,8 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "dense_dot_product_function.h" -#include <vespa/eval/eval/operation.h> +#include <vespa/eval/eval/inline_operation.h> #include <vespa/eval/eval/value.h> -#include <cblas.h> namespace vespalib::eval { @@ -16,26 +15,7 @@ template <typename LCT, typename RCT> void my_dot_product_op(InterpretedFunction::State &state, uint64_t) { auto lhs_cells = state.peek(1).cells().typify<LCT>(); auto rhs_cells = state.peek(0).cells().typify<RCT>(); - double result = 0.0; - const LCT *lhs = lhs_cells.cbegin(); - const RCT *rhs = rhs_cells.cbegin(); - for (size_t i = 0; i < lhs_cells.size(); ++i) { - result += ((*lhs++) * (*rhs++)); - } - state.pop_pop_push(state.stash.create<DoubleValue>(result)); -} - -void my_cblas_double_dot_product_op(InterpretedFunction::State &state, uint64_t) { - auto lhs_cells = state.peek(1).cells().typify<double>(); - auto rhs_cells = state.peek(0).cells().typify<double>(); - double result = cblas_ddot(lhs_cells.size(), lhs_cells.cbegin(), 1, rhs_cells.cbegin(), 1); - state.pop_pop_push(state.stash.create<DoubleValue>(result)); -} - -void my_cblas_float_dot_product_op(InterpretedFunction::State &state, uint64_t) { - auto lhs_cells = state.peek(1).cells().typify<float>(); - auto rhs_cells = state.peek(0).cells().typify<float>(); - double result = cblas_sdot(lhs_cells.size(), lhs_cells.cbegin(), 1, rhs_cells.cbegin(), 1); + double result = DotProduct<LCT,RCT>::apply(lhs_cells.cbegin(), rhs_cells.cbegin(), lhs_cells.size()); state.pop_pop_push(state.stash.create<DoubleValue>(result)); } @@ -44,19 +24,6 @@ struct MyDotProductOp { static auto invoke() { return my_dot_product_op<LCT,RCT>; } }; -InterpretedFunction::op_function my_select(CellType lct, CellType rct) { - if (lct == rct) { - if (lct == CellType::DOUBLE) { - return my_cblas_double_dot_product_op; - } - if (lct == CellType::FLOAT) { - return my_cblas_float_dot_product_op; - } - } - using MyTypify = TypifyCellType; - return typify_invoke<2,MyTypify,MyDotProductOp>(lct, rct); -} - } // namespace <unnamed> DenseDotProductFunction::DenseDotProductFunction(const TensorFunction &lhs_in, @@ -68,7 +35,8 @@ DenseDotProductFunction::DenseDotProductFunction(const TensorFunction &lhs_in, InterpretedFunction::Instruction DenseDotProductFunction::compile_self(const ValueBuilderFactory &, Stash &) const { - auto op = my_select(lhs().result_type().cell_type(), rhs().result_type().cell_type()); + auto op = typify_invoke<2,TypifyCellType,MyDotProductOp>(lhs().result_type().cell_type(), + rhs().result_type().cell_type()); return InterpretedFunction::Instruction(op); } diff --git a/eval/src/vespa/eval/instruction/mixed_112_dot_product.cpp b/eval/src/vespa/eval/instruction/mixed_112_dot_product.cpp index 8bfa4b07980..47e1dbb58ed 100644 --- a/eval/src/vespa/eval/instruction/mixed_112_dot_product.cpp +++ b/eval/src/vespa/eval/instruction/mixed_112_dot_product.cpp @@ -5,7 +5,6 @@ #include <vespa/vespalib/util/typify.h> #include <vespa/vespalib/util/require.h> #include <vespa/eval/eval/visit_stuff.h> -#include <cblas.h> #include <algorithm> #include <optional> @@ -17,14 +16,6 @@ using namespace instruction; namespace { -template <typename CT> double my_dot_product(const CT * lhs, const CT * rhs, size_t count); -template <> double my_dot_product<double>(const double * lhs, const double * rhs, size_t count) { - return cblas_ddot(count, lhs, 1, rhs, 1); -} -template <> double my_dot_product<float>(const float * lhs, const float * rhs, size_t count) { - return cblas_sdot(count, lhs, 1, rhs, 1); -} - template <typename T, size_t N> ConstArrayRef<const T *> as_ccar(std::array<T *, N> &array) { return {array.data(), array.size()}; @@ -54,10 +45,11 @@ double my_mixed_112_dot_product_fallback(const Value::Index &a_idx, const Value: auto outer = a_idx.create_view({}); auto model = c_idx.create_view({&single_dim[0], 1}); outer->lookup({}); + using dot_product = DotProduct<CT,CT>; while (outer->next_result(as_car(c_addr_ref[0]), a_space)) { model->lookup(as_ccar(c_addr_ref)); if (model->next_result({}, c_space)) { - result += my_dot_product<CT>(b_cells, c_cells + (c_space * dense_size), dense_size) * a_cells[a_space]; + result += dot_product::apply(b_cells, c_cells + (c_space * dense_size), dense_size) * a_cells[a_space]; } } return result; @@ -70,11 +62,12 @@ double my_fast_mixed_112_dot_product(const FastAddrMap *a_map, const FastAddrMap { double result = 0.0; const auto &a_labels = a_map->labels(); + using dot_product = DotProduct<CT,CT>; for (size_t a_space = 0; a_space < a_labels.size(); ++a_space) { if (a_cells[a_space] != 0.0) { // handle pseudo-sparse input auto c_space = c_map->lookup_singledim(a_labels[a_space]); if (c_space != FastAddrMap::npos()) { - result += my_dot_product<CT>(b_cells, c_cells + (c_space * dense_size), dense_size) * a_cells[a_space]; + result += dot_product::apply(b_cells, c_cells + (c_space * dense_size), dense_size) * a_cells[a_space]; } } } diff --git a/eval/src/vespa/eval/instruction/mixed_inner_product_function.cpp b/eval/src/vespa/eval/instruction/mixed_inner_product_function.cpp index 248f909fcf5..5880a90a2cd 100644 --- a/eval/src/vespa/eval/instruction/mixed_inner_product_function.cpp +++ b/eval/src/vespa/eval/instruction/mixed_inner_product_function.cpp @@ -1,9 +1,8 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "mixed_inner_product_function.h" -#include <vespa/eval/eval/operation.h> +#include <vespa/eval/eval/inline_operation.h> #include <vespa/eval/eval/value.h> -#include <cblas.h> namespace vespalib::eval { @@ -12,31 +11,6 @@ using namespace operation; namespace { -template <typename LCT, typename RCT> -struct MyDotProduct { - static double apply(const LCT * lhs, const RCT * rhs, size_t count) { - double result = 0.0; - for (size_t i = 0; i < count; ++i) { - result += lhs[i] * rhs[i]; - } - return result; - } -}; - -template <> -struct MyDotProduct<double,double> { - static double apply(const double * lhs, const double * rhs, size_t count) { - return cblas_ddot(count, lhs, 1, rhs, 1); - } -}; - -template <> -struct MyDotProduct<float,float> { - static float apply(const float * lhs, const float * rhs, size_t count) { - return cblas_sdot(count, lhs, 1, rhs, 1); - } -}; - struct MixedInnerProductParam { ValueType res_type; size_t vector_size; @@ -66,8 +40,9 @@ void my_mixed_inner_product_op(InterpretedFunction::State &state, uint64_t param ArrayRef<OCT> out_cells = state.stash.create_uninitialized_array<OCT>(num_output_cells); const MCT *m_cp = m_cells.begin(); const VCT *v_cp = v_cells.begin(); + using dot_product = DotProduct<MCT,VCT>; for (OCT &out : out_cells) { - out = MyDotProduct<MCT,VCT>::apply(m_cp, v_cp, param.vector_size); + out = dot_product::apply(m_cp, v_cp, param.vector_size); m_cp += param.vector_size; } assert(m_cp == m_cells.end()); diff --git a/eval/src/vespa/eval/instruction/sum_max_dot_product_function.cpp b/eval/src/vespa/eval/instruction/sum_max_dot_product_function.cpp index a76eaa38925..41017bc3687 100644 --- a/eval/src/vespa/eval/instruction/sum_max_dot_product_function.cpp +++ b/eval/src/vespa/eval/instruction/sum_max_dot_product_function.cpp @@ -1,9 +1,8 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "sum_max_dot_product_function.h" -#include <vespa/eval/eval/operation.h> +#include <vespa/eval/eval/inline_operation.h> #include <vespa/eval/eval/value.h> -#include <cblas.h> namespace vespalib::eval { @@ -16,11 +15,12 @@ void my_sum_max_dot_product_op(InterpretedFunction::State &state, uint64_t dp_si double result = 0.0; auto query_cells = state.peek(1).cells().typify<float>(); auto document_cells = state.peek(0).cells().typify<float>(); + using dot_product = DotProduct<float,float>; if ((query_cells.size() > 0) && (document_cells.size() > 0)) { for (const float *query = query_cells.begin(); query < query_cells.end(); query += dp_size) { float max_dp = aggr::Max<float>::null_value(); for (const float *document = document_cells.begin(); document < document_cells.end(); document += dp_size) { - max_dp = aggr::Max<float>::combine(max_dp, cblas_sdot(dp_size, query, 1, document, 1)); + max_dp = aggr::Max<float>::combine(max_dp, dot_product::apply(query, document, dp_size)); } result += max_dp; } diff --git a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/EmptyFileReferenceData.java b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/EmptyFileReferenceData.java deleted file mode 100644 index ea8461b42f3..00000000000 --- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/EmptyFileReferenceData.java +++ /dev/null @@ -1,55 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.filedistribution; - -import com.yahoo.config.FileReference; - -import java.nio.ByteBuffer; - -public class EmptyFileReferenceData extends FileReferenceData { - - private final byte[] content; - private final long xxhash; - private int contentRead = 0; - - private EmptyFileReferenceData(FileReference fileReference, String filename, Type type, byte[] content, long xxhash) { - super(fileReference, filename, type, CompressionType.gzip); - this.content = content; - this.xxhash = xxhash; - } - - public static FileReferenceData empty(FileReference fileReference, String filename) { - return new EmptyFileReferenceData(fileReference, filename, FileReferenceData.Type.file, new byte[0], 0); - } - - public ByteBuffer content() { - return ByteBuffer.wrap(content); - } - - @Override - public int nextContent(ByteBuffer bb) { - if (contentRead >= content.length) { - return -1; - } else { - int left = content.length - contentRead; - int size = Math.min(bb.remaining(), left); - bb.put(content, contentRead, size); - contentRead += size; - return size; - } - } - - @Override - public long xxhash() { - return xxhash; - } - - @Override - public long size() { - return content.length; - } - - @Override - public void close() { - // no-op - } -} diff --git a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReceiver.java b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReceiver.java index b37fe02226b..a567a3bc4b3 100644 --- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReceiver.java +++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReceiver.java @@ -243,7 +243,7 @@ public class FileReceiver { synchronized (sessions) { if (sessions.containsKey(sessionId)) { retval = 1; - log.severe("Session id " + sessionId + " already exist, impossible. Request from(" + req.target() + ")"); + log.severe("Session id " + sessionId + " already exist, impossible. Request from " + req.target()); } else { try { sessions.put(sessionId, new Session(downloadDirectory, sessionId, reference, diff --git a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceData.java b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceData.java index 3f83cbea506..87f45db5221 100644 --- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceData.java +++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceData.java @@ -10,7 +10,7 @@ import java.nio.ByteBuffer; * * @author hmusum */ -public abstract class FileReferenceData { +public abstract class FileReferenceData implements AutoCloseable { public enum Type { file, compressed } public enum CompressionType { gzip, lz4, zstd } diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index c6d141764fb..4e995e9b392 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -308,7 +308,7 @@ public class Flags { HOSTNAME); public static final UnboundBooleanFlag ALLOW_MORE_THAN_ONE_CONTENT_GROUP_DOWN = defineFeatureFlag( - "allow-more-than-one-content-group-down", false, List.of("hmusum"), "2023-04-14", "2023-08-15", + "allow-more-than-one-content-group-down", true, List.of("hmusum"), "2023-04-14", "2023-09-01", "Whether to enable possible configuration of letting more than one content group down", "Takes effect at redeployment", APPLICATION_ID); @@ -331,7 +331,7 @@ public class Flags { "Takes effect at next host-admin tick"); public static final UnboundBooleanFlag ENABLE_THE_ONE_THAT_SHOULD_NOT_BE_NAMED = defineFeatureFlag( - "enable-the-one-that-should-not-be-named", false, List.of("hmusum"), "2023-05-08", "2023-08-15", + "enable-the-one-that-should-not-be-named", false, List.of("hmusum"), "2023-05-08", "2023-09-15", "Whether to enable the one program that should not be named", "Takes effect at next host-admin tick"); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java index 7fc248024c3..284306e1e8c 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java @@ -17,7 +17,6 @@ import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeState; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.reports.DropDocumentsReport; import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.Orchestrator; -import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.OrchestratorException; import com.yahoo.vespa.hosted.node.admin.container.Container; import com.yahoo.vespa.hosted.node.admin.container.ContainerOperations; import com.yahoo.vespa.hosted.node.admin.container.ContainerResources; @@ -484,6 +483,11 @@ public class NodeAgentImpl implements NodeAgent { lastNode = node; } + // Run this here and now, even though we may immediately remove the container below. + // This ensures these maintainers are run even if something fails or returns early. + // These maintainers should also run immediately after starting the container (see below). + container.ifPresent(c -> runImportantContainerMaintainers(context, c)); + switch (node.state()) { case ready, reserved, failed, inactive, parked -> { storageMaintainer.syncLogs(context, true); @@ -508,13 +512,11 @@ public class NodeAgentImpl implements NodeAgent { containerState = STARTING; container = Optional.of(startContainer(context)); containerState = UNKNOWN; + runImportantContainerMaintainers(context, container.get()); } else { container = Optional.of(updateContainerIfNeeded(context, container.get())); } - aclMaintainer.ifPresent(maintainer -> maintainer.converge(context)); - final Optional<Container> finalContainer = container; - wireguardTasks.forEach(task -> task.converge(context, finalContainer.get().id())); startServicesIfNeeded(context); resumeNodeIfNeeded(context); if (healthChecker.isPresent()) { @@ -559,6 +561,11 @@ public class NodeAgentImpl implements NodeAgent { } } + private void runImportantContainerMaintainers(NodeAgentContext context, Container container) { + aclMaintainer.ifPresent(maintainer -> maintainer.converge(context)); + wireguardTasks.forEach(task -> task.converge(context, container.id())); + } + private static void logChangesToNodeSpec(NodeAgentContext context, NodeSpec lastNode, NodeSpec node) { StringBuilder builder = new StringBuilder(); appendIfDifferent(builder, "state", lastNode, node, NodeSpec::state); @@ -600,23 +607,8 @@ public class NodeAgentImpl implements NodeAgent { if (context.node().state() != NodeState.active) return; context.log(logger, "Ask Orchestrator for permission to suspend node"); - try { - orchestrator.suspend(context.hostname().value()); - suspendedInOrchestrator = true; - } catch (OrchestratorException e) { - // Ensure the ACLs are up to date: The reason we're unable to suspend may be because some other - // node is unable to resume because the ACL rules of SOME Docker container is wrong... - // Same can happen with stale WireGuard config, so update that too - try { - aclMaintainer.ifPresent(maintainer -> maintainer.converge(context)); - wireguardTasks.forEach(task -> getContainer(context).ifPresent(c -> task.converge(context, c.id()))); - } catch (RuntimeException suppressed) { - logger.log(Level.WARNING, "Suppressing ACL update failure: " + suppressed); - e.addSuppressed(suppressed); - } - - throw e; - } + orchestrator.suspend(context.hostname().value()); + suspendedInOrchestrator = true; } protected void writeContainerData(NodeAgentContext context, ContainerData containerData) { } |