diff options
141 files changed, 1816 insertions, 1199 deletions
diff --git a/application-model/pom.xml b/application-model/pom.xml index 3abf9851d5c..7eac247e249 100644 --- a/application-model/pom.xml +++ b/application-model/pom.xml @@ -23,6 +23,12 @@ </dependency> <dependency> <groupId>com.yahoo.vespa</groupId> + <artifactId>vespajlib</artifactId> + <version>${project.version}</version> + <scope>provided</scope> + </dependency> + <dependency> + <groupId>com.yahoo.vespa</groupId> <artifactId>config-provisioning</artifactId> <version>${project.version}</version> <scope>provided</scope> diff --git a/application-model/src/main/java/com/yahoo/vespa/applicationmodel/TenantId.java b/application-model/src/main/java/com/yahoo/vespa/applicationmodel/TenantId.java index 7d94a0418e4..20aca379015 100644 --- a/application-model/src/main/java/com/yahoo/vespa/applicationmodel/TenantId.java +++ b/application-model/src/main/java/com/yahoo/vespa/applicationmodel/TenantId.java @@ -31,8 +31,6 @@ public class TenantId { return id; } - public TenantName toName() { return TenantName.from(id); } - @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/bundle-plugin/pom.xml b/bundle-plugin/pom.xml index b1b03b60ce6..d53c2c94d5c 100644 --- a/bundle-plugin/pom.xml +++ b/bundle-plugin/pom.xml @@ -21,12 +21,12 @@ <dependency> <groupId>org.apache.maven</groupId> <artifactId>maven-plugin-api</artifactId> - <version>3.5.0</version> + <version>3.8.5</version> </dependency> <dependency> <groupId>org.apache.maven</groupId> <artifactId>maven-archiver</artifactId> - <version>3.5.0</version> + <version>3.5.2</version> </dependency> <dependency> <groupId>org.apache.maven.plugin-tools</groupId> diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/util/TestBundleDependencyScopeTranslator.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/util/TestBundleDependencyScopeTranslator.java index d922b63c24c..bd6151aea9f 100644 --- a/bundle-plugin/src/main/java/com/yahoo/container/plugin/util/TestBundleDependencyScopeTranslator.java +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/util/TestBundleDependencyScopeTranslator.java @@ -35,7 +35,10 @@ public class TestBundleDependencyScopeTranslator implements Artifacts.ScopeTrans this.dependencyScopes = dependencyScopes; } - @Override public String scopeOf(Artifact artifact) { return Objects.requireNonNull(dependencyScopes.get(artifact)); } + @Override + public String scopeOf(Artifact artifact) { + return Objects.requireNonNull(dependencyScopes.get(artifact), () -> "Could not lookup scope for " + artifact); + } public static TestBundleDependencyScopeTranslator from(Map<String, Artifact> dependencies, String rawConfig) { List<DependencyOverride> dependencyOverrides = toDependencyOverrides(rawConfig); diff --git a/cloud-tenant-base-dependencies-enforcer/pom.xml b/cloud-tenant-base-dependencies-enforcer/pom.xml index 21e5de31534..b5281050459 100644 --- a/cloud-tenant-base-dependencies-enforcer/pom.xml +++ b/cloud-tenant-base-dependencies-enforcer/pom.xml @@ -236,6 +236,7 @@ <include>org.antlr:antlr-runtime:3.5.2:jar:test</include> <include>org.antlr:antlr4-runtime:4.9.3:jar:test</include> <include>org.apache.commons:commons-exec:1.3:jar:test</include> + <include>org.apache.commons:commons-compress:1.21:jar:test</include> <include>org.apache.commons:commons-math3:3.6.1:jar:test</include> <include>org.apache.httpcomponents.client5:httpclient5:${httpclient5.version}:jar:test</include> <include>org.apache.httpcomponents.core5:httpcore5:${httpclient5.version}:jar:test</include> diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java index a410b025e6b..9453f489029 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java @@ -120,7 +120,7 @@ public interface ModelContext { @ModelFeatureFlag(owners = {"arnej"}) default boolean avoidRenamingSummaryFeatures() { return false; } @ModelFeatureFlag(owners = {"bjorncs", "baldersheim"}, removeAfter = "7.569") default boolean mergeGroupingResultInSearchInvoker() { return true; } @ModelFeatureFlag(owners = {"arnej"}) default boolean experimentalSdParsing() { return false; } - @ModelFeatureFlag(owners = {"hmusum"}, removeAfter = "7.564") default String adminClusterNodeArchitecture() { return adminClusterArchitecture().name(); } + @ModelFeatureFlag(owners = {"hmusum"}, removeAfter = "7.571") default String adminClusterNodeArchitecture() { return adminClusterArchitecture().name(); } @ModelFeatureFlag(owners = {"hmusum"}) default Architecture adminClusterArchitecture() { return Architecture.getDefault(); } } diff --git a/config-model/.gitignore b/config-model/.gitignore index 4cf50da0853..6edd041cbe8 100644 --- a/config-model/.gitignore +++ b/config-model/.gitignore @@ -5,3 +5,4 @@ /src/test/integration/*/copy/ /src/test/integration/*/models.generated/ *.cfg.actual +/var/ diff --git a/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java b/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java index 824bf248b5c..1838f1e36b7 100644 --- a/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java +++ b/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java @@ -99,7 +99,7 @@ public class MockApplicationPackage implements ApplicationPackage { @SuppressWarnings("deprecation") // not redundant @Override public String getApplicationName() { - return "mock application"; + return "mock-application"; } @Override diff --git a/config-model/src/main/java/com/yahoo/vespa/model/clients/Clients.java b/config-model/src/main/java/com/yahoo/vespa/model/clients/Clients.java index 47bcc64f663..8b8b9e5f40d 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/clients/Clients.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/clients/Clients.java @@ -13,6 +13,7 @@ import com.yahoo.documentapi.messagebus.loadtypes.LoadTypeSet; * * @author Gunnar Gauslaa Bergem */ +@SuppressWarnings("removal") // TODO: Remove on Vespa 8 public class Clients extends ConfigModel { private static final long serialVersionUID = 1L; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java index ce2bc351d2b..73bd064b967 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java @@ -62,7 +62,6 @@ import java.util.Set; import java.util.TreeMap; import java.util.logging.Level; -import static com.yahoo.config.provision.NodeResources.Architecture; import static com.yahoo.config.provision.NodeResources.DiskSpeed; import static com.yahoo.config.provision.NodeResources.StorageType; import static java.util.stream.Collectors.toList; @@ -337,8 +336,7 @@ public class ContentCluster extends AbstractConfigProducer<AbstractConfigProduce DeployState deployState, String clusterName) { if (admin.getClusterControllers() == null) { - NodeResources nodeResources = clusterControllerResources - .with(Architecture.valueOf(deployState.featureFlags().adminClusterNodeArchitecture())); + NodeResources nodeResources = clusterControllerResources.with(deployState.featureFlags().adminClusterArchitecture()); NodesSpecification spec = NodesSpecification.requiredFromSharedParents(deployState.zone().environment().isProduction() ? 3 : 1, nodeResources, contentElement, diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ApplicationId.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ApplicationId.java index 76d4ae3f1a6..aa70bf4d26a 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/ApplicationId.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ApplicationId.java @@ -3,6 +3,10 @@ package com.yahoo.config.provision; import com.yahoo.cloud.config.ApplicationIdConfig; +import java.util.Comparator; +import java.util.Objects; +import java.util.regex.Pattern; + /** * A complete, immutable identification of an application instance. * @@ -10,27 +14,40 @@ import com.yahoo.cloud.config.ApplicationIdConfig; * @author vegard * @author bratseth */ -public final class ApplicationId implements Comparable<ApplicationId> { +public class ApplicationId implements Comparable<ApplicationId> { + + // TODO: remove '.' and '*' from this pattern. + static final Pattern namePattern = Pattern.compile("(?!\\.\\.)[a-zA-Z0-9_.*-]{1,256}"); + + private static final ApplicationId global = new ApplicationId(TenantName.from("*"), + ApplicationName.from("*"), + InstanceName.from("*")) { + @Override public boolean equals(Object other) { return this == other; } + }; + + private static final Comparator<ApplicationId> comparator = Comparator.comparing(ApplicationId::tenant) + .thenComparing(ApplicationId::application) + .thenComparing(ApplicationId::instance) + .thenComparing(global::equals, Boolean::compare); private final TenantName tenant; private final ApplicationName application; private final InstanceName instance; - - private final String stringValue; private final String serializedForm; - public ApplicationId(ApplicationIdConfig config) { - this(TenantName.from(config.tenant()), ApplicationName.from(config.application()), InstanceName.from(config.instance())); - } - private ApplicationId(TenantName tenant, ApplicationName applicationName, InstanceName instanceName) { this.tenant = tenant; this.application = applicationName; this.instance = instanceName; - this.stringValue = toStringValue(); this.serializedForm = toSerializedForm(); } + public static ApplicationId from(ApplicationIdConfig config) { + return from(TenantName.from(config.tenant()), + ApplicationName.from(config.application()), + InstanceName.from(config.instance())); + } + public static ApplicationId from(TenantName tenant, ApplicationName application, InstanceName instance) { return new ApplicationId(tenant, application, instance); } @@ -44,7 +61,7 @@ public final class ApplicationId implements Comparable<ApplicationId> { if (parts.length < 3) throw new IllegalArgumentException("Application ids must be on the form tenant:application:instance, but was " + idString); - return new Builder().tenant(parts[0]).applicationName(parts[1]).instanceName(parts[2]).build(); + return from(parts[0], parts[1], parts[2]); } public static ApplicationId fromFullString(String idString) { @@ -52,11 +69,11 @@ public final class ApplicationId implements Comparable<ApplicationId> { if (parts.length < 3) throw new IllegalArgumentException("Application ids must be on the form tenant.application.instance, but was " + idString); - return new Builder().tenant(parts[0]).applicationName(parts[1]).instanceName(parts[2]).build(); + return from(parts[0], parts[1], parts[2]); } @Override - public int hashCode() { return stringValue.hashCode(); } + public int hashCode() { return Objects.hash(tenant, application, instance); } @Override public boolean equals(Object other) { @@ -72,10 +89,6 @@ public final class ApplicationId implements Comparable<ApplicationId> { /** Returns a serialized form of the content of this: tenant:application:instance */ public String serializedForm() { return serializedForm; } - private String toStringValue() { - return "tenant '" + tenant + "', application '" + application + "', instance '" + instance + "'"; - } - /** Returns "dotted" string (tenant.application.instance) with instance name omitted if it is "default" */ public String toShortString() { return tenant().value() + "." + application().value() + @@ -88,7 +101,7 @@ public final class ApplicationId implements Comparable<ApplicationId> { } private String toSerializedForm() { - return tenant + ":" + application + ":" + instance; + return tenant.value() + ":" + application.value() + ":" + instance.value(); } @Override @@ -100,18 +113,7 @@ public final class ApplicationId implements Comparable<ApplicationId> { @Override public int compareTo(ApplicationId other) { - int diff; - - diff = tenant.compareTo(other.tenant); - if (diff != 0) { return diff; } - - diff = application.compareTo(other.application); - if (diff != 0) { return diff; } - - diff = instance.compareTo(other.instance); - if (diff != 0) { return diff; } - - return 0; + return comparator.compare(this, other); } /** Returns an application id where all fields are "default" */ @@ -119,12 +121,10 @@ public final class ApplicationId implements Comparable<ApplicationId> { return new ApplicationId(TenantName.defaultName(), ApplicationName.defaultName(), InstanceName.defaultName()); } - /** Returns an application id where all fields are "*" */ + // TODO: kill this + /** Returns a very special application id, which is not equal to any other id. */ public static ApplicationId global() { - return new Builder().tenant("*") - .applicationName("*") - .instanceName("*") - .build(); + return global; } public static class Builder { diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ApplicationName.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ApplicationName.java index f16c126dec2..f2585913015 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/ApplicationName.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ApplicationName.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.provision; -import java.util.Objects; +import ai.vespa.validation.PatternedStringWrapper; /** * Represents an applications name, which may be any kind of string or default. This type is defined @@ -10,28 +10,12 @@ import java.util.Objects; * @author Ulf Lilleengen * @since 5.25 */ -public class ApplicationName implements Comparable<ApplicationName> { +public class ApplicationName extends PatternedStringWrapper<ApplicationName> { - private final String applicationName; + private static final ApplicationName defaultName = new ApplicationName("default"); - private ApplicationName(String applicationName) { - this.applicationName = applicationName; - } - - @Override - public int hashCode() { - return applicationName.hashCode(); - } - - @Override - public boolean equals(Object obj) { - if (!(obj instanceof ApplicationName)) return false; - return Objects.equals(((ApplicationName) obj).applicationName, applicationName); - } - - @Override - public String toString() { - return applicationName; + private ApplicationName(String name) { + super(name, ApplicationId.namePattern, "application name"); } public static ApplicationName from(String name) { @@ -39,20 +23,11 @@ public class ApplicationName implements Comparable<ApplicationName> { } public static ApplicationName defaultName() { - return new ApplicationName("default"); + return defaultName; } public boolean isDefault() { - return equals(ApplicationName.defaultName()); - } - - public String value() { - return applicationName; - } - - @Override - public int compareTo(ApplicationName name) { - return this.applicationName.compareTo(name.applicationName); + return equals(defaultName); } } diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/InstanceName.java b/config-provisioning/src/main/java/com/yahoo/config/provision/InstanceName.java index 8101b70b943..fc40d351465 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/InstanceName.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/InstanceName.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.provision; -import java.util.Objects; +import ai.vespa.validation.PatternedStringWrapper; /** * Represents an applications instance name, which may be any kind of string or default. This type is defined @@ -9,30 +9,12 @@ import java.util.Objects; * * @author Ulf Lilleengen */ -public class InstanceName implements Comparable<InstanceName> { +public class InstanceName extends PatternedStringWrapper<InstanceName> { - private static final InstanceName defaultInstance = new InstanceName("default"); + private static final InstanceName defaultName = new InstanceName("default"); - private final String instanceName; - - private InstanceName(String instanceName) { - this.instanceName = instanceName; - } - - @Override - public int hashCode() { - return instanceName.hashCode(); - } - - @Override - public boolean equals(Object obj) { - if (!(obj instanceof InstanceName)) return false; - return Objects.equals(((InstanceName) obj).instanceName, instanceName); - } - - @Override - public String toString() { - return instanceName; + private InstanceName(String name) { + super(name, ApplicationId.namePattern, "instance name"); } public static InstanceName from(String name) { @@ -40,22 +22,15 @@ public class InstanceName implements Comparable<InstanceName> { } public static InstanceName defaultName() { - return defaultInstance; + return defaultName; } public boolean isDefault() { - return equals(InstanceName.defaultName()); + return equals(defaultName); } public boolean isTester() { return value().endsWith("-t"); } - public String value() { return instanceName; } - - @Override - public int compareTo(InstanceName instance) { - return instanceName.compareTo(instance.instanceName); - } - } diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/TenantName.java b/config-provisioning/src/main/java/com/yahoo/config/provision/TenantName.java index 92fe5345b4e..9909ab360a0 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/TenantName.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/TenantName.java @@ -1,56 +1,31 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.provision; -import java.util.Objects; +import ai.vespa.validation.PatternedStringWrapper; /** * Represents a tenant in the provision API. * * @author Ulf Lilleengen */ -public class TenantName implements Comparable<TenantName> { +public class TenantName extends PatternedStringWrapper<TenantName> { - private final String name; + private static final TenantName defaultName = new TenantName("default"); private TenantName(String name) { - this.name = name; + super(name, ApplicationId.namePattern, "tenant name"); } - public String value() { return name; } - - /** - * Create a {@link TenantName} with a given name. - * - * @param name Name of tenant. - * @return instance of {@link TenantName}. - */ public static TenantName from(String name) { return new TenantName(name); } - @Override - public int hashCode() { - return name.hashCode(); - } - - @Override - public boolean equals(Object obj) { - if (!(obj instanceof TenantName)) return false; - return Objects.equals(((TenantName)obj).value(), value()); - } - - @Override - public String toString() { - return name; - } - public static TenantName defaultName() { - return from("default"); + return defaultName; } - @Override - public int compareTo(TenantName that) { - return this.name.compareTo(that.name); + public boolean isDefault() { + return equals(defaultName); } } diff --git a/config-provisioning/src/test/java/com/yahoo/config/provision/ApplicationIdTest.java b/config-provisioning/src/test/java/com/yahoo/config/provision/ApplicationIdTest.java index c82230f7edf..01904b5eece 100644 --- a/config-provisioning/src/test/java/com/yahoo/config/provision/ApplicationIdTest.java +++ b/config-provisioning/src/test/java/com/yahoo/config/provision/ApplicationIdTest.java @@ -106,7 +106,7 @@ public class ApplicationIdTest { builder.tenant("a"); builder.application("b"); builder.instance("c"); - ApplicationId applicationId = new ApplicationId(new ApplicationIdConfig(builder)); + ApplicationId applicationId = ApplicationId.from(new ApplicationIdConfig(builder)); assertEquals("a", applicationId.tenant().value()); assertEquals("b", applicationId.application().value()); assertEquals("c", applicationId.instance().value()); diff --git a/configdefinitions/src/vespa/configserver.def b/configdefinitions/src/vespa/configserver.def index 05143bfef9f..64ca1e522f5 100644 --- a/configdefinitions/src/vespa/configserver.def +++ b/configdefinitions/src/vespa/configserver.def @@ -20,6 +20,10 @@ configServerDBDir string default="var/db/vespa/config_server/serverdb/" configDefinitionsDir string default="share/vespa/configdefinitions/" fileReferencesDir string default="var/db/vespa/filedistribution/" +# Application package +# The maximum decompressed size of an application package, in bytes. Defaults to 8 GB +maxApplicationPackageSize long default=8589934592 + # Misc sessionLifetime long default=3600 # in seconds masterGeneration long default=0 diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java index 7a754dd84cd..cb1c1a461e3 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java @@ -1053,7 +1053,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye private File decompressApplication(InputStream in, String contentType, File tempDir) { try (CompressedApplicationInputStream application = - CompressedApplicationInputStream.createFromCompressedStream(in, contentType)) { + CompressedApplicationInputStream.createFromCompressedStream(in, contentType, configserverConfig.maxApplicationPackageSize())) { return decompressApplication(application, tempDir); } catch (IOException e) { throw new IllegalArgumentException("Unable to decompress data in body", e); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/CompressedApplicationInputStream.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/CompressedApplicationInputStream.java index 0672f13fd6a..443ab47e786 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/CompressedApplicationInputStream.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/CompressedApplicationInputStream.java @@ -1,23 +1,21 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.application; -import com.google.common.io.ByteStreams; +import com.yahoo.compress.ArchiveStreamReader; +import com.yahoo.compress.ArchiveStreamReader.Options; +import com.yahoo.vespa.config.server.http.BadRequestException; +import com.yahoo.vespa.config.server.http.InternalServerException; +import com.yahoo.vespa.config.server.http.v2.ApplicationApiHandler; import java.io.File; -import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; +import java.io.OutputStream; +import java.io.UncheckedIOException; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.logging.Level; -import com.yahoo.vespa.config.server.http.BadRequestException; -import com.yahoo.vespa.config.server.http.InternalServerException; -import com.yahoo.vespa.config.server.http.v2.ApplicationApiHandler; -import org.apache.commons.compress.archivers.ArchiveEntry; -import org.apache.commons.compress.archivers.ArchiveInputStream; -import org.apache.commons.compress.archivers.tar.TarArchiveInputStream; -import org.apache.commons.compress.archivers.zip.ZipArchiveInputStream; - import java.util.logging.Logger; -import java.util.zip.GZIPInputStream; import static com.yahoo.yolean.Exceptions.uncheck; @@ -29,53 +27,43 @@ import static com.yahoo.yolean.Exceptions.uncheck; public class CompressedApplicationInputStream implements AutoCloseable { private static final Logger log = Logger.getLogger(CompressedApplicationInputStream.class.getPackage().getName()); - private final ArchiveInputStream ais; + + private final ArchiveStreamReader reader; + + private CompressedApplicationInputStream(ArchiveStreamReader reader) { + this.reader = reader; + } /** * Create an instance of a compressed application from an input stream. * * @param is the input stream containing the compressed files. * @param contentType the content type for determining what kind of compressed stream should be used. + * @param maxSizeInBytes the maximum allowed size of the decompressed content * @return An instance of an unpacked application. */ - public static CompressedApplicationInputStream createFromCompressedStream(InputStream is, String contentType) { + public static CompressedApplicationInputStream createFromCompressedStream(InputStream is, String contentType, long maxSizeInBytes) { try { - ArchiveInputStream ais = getArchiveInputStream(is, contentType); - return createFromCompressedStream(ais); - } catch (IOException e) { + Options options = Options.standard().maxSize(maxSizeInBytes).allowDotSegment(true); + switch (contentType) { + case ApplicationApiHandler.APPLICATION_X_GZIP: + return new CompressedApplicationInputStream(ArchiveStreamReader.ofTarGzip(is, options)); + case ApplicationApiHandler.APPLICATION_ZIP: + return new CompressedApplicationInputStream(ArchiveStreamReader.ofZip(is, options)); + default: + throw new BadRequestException("Unable to decompress"); + } + } catch (UncheckedIOException e) { throw new InternalServerException("Unable to create compressed application stream", e); } } - static CompressedApplicationInputStream createFromCompressedStream(ArchiveInputStream ais) { - return new CompressedApplicationInputStream(ais); - } - - private static ArchiveInputStream getArchiveInputStream(InputStream is, String contentTypeHeader) throws IOException { - ArchiveInputStream ais; - switch (contentTypeHeader) { - case ApplicationApiHandler.APPLICATION_X_GZIP: - ais = new TarArchiveInputStream(new GZIPInputStream(is)); - break; - case ApplicationApiHandler.APPLICATION_ZIP: - ais = new ZipArchiveInputStream(is); - break; - default: - throw new BadRequestException("Unable to decompress"); - } - return ais; - } - - private CompressedApplicationInputStream(ArchiveInputStream ais) { - this.ais = ais; - } - /** * Close this stream. * @throws IOException if the stream could not be closed */ public void close() throws IOException { - ais.close(); + reader.close(); } File decompress() throws IOException { @@ -83,45 +71,44 @@ public class CompressedApplicationInputStream implements AutoCloseable { } public File decompress(File dir) throws IOException { - decompressInto(dir); + decompressInto(dir.toPath()); dir = findActualApplicationDir(dir); return dir; } - private void decompressInto(File application) throws IOException { - log.log(Level.FINE, () -> "Application is in " + application.getAbsolutePath()); + private void decompressInto(Path dir) throws IOException { + if (!Files.isDirectory(dir)) throw new IllegalArgumentException("Not a directory: " + dir.toAbsolutePath()); + log.log(Level.FINE, () -> "Application is in " + dir.toAbsolutePath()); int entries = 0; - ArchiveEntry entry; - while ((entry = ais.getNextEntry()) != null) { - log.log(Level.FINE, "Unpacking %s", entry.getName()); - File outFile = new File(application, entry.getName()); - // FIXME/TODO: write more tests that break this logic. I have a feeling it is not very robust. - if (entry.isDirectory()) { - if (!(outFile.exists() && outFile.isDirectory())) { - log.log(Level.FINE, () -> "Creating dir: " + outFile.getAbsolutePath()); - boolean res = outFile.mkdirs(); - if (!res) { - log.log(Level.WARNING, "Could not create dir " + entry.getName()); - } - } - } else { - log.log(Level.FINE, () -> "Creating output file: " + outFile.getAbsolutePath()); - - // Create parent dir if necessary - String parent = outFile.getParent(); - new File(parent).mkdirs(); - - FileOutputStream fos = new FileOutputStream(outFile); - ByteStreams.copy(ais, fos); - fos.close(); + Path tmpFile = null; + OutputStream tmpStream = null; + try { + tmpFile = createTempFile(dir); + tmpStream = Files.newOutputStream(tmpFile); + ArchiveStreamReader.ArchiveFile file; + while ((file = reader.readNextTo(tmpStream)) != null) { + tmpStream.close(); + log.log(Level.FINE, "Creating output file: " + file.path()); + Path dstFile = dir.resolve(file.path().toString()).normalize(); + Files.createDirectories(dstFile.getParent()); + Files.move(tmpFile, dstFile); + tmpFile = createTempFile(dir); + tmpStream = Files.newOutputStream(tmpFile); + entries++; } - entries++; + } finally { + if (tmpStream != null) tmpStream.close(); + if (tmpFile != null) Files.deleteIfExists(tmpFile); } if (entries == 0) { - log.log(Level.WARNING, "Not able to read any entries from " + application.getName()); + log.log(Level.WARNING, "Not able to decompress any entries to " + dir); } } + private static Path createTempFile(Path applicationDir) throws IOException { + return Files.createTempFile(applicationDir, "application", null); + } + private File findActualApplicationDir(File application) { // If application is in e.g. application/, use that as root for UnpackedApplication // TODO: Vespa 8: Remove application/ directory support @@ -131,4 +118,5 @@ public class CompressedApplicationInputStream implements AutoCloseable { } return application; } + } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java index 0ee9bc84ff7..fe9ab82637f 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java @@ -211,7 +211,7 @@ public class ModelContextImpl implements ModelContext { private final boolean useQrserverServiceName; private final boolean avoidRenamingSummaryFeatures; private final boolean experimentalSdParsing; - private final Architecture adminClusterNodeResourcesArchitecture; + private final Architecture adminClusterArchitecture; public FeatureFlags(FlagSource source, ApplicationId appId, Version version) { this.defaultTermwiseLimit = flagValue(source, appId, version, Flags.DEFAULT_TERM_WISE_LIMIT); @@ -260,7 +260,7 @@ public class ModelContextImpl implements ModelContext { this.useQrserverServiceName = flagValue(source, appId, version, Flags.USE_QRSERVER_SERVICE_NAME); this.avoidRenamingSummaryFeatures = flagValue(source, appId, version, Flags.AVOID_RENAMING_SUMMARY_FEATURES); this.experimentalSdParsing = flagValue(source, appId, version, Flags.EXPERIMENTAL_SD_PARSING); - this.adminClusterNodeResourcesArchitecture = Architecture.valueOf(flagValue(source, appId, version, PermanentFlags.ADMIN_CLUSTER_NODE_ARCHITECTURE)); + this.adminClusterArchitecture = Architecture.valueOf(flagValue(source, appId, version, PermanentFlags.ADMIN_CLUSTER_NODE_ARCHITECTURE)); } @Override public double defaultTermwiseLimit() { return defaultTermwiseLimit; } @@ -311,8 +311,7 @@ public class ModelContextImpl implements ModelContext { @Override public boolean useQrserverServiceName() { return useQrserverServiceName; } @Override public boolean avoidRenamingSummaryFeatures() { return avoidRenamingSummaryFeatures; } @Override public boolean experimentalSdParsing() { return experimentalSdParsing; } - @Override public String adminClusterNodeArchitecture() { return adminClusterArchitecture().name(); } - @Override public Architecture adminClusterArchitecture() { return adminClusterNodeResourcesArchitecture; } + @Override public Architecture adminClusterArchitecture() { return adminClusterArchitecture; } private static <V> V flagValue(FlagSource source, ApplicationId appId, Version vespaVersion, UnboundFlag<? extends V, ?, ?> flag) { return flag.bindTo(source) diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationApiHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationApiHandler.java index 1a8b36ee19f..c8953d5996c 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationApiHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationApiHandler.java @@ -49,9 +49,11 @@ public class ApplicationApiHandler extends SessionHandler { public final static String MULTIPART_PARAMS = "prepareParams"; public final static String MULTIPART_APPLICATION_PACKAGE = "applicationPackage"; public final static String contentTypeHeader = "Content-Type"; + private final TenantRepository tenantRepository; private final Duration zookeeperBarrierTimeout; private final Zone zone; + private final long maxApplicationPackageSize; @Inject public ApplicationApiHandler(Context ctx, @@ -61,6 +63,7 @@ public class ApplicationApiHandler extends SessionHandler { super(ctx, applicationRepository); this.tenantRepository = applicationRepository.tenantRepository(); this.zookeeperBarrierTimeout = Duration.ofSeconds(configserverConfig.zookeeper().barrierTimeout()); + this.maxApplicationPackageSize = configserverConfig.maxApplicationPackageSize(); this.zone = zone; } @@ -85,14 +88,14 @@ public class ApplicationApiHandler extends SessionHandler { log.log(Level.FINE, "Deploy parameters: [{0}]", new String(params, StandardCharsets.UTF_8)); prepareParams = PrepareParams.fromJson(params, tenantName, zookeeperBarrierTimeout); Part appPackagePart = parts.get(MULTIPART_APPLICATION_PACKAGE); - compressedStream = createFromCompressedStream(appPackagePart.getInputStream(), appPackagePart.getContentType()); + compressedStream = createFromCompressedStream(appPackagePart.getInputStream(), appPackagePart.getContentType(), maxApplicationPackageSize); } catch (IOException e) { log.log(Level.WARNING, "Unable to parse multipart in deploy", e); throw new BadRequestException("Request contains invalid data"); } } else { prepareParams = PrepareParams.fromHttpRequest(request, tenantName, zookeeperBarrierTimeout); - compressedStream = createFromCompressedStream(request.getData(), request.getHeader(contentTypeHeader)); + compressedStream = createFromCompressedStream(request.getData(), request.getHeader(contentTypeHeader), maxApplicationPackageSize); } PrepareResult result = applicationRepository.deploy(compressedStream, prepareParams); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/DelayedConfigResponses.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/DelayedConfigResponses.java index d3976464bde..7e6fccb6d2f 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/DelayedConfigResponses.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/DelayedConfigResponses.java @@ -193,10 +193,9 @@ public class DelayedConfigResponses { } private synchronized void metricDelayedResponses(ApplicationId app, int elems) { - if ( ! metrics.containsKey(app)) { - metrics.put(app, rpcServer.metricUpdaterFactory().getOrCreateMetricUpdater(Metrics.createDimensions(app))); - } - metrics.get(app).setDelayedResponses(elems); + metrics.computeIfAbsent(app, key -> rpcServer.metricUpdaterFactory() + .getOrCreateMetricUpdater(Metrics.createDimensions(key))) + .setDelayedResponses(elems); } private synchronized void createQueueIfNotExists(GetConfigContext context) { 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 99ffff6403b..ebf1fb32141 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 @@ -243,13 +243,9 @@ public class RpcServer implements Runnable, ReloadListener, TenantListener { } private ApplicationState getState(ApplicationId id) { - ApplicationState state = applicationStateMap.get(id); - if (state == null) { - applicationStateMap.putIfAbsent(id, new ApplicationState(0)); - state = applicationStateMap.get(id); - } - return state; + return applicationStateMap.computeIfAbsent(id, __ -> new ApplicationState(0)); } + boolean hasNewerGeneration(ApplicationId id, long generation) { return getState(id).getActiveGeneration() > generation; } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/CompressedApplicationInputStreamTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/CompressedApplicationInputStreamTest.java index 23444ac53d6..1e8005c8af6 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/CompressedApplicationInputStreamTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/CompressedApplicationInputStreamTest.java @@ -2,10 +2,10 @@ package com.yahoo.vespa.config.server.application; import com.google.common.io.ByteStreams; +import com.yahoo.vespa.config.server.http.InternalServerException; +import com.yahoo.yolean.Exceptions; import org.apache.commons.compress.archivers.ArchiveOutputStream; -import org.apache.commons.compress.archivers.tar.TarArchiveInputStream; import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream; -import org.apache.commons.compress.archivers.zip.ZipArchiveInputStream; import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream; import org.junit.Test; @@ -15,11 +15,10 @@ import java.io.FileOutputStream; import java.io.IOException; import java.util.Arrays; import java.util.List; -import java.util.zip.GZIPInputStream; import java.util.zip.GZIPOutputStream; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; /** @@ -64,9 +63,10 @@ public class CompressedApplicationInputStreamTest { @Test public void require_that_valid_tar_application_can_be_unpacked() throws IOException { File outFile = createTarFile(); - CompressedApplicationInputStream unpacked = CompressedApplicationInputStream.createFromCompressedStream(new TarArchiveInputStream(new GZIPInputStream(new FileInputStream(outFile)))); - File outApp = unpacked.decompress(); - assertTestApp(outApp); + try (CompressedApplicationInputStream unpacked = streamFromTarGz(outFile)) { + File outApp = unpacked.decompress(); + assertTestApp(outApp); + } } @Test @@ -91,48 +91,39 @@ public class CompressedApplicationInputStreamTest { archiveOutputStream.close(); - CompressedApplicationInputStream unpacked = CompressedApplicationInputStream.createFromCompressedStream(new TarArchiveInputStream(new GZIPInputStream(new FileInputStream(outFile)))); - File outApp = unpacked.decompress(); - assertEquals("application", outApp.getName()); // gets the name of the subdir - assertTestApp(outApp); + try (CompressedApplicationInputStream unpacked = streamFromTarGz(outFile)) { + File outApp = unpacked.decompress(); + assertEquals("application", outApp.getName()); // gets the name of the subdir + assertTestApp(outApp); + } } @Test public void require_that_valid_zip_application_can_be_unpacked() throws IOException { File outFile = createZipFile(); - CompressedApplicationInputStream unpacked = CompressedApplicationInputStream.createFromCompressedStream( - new ZipArchiveInputStream(new FileInputStream(outFile))); - File outApp = unpacked.decompress(); - assertTestApp(outApp); + try (CompressedApplicationInputStream unpacked = streamFromZip(outFile)) { + File outApp = unpacked.decompress(); + assertTestApp(outApp); + } } @Test public void require_that_gnu_tared_file_can_be_unpacked() throws IOException, InterruptedException { - File tmpTar = File.createTempFile("myapp", ".tar"); - Process p = new ProcessBuilder("tar", "-C", "src/test/resources/deploy/validapp", "--exclude=.svn", "-cvf", tmpTar.getAbsolutePath(), ".").start(); - p.waitFor(); - p = new ProcessBuilder("gzip", tmpTar.getAbsolutePath()).start(); - p.waitFor(); - File gzFile = new File(tmpTar.getAbsolutePath() + ".gz"); + File gzFile = createTarGz("src/test/resources/deploy/validapp"); assertTrue(gzFile.exists()); - CompressedApplicationInputStream unpacked = CompressedApplicationInputStream.createFromCompressedStream( - new TarArchiveInputStream(new GZIPInputStream(new FileInputStream(gzFile)))); + CompressedApplicationInputStream unpacked = CompressedApplicationInputStream.createFromCompressedStream(new FileInputStream(gzFile), "application/x-gzip", Long.MAX_VALUE); File outApp = unpacked.decompress(); assertTestApp(outApp); } @Test public void require_that_nested_app_can_be_unpacked() throws IOException, InterruptedException { - File tmpTar = File.createTempFile("myapp", ".tar"); - Process p = new ProcessBuilder("tar", "-C", "src/test/resources/deploy/advancedapp", "--exclude=.svn", "-cvf", tmpTar.getAbsolutePath(), ".").start(); - p.waitFor(); - p = new ProcessBuilder("gzip", tmpTar.getAbsolutePath()).start(); - p.waitFor(); - File gzFile = new File(tmpTar.getAbsolutePath() + ".gz"); + File gzFile = createTarGz("src/test/resources/deploy/advancedapp"); assertTrue(gzFile.exists()); - CompressedApplicationInputStream unpacked = CompressedApplicationInputStream.createFromCompressedStream( - new TarArchiveInputStream(new GZIPInputStream(new FileInputStream(gzFile)))); - File outApp = unpacked.decompress(); + File outApp; + try (CompressedApplicationInputStream unpacked = streamFromTarGz(gzFile)) { + outApp = unpacked.decompress(); + } List<File> files = Arrays.asList(outApp.listFiles()); assertEquals(5, files.size()); assertTrue(files.contains(new File(outApp, "services.xml"))); @@ -164,11 +155,29 @@ public class CompressedApplicationInputStreamTest { assertEquals(new File(bar, "lol").getAbsolutePath(), bar.listFiles()[0].getAbsolutePath()); } - - @Test(expected = IOException.class) - public void require_that_invalid_application_returns_error_when_unpacked() throws IOException { + @Test(expected = InternalServerException.class) + public void require_that_invalid_application_returns_error_when_unpacked() throws Exception { File app = new File("src/test/resources/deploy/validapp/services.xml"); - CompressedApplicationInputStream.createFromCompressedStream( - new TarArchiveInputStream(new GZIPInputStream(new FileInputStream(app)))); + streamFromTarGz(app).close(); + } + + private static File createTarGz(String appDir) throws IOException, InterruptedException { + File tmpTar = File.createTempFile("myapp", ".tar"); + Process p = new ProcessBuilder("tar", "-C", appDir, "-cvf", tmpTar.getAbsolutePath(), ".").start(); + p.waitFor(); + p = new ProcessBuilder("gzip", tmpTar.getAbsolutePath()).start(); + p.waitFor(); + File gzFile = new File(tmpTar.getAbsolutePath() + ".gz"); + assertTrue(gzFile.exists()); + return gzFile; + } + + private static CompressedApplicationInputStream streamFromZip(File zipFile) { + return Exceptions.uncheck(() -> CompressedApplicationInputStream.createFromCompressedStream(new FileInputStream(zipFile), "application/zip", Long.MAX_VALUE)); + } + + private static CompressedApplicationInputStream streamFromTarGz(File tarFile) { + return Exceptions.uncheck(() -> CompressedApplicationInputStream.createFromCompressedStream(new FileInputStream(tarFile), "application/x-gzip", Long.MAX_VALUE)); } + } diff --git a/container-core/src/main/java/com/yahoo/container/core/documentapi/DocumentAccessProvider.java b/container-core/src/main/java/com/yahoo/container/core/documentapi/DocumentAccessProvider.java index 44a70ea2f3b..be8ba669ec0 100644 --- a/container-core/src/main/java/com/yahoo/container/core/documentapi/DocumentAccessProvider.java +++ b/container-core/src/main/java/com/yahoo/container/core/documentapi/DocumentAccessProvider.java @@ -19,10 +19,10 @@ public class DocumentAccessProvider implements Provider<VespaDocumentAccess> { private final VespaDocumentAccess access; @Inject - public DocumentAccessProvider(DocumentmanagerConfig documentmanagerConfig, LoadTypeConfig loadTypeConfig, + public DocumentAccessProvider(DocumentmanagerConfig documentmanagerConfig, MessagebusConfig messagebusConfig, DocumentProtocolPoliciesConfig policiesConfig, DistributionConfig distributionConfig) { - this.access = new VespaDocumentAccess(documentmanagerConfig, loadTypeConfig, System.getProperty("config.id"), + this.access = new VespaDocumentAccess(documentmanagerConfig, System.getProperty("config.id"), messagebusConfig, policiesConfig, distributionConfig); } diff --git a/container-core/src/main/java/com/yahoo/container/core/documentapi/VespaDocumentAccess.java b/container-core/src/main/java/com/yahoo/container/core/documentapi/VespaDocumentAccess.java index 6976299cc7d..1775dbe53c1 100644 --- a/container-core/src/main/java/com/yahoo/container/core/documentapi/VespaDocumentAccess.java +++ b/container-core/src/main/java/com/yahoo/container/core/documentapi/VespaDocumentAccess.java @@ -43,13 +43,12 @@ public class VespaDocumentAccess extends DocumentAccess { private boolean shutDown = false; VespaDocumentAccess(DocumentmanagerConfig documentmanagerConfig, - LoadTypeConfig loadTypeConfig, String slobroksConfigId, MessagebusConfig messagebusConfig, DocumentProtocolPoliciesConfig policiesConfig, DistributionConfig distributionConfig) { super(new DocumentAccessParams().setDocumentmanagerConfig(documentmanagerConfig)); - this.parameters = new MessageBusParams(new LoadTypeSet(loadTypeConfig)) + this.parameters = new MessageBusParams() .setDocumentProtocolPoliciesConfig(policiesConfig, distributionConfig); this.parameters.setDocumentmanagerConfig(documentmanagerConfig); this.parameters.getRPCNetworkParams().setSlobrokConfigId(slobroksConfigId); diff --git a/container-core/src/main/java/com/yahoo/container/logging/LogFileHandler.java b/container-core/src/main/java/com/yahoo/container/logging/LogFileHandler.java index c97e128b170..4d6f470637b 100644 --- a/container-core/src/main/java/com/yahoo/container/logging/LogFileHandler.java +++ b/container-core/src/main/java/com/yahoo/container/logging/LogFileHandler.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.container.logging; -import com.yahoo.compress.ZstdOuputStream; +import com.yahoo.compress.ZstdOutputStream; import com.yahoo.io.NativeIO; import com.yahoo.log.LogFileDb; import com.yahoo.protect.Process; @@ -401,7 +401,7 @@ class LogFileHandler <LOGTYPE> { Path compressedFile = Paths.get(oldFile.toString() + ".zst"); int bufferSize = 2*1024*1024; try (FileOutputStream fileOut = AtomicFileOutputStream.create(compressedFile); - ZstdOuputStream out = new ZstdOuputStream(fileOut, bufferSize); + ZstdOutputStream out = new ZstdOutputStream(fileOut, bufferSize); FileInputStream in = new FileInputStream(oldFile.toFile())) { pageFriendlyTransfer(nativeIO, out, fileOut.getFD(), in, bufferSize); out.flush(); diff --git a/container-dev/pom.xml b/container-dev/pom.xml index 034081f4620..6268e1e6fb4 100644 --- a/container-dev/pom.xml +++ b/container-dev/pom.xml @@ -121,6 +121,10 @@ <groupId>io.airlift</groupId> <artifactId>aircompressor</artifactId> </exclusion> + <exclusion> + <groupId>org.apache.commons</groupId> + <artifactId>commons-compress</artifactId> + </exclusion> </exclusions> </dependency> <dependency> diff --git a/container-messagebus/src/main/java/com/yahoo/container/jdisc/messagebus/SessionCache.java b/container-messagebus/src/main/java/com/yahoo/container/jdisc/messagebus/SessionCache.java index 46dcaf17abc..91a3181cb68 100644 --- a/container-messagebus/src/main/java/com/yahoo/container/jdisc/messagebus/SessionCache.java +++ b/container-messagebus/src/main/java/com/yahoo/container/jdisc/messagebus/SessionCache.java @@ -69,14 +69,18 @@ public final class SessionCache extends AbstractComponent { @Inject public SessionCache(NetworkMultiplexerProvider nets, ContainerMbusConfig containerMbusConfig, DocumentTypeManager documentTypeManager, - LoadTypeConfig loadTypeConfig, MessagebusConfig messagebusConfig, + MessagebusConfig messagebusConfig, DocumentProtocolPoliciesConfig policiesConfig, DistributionConfig distributionConfig) { this(nets::net, containerMbusConfig, documentTypeManager, - loadTypeConfig, messagebusConfig, policiesConfig, distributionConfig); + null/*TODO: Remove on Vespa 8*/, messagebusConfig, policiesConfig, distributionConfig); } + /** + * @deprecated load types are deprecated. Use constructor without LoadTypeSet instead. + */ + @Deprecated(forRemoval = true) // TODO: Remove on Vespa 8 public SessionCache(Supplier<NetworkMultiplexer> net, ContainerMbusConfig containerMbusConfig, DocumentTypeManager documentTypeManager, LoadTypeConfig loadTypeConfig, MessagebusConfig messagebusConfig, @@ -86,7 +90,6 @@ public final class SessionCache extends AbstractComponent { containerMbusConfig, messagebusConfig, new DocumentProtocol(documentTypeManager, - new LoadTypeSet(loadTypeConfig), policiesConfig, distributionConfig)); } diff --git a/container-messagebus/src/test/java/com/yahoo/container/jdisc/messagebus/MbusClientProviderTest.java b/container-messagebus/src/test/java/com/yahoo/container/jdisc/messagebus/MbusClientProviderTest.java index c509fb917fa..b9f33506894 100644 --- a/container-messagebus/src/test/java/com/yahoo/container/jdisc/messagebus/MbusClientProviderTest.java +++ b/container-messagebus/src/test/java/com/yahoo/container/jdisc/messagebus/MbusClientProviderTest.java @@ -36,6 +36,7 @@ public class MbusClientProviderTest { testClient(new SessionConfig(builder)); } + @SuppressWarnings("removal") // TODO: Remove on Vespa 8 private void testClient(SessionConfig config) { SessionCache cache = new SessionCache(() -> NetworkMultiplexer.dedicated(new NullNetwork()), new ContainerMbusConfig.Builder().build(), diff --git a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcher.java b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcher.java index 489214bebf8..71a93607b4b 100644 --- a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcher.java +++ b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsStreamingSearcher.java @@ -90,6 +90,7 @@ public class VdsStreamingSearcher extends VespaBackEndSearcher { } @Override + @SuppressWarnings("removal") // TODO: Remove on Vespa 8 public LoadTypeSet getLoadTypeSet() { return ((MessageBusDocumentAccess) access.delegate()).getParams().getLoadTypes(); } diff --git a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsVisitor.java b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsVisitor.java index b2e4821f164..9330e43eaf7 100644 --- a/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsVisitor.java +++ b/container-search/src/main/java/com/yahoo/vespa/streamingvisitors/VdsVisitor.java @@ -76,6 +76,12 @@ class VdsVisitor extends VisitorDataHandler implements Visitor { public interface VisitorSessionFactory { VisitorSession createVisitorSession(VisitorParameters params) throws ParseException; + + /** + * @deprecated load types are deprecated + */ + @Deprecated(forRemoval = true) // TODO: Remove on Vespa 8 + @SuppressWarnings("removal") // TODO: Remove on Vespa 8 LoadTypeSet getLoadTypeSet(); } @@ -119,6 +125,7 @@ class VdsVisitor extends VisitorDataHandler implements Visitor { return query.properties().getString(streamingSelection); } + @SuppressWarnings("removal") // TODO: Remove on Vespa 8 private void setVisitorParameters(String searchCluster, Route route, String documentType) { params.setDocumentSelection(createSelectionString(documentType, createQuerySelectionString())); params.setTimeoutMs(query.getTimeout()); // Per bucket visitor timeout @@ -134,6 +141,7 @@ class VdsVisitor extends VisitorDataHandler implements Visitor { params.visitInconsistentBuckets(true); params.setPriority(DocumentProtocol.Priority.VERY_HIGH); + // TODO remove on Vespa 8 if (query.properties().getString(streamingLoadtype) != null) { LoadType loadType = visitorSessionFactory.getLoadTypeSet().getNameMap().get(query.properties().getString(streamingLoadtype)); if (loadType != null) { diff --git a/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/VdsVisitorTestCase.java b/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/VdsVisitorTestCase.java index 1d07cafeda9..b1bc926daed 100644 --- a/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/VdsVisitorTestCase.java +++ b/container-search/src/test/java/com/yahoo/vespa/streamingvisitors/VdsVisitorTestCase.java @@ -31,8 +31,9 @@ import static org.junit.Assert.*; /** * @author <a href="mailto:ulf@yahoo-inc.com">Ulf Carlin</a> */ +@SuppressWarnings("removal") // TODO: Remove on Vespa 8 public class VdsVisitorTestCase { - private LoadTypeSet loadTypeSet = new LoadTypeSet(); + private LoadTypeSet loadTypeSet = new LoadTypeSet(); // TODO remove on Vespa 8 public VdsVisitorTestCase() { loadTypeSet.addLoadType(1, "low", DocumentProtocol.Priority.LOW_1); @@ -489,7 +490,7 @@ public class VdsVisitorTestCase { private static class MockVisitorSessionFactory implements VdsVisitor.VisitorSessionFactory { private VisitorParameters params; - private LoadTypeSet loadTypeSet; + private LoadTypeSet loadTypeSet; // TODO remove on Vespa 8 private boolean timeoutQuery = false; private boolean failQuery = false; @@ -504,6 +505,7 @@ public class VdsVisitorTestCase { } @Override + // TODO: Remove on Vespa 8 public LoadTypeSet getLoadTypeSet() { return loadTypeSet; } diff --git a/container-test/pom.xml b/container-test/pom.xml index 7c739faad26..d4ea39fa3c9 100644 --- a/container-test/pom.xml +++ b/container-test/pom.xml @@ -92,5 +92,10 @@ <artifactId>aircompressor</artifactId> <scope>compile</scope> </dependency> + <dependency> + <groupId>org.apache.commons</groupId> + <artifactId>commons-compress</artifactId> + <scope>compile</scope> + </dependency> </dependencies> </project> diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepoStats.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepoStats.java index fe7beb538da..68ebc5e86aa 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepoStats.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/configserver/NodeRepoStats.java @@ -20,6 +20,6 @@ public class NodeRepoStats { public Load load() { return load; } public Load activeLoad() { return activeLoad; } - public List<ApplicationStats> applicationStats() { return applicationStats; } + public List<ApplicationStats> applicationStats() { return applicationStats; } } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MailerException.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MailerException.java new file mode 100644 index 00000000000..0febc296fc8 --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/MailerException.java @@ -0,0 +1,14 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.api.integration.organization; + +/** + * MailerException wrap all possible Mailer implementation exceptions + * + * @author enygaard + */ +public class MailerException extends RuntimeException { + + public MailerException(Throwable ex) { + super(ex); + } +} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockMailer.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockMailer.java index 6050f238da7..cb2b76d845c 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockMailer.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockMailer.java @@ -12,9 +12,25 @@ import java.util.Map; public class MockMailer implements Mailer { public final Map<String, List<Mail>> mails = new HashMap<>(); + public final boolean blackhole; + + public MockMailer() { + this(false); + } + + MockMailer(boolean blackhole) { + this.blackhole = blackhole; + } + + public static MockMailer blackhole() { + return new MockMailer(true); + } @Override public void send(Mail mail) { + if (blackhole) { + return; + } for (String recipient : mail.recipients()) { mails.putIfAbsent(recipient, new ArrayList<>()); mails.get(recipient).add(mail); @@ -33,7 +49,10 @@ public class MockMailer implements Mailer { /** Returns the list of mails sent to the given recipient. Modifications affect the set of mails stored in this. */ public List<Mail> inbox(String recipient) { - return mails.get(recipient); + return mails.getOrDefault(recipient, List.of()); } + public void reset() { + mails.clear(); + } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java index 6afeab9b4e6..c35e8c5a7ac 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java @@ -23,6 +23,7 @@ import com.yahoo.vespa.hosted.controller.config.ControllerConfig; import com.yahoo.vespa.hosted.controller.deployment.JobController; import com.yahoo.vespa.hosted.controller.dns.NameServiceForwarder; import com.yahoo.vespa.hosted.controller.notification.NotificationsDb; +import com.yahoo.vespa.hosted.controller.notify.Notifier; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import com.yahoo.vespa.hosted.controller.persistence.JobControlFlags; import com.yahoo.vespa.hosted.controller.security.AccessControl; @@ -88,6 +89,7 @@ public class Controller extends AbstractComponent { private final CuratorArchiveBucketDb archiveBucketDb; private final NotificationsDb notificationsDb; private final SupportAccessControl supportAccessControl; + private final Notifier notifier; /** * Creates a controller @@ -126,6 +128,7 @@ public class Controller extends AbstractComponent { auditLogger = new AuditLogger(curator, clock); jobControl = new JobControl(new JobControlFlags(curator, flagSource)); archiveBucketDb = new CuratorArchiveBucketDb(this); + notifier = new Notifier(curator, serviceRegistry.mailer()); notificationsDb = new NotificationsDb(this); supportAccessControl = new SupportAccessControl(this); @@ -330,4 +333,7 @@ public class Controller extends AbstractComponent { return supportAccessControl; } + public Notifier notifier() { + return notifier; + } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackage.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackage.java index 5c7ae5041fe..258884a4d11 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackage.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackage.java @@ -4,6 +4,9 @@ package com.yahoo.vespa.hosted.controller.application.pkg; import com.google.common.hash.Funnel; import com.google.common.hash.Hashing; import com.yahoo.component.Version; +import com.yahoo.compress.ArchiveStreamReader; +import com.yahoo.compress.ArchiveStreamReader.ArchiveFile; +import com.yahoo.compress.ArchiveStreamReader.Options; import com.yahoo.config.application.FileSystemWrapper; import com.yahoo.config.application.FileSystemWrapper.FileWrapper; import com.yahoo.config.application.XmlPreProcessor; @@ -24,6 +27,7 @@ import com.yahoo.yolean.Exceptions; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.InputStreamReader; +import java.io.OutputStream; import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.Paths; @@ -40,6 +44,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.SortedMap; +import java.util.TreeMap; import java.util.concurrent.ConcurrentSkipListMap; import java.util.function.Function; import java.util.function.Predicate; @@ -108,7 +113,7 @@ public class ApplicationPackage { this.trustedCertificates = files.get(trustedCertificatesFile).map(bytes -> X509CertificateUtils.certificateListFromPem(new String(bytes, UTF_8))).orElse(List.of()); - this.bundleHash = calculateBundleHash(); + this.bundleHash = calculateBundleHash(zippedContent); preProcessAndPopulateCache(); } @@ -120,7 +125,7 @@ public class ApplicationPackage { byte[] certificatesBytes = X509CertificateUtils.toPem(trustedCertificates).getBytes(UTF_8); ByteArrayOutputStream modified = new ByteArrayOutputStream(zippedContent.length + certificatesBytes.length); - ZipStreamReader.transferAndWrite(modified, new ByteArrayInputStream(zippedContent), trustedCertificatesFile, certificatesBytes); + ZipEntries.transferAndWrite(modified, new ByteArrayInputStream(zippedContent), trustedCertificatesFile, certificatesBytes); return new ApplicationPackage(modified.toByteArray()); } @@ -227,15 +232,23 @@ public class ApplicationPackage { } // Hashes all files and settings that require a deployment to be forwarded to configservers - private String calculateBundleHash() { + private String calculateBundleHash(byte[] zippedContent) { Predicate<String> entryMatcher = name -> ! name.endsWith(deploymentFile) && ! name.endsWith(buildMetaFile); - SortedMap<String, Long> entryCRCs = ZipStreamReader.getEntryCRCs(new ByteArrayInputStream(zippedContent), entryMatcher); - Funnel<SortedMap<String, Long>> funnel = (from, into) -> from.entrySet().forEach(entry -> { - into.putBytes(entry.getKey().getBytes()); - into.putLong(entry.getValue()); + SortedMap<String, Long> crcByEntry = new TreeMap<>(); + Options options = Options.standard().pathPredicate(entryMatcher); + ArchiveFile file; + try (ArchiveStreamReader reader = ArchiveStreamReader.ofZip(new ByteArrayInputStream(zippedContent), options)) { + OutputStream discard = OutputStream.nullOutputStream(); + while ((file = reader.readNextTo(discard)) != null) { + crcByEntry.put(file.path().toString(), file.crc32().orElse(-1)); + } + } + Funnel<SortedMap<String, Long>> funnel = (from, into) -> from.forEach((key, value) -> { + into.putBytes(key.getBytes()); + into.putLong(value); }); return Hashing.sha1().newHasher() - .putObject(entryCRCs, funnel) + .putObject(crcByEntry, funnel) .putInt(deploymentSpec.deployableHashCode()) .hash().toString(); } @@ -285,13 +298,13 @@ public class ApplicationPackage { } private Map<Path, Optional<byte[]>> read(Collection<String> names) { - var entries = new ZipStreamReader(new ByteArrayInputStream(zip), - name -> names.contains(withoutLegacyDir(name)), - maxSize, - true) - .entries().stream() - .collect(toMap(entry -> Paths.get(withoutLegacyDir(entry.zipEntry().getName())).normalize(), - ZipStreamReader.ZipEntryWithContent::content)); + var entries = ZipEntries.from(zip, + name -> names.contains(withoutLegacyDir(name)), + maxSize, + true) + .asList().stream() + .collect(toMap(entry -> Paths.get(withoutLegacyDir(entry.name())).normalize(), + ZipEntries.ZipEntryWithContent::content)); names.stream().map(Paths::get).forEach(path -> entries.putIfAbsent(path.normalize(), Optional.empty())); return entries; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageDiff.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageDiff.java index 97810b9de80..e18f6247cb1 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageDiff.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageDiff.java @@ -15,7 +15,7 @@ import java.util.Optional; import java.util.stream.Collectors; import java.util.stream.Stream; -import static com.yahoo.vespa.hosted.controller.application.pkg.ZipStreamReader.ZipEntryWithContent; +import static com.yahoo.vespa.hosted.controller.application.pkg.ZipEntries.ZipEntryWithContent; /** * @author freva @@ -75,8 +75,8 @@ public class ApplicationPackageDiff { } private static Map<String, ZipEntryWithContent> readContents(ApplicationPackage app, int maxFileSizeToDiff) { - return new ZipStreamReader(new ByteArrayInputStream(app.zippedContent()), entry -> true, maxFileSizeToDiff, false).entries().stream() - .collect(Collectors.toMap(entry -> entry.zipEntry().getName(), e -> e)); + return ZipEntries.from(app.zippedContent(), entry -> true, maxFileSizeToDiff, false).asList().stream() + .collect(Collectors.toMap(ZipEntryWithContent::name, e -> e)); } private static List<String> lines(byte[] data) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipEntries.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipEntries.java new file mode 100644 index 00000000000..a6cb7f23fc3 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipEntries.java @@ -0,0 +1,99 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.application.pkg; + +import com.yahoo.compress.ArchiveStreamReader; +import com.yahoo.compress.ArchiveStreamReader.ArchiveFile; +import com.yahoo.compress.ArchiveStreamReader.Options; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.io.UncheckedIOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.function.Predicate; +import java.util.zip.ZipEntry; +import java.util.zip.ZipInputStream; +import java.util.zip.ZipOutputStream; + +/** + * A list of entries read from a ZIP archive, and their contents. + * + * @author bratseth + */ +public class ZipEntries { + + private final List<ZipEntryWithContent> entries; + + private ZipEntries(List<ZipEntryWithContent> entries) { + this.entries = List.copyOf(Objects.requireNonNull(entries)); + } + + /** Copies the zipped content from in to out, adding/overwriting an entry with the given name and content. */ + public static void transferAndWrite(OutputStream out, InputStream in, String name, byte[] content) { + try (ZipOutputStream zipOut = new ZipOutputStream(out); + ZipInputStream zipIn = new ZipInputStream(in)) { + for (ZipEntry entry = zipIn.getNextEntry(); entry != null; entry = zipIn.getNextEntry()) { + if (entry.getName().equals(name)) + continue; + + zipOut.putNextEntry(new ZipEntry(entry.getName())); + zipIn.transferTo(zipOut); + zipOut.closeEntry(); + } + zipOut.putNextEntry(new ZipEntry(name)); + zipOut.write(content); + zipOut.closeEntry(); + } + catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + /** Read ZIP entries from inputStream */ + public static ZipEntries from(byte[] zip, Predicate<String> entryNameMatcher, int maxEntrySizeInBytes, boolean throwIfEntryExceedsMaxSize) { + Options options = Options.standard() + .pathPredicate(entryNameMatcher) + .maxSize(2 * (long) Math.pow(1024, 3)) // 2 GB + .maxEntrySize(maxEntrySizeInBytes) + .maxEntries(1024) + .truncateEntry(!throwIfEntryExceedsMaxSize); + List<ZipEntryWithContent> entries = new ArrayList<>(); + try (ArchiveStreamReader reader = ArchiveStreamReader.ofZip(new ByteArrayInputStream(zip), options)) { + ArchiveFile file; + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + while ((file = reader.readNextTo(baos)) != null) { + entries.add(new ZipEntryWithContent(file.path().toString(), + Optional.of(baos.toByteArray()).filter(b -> b.length > 0), + file.size())); + baos.reset(); + } + } + return new ZipEntries(entries); + } + + public List<ZipEntryWithContent> asList() { return entries; } + + public static class ZipEntryWithContent { + + private final String name; + private final Optional<byte[]> content; + private final long size; + + public ZipEntryWithContent(String name, Optional<byte[]> content, long size) { + this.name = name; + this.content = content; + this.size = size; + } + + public String name() { return name; } + public byte[] contentOrThrow() { return content.orElseThrow(); } + public Optional<byte[]> content() { return content; } + public long size() { return size; } + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipStreamReader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipStreamReader.java deleted file mode 100644 index 174ac4cb8b0..00000000000 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipStreamReader.java +++ /dev/null @@ -1,138 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.application.pkg; - -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import java.io.UncheckedIOException; -import java.nio.file.Path; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.SortedMap; -import java.util.TreeMap; -import java.util.function.Predicate; -import java.util.zip.ZipEntry; -import java.util.zip.ZipInputStream; -import java.util.zip.ZipOutputStream; - -/** - * @author bratseth - */ -public class ZipStreamReader { - - private final List<ZipEntryWithContent> entries = new ArrayList<>(); - private final int maxEntrySizeInBytes; - - public ZipStreamReader(InputStream input, Predicate<String> entryNameMatcher, int maxEntrySizeInBytes, boolean throwIfEntryExceedsMaxSize) { - this.maxEntrySizeInBytes = maxEntrySizeInBytes; - try (ZipInputStream zipInput = new ZipInputStream(input)) { - ZipEntry zipEntry; - - while (null != (zipEntry = zipInput.getNextEntry())) { - if (!entryNameMatcher.test(requireName(zipEntry.getName()))) continue; - entries.add(readContent(zipEntry, zipInput, throwIfEntryExceedsMaxSize)); - } - } catch (IOException e) { - throw new UncheckedIOException("IO error reading zip content", e); - } - } - - /** Copies the zipped content from in to out, adding/overwriting an entry with the given name and content. */ - public static void transferAndWrite(OutputStream out, InputStream in, String name, byte[] content) { - try (ZipOutputStream zipOut = new ZipOutputStream(out); - ZipInputStream zipIn = new ZipInputStream(in)) { - for (ZipEntry entry = zipIn.getNextEntry(); entry != null; entry = zipIn.getNextEntry()) { - if (entry.getName().equals(name)) - continue; - - zipOut.putNextEntry(new ZipEntry(entry.getName())); - zipIn.transferTo(zipOut); - zipOut.closeEntry(); - } - zipOut.putNextEntry(new ZipEntry(name)); - zipOut.write(content); - zipOut.closeEntry(); - } - catch (IOException e) { - throw new UncheckedIOException(e); - } - } - - public static SortedMap<String, Long> getEntryCRCs(InputStream in, Predicate<String> entryNameMatcher) { - SortedMap<String, Long> entryCRCs = new TreeMap<>(); - byte[] buffer = new byte[2048]; - try (ZipInputStream zipIn = new ZipInputStream(in)) { - for (ZipEntry entry = zipIn.getNextEntry(); entry != null; entry = zipIn.getNextEntry()) { - if ( ! entryNameMatcher.test(entry.getName())) - continue; - // CRC is not set until entry is read - while ( -1 != zipIn.read(buffer)){} - entryCRCs.put(entry.getName(), entry.getCrc()); - } - } catch (IOException e) { - throw new UncheckedIOException(e); - } - return entryCRCs; - } - - private ZipEntryWithContent readContent(ZipEntry zipEntry, ZipInputStream zipInput, boolean throwIfEntryExceedsMaxSize) { - try (ByteArrayOutputStream bis = new ByteArrayOutputStream()) { - byte[] buffer = new byte[2048]; - int read; - long size = 0; - while ( -1 != (read = zipInput.read(buffer))) { - size += read; - if (size > maxEntrySizeInBytes) { - if (throwIfEntryExceedsMaxSize) throw new IllegalArgumentException( - "Entry in zip content exceeded size limit of " + maxEntrySizeInBytes + " bytes"); - } else bis.write(buffer, 0, read); - } - - boolean hasContent = size <= maxEntrySizeInBytes; - return new ZipEntryWithContent(zipEntry, - Optional.of(bis).filter(__ -> hasContent).map(ByteArrayOutputStream::toByteArray), - size); - } catch (IOException e) { - throw new UncheckedIOException("Failed reading from zipped content", e); - } - } - - public List<ZipEntryWithContent> entries() { return Collections.unmodifiableList(entries); } - - private static String requireName(String name) { - if (List.of(name.split("/")).contains("..") || - !trimTrailingSlash(name).equals(Path.of(name).normalize().toString())) { - throw new IllegalArgumentException("Unexpected non-normalized path found in zip content: '" + name + "'"); - } - return name; - } - - private static String trimTrailingSlash(String name) { - if (name.endsWith("/")) return name.substring(0, name.length() - 1); - return name; - } - - public static class ZipEntryWithContent { - - private final ZipEntry zipEntry; - private final Optional<byte[]> content; - private final long size; - - public ZipEntryWithContent(ZipEntry zipEntry, Optional<byte[]> content, long size) { - this.zipEntry = zipEntry; - this.content = content; - this.size = size; - } - - public ZipEntry zipEntry() { return zipEntry; } - public byte[] contentOrThrow() { return content.orElseThrow(); } - public Optional<byte[]> content() { return content; } - public long size() { return size; } - } - -} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDb.java index c0bd1ac03ff..5244d46d0a9 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDb.java @@ -9,6 +9,7 @@ import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.api.application.v4.model.ClusterMetrics; import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; +import com.yahoo.vespa.hosted.controller.notify.Notifier; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import java.time.Clock; @@ -32,14 +33,16 @@ public class NotificationsDb { private final Clock clock; private final CuratorDb curatorDb; + private final Notifier notifier; public NotificationsDb(Controller controller) { - this(controller.clock(), controller.curator()); + this(controller.clock(), controller.curator(), controller.notifier()); } - NotificationsDb(Clock clock, CuratorDb curatorDb) { + NotificationsDb(Clock clock, CuratorDb curatorDb, Notifier notifier) { this.clock = clock; this.curatorDb = curatorDb; + this.notifier = notifier; } public List<TenantName> listTenantsWithNotifications() { @@ -61,13 +64,24 @@ public class NotificationsDb { * already exists, it'll be replaced by this one instead */ public void setNotification(NotificationSource source, Type type, Level level, List<String> messages) { + Optional<Notification> changed = Optional.empty(); try (Lock lock = curatorDb.lockNotifications(source.tenant())) { - List<Notification> notifications = curatorDb.readNotifications(source.tenant()).stream() + var existingNotifications = curatorDb.readNotifications(source.tenant()); + List<Notification> notifications = existingNotifications.stream() .filter(notification -> !source.equals(notification.source()) || type != notification.type()) .collect(Collectors.toCollection(ArrayList::new)); - notifications.add(new Notification(clock.instant(), type, level, source, messages)); + var notification = new Notification(clock.instant(), type, level, source, messages); + // Be conservative for now, only dispatch notifications if they are from new source or with new type. + // the message content and level is ignored for now + if (!existingNotifications.stream().anyMatch(n -> n.source().equals(source) && n.type().equals(type))) { + changed = Optional.of(notification); + } + notifications.add(notification); curatorDb.writeNotifications(source.tenant(), notifications); } + if (changed.isPresent()) { + notifier.dispatch(changed.get()); + } } /** Remove the notification with the given source and type */ @@ -131,8 +145,9 @@ public class NotificationsDb { newNotifications.stream()) .collect(Collectors.toUnmodifiableList()); - if (!initial.equals(updated)) + if (!initial.equals(updated)) { curatorDb.writeNotifications(deploymentSource.tenant(), updated); + } } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notify/Notifier.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notify/Notifier.java new file mode 100644 index 00000000000..46e1fd904ed --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notify/Notifier.java @@ -0,0 +1,76 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.notify; + +import com.yahoo.text.Text; +import com.yahoo.vespa.hosted.controller.api.integration.organization.Mail; +import com.yahoo.vespa.hosted.controller.api.integration.organization.Mailer; +import com.yahoo.vespa.hosted.controller.api.integration.organization.MailerException; +import com.yahoo.vespa.hosted.controller.notification.Notification; +import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; +import com.yahoo.vespa.hosted.controller.tenant.CloudTenant; +import com.yahoo.vespa.hosted.controller.tenant.TenantContacts; + +import java.util.Collection; +import java.util.Objects; +import java.util.logging.Level; +import java.util.logging.Logger; +import java.util.stream.Collectors; + +/** + * Notifier is responsible for dispatching user notifications to their chosen Contact points. + * + * @author enygaard + */ +public class Notifier { + private final CuratorDb curatorDb; + private final Mailer mailer; + + private static final Logger log = Logger.getLogger(Notifier.class.getName()); + + public Notifier(CuratorDb curatorDb, Mailer mailer) { + this.curatorDb = Objects.requireNonNull(curatorDb); + this.mailer = Objects.requireNonNull(mailer); + } + + public void dispatch(Notification notification) { + var tenant = curatorDb.readTenant(notification.source().tenant()); + tenant.stream().forEach(t -> { + if (t instanceof CloudTenant) { + var ct = (CloudTenant) t; + ct.info().contacts().all().stream() + .filter(c -> c.audiences().contains(TenantContacts.Audience.NOTIFICATIONS)) + .collect(Collectors.groupingBy(TenantContacts.Contact::type, Collectors.toList())) + .entrySet() + .forEach(e -> dispatch(notification, e.getKey(), e.getValue())); + } + }); + } + + private void dispatch(Notification notification, TenantContacts.Type type, Collection<? extends TenantContacts.Contact> contacts) { + switch (type) { + case EMAIL: + dispatch(notification, contacts.stream().map(c -> (TenantContacts.EmailContact) c).collect(Collectors.toList())); + break; + default: + throw new IllegalArgumentException("Unknown TenantContacts type " + type.name()); + } + } + + private void dispatch(Notification notification, Collection<TenantContacts.EmailContact> contacts) { + try { + mailer.send(mailOf(notification, contacts.stream().map(c -> c.email()).collect(Collectors.toList()))); + } catch (MailerException e) { + log.log(Level.SEVERE, "Failed sending email", e); + } + } + + private Mail mailOf(Notification n, Collection<String> recipients) { + var subject = Text.format("[%s] Vespa Notification for %s", n.level().toString().toUpperCase(), n.type().name()); + var body = new StringBuilder(); + body.append("Source: ").append(n.source().toString()).append("\n") + .append("\n") + .append(String.join("\n", n.messages())); + return new Mail(recipients, subject.toString(), body.toString()); + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java index 158cc6caede..1a4a42cb521 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java @@ -30,6 +30,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.TreeMap; import java.util.logging.Level; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -132,7 +133,7 @@ public class DeploymentApiHandler extends ThreadedHttpRequestHandler { entry -> entry.getValue().instanceJobs().get(entry.getKey()))); Cursor productionArray = versionObject.setArray("productionApplications"); statistics.productionSuccesses().stream() - .collect(groupingBy(run -> run.id().application())) + .collect(groupingBy(run -> run.id().application(), TreeMap::new, toList())) .forEach((id, runs) -> { Cursor applicationObject = productionArray.addObject(); toSlime(applicationObject, id, request); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java index 709a7967a5e..4e155e937b9 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java @@ -5,7 +5,6 @@ import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.application.api.ValidationId; import org.junit.Test; -import java.io.ByteArrayInputStream; import java.nio.file.Files; import java.nio.file.Path; import java.time.Instant; @@ -154,9 +153,9 @@ public class ApplicationPackageTest { } private static Map<String, String> unzip(byte[] zip) { - return new ZipStreamReader(new ByteArrayInputStream(zip), __ -> true, 1 << 10, true) - .entries().stream() - .collect(Collectors.toMap(entry -> entry.zipEntry().getName(), + return ZipEntries.from(zip, __ -> true, 1 << 10, true) + .asList().stream() + .collect(Collectors.toMap(ZipEntries.ZipEntryWithContent::name, entry -> new String(entry.contentOrThrow(), UTF_8))); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipEntriesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipEntriesTest.java new file mode 100644 index 00000000000..6908464640b --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipEntriesTest.java @@ -0,0 +1,50 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.application.pkg; + +import com.yahoo.security.KeyAlgorithm; +import com.yahoo.security.KeyUtils; +import com.yahoo.security.SignatureAlgorithm; +import com.yahoo.security.X509CertificateBuilder; +import org.junit.Test; + +import javax.security.auth.x500.X500Principal; +import java.math.BigInteger; +import java.security.KeyPair; +import java.security.cert.X509Certificate; +import java.time.Instant; +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +import static org.junit.Assert.assertEquals; + +/** + * @author mpolden + */ +public class ZipEntriesTest { + + @Test + public void test_replacement() { + ApplicationPackage applicationPackage = new ApplicationPackage(new byte[0]); + List<X509Certificate> certificates = IntStream.range(0, 3) + .mapToObj(i -> { + KeyPair keyPair = KeyUtils.generateKeypair(KeyAlgorithm.EC, 256); + X500Principal subject = new X500Principal("CN=subject" + i); + return X509CertificateBuilder.fromKeypair(keyPair, + subject, + Instant.now(), + Instant.now().plusSeconds(1), + SignatureAlgorithm.SHA512_WITH_ECDSA, + BigInteger.valueOf(1)) + .build(); + }) + .collect(Collectors.toUnmodifiableList()); + + assertEquals(List.of(), applicationPackage.trustedCertificates()); + for (int i = 0; i < certificates.size(); i++) { + applicationPackage = applicationPackage.withTrustedCertificate(certificates.get(i)); + assertEquals(certificates.subList(0, i + 1), applicationPackage.trustedCertificates()); + } + } + +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipStreamReaderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipStreamReaderTest.java deleted file mode 100644 index 33c18d123d2..00000000000 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ZipStreamReaderTest.java +++ /dev/null @@ -1,114 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.application.pkg; - -import com.yahoo.security.KeyAlgorithm; -import com.yahoo.security.KeyUtils; -import com.yahoo.security.SignatureAlgorithm; -import com.yahoo.security.X509CertificateBuilder; -import org.junit.Test; - -import javax.security.auth.x500.X500Principal; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.UncheckedIOException; -import java.math.BigInteger; -import java.nio.charset.StandardCharsets; -import java.security.KeyPair; -import java.security.cert.X509Certificate; -import java.time.Instant; -import java.util.List; -import java.util.Map; -import java.util.stream.Collectors; -import java.util.stream.IntStream; -import java.util.zip.ZipEntry; -import java.util.zip.ZipOutputStream; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - -/** - * @author mpolden - */ -public class ZipStreamReaderTest { - - @Test - public void test_size_limit() { - Map<String, String> entries = Map.of("foo.xml", "foobar"); - try { - new ZipStreamReader(new ByteArrayInputStream(zip(entries)), "foo.xml"::equals, 1, true); - fail("Expected exception"); - } catch (IllegalArgumentException ignored) {} - - entries = Map.of("foo.xml", "foobar", - "foo.jar", "0".repeat(100) // File not extracted and thus not subject to size limit - ); - ZipStreamReader reader = new ZipStreamReader(new ByteArrayInputStream(zip(entries)), "foo.xml"::equals, 10, true); - byte[] extracted = reader.entries().get(0).contentOrThrow(); - assertEquals("foobar", new String(extracted, StandardCharsets.UTF_8)); - } - - @Test - public void test_paths() { - Map<String, Boolean> tests = Map.of( - "../../services.xml", true, - "/../.././services.xml", true, - "./application/././services.xml", true, - "application//services.xml", true, - "artifacts/", false, // empty dir - "services..xml", false, - "application/services.xml", false, - "components/foo-bar-deploy.jar", false, - "services.xml", false - ); - tests.forEach((name, expectException) -> { - try { - new ZipStreamReader(new ByteArrayInputStream(zip(Map.of(name, "foo"))), name::equals, 1024, true); - assertFalse("Expected exception for '" + name + "'", expectException); - } catch (IllegalArgumentException ignored) { - assertTrue("Unexpected exception for '" + name + "'", expectException); - } - }); - } - - @Test - public void test_replacement() { - ApplicationPackage applicationPackage = new ApplicationPackage(new byte[0]); - List<X509Certificate> certificates = IntStream.range(0, 3) - .mapToObj(i -> { - KeyPair keyPair = KeyUtils.generateKeypair(KeyAlgorithm.EC, 256); - X500Principal subject = new X500Principal("CN=subject" + i); - return X509CertificateBuilder.fromKeypair(keyPair, - subject, - Instant.now(), - Instant.now().plusSeconds(1), - SignatureAlgorithm.SHA512_WITH_ECDSA, - BigInteger.valueOf(1)) - .build(); - }) - .collect(Collectors.toUnmodifiableList()); - - assertEquals(List.of(), applicationPackage.trustedCertificates()); - for (int i = 0; i < certificates.size(); i++) { - applicationPackage = applicationPackage.withTrustedCertificate(certificates.get(i)); - assertEquals(certificates.subList(0, i + 1), applicationPackage.trustedCertificates()); - } - } - - private static byte[] zip(Map<String, String> entries) { - ByteArrayOutputStream zip = new ByteArrayOutputStream(); - try (ZipOutputStream out = new ZipOutputStream(zip)) { - for (Map.Entry<String, String> entry : entries.entrySet()) { - out.putNextEntry(new ZipEntry(entry.getKey())); - out.write(entry.getValue().getBytes(StandardCharsets.UTF_8)); - out.closeEntry(); - } - } catch (IOException e) { - throw new UncheckedIOException(e); - } - return zip.toByteArray(); - } - -} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java index 640e6860eb6..a464e3d7e9b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/ApplicationPackageBuilder.java @@ -264,36 +264,38 @@ public class ApplicationPackageBuilder { xml.append(athenzIdentityAttributes); } xml.append(">\n"); - xml.append(" <instance id='").append(instances).append("'>\n"); - if (upgradePolicy != null || revisionTarget != null || revisionChange != null || upgradeRollout != null) { - xml.append(" <upgrade "); - if (upgradePolicy != null) xml.append("policy='").append(upgradePolicy).append("' "); - if (revisionTarget != null) xml.append("revision-target='").append(revisionTarget).append("' "); - if (revisionChange != null) xml.append("revision-change='").append(revisionChange).append("' "); - if (upgradeRollout != null) xml.append("rollout='").append(upgradeRollout).append("' "); - xml.append("/>\n"); - } - xml.append(notifications); - if (explicitSystemTest) - xml.append(" <test />\n"); - if (explicitStagingTest) - xml.append(" <staging />\n"); - xml.append(blockChange); - xml.append(" <prod"); - if (globalServiceId != null) { - xml.append(" global-service-id='"); - xml.append(globalServiceId); - xml.append("'"); - } - xml.append(">\n"); - xml.append(prodBody); - xml.append(" </prod>\n"); - if (endpointsBody.length() > 0 ) { - xml.append(" <endpoints>\n"); - xml.append(endpointsBody); - xml.append(" </endpoints>\n"); + for (String instance : instances.split(",")) { + xml.append(" <instance id='").append(instance).append("'>\n"); + if (upgradePolicy != null || revisionTarget != null || revisionChange != null || upgradeRollout != null) { + xml.append(" <upgrade "); + if (upgradePolicy != null) xml.append("policy='").append(upgradePolicy).append("' "); + if (revisionTarget != null) xml.append("revision-target='").append(revisionTarget).append("' "); + if (revisionChange != null) xml.append("revision-change='").append(revisionChange).append("' "); + if (upgradeRollout != null) xml.append("rollout='").append(upgradeRollout).append("' "); + xml.append("/>\n"); + } + xml.append(notifications); + if (explicitSystemTest) + xml.append(" <test />\n"); + if (explicitStagingTest) + xml.append(" <staging />\n"); + xml.append(blockChange); + xml.append(" <prod"); + if (globalServiceId != null) { + xml.append(" global-service-id='"); + xml.append(globalServiceId); + xml.append("'"); + } + xml.append(">\n"); + xml.append(prodBody); + xml.append(" </prod>\n"); + if (endpointsBody.length() > 0) { + xml.append(" <endpoints>\n"); + xml.append(endpointsBody); + xml.append(" </endpoints>\n"); + } + xml.append(" </instance>\n"); } - xml.append(" </instance>\n"); if (applicationEndpointsBody.length() > 0) { xml.append(" <endpoints>\n"); xml.append(applicationEndpointsBody); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java index 989a7c31821..86c21839c96 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java @@ -32,7 +32,6 @@ import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.EndpointId; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; -import com.yahoo.vespa.hosted.controller.application.pkg.ZipStreamReader; import com.yahoo.vespa.hosted.controller.integration.ConfigServerMock; import com.yahoo.vespa.hosted.controller.maintenance.JobRunner; import com.yahoo.vespa.hosted.controller.maintenance.NameServiceDispatcher; @@ -41,8 +40,6 @@ import com.yahoo.vespa.hosted.controller.routing.RoutingPolicy; import com.yahoo.vespa.hosted.controller.routing.RoutingPolicyId; import javax.security.auth.x500.X500Principal; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; import java.math.BigInteger; import java.security.KeyPair; import java.security.cert.X509Certificate; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java index a2a1b4ba0a1..7b67db39350 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/NodeRepositoryMock.java @@ -26,6 +26,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.TreeMap; import java.util.function.UnaryOperator; import java.util.stream.Collectors; @@ -224,7 +225,7 @@ public class NodeRepositoryMock implements NodeRepository { } public void putApplication(ZoneId zone, Application application) { - applications.computeIfAbsent(zone, (k) -> new HashMap<>()) + applications.computeIfAbsent(zone, (k) -> new TreeMap<>()) .put(application.id(), application); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java index d51856b329d..a5655d2e6eb 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.notification; +import com.google.common.collect.ImmutableBiMap; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.TenantName; @@ -11,8 +12,14 @@ import com.yahoo.vespa.hosted.controller.api.application.v4.model.ClusterMetrics import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; +import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockMailer; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; +import com.yahoo.vespa.hosted.controller.notify.Notifier; import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb; +import com.yahoo.vespa.hosted.controller.tenant.CloudTenant; +import com.yahoo.vespa.hosted.controller.tenant.LastLoginInfo; +import com.yahoo.vespa.hosted.controller.tenant.TenantContacts; +import com.yahoo.vespa.hosted.controller.tenant.TenantInfo; import org.junit.Before; import org.junit.Test; @@ -22,6 +29,7 @@ import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.stream.Collectors; import static com.yahoo.vespa.hosted.controller.notification.Notification.Level; @@ -36,6 +44,19 @@ import static org.junit.Assert.assertTrue; public class NotificationsDbTest { private static final TenantName tenant = TenantName.from("tenant1"); + private static final String email = "user1@example.com"; + private static final CloudTenant cloudTenant = new CloudTenant(tenant, + Instant.now(), + LastLoginInfo.EMPTY, + Optional.empty(), + ImmutableBiMap.of(), + TenantInfo.empty() + .withContacts(new TenantContacts( + List.of(new TenantContacts.EmailContact( + List.of(TenantContacts.Audience.NOTIFICATIONS), + email)))), + List.of(), + Optional.empty()); private static final List<Notification> notifications = List.of( notification(1001, Type.deployment, Level.error, NotificationSource.from(tenant), "tenant msg"), notification(1101, Type.applicationPackage, Level.warning, NotificationSource.from(TenantAndApplicationId.from(tenant.value(), "app1")), "app msg"), @@ -46,7 +67,8 @@ public class NotificationsDbTest { private final ManualClock clock = new ManualClock(Instant.ofEpochSecond(12345)); private final MockCuratorDb curatorDb = new MockCuratorDb(); - private final NotificationsDb notificationsDb = new NotificationsDb(clock, curatorDb); + private final MockMailer mailer = new MockMailer(); + private final NotificationsDb notificationsDb = new NotificationsDb(clock, curatorDb, new Notifier(curatorDb, mailer)); @Test public void list_test() { @@ -75,6 +97,29 @@ public class NotificationsDbTest { } @Test + public void notifier_test() { + Notification notification1 = notification(12345, Type.deployment, Level.warning, NotificationSource.from(ApplicationId.from(tenant.value(), "app2", "instance2")), "instance msg #2"); + Notification notification2 = notification(12345, Type.deployment, Level.error, NotificationSource.from(ApplicationId.from(tenant.value(), "app3", "instance2")), "instance msg #3"); + Notification notification3 = notification(12345, Type.reindex, Level.warning, NotificationSource.from(ApplicationId.from(tenant.value(), "app2", "instance2")), "instance msg #2"); + + var a = notifications.get(0); + notificationsDb.setNotification(a.source(), a.type(), a.level(), a.messages()); + assertEquals(0, mailer.inbox(email).size()); + + // Replace the 3rd notification. but don't change source or type + notificationsDb.setNotification(notification1.source(), notification1.type(), notification1.level(), notification1.messages()); + assertEquals(0, mailer.inbox(email).size()); + + // Notification for a new app, add without replacement + notificationsDb.setNotification(notification2.source(), notification2.type(), notification2.level(), notification2.messages()); + assertEquals(1, mailer.inbox(email).size()); + + // Notification for new type on existing app + notificationsDb.setNotification(notification3.source(), notification3.type(), notification3.level(), notification3.messages()); + assertEquals(2, mailer.inbox(email).size()); + } + + @Test public void remove_single_test() { // Remove the 3rd notification notificationsDb.removeNotification(NotificationSource.from(ApplicationId.from(tenant.value(), "app2", "instance2")), Type.deployment); @@ -160,6 +205,8 @@ public class NotificationsDbTest { @Before public void init() { curatorDb.writeNotifications(tenant, notifications); + curatorDb.writeTenant(cloudTenant); + mailer.reset(); } private static List<Notification> notificationIndices(int... indices) { diff --git a/docproc/src/main/java/com/yahoo/docproc/jdisc/messagebus/MessageFactory.java b/docproc/src/main/java/com/yahoo/docproc/jdisc/messagebus/MessageFactory.java index 4668942b61e..1fe6d4cc86c 100644 --- a/docproc/src/main/java/com/yahoo/docproc/jdisc/messagebus/MessageFactory.java +++ b/docproc/src/main/java/com/yahoo/docproc/jdisc/messagebus/MessageFactory.java @@ -21,12 +21,13 @@ import java.util.logging.Logger; /** * @author Einar M R Rosenvinge */ +@SuppressWarnings("removal") // TODO: Remove on Vespa 8 class MessageFactory { private final static Logger log = Logger.getLogger(MessageFactory.class.getName()); private final Message requestMsg; - private final LoadType loadType; - private final DocumentProtocol.Priority priority; + private final LoadType loadType; // TODO: Remove on Vespa 8 + private final DocumentProtocol.Priority priority; // TODO: Remove on Vespa 8 @SuppressWarnings("removal") // TODO: Remove on Vespa 8 public MessageFactory(DocumentMessage requestMsg) { diff --git a/document/src/main/java/com/yahoo/document/datatypes/StringFieldValue.java b/document/src/main/java/com/yahoo/document/datatypes/StringFieldValue.java index de96c548652..51b068b4712 100644 --- a/document/src/main/java/com/yahoo/document/datatypes/StringFieldValue.java +++ b/document/src/main/java/com/yahoo/document/datatypes/StringFieldValue.java @@ -17,7 +17,7 @@ import com.yahoo.vespa.objects.Ids; import java.util.Collection; import java.util.HashMap; import java.util.Map; -import java.util.OptionalInt; +import java.util.Objects; /** * A StringFieldValue is a wrapper class that holds a String in {@link com.yahoo.document.Document}s and @@ -57,10 +57,9 @@ public class StringFieldValue extends FieldValue { } private static void validateTextString(String value) { - OptionalInt illegalCodePoint = Text.validateTextString(value); - if (illegalCodePoint.isPresent()) { + if ( ! Text.isValidTextString(value)) { throw new IllegalArgumentException("The string field value contains illegal code point 0x" + - Integer.toHexString(illegalCodePoint.getAsInt()).toUpperCase()); + Integer.toHexString(Text.validateTextString(value).getAsInt()).toUpperCase()); } } @@ -88,7 +87,7 @@ public class StringFieldValue extends FieldValue { public StringFieldValue clone() { StringFieldValue strfval = (StringFieldValue) super.clone(); if (spanTrees != null) { - strfval.spanTrees = new HashMap<String, SpanTree>(spanTrees.size()); + strfval.spanTrees = new HashMap<>(spanTrees.size()); for (Map.Entry<String, SpanTree> entry : spanTrees.entrySet()) { strfval.spanTrees.put(entry.getKey(), new SpanTree(entry.getValue())); } @@ -240,8 +239,8 @@ public class StringFieldValue extends FieldValue { if (!(o instanceof StringFieldValue)) return false; if (!super.equals(o)) return false; StringFieldValue that = (StringFieldValue) o; - if ((spanTrees != null) ? !spanTrees.equals(that.spanTrees) : that.spanTrees != null) return false; - if ((value != null) ? !value.equals(that.value) : that.value != null) return false; + if (!Objects.equals(spanTrees, that.spanTrees)) return false; + if (!Objects.equals(value, that.value)) return false; return true; } diff --git a/document/src/main/java/com/yahoo/document/idstring/IdString.java b/document/src/main/java/com/yahoo/document/idstring/IdString.java index 55beff9eef9..40ac19dec6d 100644 --- a/document/src/main/java/com/yahoo/document/idstring/IdString.java +++ b/document/src/main/java/com/yahoo/document/idstring/IdString.java @@ -5,8 +5,6 @@ import com.yahoo.api.annotations.Beta; import com.yahoo.text.Text; import com.yahoo.text.Utf8String; -import java.util.OptionalInt; - /** * To be used with DocumentId constructor. * @@ -81,10 +79,9 @@ public abstract class IdString { } private static void validateTextString(String id) { - OptionalInt illegalCodePoint = Text.validateTextString(id); - if (illegalCodePoint.isPresent()) { + if ( ! Text.isValidTextString(id)) { throw new IllegalArgumentException("Unparseable id '" + id + "': Contains illegal code point 0x" + - Integer.toHexString(illegalCodePoint.getAsInt()).toUpperCase()); + Integer.toHexString(Text.validateTextString(id).getAsInt()).toUpperCase()); } } diff --git a/document/src/tests/repo/doctype_config_test.cpp b/document/src/tests/repo/doctype_config_test.cpp index 84ec1414fcc..eab40e04617 100644 --- a/document/src/tests/repo/doctype_config_test.cpp +++ b/document/src/tests/repo/doctype_config_test.cpp @@ -659,4 +659,14 @@ TEST("Tensor fields have tensor types") { EXPECT_TRUE(&tensorField1.getDataType() == tensorFieldValue1->getDataType()); } +TEST("requireThatImportedFieldsWorks") { + DocumentTypeRepo repo(readDocumenttypesConfig(TEST_PATH("import-dt.cfg"))); + ASSERT_TRUE(repo.getDocumentType("document")); + ASSERT_TRUE(repo.getDocumentType("grandparent")); + ASSERT_TRUE(repo.getDocumentType("parent_a")); + ASSERT_TRUE(repo.getDocumentType("parent_b")); + ASSERT_TRUE(repo.getDocumentType("child")); +} + + TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/document/src/tests/repo/import-dt.cfg b/document/src/tests/repo/import-dt.cfg new file mode 100644 index 00000000000..742f377e65c --- /dev/null +++ b/document/src/tests/repo/import-dt.cfg @@ -0,0 +1,139 @@ +enablecompression false +usev8geopositions false +doctype[0].name "document" +doctype[0].idx 10000 +doctype[0].internalid 8 +doctype[0].contentstruct 10001 +doctype[0].primitivetype[0].idx 10002 +doctype[0].primitivetype[0].name "bool" +doctype[0].primitivetype[1].idx 10003 +doctype[0].primitivetype[1].name "byte" +doctype[0].primitivetype[2].idx 10004 +doctype[0].primitivetype[2].name "double" +doctype[0].primitivetype[3].idx 10005 +doctype[0].primitivetype[3].name "float" +doctype[0].primitivetype[4].idx 10006 +doctype[0].primitivetype[4].name "float16" +doctype[0].primitivetype[5].idx 10007 +doctype[0].primitivetype[5].name "int" +doctype[0].primitivetype[6].idx 10008 +doctype[0].primitivetype[6].name "long" +doctype[0].primitivetype[7].idx 10010 +doctype[0].primitivetype[7].name "predicate" +doctype[0].primitivetype[8].idx 10011 +doctype[0].primitivetype[8].name "raw" +doctype[0].primitivetype[9].idx 10012 +doctype[0].primitivetype[9].name "string" +doctype[0].primitivetype[10].idx 10014 +doctype[0].primitivetype[10].name "uri" +doctype[0].wsettype[0].idx 10013 +doctype[0].wsettype[0].elementtype 10012 +doctype[0].wsettype[0].createifnonexistent true +doctype[0].wsettype[0].removeifzero true +doctype[0].wsettype[0].internalid 18 +doctype[0].structtype[0].idx 10001 +doctype[0].structtype[0].name "document.header" +doctype[0].structtype[0].internalid -284186494 +doctype[0].structtype[1].idx 10009 +doctype[0].structtype[1].name "position" +doctype[0].structtype[1].field[0].name "x" +doctype[0].structtype[1].field[0].internalid 914677694 +doctype[0].structtype[1].field[0].type 10007 +doctype[0].structtype[1].field[1].name "y" +doctype[0].structtype[1].field[1].internalid 900009410 +doctype[0].structtype[1].field[1].type 10007 +doctype[0].structtype[1].internalid 1381038251 +doctype[1].name "child" +doctype[1].idx 10015 +doctype[1].internalid 746267614 +doctype[1].inherits[0].idx 10000 +doctype[1].contentstruct 10016 +doctype[1].fieldsets{myfieldset}.fields[0] "my_ancient_int_field" +doctype[1].fieldsets{myfieldset}.fields[1] "my_int_field" +doctype[1].fieldsets{myfieldset}.fields[2] "my_string_field" +doctype[1].fieldsets{[document]}.fields[0] "a_ref" +doctype[1].fieldsets{[document]}.fields[1] "b_ref" +doctype[1].fieldsets{[document]}.fields[2] "b_ref_with_summary" +doctype[1].importedfield[0].name "my_int_field" +doctype[1].importedfield[1].name "my_string_field" +doctype[1].importedfield[2].name "my_int_array_field" +doctype[1].importedfield[3].name "my_int_wset_field" +doctype[1].importedfield[4].name "my_ancient_int_field" +doctype[1].documentref[0].idx 10017 +doctype[1].documentref[0].targettype 10018 +doctype[1].documentref[0].internalid -1586898847 +doctype[1].documentref[1].idx 10019 +doctype[1].documentref[1].targettype 10020 +doctype[1].documentref[1].internalid -1586898816 +doctype[1].structtype[0].idx 10016 +doctype[1].structtype[0].name "child.header" +doctype[1].structtype[0].field[0].name "a_ref" +doctype[1].structtype[0].field[0].internalid 16961427 +doctype[1].structtype[0].field[0].type 10017 +doctype[1].structtype[0].field[1].name "b_ref" +doctype[1].structtype[0].field[1].internalid 590403512 +doctype[1].structtype[0].field[1].type 10019 +doctype[1].structtype[0].field[2].name "b_ref_with_summary" +doctype[1].structtype[0].field[2].internalid 232360236 +doctype[1].structtype[0].field[2].type 10019 +doctype[1].structtype[0].internalid 81425825 +doctype[2].name "grandparent" +doctype[2].idx 10021 +doctype[2].internalid -154107656 +doctype[2].inherits[0].idx 10000 +doctype[2].contentstruct 10022 +doctype[2].fieldsets{[document]}.fields[0] "int_field" +doctype[2].structtype[0].idx 10022 +doctype[2].structtype[0].name "grandparent.header" +doctype[2].structtype[0].field[0].name "int_field" +doctype[2].structtype[0].field[0].internalid 2128822283 +doctype[2].structtype[0].field[0].type 10007 +doctype[2].structtype[0].internalid 990971719 +doctype[3].name "parent_a" +doctype[3].idx 10018 +doctype[3].internalid -244366130 +doctype[3].inherits[0].idx 10000 +doctype[3].contentstruct 10023 +doctype[3].fieldsets{[document]}.fields[0] "grandparent_ref" +doctype[3].fieldsets{[document]}.fields[1] "int_array_field" +doctype[3].fieldsets{[document]}.fields[2] "int_field" +doctype[3].fieldsets{[document]}.fields[3] "int_wset_field" +doctype[3].importedfield[0].name "ancient_int_field" +doctype[3].arraytype[0].idx 10025 +doctype[3].arraytype[0].elementtype 10007 +doctype[3].arraytype[0].internalid -1245117006 +doctype[3].wsettype[0].idx 10026 +doctype[3].wsettype[0].elementtype 10007 +doctype[3].wsettype[0].createifnonexistent false +doctype[3].wsettype[0].removeifzero false +doctype[3].wsettype[0].internalid 519906144 +doctype[3].documentref[0].idx 10024 +doctype[3].documentref[0].targettype 10021 +doctype[3].documentref[0].internalid -1714181319 +doctype[3].structtype[0].idx 10023 +doctype[3].structtype[0].name "parent_a.header" +doctype[3].structtype[0].field[0].name "grandparent_ref" +doctype[3].structtype[0].field[0].internalid 29565679 +doctype[3].structtype[0].field[0].type 10024 +doctype[3].structtype[0].field[1].name "int_field" +doctype[3].structtype[0].field[1].internalid 2128822283 +doctype[3].structtype[0].field[1].type 10007 +doctype[3].structtype[0].field[2].name "int_array_field" +doctype[3].structtype[0].field[2].internalid 85807681 +doctype[3].structtype[0].field[2].type 10025 +doctype[3].structtype[0].field[3].name "int_wset_field" +doctype[3].structtype[0].field[3].internalid 1945161474 +doctype[3].structtype[0].field[3].type 10026 +doctype[3].structtype[0].internalid 236742321 +doctype[4].name "parent_b" +doctype[4].idx 10020 +doctype[4].internalid -244365169 +doctype[4].inherits[0].idx 10000 +doctype[4].contentstruct 10027 +doctype[4].fieldsets{[document]}.fields[0] "string_field" +doctype[4].structtype[0].idx 10027 +doctype[4].structtype[0].name "parent_b.header" +doctype[4].structtype[0].field[0].name "string_field" +doctype[4].structtype[0].field[0].internalid 1222457840 +doctype[4].structtype[0].field[0].type 10012 +doctype[4].structtype[0].internalid 40228816 diff --git a/document/src/vespa/document/repo/documenttyperepo.cpp b/document/src/vespa/document/repo/documenttyperepo.cpp index d8f272d5d55..147e79bbf32 100644 --- a/document/src/vespa/document/repo/documenttyperepo.cpp +++ b/document/src/vespa/document/repo/documenttyperepo.cpp @@ -37,12 +37,20 @@ namespace document { namespace internal { -using DocumentTypeMapT = vespalib::hash_map<int32_t, DataTypeRepo *>; +using DocumentTypeMapT = std::map<int32_t, std::unique_ptr<DataTypeRepo>>; class DocumentTypeMap : public DocumentTypeMapT { public: using DocumentTypeMapT::DocumentTypeMapT; + DataTypeRepo * findRepo(int32_t doc_type_id) const { + auto iter = find(doc_type_id); + if (iter == end()) { + return nullptr; + } else { + return iter->second.get(); + } + } }; } @@ -50,28 +58,16 @@ public: using DocumentTypeMap = internal::DocumentTypeMap; namespace { -template <typename Container> -void DeleteContent(Container &c) { - for (auto ptr : c) { - delete ptr; - } -} -template <typename Map> -void DeleteMapContent(Map &m) { - for (auto & entry : m) { - delete entry.second; - } -} // A collection of data types. class Repo { - vector<const DataType *> _owned_types; - hash_map<int32_t, const DataType *> _types; + vector<std::unique_ptr<const DataType>> _owned_types; + hash_map<int32_t, const DataType *> _id_map; hash_map<string, const DataType *> _tensorTypes; hash_map<string, const DataType *> _name_map; public: - ~Repo() { DeleteContent(_owned_types); } + ~Repo() {} void inherit(const Repo &parent); bool addDataType(const DataType &type); @@ -85,14 +81,14 @@ public: }; void Repo::inherit(const Repo &parent) { - _types.insert(parent._types.begin(), parent._types.end()); + _id_map.insert(parent._id_map.begin(), parent._id_map.end()); _tensorTypes.insert(parent._tensorTypes.begin(), parent._tensorTypes.end()); _name_map.insert(parent._name_map.begin(), parent._name_map.end()); } // Returns true if a reference to type is stored. bool Repo::addDataType(const DataType &type) { - const DataType *& data_type = _types[type.getId()]; + const DataType *& data_type = _id_map[type.getId()]; if (data_type) { if (data_type->equals(type) && (data_type->getName() == type.getName())) { return false; // Redefinition of identical type is ok. @@ -117,9 +113,9 @@ template <typename T> const DataType* Repo::addDataType(unique_ptr<T> type) { int id = type->getId(); if (addDataType(*type)) { - _owned_types.push_back(type.release()); + _owned_types.emplace_back(std::move(type)); } - return _types[id]; + return _id_map[id]; } @@ -127,28 +123,21 @@ const DataType & Repo::addTensorType(const string &spec) { auto type = TensorDataType::fromSpec(spec); - auto insres = _tensorTypes.insert(std::make_pair(spec, type.get())); - if (insres.second) { - _owned_types.push_back(type.release()); + auto [ iter, inserted ] = _tensorTypes.insert(std::make_pair(spec, type.get())); + if (inserted) { + _owned_types.emplace_back(std::move(type)); } - return *insres.first->second; -} - -template <typename Map> -typename Map::mapped_type FindPtr(const Map &m, typename Map::key_type key) { - typename Map::const_iterator it = m.find(key); - if (it != m.end()) { - return it->second; - } - return typename Map::mapped_type(); + return *iter->second; } const DataType *Repo::lookup(int32_t id) const { - return FindPtr(_types, id); + auto iter = _id_map.find(id); + return (iter == _id_map.end()) ? nullptr : iter->second; } const DataType *Repo::lookup(stringref n) const { - return FindPtr(_name_map, n); + auto iter = _name_map.find(n); + return (iter == _name_map.end()) ? nullptr : iter->second; } const DataType &Repo::findOrThrow(int32_t id) const { @@ -169,11 +158,11 @@ Repo::findOrThrowOrCreate(int32_t id, const string &detailedType) } class AnnotationTypeRepo { - vector<const AnnotationType *> _owned_types; + vector<std::unique_ptr<const AnnotationType>> _owned_types; hash_map<int32_t, AnnotationType *> _annotation_types; public: - ~AnnotationTypeRepo() { DeleteContent(_owned_types); } + ~AnnotationTypeRepo() {} void inherit(const AnnotationTypeRepo &parent); AnnotationType * addAnnotationType(AnnotationType::UP annotation_type); @@ -196,7 +185,7 @@ AnnotationType * AnnotationTypeRepo::addAnnotationType(AnnotationType::UP type) } } else { a_type = type.get(); - _owned_types.push_back(type.release()); + _owned_types.emplace_back(std::move(type)); } return a_type; } @@ -215,7 +204,8 @@ void AnnotationTypeRepo::setAnnotationDataType(int32_t id, const DataType &d) { } const AnnotationType *AnnotationTypeRepo::lookup(int32_t id) const { - return FindPtr(_annotation_types, id); + auto iter = _annotation_types.find(id); + return (iter == _annotation_types.end()) ? nullptr : iter->second; } } // namespace @@ -225,12 +215,14 @@ const AnnotationType *AnnotationTypeRepo::lookup(int32_t id) const { struct DataTypeRepo { typedef unique_ptr<DataTypeRepo> UP; - DocumentType *doc_type; + std::unique_ptr<DocumentType> doc_type; Repo repo; AnnotationTypeRepo annotations; - DataTypeRepo() : doc_type(nullptr) {} - ~DataTypeRepo() { delete doc_type; } + DataTypeRepo() : doc_type() {} + ~DataTypeRepo() {} + + DocumentType * doc() const { return doc_type.get(); } }; namespace { @@ -363,13 +355,15 @@ void addDataTypes(const vector<Datatype> &types, Repo &repo, const AnnotationTyp } void addDocumentTypes(const DocumentTypeMap &type_map, Repo &repo) { - for (const auto & entry : type_map) { - repo.addDataType(*entry.second->doc_type); + for (const auto & [ key, data_type_repo ] : type_map) { + repo.addDataType(*data_type_repo->doc()); } } const DocumentType * addDefaultDocument(DocumentTypeMap &type_map) { + const uint32_t typeId = DataType::T_DOCUMENT; + auto data_types = std::make_unique<DataTypeRepo>(); vector<const DataType *> default_types = DataType::getDefaultDataTypes(); for (size_t i = 0; i < default_types.size(); ++i) { @@ -377,25 +371,23 @@ addDefaultDocument(DocumentTypeMap &type_map) { } data_types->repo.addDataType(UrlDataType::getInstance()); data_types->repo.addDataType(PositionDataType::getInstance()); - data_types->doc_type = new DocumentType("document", 8); + data_types->doc_type = std::make_unique<DocumentType>("document", typeId); vector<const AnnotationType *> annotation_types(AnnotationType::getDefaultAnnotationTypes()); for(size_t i(0); i < annotation_types.size(); ++i) { data_types->annotations.addAnnotationType(std::make_unique<AnnotationType>(*annotation_types[i])); } - - uint32_t typeId = data_types->doc_type->getId(); - const DocumentType * docType = data_types->doc_type; - type_map[typeId] = data_types.release(); + const DocumentType * docType = data_types->doc(); + type_map[typeId] = std::move(data_types); return docType; } const DataTypeRepo &lookupRepo(int32_t id, const DocumentTypeMap &type_map) { - DocumentTypeMap::const_iterator it = type_map.find(id); - if (it == type_map.end()) { + if (const auto * p = type_map.findRepo(id)) { + return *p; + } else { throw IllegalArgumentException(make_string("Unable to find document type %d.", id)); } - return *it->second; } void inheritDataTypes(const vector<DocumenttypesConfig::Documenttype::Inherits> &base_types, @@ -427,7 +419,7 @@ makeDataTypeRepo(const DocumentType &doc_type, const DocumentTypeMap &type_map) auto data_types = std::make_unique<DataTypeRepo>(); data_types->repo.inherit(lookupRepo(DataType::T_DOCUMENT, type_map).repo); data_types->annotations.inherit(lookupRepo(DataType::T_DOCUMENT, type_map).annotations); - data_types->doc_type = new DocumentType(doc_type); + data_types->doc_type = std::make_unique<DocumentType>(doc_type); return data_types; } @@ -446,8 +438,8 @@ void addReferenceTypes(const vector<DocumenttypesConfig::Documenttype::Reference Repo& data_type_repo, const DocumentTypeMap& doc_type_map) { for (const auto& ref_type : ref_types) { - const auto* target_doc_type = lookupRepo(ref_type.targetTypeId, doc_type_map).doc_type; - data_type_repo.addDataType(std::make_unique<ReferenceDataType>(*target_doc_type, ref_type.id)); + const auto & repo = lookupRepo(ref_type.targetTypeId, doc_type_map); + data_type_repo.addDataType(std::make_unique<ReferenceDataType>(*repo.doc_type, ref_type.id)); } } @@ -460,31 +452,31 @@ void add_imported_fields(const DocumenttypesConfig::Documenttype::ImportedfieldV } void configureDataTypeRepo(const DocumenttypesConfig::Documenttype &doc_type, DocumentTypeMap &type_map) { - DataTypeRepo *data_types = type_map[doc_type.id]; + const auto & data_types = type_map[doc_type.id]; inheritAnnotationTypes(doc_type.inherits, type_map, data_types->annotations); addAnnotationTypes(doc_type.annotationtype, data_types->annotations); inheritDataTypes(doc_type.inherits, type_map, data_types->repo); addReferenceTypes(doc_type.referencetype, data_types->repo, type_map); addDataTypes(doc_type.datatype, data_types->repo, data_types->annotations); setAnnotationDataTypes(doc_type.annotationtype, data_types->annotations, data_types->repo); - inheritDocumentTypes(doc_type.inherits, type_map, *data_types->doc_type); - addFieldSet(doc_type.fieldsets, *data_types->doc_type); - add_imported_fields(doc_type.importedfield, *data_types->doc_type); + inheritDocumentTypes(doc_type.inherits, type_map, *data_types->doc()); + addFieldSet(doc_type.fieldsets, *data_types->doc()); + add_imported_fields(doc_type.importedfield, *data_types->doc()); } void addDataTypeRepo(DataTypeRepo::UP data_types, DocumentTypeMap &doc_types) { - DataTypeRepo *& p = doc_types[data_types->doc_type->getId()]; + auto & p = doc_types[data_types->doc()->getId()]; if (p) { - LOG(warning, "Type repo already exists for id %d.", data_types->doc_type->getId()); + LOG(warning, "Type repo already exists for id %d.", data_types->doc()->getId()); throw IllegalArgumentException("Trying to redefine a document type."); } - p = data_types.release(); + p = std::move(data_types); } DataTypeRepo::UP makeSkeletonDataTypeRepo(const DocumenttypesConfig::Documenttype &type) { auto data_types = std::make_unique<DataTypeRepo>(); auto type_ap = std::make_unique<StructDataType>(type.name + ".header", type.headerstruct); - data_types->doc_type = new DocumentType(type.name, type.id, *type_ap); + data_types->doc_type = std::make_unique<DocumentType>(type.name, type.id, *type_ap); data_types->repo.addDataType(std::move(type_ap)); return data_types; } @@ -534,22 +526,23 @@ private: struct DocTypeInProgress { const CDocType & cfg; - DataTypeRepo * data_type_repo; - DocumentType * dtype = nullptr; + DataTypeRepo * data_type_repo = nullptr; bool builtin = false; DocTypeInProgress(const CDocType & config, DocumentTypeMap &doc_types) - : cfg(config), - data_type_repo(doc_types[cfg.internalid]) + : cfg(config) { - if (data_type_repo) { + auto iter = doc_types.find(cfg.internalid); + if (iter != doc_types.end()) { LOG(debug, "old doct : %s [%d]", cfg.name.c_str(), cfg.internalid); builtin = true; } else { LOG(debug, "new doct : %s [%d]", cfg.name.c_str(), cfg.internalid); - data_type_repo = new DataTypeRepo(); - doc_types[cfg.internalid] = data_type_repo; + doc_types.emplace(cfg.internalid, std::make_unique<DataTypeRepo>()); } + iter = doc_types.find(cfg.internalid); + LOG_ASSERT(iter != doc_types.end()); + data_type_repo = iter->second.get(); } Repo& repo() { return data_type_repo->repo; } @@ -587,14 +580,13 @@ private: createEmptyStructs(dtInP); initializeDocTypeAndInheritAnnotations(dtInP); createEmptyAnnotationTypes(dtInP); + } + for (auto & [id, dtInP] : _doc_types_in_progress) { createReferenceTypes(dtInP); } createComplexTypes(); fillStructs(); - for (const CDocType & docT : _input) { - auto iter = _doc_types_in_progress.find(docT.idx); - LOG_ASSERT(iter != _doc_types_in_progress.end()); - auto & dtInP = iter->second; + for (auto & [id, dtInP] : _doc_types_in_progress) { fillDocument(dtInP); fillAnnotationTypes(dtInP); } @@ -674,15 +666,15 @@ private: void initializeDocTypeAndInheritAnnotations(DocTypeInProgress & dtInP) { if (dtInP.builtin) { - madeType(dtInP.data_type_repo->doc_type, dtInP.cfg.idx); + madeType(dtInP.data_type_repo->doc(), dtInP.cfg.idx); return; } - LOG_ASSERT(dtInP.data_type_repo->doc_type == nullptr); + LOG_ASSERT(dtInP.data_type_repo->doc() == nullptr); const auto & docT = dtInP.cfg; const StructDataType * fields = findStruct(docT.contentstruct); if (fields != nullptr) { - dtInP.data_type_repo->doc_type = new DocumentType(docT.name, docT.internalid, *fields); - madeType(dtInP.data_type_repo->doc_type, docT.idx); + dtInP.data_type_repo->doc_type = std::make_unique<DocumentType>(docT.name, docT.internalid, *fields); + madeType(dtInP.data_type_repo->doc(), docT.idx); } else { LOG(error, "Missing content struct for '%s' (idx %d not found)", docT.name.c_str(), docT.contentstruct); @@ -696,7 +688,7 @@ private: inheritD.idx, docT.name.c_str()); throw IllegalArgumentException("Unable to find document for inheritance"); } - DataTypeRepo * parentRepo = FindPtr(_output, dt->getId()); + const DataTypeRepo *parentRepo = _output.findRepo(dt->getId()); if (parentRepo == nullptr) { LOG(error, "parent repo [id %d] missing for document %s", dt->getId(), docT.name.c_str()); @@ -822,7 +814,7 @@ private: return; } const CDocType & docT = dtInP.cfg; - auto * doc_type = dtInP.data_type_repo->doc_type; + auto * doc_type = dtInP.data_type_repo->doc(); LOG_ASSERT(doc_type != nullptr); for (const auto & importD : docT.importedfield) { doc_type->add_imported_field_name(importD.name); @@ -1011,7 +1003,6 @@ DocumentTypeRepo::DocumentTypeRepo(const DocumentType & type) : try { addDataTypeRepo(makeDataTypeRepo(type, *_doc_types), *_doc_types); } catch (...) { - DeleteMapContent(*_doc_types); throw; } } @@ -1029,31 +1020,32 @@ DocumentTypeRepo::DocumentTypeRepo(const DocumenttypesConfig &config) : configureAllRepos(config.documenttype, *_doc_types); } } catch (...) { - DeleteMapContent(*_doc_types); throw; } } DocumentTypeRepo::~DocumentTypeRepo() { - DeleteMapContent(*_doc_types); +} + +DataTypeRepo *DocumentTypeRepo::findRepo(int32_t doc_type_id) const { + return _doc_types->findRepo(doc_type_id); } const DocumentType * DocumentTypeRepo::getDocumentType(int32_t type_id) const noexcept { - const DataTypeRepo *repo = FindPtr(*_doc_types, type_id); - return repo ? repo->doc_type : nullptr; + const DataTypeRepo *repo = findRepo(type_id); + return repo ? repo->doc() : nullptr; } const DocumentType * DocumentTypeRepo::getDocumentType(stringref name) const noexcept { - DocumentTypeMap::const_iterator it = _doc_types->find(DocumentType::createId(name)); - - if (it != _doc_types->end() && it->second->doc_type->getName() == name) { - return it->second->doc_type; + const auto * rp = findRepo(DocumentType::createId(name)); + if (rp && rp->doc()->getName() == name) { + return rp->doc(); } - for (it = _doc_types->begin(); it != _doc_types->end(); ++it) { - if (it->second->doc_type->getName() == name) { - return it->second->doc_type; + for (const auto & [ id, p ] : *_doc_types) { + if (p->doc()->getName() == name) { + return p->doc(); } } return nullptr; @@ -1061,26 +1053,26 @@ DocumentTypeRepo::getDocumentType(stringref name) const noexcept { const DataType * DocumentTypeRepo::getDataType(const DocumentType &doc_type, int32_t id) const { - const DataTypeRepo *dt_repo = FindPtr(*_doc_types, doc_type.getId()); + const DataTypeRepo *dt_repo = findRepo(doc_type.getId()); return dt_repo ? dt_repo->repo.lookup(id) : nullptr; } const DataType * DocumentTypeRepo::getDataType(const DocumentType &doc_type, stringref name) const { - const DataTypeRepo *dt_repo = FindPtr(*_doc_types, doc_type.getId()); + const DataTypeRepo *dt_repo = findRepo(doc_type.getId()); return dt_repo ? dt_repo->repo.lookup(name) : nullptr; } const AnnotationType * DocumentTypeRepo::getAnnotationType(const DocumentType &doc_type, int32_t id) const { - const DataTypeRepo *dt_repo = FindPtr(*_doc_types, doc_type.getId()); + const DataTypeRepo *dt_repo = findRepo(doc_type.getId()); return dt_repo ? dt_repo->annotations.lookup(id) : nullptr; } void DocumentTypeRepo::forEachDocumentType(std::function<void(const DocumentType &)> handler) const { - for (const auto & entry : *_doc_types) { - handler(*entry.second->doc_type); + for (const auto & [ it, rp ] : *_doc_types) { + handler(*rp->doc()); } } diff --git a/document/src/vespa/document/repo/documenttyperepo.h b/document/src/vespa/document/repo/documenttyperepo.h index 1a3898a1b65..78f7edd078f 100644 --- a/document/src/vespa/document/repo/documenttyperepo.h +++ b/document/src/vespa/document/repo/documenttyperepo.h @@ -37,9 +37,10 @@ public: void forEachDocumentType(std::function<void(const DocumentType &)> handler) const; const DocumentType *getDefaultDocType() const { return _default; } private: - std::unique_ptr<internal::DocumentTypeMap> _doc_types; const DocumentType * _default; + + DataTypeRepo * findRepo(int32_t doc_type_id) const; }; } // namespace document diff --git a/documentapi/abi-spec.json b/documentapi/abi-spec.json index 2e68d4803cb..1a2afa3621f 100644 --- a/documentapi/abi-spec.json +++ b/documentapi/abi-spec.json @@ -1844,6 +1844,7 @@ "public static com.yahoo.documentapi.messagebus.protocol.DocumentProtocol$Priority getPriorityByName(java.lang.String)", "public void <init>(com.yahoo.document.DocumentTypeManager)", "public void <init>(com.yahoo.document.DocumentTypeManager, java.lang.String)", + "public void <init>(com.yahoo.document.DocumentTypeManager, com.yahoo.documentapi.messagebus.protocol.DocumentProtocolPoliciesConfig, com.yahoo.vespa.config.content.DistributionConfig)", "public void <init>(com.yahoo.document.DocumentTypeManager, com.yahoo.documentapi.messagebus.loadtypes.LoadTypeSet, com.yahoo.documentapi.messagebus.protocol.DocumentProtocolPoliciesConfig, com.yahoo.vespa.config.content.DistributionConfig)", "public void <init>(com.yahoo.document.DocumentTypeManager, java.lang.String, com.yahoo.documentapi.messagebus.loadtypes.LoadTypeSet)", "public com.yahoo.documentapi.messagebus.protocol.DocumentProtocol putRoutingPolicyFactory(java.lang.String, com.yahoo.documentapi.messagebus.protocol.RoutingPolicyFactory)", @@ -3123,7 +3124,8 @@ ], "methods": [ "public abstract boolean encode(com.yahoo.messagebus.Routable, com.yahoo.document.serialization.DocumentSerializer)", - "public abstract com.yahoo.messagebus.Routable decode(com.yahoo.document.serialization.DocumentDeserializer, com.yahoo.documentapi.messagebus.loadtypes.LoadTypeSet)" + "public abstract com.yahoo.messagebus.Routable decode(com.yahoo.document.serialization.DocumentDeserializer, com.yahoo.documentapi.messagebus.loadtypes.LoadTypeSet)", + "public com.yahoo.messagebus.Routable decode(com.yahoo.document.serialization.DocumentDeserializer)" ], "fields": [] }, diff --git a/documentapi/src/main/java/com/yahoo/documentapi/VisitorParameters.java b/documentapi/src/main/java/com/yahoo/documentapi/VisitorParameters.java index a9dfe0128e0..7446c681dec 100644 --- a/documentapi/src/main/java/com/yahoo/documentapi/VisitorParameters.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/VisitorParameters.java @@ -20,6 +20,7 @@ import java.util.TreeMap; * * @author Håkon Humberset */ +@SuppressWarnings("removal") // TODO: Remove on Vespa 8 public class VisitorParameters extends Parameters { private String documentSelection; @@ -47,7 +48,7 @@ public class VisitorParameters extends Parameters { private int maxBucketsPerVisitor = 1; private boolean dynamicallyIncreaseMaxBucketsPerVisitor = false; private float dynamicMaxBucketsIncreaseFactor = 2; - private LoadType loadType = LoadType.DEFAULT; + private LoadType loadType = LoadType.DEFAULT; // TODO: Remove on Vespa 8 private DocumentProtocol.Priority priority = null; private int traceLevel = 0; private ThrottlePolicy throttlePolicy = null; @@ -332,10 +333,18 @@ public class VisitorParameters extends Parameters { throttlePolicy = policy; } + /** + * @deprecated load types are deprecated + */ + @Deprecated(forRemoval = true) // TODO: Remove on Vespa 8 public void setLoadType(LoadType loadType) { this.loadType = loadType; } + /** + * @deprecated load types are deprecated + */ + @Deprecated(forRemoval = true) // TODO: Remove on Vespa 8 public LoadType getLoadType() { return loadType; } diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusDocumentAccess.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusDocumentAccess.java index de09049b49e..c2e6dde7f60 100644 --- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusDocumentAccess.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusDocumentAccess.java @@ -53,6 +53,7 @@ public class MessageBusDocumentAccess extends DocumentAccess { * * @param params All parameters for construction. */ + @SuppressWarnings("removal") // TODO: Remove on Vespa 8 public MessageBusDocumentAccess(MessageBusParams params) { super(params); this.params = params; diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusParams.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusParams.java index 628bca4098f..bb8c3a3b1b1 100755 --- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusParams.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusParams.java @@ -14,6 +14,7 @@ import static java.util.Objects.requireNonNull; /** * @author Einar M R Rosenvinge */ +@SuppressWarnings("removal") // TODO: Remove on Vespa 8 public class MessageBusParams extends DocumentAccessParams { private String routingConfigId = null; @@ -26,12 +27,16 @@ public class MessageBusParams extends DocumentAccessParams { private RPCNetworkParams rpcNetworkParams = new RPCNetworkParams(); private com.yahoo.messagebus.MessageBusParams mbusParams = new com.yahoo.messagebus.MessageBusParams(); private SourceSessionParams sourceSessionParams = new SourceSessionParams(); - private LoadTypeSet loadTypes; + private LoadTypeSet loadTypes; // TODO remove on Vespa 8 public MessageBusParams() { this(new LoadTypeSet()); } + /** + * @deprecated load types are deprecated. Use default constructor instead + */ + @Deprecated(forRemoval = true) // TODO: Remove on Vespa 8 public MessageBusParams(LoadTypeSet loadTypes) { this.loadTypes = loadTypes; } @@ -39,7 +44,9 @@ public class MessageBusParams extends DocumentAccessParams { /** * * @return Returns the set of load types accepted by this Vespa installation + * @deprecated load types are deprecated */ + @Deprecated(forRemoval = true) // TODO: Remove on Vespa 8 public LoadTypeSet getLoadTypes() { return loadTypes; } diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/loadtypes/LoadType.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/loadtypes/LoadType.java index 53c09dcbcb6..133736a8542 100644 --- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/loadtypes/LoadType.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/loadtypes/LoadType.java @@ -4,8 +4,10 @@ package com.yahoo.documentapi.messagebus.loadtypes; import com.yahoo.documentapi.messagebus.protocol.DocumentProtocol; /** + * @deprecated load types are deprecated * @author thomasg */ +@Deprecated(forRemoval = true) // TODO: Remove on Vespa 8 public class LoadType { private int id; private String name; diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/loadtypes/LoadTypeSet.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/loadtypes/LoadTypeSet.java index e28d760eddf..a3fbed472f0 100644 --- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/loadtypes/LoadTypeSet.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/loadtypes/LoadTypeSet.java @@ -20,9 +20,15 @@ import java.util.TreeMap; * * For testing, you may want to use the empty constructor and add * load types yourself with addType(). + * + * @deprecated load types are deprecated */ +@Deprecated(forRemoval = true) // TODO: Remove on Vespa 8 +@SuppressWarnings("removal") // TODO: Remove on Vespa 8 public class LoadTypeSet { + public static final LoadTypeSet EMPTY = new LoadTypeSet(); + class DualMap { Map<String, LoadType> nameMap = new TreeMap<String, LoadType>(); Map<Integer, LoadType> idMap = new HashMap<Integer, LoadType>(); diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/DocumentMessage.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/DocumentMessage.java index d1e3b61f998..21f7c243c6f 100755 --- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/DocumentMessage.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/DocumentMessage.java @@ -9,10 +9,11 @@ import com.yahoo.text.Utf8String; /** * @author Simon Thoresen Hult */ +@SuppressWarnings("removal") // TODO: Remove on Vespa 8 public abstract class DocumentMessage extends Message { private DocumentProtocol.Priority priority = DocumentProtocol.Priority.NORMAL_3; - private LoadType loadType = LoadType.DEFAULT; + private LoadType loadType = LoadType.DEFAULT; // TODO: Remove on Vespa 8 /** * Constructs a new message with no content. @@ -65,10 +66,20 @@ public abstract class DocumentMessage extends Message { this.priority = priority; } + /** + * @deprecated load types are deprecated + */ + @Deprecated(forRemoval = true) // TODO: Remove on Vespa 8 + @SuppressWarnings("removal") // TODO: Remove on Vespa 8 public LoadType getLoadType() { return loadType; } + /** + * @deprecated load types are deprecated + */ + @Deprecated(forRemoval = true) // TODO: Remove on Vespa 8 + @SuppressWarnings("removal") // TODO: Remove on Vespa 8 public void setLoadType(LoadType loadType) { if (loadType != null) { this.loadType = loadType; @@ -79,7 +90,7 @@ public abstract class DocumentMessage extends Message { @Override public int getApproxSize() { - return 4 + 1; // type + priority + return 4 + 1; // type + priority // TODO update on Vespa 8 to not include deprecated fields } @Override diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/DocumentProtocol.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/DocumentProtocol.java index ac946b80429..5db426a5db4 100755 --- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/DocumentProtocol.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/DocumentProtocol.java @@ -32,6 +32,7 @@ import static java.util.Objects.requireNonNull; * * @author Simon Thoresen Hult */ +@SuppressWarnings("removal") // TODO: Remove on Vespa 8 public class DocumentProtocol implements Protocol { private static final Logger log = Logger.getLogger(DocumentProtocol.class.getName()); @@ -246,12 +247,27 @@ public class DocumentProtocol implements Protocol { this(docMan, configId, new LoadTypeSet()); } + public DocumentProtocol(DocumentTypeManager documentTypeManager, + DocumentProtocolPoliciesConfig policiesConfig, + DistributionConfig distributionConfig) { + this(requireNonNull(documentTypeManager), null, new LoadTypeSet(), + requireNonNull(policiesConfig), requireNonNull(distributionConfig)); + } + + /** + * @deprecated load types are deprecated. Use constructor without LoadTypeSet instead. + */ + @Deprecated(forRemoval = true) // TODO: Remove on Vespa 8 public DocumentProtocol(DocumentTypeManager documentTypeManager, LoadTypeSet loadTypes, DocumentProtocolPoliciesConfig policiesConfig, DistributionConfig distributionConfig) { this(requireNonNull(documentTypeManager), null, requireNonNull(loadTypes), - requireNonNull(policiesConfig), requireNonNull(distributionConfig)); + requireNonNull(policiesConfig), requireNonNull(distributionConfig)); } + /** + * @deprecated load types are deprecated. Use constructor without LoadTypeSet instead. + */ + @Deprecated(forRemoval = true) // TODO: Remove on Vespa 8 public DocumentProtocol(DocumentTypeManager docMan, String configId, LoadTypeSet set) { this(docMan, configId == null ? "client" : configId, set, null, null); } diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/RoutableFactories60.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/RoutableFactories60.java index 60c8a613bb5..42172b04b90 100644 --- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/RoutableFactories60.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/RoutableFactories60.java @@ -6,22 +6,17 @@ import com.yahoo.document.Document; import com.yahoo.document.DocumentId; import com.yahoo.document.DocumentPut; import com.yahoo.document.DocumentUpdate; -import com.yahoo.document.FixedBucketSpaces; import com.yahoo.document.TestAndSetCondition; import com.yahoo.document.serialization.DocumentDeserializer; import com.yahoo.document.serialization.DocumentSerializer; -import com.yahoo.document.serialization.DocumentSerializerFactory; import com.yahoo.documentapi.messagebus.loadtypes.LoadTypeSet; -import java.util.logging.Level; import com.yahoo.messagebus.Routable; import com.yahoo.vdslib.DocumentSummary; import com.yahoo.vdslib.SearchResult; import com.yahoo.vdslib.VisitorStatistics; import com.yahoo.vespa.objects.Deserializer; -import com.yahoo.vespa.objects.Serializer; import java.util.Map; -import java.util.logging.Logger; import static com.yahoo.documentapi.messagebus.protocol.AbstractRoutableFactory.decodeString; import static com.yahoo.documentapi.messagebus.protocol.AbstractRoutableFactory.encodeString; @@ -86,7 +81,7 @@ public abstract class RoutableFactories60 { DocumentMessage msg = doDecode(in); if (msg != null) { msg.setPriority(DocumentProtocol.getPriority(pri)); - msg.setLoadType(loadTypes.getIdMap().get(loadType)); + msg.setLoadType(loadTypes.getIdMap().get(loadType)); // TODO: ignore on Vespa 8 } return msg; } @@ -136,6 +131,7 @@ public abstract class RoutableFactories60 { return doEncode(reply, out); } + @SuppressWarnings("removal") // TODO: Remove on Vespa 8 public Routable decode(DocumentDeserializer in, LoadTypeSet loadTypes) { byte pri = in.getByte(null); DocumentReply reply = doDecode(in); diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/RoutableFactory.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/RoutableFactory.java index d38671fa313..7baa41e5c6a 100755 --- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/RoutableFactory.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/RoutableFactory.java @@ -22,7 +22,7 @@ public interface RoutableFactory { /** * <p>This method encodes the content of the given routable into a byte buffer that can later be decoded using the - * {@link #decode(DocumentDeserializer, com.yahoo.documentapi.messagebus.loadtypes.LoadTypeSet)} method.</p> <p>Return false to signal failure.</p> + * {@link #decode(DocumentDeserializer)} method.</p> <p>Return false to signal failure.</p> * <p>This method is NOT exception safe.</p> * * @param obj The routable to encode. @@ -38,7 +38,15 @@ public interface RoutableFactory { * @param in The buffer to read from. * @param loadTypes The LoadTypeSet to inject into the Routable. * @return The decoded routable. + * @deprecated load types are deprecated. Use method without LoadTypeSet instead */ + @Deprecated(forRemoval = true) // TODO: Remove on Vespa 8 + @SuppressWarnings("removal") // TODO: Remove on Vespa 8 Routable decode(DocumentDeserializer in, LoadTypeSet loadTypes); + @SuppressWarnings("removal") // TODO: Remove on Vespa 8 + default Routable decode(DocumentDeserializer in) { + return decode(in, LoadTypeSet.EMPTY); + } + } diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/RoutableRepository.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/RoutableRepository.java index 24677a9a322..2360cbe8bc3 100755 --- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/RoutableRepository.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/RoutableRepository.java @@ -28,13 +28,21 @@ import java.util.logging.Logger; * * @author Simon Thoresen Hult */ +@SuppressWarnings("removal") // TODO: Remove on Vespa 8 final class RoutableRepository { private static final Logger log = Logger.getLogger(RoutableRepository.class.getName()); private final CopyOnWriteHashMap<Integer, VersionMap> factoryTypes = new CopyOnWriteHashMap<>(); private final CopyOnWriteHashMap<CacheKey, RoutableFactory> cache = new CopyOnWriteHashMap<>(); - private LoadTypeSet loadTypes; + private LoadTypeSet loadTypes; // TODO remove on Vespa 8 + public RoutableRepository() {} + + /** + * @deprecated load types are deprecated. Use default constructor instead. + */ + @Deprecated(forRemoval = true) // TODO: Remove on Vespa 8 + @SuppressWarnings("removal") // TODO: Remove on Vespa 8 public RoutableRepository(LoadTypeSet set) { loadTypes = set; } diff --git a/documentapi/src/test/java/com/yahoo/documentapi/VisitorParametersTestCase.java b/documentapi/src/test/java/com/yahoo/documentapi/VisitorParametersTestCase.java index caed3867d99..b1187d48374 100644 --- a/documentapi/src/test/java/com/yahoo/documentapi/VisitorParametersTestCase.java +++ b/documentapi/src/test/java/com/yahoo/documentapi/VisitorParametersTestCase.java @@ -7,7 +7,9 @@ import com.yahoo.documentapi.messagebus.protocol.DocumentProtocol; import org.junit.Test; import static org.junit.Assert.*; +@SuppressWarnings("removal") // TODO: Remove on Vespa 8 public class VisitorParametersTestCase { + // TODO: Remove on Vespa 8 private LoadType loadType = new LoadType(3, "samnmax", DocumentProtocol.Priority.HIGH_3); @SuppressWarnings("removal")// TODO: Vespa 8: remove @@ -21,12 +23,12 @@ public class VisitorParametersTestCase { params.setLibraryParameter("groovy", "dudes"); params.setLibraryParameter("ninja", "turtles"); params.setMaxBucketsPerVisitor(55); - params.setPriority(DocumentProtocol.Priority.HIGHEST); + params.setPriority(DocumentProtocol.Priority.HIGHEST); // TODO: Remove on Vespa 8 params.setRoute("extraterrestrial/highway"); params.setTimeoutMs(1337); params.setMaxPending(111); params.setFieldSet(AllFields.NAME); - params.setLoadType(loadType); + params.setLoadType(loadType); // TODO: Remove on Vespa 8 params.setVisitRemoves(true); params.setVisitInconsistentBuckets(true); params.setTraceLevel(9); diff --git a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/loadtypes/test/LoadTypesTestCase.java b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/loadtypes/test/LoadTypesTestCase.java index 73c343174d4..18269971f88 100644 --- a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/loadtypes/test/LoadTypesTestCase.java +++ b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/loadtypes/test/LoadTypesTestCase.java @@ -9,6 +9,8 @@ import static org.junit.Assert.assertEquals; /** * @author thomasg */ +@SuppressWarnings("removal") // TODO: Remove on Vespa 8 +// TODO Vespa 8: remove test case once load types are gone public class LoadTypesTestCase { @Test diff --git a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/Messages60TestCase.java b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/Messages60TestCase.java index 5250a6b1db7..deecba96aa6 100644 --- a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/Messages60TestCase.java +++ b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/Messages60TestCase.java @@ -143,6 +143,7 @@ public class Messages60TestCase extends MessagesTestBase { private static final String BUCKET_SPACE = "beartato"; @Override + @SuppressWarnings("removal") // TODO: Remove on Vespa 8 public void run() { GetBucketListMessage msg = new GetBucketListMessage(new BucketId(16, 123)); msg.setBucketSpace(BUCKET_SPACE); @@ -151,7 +152,7 @@ public class Messages60TestCase extends MessagesTestBase { for (Language lang : LANGUAGES) { msg = (GetBucketListMessage)deserialize("GetBucketListMessage", DocumentProtocol.MESSAGE_GETBUCKETLIST, lang); assertEquals(new BucketId(16, 123), msg.getBucketId()); - assertEquals("default", msg.getLoadType().getName()); + assertEquals("default", msg.getLoadType().getName()); // TODO: Remove on Vespa 8 assertEquals(BUCKET_SPACE, msg.getBucketSpace()); } } @@ -162,9 +163,10 @@ public class Messages60TestCase extends MessagesTestBase { private static final String BUCKET_SPACE = "andrei"; @Override + @SuppressWarnings("removal") // TODO: Remove on Vespa 8 public void run() { StatBucketMessage msg = new StatBucketMessage(new BucketId(16, 123), "id.user=123"); - msg.setLoadType(null); + msg.setLoadType(null); // TODO: Remove on Vespa 8 msg.setBucketSpace(BUCKET_SPACE); assertEquals(BASE_MESSAGE_LENGTH + 27 + serializedLength(BUCKET_SPACE), serialize("StatBucketMessage", msg)); @@ -172,7 +174,7 @@ public class Messages60TestCase extends MessagesTestBase { msg = (StatBucketMessage)deserialize("StatBucketMessage", DocumentProtocol.MESSAGE_STATBUCKET, lang); assertEquals(new BucketId(16, 123), msg.getBucketId()); assertEquals("id.user=123", msg.getDocumentSelection()); - assertEquals("default", msg.getLoadType().getName()); + assertEquals("default", msg.getLoadType().getName()); // TODO: Remove on Vespa 8 assertEquals(BUCKET_SPACE, msg.getBucketSpace()); } } diff --git a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/MessagesTestBase.java b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/MessagesTestBase.java index 19f77ee1335..74d06c05383 100755 --- a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/MessagesTestBase.java +++ b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/MessagesTestBase.java @@ -19,6 +19,7 @@ import static org.junit.Assert.*; /** * @author Simon Thoresen Hult */ +@SuppressWarnings("removal") // TODO: Remove on Vespa 8 public abstract class MessagesTestBase { protected enum Language { @@ -28,7 +29,7 @@ public abstract class MessagesTestBase { protected static final Set<Language> LANGUAGES = EnumSet.allOf(Language.class); protected final DocumentTypeManager docMan = new DocumentTypeManager(); - protected final LoadTypeSet loadTypes = new LoadTypeSet(); + protected final LoadTypeSet loadTypes = new LoadTypeSet(); // TODO remove on Vespa 8 protected final DocumentProtocol protocol = new DocumentProtocol(docMan, null, loadTypes); public MessagesTestBase() { diff --git a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/test/MessageBusVisitorSessionTestCase.java b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/test/MessageBusVisitorSessionTestCase.java index 0c66c05f35e..ab881e143b7 100755 --- a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/test/MessageBusVisitorSessionTestCase.java +++ b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/test/MessageBusVisitorSessionTestCase.java @@ -697,6 +697,7 @@ public class MessageBusVisitorSessionTestCase { } @Test + @SuppressWarnings("removal") // TODO: Remove on Vespa 8 public void testMessageParameters() { MockSender sender = new MockSender(); MockReceiver receiver = new MockReceiver(); @@ -716,7 +717,7 @@ public class MessageBusVisitorSessionTestCase { params.setTimeoutMs(1337); params.setMaxPending(111); params.setFieldSet(DocIdOnly.NAME); - params.setLoadType(new LoadType(3, "samnmax", DocumentProtocol.Priority.HIGH_3)); + params.setLoadType(new LoadType(3, "samnmax", DocumentProtocol.Priority.HIGH_3)); // TODO: Remove on Vespa 8 params.setVisitRemoves(true); params.setVisitInconsistentBuckets(true); params.setTraceLevel(9); diff --git a/eval/src/tests/tensor/onnx_wrapper/onnx_wrapper_test.cpp b/eval/src/tests/tensor/onnx_wrapper/onnx_wrapper_test.cpp index e50c41e2e09..021b20149d1 100644 --- a/eval/src/tests/tensor/onnx_wrapper/onnx_wrapper_test.cpp +++ b/eval/src/tests/tensor/onnx_wrapper/onnx_wrapper_test.cpp @@ -482,7 +482,11 @@ TEST(OnnxTest, inspect_float_to_int8_conversion) { TEST(OnnxTest, default_allocator_type) { Ort::AllocatorWithDefaultOptions default_alloc; +#if ORT_API_VERSION >= 10 + OrtAllocatorType res = OrtInvalidAllocator; +#else OrtAllocatorType res = Invalid; +#endif Ort::ThrowOnError(Ort::GetApi().MemoryInfoGetType(default_alloc.GetInfo(), &res)); fprintf(stderr, "default allocator type: %d\n", int(res)); } diff --git a/fastos/src/vespa/fastos/app.cpp b/fastos/src/vespa/fastos/app.cpp index 05e885a7d37..8477f79f00d 100644 --- a/fastos/src/vespa/fastos/app.cpp +++ b/fastos/src/vespa/fastos/app.cpp @@ -16,18 +16,6 @@ FastOS_ApplicationInterface::FastOS_ApplicationInterface() : _argc(0), _argv(nullptr) { -#ifdef __linux__ - char * fadvise = getenv("VESPA_FADVISE_OPTIONS"); - if (fadvise != nullptr) { - int fadviseOptions(0); - if (strstr(fadvise, "SEQUENTIAL")) { fadviseOptions |= POSIX_FADV_SEQUENTIAL; } - if (strstr(fadvise, "RANDOM")) { fadviseOptions |= POSIX_FADV_RANDOM; } - if (strstr(fadvise, "WILLNEED")) { fadviseOptions |= POSIX_FADV_WILLNEED; } - if (strstr(fadvise, "DONTNEED")) { fadviseOptions |= POSIX_FADV_DONTNEED; } - if (strstr(fadvise, "NOREUSE")) { fadviseOptions |= POSIX_FADV_NOREUSE; } - FastOS_FileInterface::setDefaultFAdviseOptions(fadviseOptions); - } -#endif } FastOS_ApplicationInterface::~FastOS_ApplicationInterface () = default; 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 5c76eb274df..4ad4d52f094 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -209,7 +209,7 @@ public class Flags { public static final UnboundBooleanFlag ENABLED_HORIZON_DASHBOARD = defineFeatureFlag( "enabled-horizon-dashboard", false, - List.of("olaa"), "2021-09-13", "2022-04-01", + List.of("olaa"), "2021-09-13", "2022-07-01", "Enable Horizon dashboard", "Takes effect immediately", TENANT_ID, CONSOLE_USER_EMAIL diff --git a/fnet/src/vespa/fnet/controlpacket.cpp b/fnet/src/vespa/fnet/controlpacket.cpp index 9ff69a76210..9aa03ad5e3a 100644 --- a/fnet/src/vespa/fnet/controlpacket.cpp +++ b/fnet/src/vespa/fnet/controlpacket.cpp @@ -95,7 +95,10 @@ FNET_ControlPacket FNET_ControlPacket::IOCClose(FNET_CMD_IOC_CLOSE); FNET_ControlPacket -FNET_ControlPacket::DetachServerAdapter(FNET_CMD_DETACH_SERVER_ADAPTER); +FNET_ControlPacket::DetachServerAdapterInit(FNET_CMD_DETACH_SERVER_ADAPTER_INIT); + +FNET_ControlPacket +FNET_ControlPacket::DetachServerAdapterFini(FNET_CMD_DETACH_SERVER_ADAPTER_FINI); FNET_ControlPacket FNET_ControlPacket::Execute(FNET_CMD_EXECUTE); diff --git a/fnet/src/vespa/fnet/controlpacket.h b/fnet/src/vespa/fnet/controlpacket.h index 8dd2d034ae6..ad846d37c30 100644 --- a/fnet/src/vespa/fnet/controlpacket.h +++ b/fnet/src/vespa/fnet/controlpacket.h @@ -38,7 +38,8 @@ public: FNET_CMD_IOC_ENABLE_WRITE, FNET_CMD_IOC_HANDSHAKE_ACT, FNET_CMD_IOC_CLOSE, - FNET_CMD_DETACH_SERVER_ADAPTER, + FNET_CMD_DETACH_SERVER_ADAPTER_INIT, + FNET_CMD_DETACH_SERVER_ADAPTER_FINI, FNET_CMD_EXECUTE, FNET_CMD_TIMEOUT, FNET_CMD_BAD_PACKET, @@ -51,7 +52,8 @@ public: static FNET_ControlPacket IOCEnableWrite; static FNET_ControlPacket IOCHandshakeACT; static FNET_ControlPacket IOCClose; - static FNET_ControlPacket DetachServerAdapter; + static FNET_ControlPacket DetachServerAdapterInit; + static FNET_ControlPacket DetachServerAdapterFini; static FNET_ControlPacket Execute; static FNET_ControlPacket Timeout; static FNET_ControlPacket BadPacket; diff --git a/fnet/src/vespa/fnet/frt/supervisor.cpp b/fnet/src/vespa/fnet/frt/supervisor.cpp index 0a31b9d882b..1681321b239 100644 --- a/fnet/src/vespa/fnet/frt/supervisor.cpp +++ b/fnet/src/vespa/fnet/frt/supervisor.cpp @@ -30,12 +30,10 @@ FRT_Supervisor::FRT_Supervisor(FNET_Transport *transport) FRT_Supervisor::~FRT_Supervisor() { + _transport->detach(this); if (_connector != nullptr) { - _connector->Owner()->Close(_connector, /* needref */ false); + _connector->SubRef(); } - _transport->wait_for_pending_resolves(); - _transport->detach(this); - _transport->sync(); } FNET_Scheduler * diff --git a/fnet/src/vespa/fnet/transport.cpp b/fnet/src/vespa/fnet/transport.cpp index 0a79f324bb9..1130b6d3e5e 100644 --- a/fnet/src/vespa/fnet/transport.cpp +++ b/fnet/src/vespa/fnet/transport.cpp @@ -125,6 +125,11 @@ TransportConfig::time_tools() const { } // fnet +void +FNET_Transport::wait_for_pending_resolves() { + _async_resolver->wait_for_pending_resolves(); +} + FNET_Transport::FNET_Transport(const fnet::TransportConfig &cfg) : _async_resolver(cfg.resolver()), _crypto_engine(cfg.crypto()), @@ -158,11 +163,6 @@ FNET_Transport::resolve_async(const vespalib::string &spec, _async_resolver->resolve_async(spec, std::move(result_handler)); } -void -FNET_Transport::wait_for_pending_resolves() { - _async_resolver->wait_for_pending_resolves(); -} - vespalib::CryptoSocket::UP FNET_Transport::create_client_crypto_socket(vespalib::SocketHandle socket, const vespalib::SocketSpec &spec) { @@ -221,8 +221,14 @@ void FNET_Transport::detach(FNET_IServerAdapter *server_adapter) { for (const auto &thread: _threads) { - thread->detach(server_adapter); + thread->init_detach(server_adapter); + } + wait_for_pending_resolves(); + sync(); + for (const auto &thread: _threads) { + thread->fini_detach(server_adapter); } + sync(); } FNET_Scheduler * diff --git a/fnet/src/vespa/fnet/transport.h b/fnet/src/vespa/fnet/transport.h index 7dbfd80dfe7..d6e4aefb02b 100644 --- a/fnet/src/vespa/fnet/transport.h +++ b/fnet/src/vespa/fnet/transport.h @@ -113,6 +113,11 @@ private: Threads _threads; const FNET_Config _config; + /** + * Wait for all pending resolve requests. + **/ + void wait_for_pending_resolves(); + public: FNET_Transport(const FNET_Transport &) = delete; FNET_Transport & operator = (const FNET_Transport &) = delete; @@ -160,11 +165,6 @@ public: vespalib::AsyncResolver::ResultHandler::WP result_handler); /** - * Wait for all pending resolve requests. - **/ - void wait_for_pending_resolves(); - - /** * Wrap a plain socket endpoint (client side) in a CryptoSocket. The * implementation will be determined by the CryptoEngine used by * this Transport. @@ -258,11 +258,8 @@ public: * Detach a server adapter from this transport. * * This will close all connectors and connections referencing the - * server adapter. Note that this is an async - * operation. 'wait_for_pending_resolves' should be called before - * this to make sure any in-flight connections are added - * first. 'sync' should be called after this to drain any pending - * call-backs. + * server adapter. Note that this function will also synchronize + * with async address resolving and underlying transport threads. **/ void detach(FNET_IServerAdapter *server_adapter); diff --git a/fnet/src/vespa/fnet/transport_thread.cpp b/fnet/src/vespa/fnet/transport_thread.cpp index 53363740bae..d46d174c670 100644 --- a/fnet/src/vespa/fnet/transport_thread.cpp +++ b/fnet/src/vespa/fnet/transport_thread.cpp @@ -162,7 +162,7 @@ FNET_TransportThread::SafeDiscardEvent(FNET_ControlPacket *cpacket, void FNET_TransportThread::handle_add_cmd(FNET_IOComponent *ioc) { - if (ioc->handle_add_event()) { + if ((_detaching.count(ioc->server_adapter()) == 0) && ioc->handle_add_event()) { AddComponent(ioc); ioc->_flags._ioc_added = true; ioc->attach_selector(_selector); @@ -186,8 +186,9 @@ FNET_TransportThread::handle_close_cmd(FNET_IOComponent *ioc) void -FNET_TransportThread::handle_detach_server_adapter_cmd(FNET_IServerAdapter *server_adapter) +FNET_TransportThread::handle_detach_server_adapter_init_cmd(FNET_IServerAdapter *server_adapter) { + _detaching.insert(server_adapter); FNET_IOComponent *component = _componentsHead; while (component != nullptr) { FNET_IOComponent *tmp = component; @@ -201,6 +202,12 @@ FNET_TransportThread::handle_detach_server_adapter_cmd(FNET_IServerAdapter *serv } +void +FNET_TransportThread::handle_detach_server_adapter_fini_cmd(FNET_IServerAdapter *server_adapter) +{ + _detaching.erase(server_adapter); +} + extern "C" { static void pipehandler(int) @@ -241,7 +248,8 @@ FNET_TransportThread::FNET_TransportThread(FNET_Transport &owner_in) _pseudo_thread(), _started(false), _shutdown(false), - _finished(false) + _finished(false), + _detaching() { trapsigpipe(); } @@ -348,9 +356,15 @@ FNET_TransportThread::Close(FNET_IOComponent *comp, bool needRef) } void -FNET_TransportThread::detach(FNET_IServerAdapter *server_adapter) +FNET_TransportThread::init_detach(FNET_IServerAdapter *server_adapter) { - PostEvent(&FNET_ControlPacket::DetachServerAdapter, FNET_Context(server_adapter)); + PostEvent(&FNET_ControlPacket::DetachServerAdapterInit, FNET_Context(server_adapter)); +} + +void +FNET_TransportThread::fini_detach(FNET_IServerAdapter *server_adapter) +{ + PostEvent(&FNET_ControlPacket::DetachServerAdapterFini, FNET_Context(server_adapter)); } bool @@ -432,8 +446,13 @@ FNET_TransportThread::handle_wakeup() continue; } - if (packet->GetCommand() == FNET_ControlPacket::FNET_CMD_DETACH_SERVER_ADAPTER) { - handle_detach_server_adapter_cmd(context._value.SERVER_ADAPTER); + if (packet->GetCommand() == FNET_ControlPacket::FNET_CMD_DETACH_SERVER_ADAPTER_INIT) { + handle_detach_server_adapter_init_cmd(context._value.SERVER_ADAPTER); + continue; + } + + if (packet->GetCommand() == FNET_ControlPacket::FNET_CMD_DETACH_SERVER_ADAPTER_FINI) { + handle_detach_server_adapter_fini_cmd(context._value.SERVER_ADAPTER); continue; } diff --git a/fnet/src/vespa/fnet/transport_thread.h b/fnet/src/vespa/fnet/transport_thread.h index c120894ac9c..b507c5dc31d 100644 --- a/fnet/src/vespa/fnet/transport_thread.h +++ b/fnet/src/vespa/fnet/transport_thread.h @@ -13,6 +13,7 @@ #include <mutex> #include <condition_variable> #include <chrono> +#include <set> namespace fnet { struct TimeTools; } class FNET_Transport; @@ -51,6 +52,7 @@ private: std::atomic<bool> _started; // event loop started ? std::atomic<bool> _shutdown; // should stop event loop ? std::atomic<bool> _finished; // event loop stopped ? + std::set<FNET_IServerAdapter*> _detaching; // server adapters being detached /** * Add an IOComponent to the list of components. This operation is @@ -143,7 +145,8 @@ private: void handle_add_cmd(FNET_IOComponent *ioc); void handle_close_cmd(FNET_IOComponent *ioc); - void handle_detach_server_adapter_cmd(FNET_IServerAdapter *server_adapter); + void handle_detach_server_adapter_init_cmd(FNET_IServerAdapter *server_adapter); + void handle_detach_server_adapter_fini_cmd(FNET_IServerAdapter *server_adapter); /** * This method is called to initialize the transport thread event @@ -336,16 +339,16 @@ public: void Close(FNET_IOComponent *comp, bool needRef = true); /** - * Detach a server adapter from this transport. - * - * This will close all connectors and connections referencing the - * server adapter. Note that this is an async - * operation. 'wait_for_pending_resolves' (on the owning - * Transport) should be called before this to make sure any - * in-flight connections are added first. 'sync' should be called - * after this to drain any pending call-backs. + * Start the operation of detaching a server adapter from this + * transport. + **/ + void init_detach(FNET_IServerAdapter *server_adapter); + + /** + * Complete the operation of detaching a server adapter from this + * transport. **/ - void detach(FNET_IServerAdapter *server_adapter); + void fini_detach(FNET_IServerAdapter *server_adapter); /** * Post an execution event on the transport event queue. The return diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java index 379bb2566df..a9abc352d8c 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java @@ -43,8 +43,8 @@ import java.util.logging.Logger; import java.util.stream.Collectors; import java.util.stream.IntStream; +import static com.yahoo.stream.CustomCollectors.toLinkedMap; import static java.util.stream.Collectors.collectingAndThen; -import static java.util.stream.Collectors.toMap; /** * Client which reads and writes nodes to a curator database. @@ -453,7 +453,8 @@ public class CuratorDatabaseClient { .map(this::readLoadBalancer) .filter(Optional::isPresent) .map(Optional::get) - .collect(collectingAndThen(toMap(LoadBalancer::id, Function.identity()), + .collect(collectingAndThen(toLinkedMap(LoadBalancer::id, + Function.identity()), Collections::unmodifiableMap)); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializerTest.java index cc121ba8104..48ee23c7b60 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializerTest.java @@ -334,6 +334,9 @@ public class NodeSerializerTest { " \"hostname\" : \"myHostname\",\n" + " \"ipAddresses\" : [\"127.0.0.1\"],\n" + " \"instance\": {\n" + + " \"tenantId\":\"t\",\n" + + " \"applicationId\":\"a\",\n" + + " \"instanceId\":\"i\",\n" + " \"serviceId\": \"content/myId/0/0/stateful\",\n" + " \"wantedVespaVersion\": \"6.42.2\"\n" + " }\n" + diff --git a/parent/pom.xml b/parent/pom.xml index 5e3ec3d20ea..d90dc7a292f 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -886,6 +886,17 @@ <version>2.3.0</version> </dependency> <dependency> + <groupId>org.xerial.snappy</groupId> + <artifactId>snappy-java</artifactId> + <version>1.1.7</version> + </dependency> + <dependency> + <groupId>io.dropwizard.metrics</groupId> + <artifactId>metrics-core</artifactId> + <version>3.2.5</version> + </dependency> + + <dependency> <groupId>uk.co.datumedge</groupId> <artifactId>hamcrest-json</artifactId> <version>0.2</version> diff --git a/routing-generator/src/main/java/com/yahoo/vespa/hosted/routing/nginx/NginxMetricsReporter.java b/routing-generator/src/main/java/com/yahoo/vespa/hosted/routing/nginx/NginxMetricsReporter.java index 79381b8c99e..b9ab1dbe9b6 100644 --- a/routing-generator/src/main/java/com/yahoo/vespa/hosted/routing/nginx/NginxMetricsReporter.java +++ b/routing-generator/src/main/java/com/yahoo/vespa/hosted/routing/nginx/NginxMetricsReporter.java @@ -58,7 +58,7 @@ public class NginxMetricsReporter extends AbstractComponent implements Runnable @Inject public NginxMetricsReporter(ApplicationIdConfig applicationId, Metric metric, HealthStatus healthStatus, RoutingGenerator routingGenerator) { - this(new ApplicationId(applicationId), metric, healthStatus, FileSystems.getDefault(), interval, routingGenerator::routingTable); + this(ApplicationId.from(applicationId), metric, healthStatus, FileSystems.getDefault(), interval, routingGenerator::routingTable); } NginxMetricsReporter(ApplicationId application, Metric metric, HealthStatus healthStatus, FileSystem fileSystem, Duration interval, diff --git a/screwdriver/release-java-artifacts.sh b/screwdriver/release-java-artifacts.sh index 8030638cf5b..8d80bb45578 100755 --- a/screwdriver/release-java-artifacts.sh +++ b/screwdriver/release-java-artifacts.sh @@ -52,6 +52,8 @@ for MODULE in $(comm -2 -3 \ echo "No javadoc available for module" > $MODULE/src/main/javadoc/README done +# Workaround for broken nexus-staging-maven-plugin instead of swapping JDK +export MAVEN_OPTS="--add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.base/java.lang.reflect=ALL-UNNAMED --add-opens=java.base/java.text=ALL-UNNAMED --add-opens=java.desktop/java.awt.font=ALL-UNNAMED" export VESPA_MAVEN_EXTRA_OPTS="--show-version --batch-mode" ./bootstrap.sh @@ -69,20 +71,12 @@ mvn $COMMON_MAVEN_OPTS --file ./maven-plugins/pom.xml -DskipStagingRepositoryClo # Deploy the rest of the artifacts mvn $COMMON_MAVEN_OPTS --threads 8 -DskipStagingRepositoryClose=true -DstagingRepositoryId=$STG_REPO deploy -# Workaround for nexus-staging-maven-plugin:1.6.12:rc-release not working with maven+jdk17 -SWAP_MAVEN_JAVA_WORKAROUND=false -if rpm -q maven-openjdk17 &> /dev/null; then SWAP_MAVEN_JAVA_WORKAROUND=true; fi -if $SWAP_MAVEN_JAVA_WORKAROUND; then dnf swap -y maven-openjdk17 maven-openjdk11; fi - # Close with checks mvn $COMMON_MAVEN_OPTS -N org.sonatype.plugins:nexus-staging-maven-plugin:1.6.12:rc-close -DnexusUrl=https://oss.sonatype.org/ -DserverId=ossrh -DstagingRepositoryId=$STG_REPO # Release if ok mvn $COMMON_MAVEN_OPTS -N org.sonatype.plugins:nexus-staging-maven-plugin:1.6.12:rc-release -DnexusUrl=https://oss.sonatype.org/ -DserverId=ossrh -DstagingRepositoryId=$STG_REPO -# Swap back if we swapped previously -if $SWAP_MAVEN_JAVA_WORKAROUND; then dnf swap -y maven-openjdk11 maven-openjdk17; fi - # Delete the GPG rings rm -rf $SD_SOURCE_DIR/screwdriver/deploy diff --git a/screwdriver/settings-publish.xml b/screwdriver/settings-publish.xml index 5524bf9d7ac..2d6dc2d187c 100644 --- a/screwdriver/settings-publish.xml +++ b/screwdriver/settings-publish.xml @@ -30,7 +30,7 @@ <gpg.publickeyring>pubring.gpg</gpg.publickeyring> <gpg.secretkeyring>secring.gpg</gpg.secretkeyring> <maven.gpg.plugin.version>1.6</maven.gpg.plugin.version> - <nexus.staging.maven.plugin.version>1.6.7</nexus.staging.maven.plugin.version> + <nexus.staging.maven.plugin.version>1.6.12</nexus.staging.maven.plugin.version> </properties> </profile> </profiles> diff --git a/searchcore/src/apps/proton/proton.cpp b/searchcore/src/apps/proton/proton.cpp index ab273fd2660..27ecbd08917 100644 --- a/searchcore/src/apps/proton/proton.cpp +++ b/searchcore/src/apps/proton/proton.cpp @@ -12,9 +12,11 @@ #include <vespa/config/common/configcontext.h> #include <vespa/fnet/transport.h> #include <vespa/fastos/thread.h> +#include <vespa/fastos/file.h> #include <vespa/fastos/app.h> #include <iostream> #include <thread> +#include <fcntl.h> #include <vespa/log/log.h> LOG_SETUP("proton"); @@ -42,6 +44,7 @@ class App : public FastOS_Application { private: static void setupSignals(); + static void setup_fadvise(); Params parseParams(); void startAndRun(FastOS_ThreadPool & threadPool, FNET_Transport & transport); public: @@ -56,6 +59,23 @@ App::setupSignals() SIG::TERM.hook(); } +void +App::setup_fadvise() +{ +#ifdef __linux__ + char * fadvise = getenv("VESPA_FADVISE_OPTIONS"); + if (fadvise != nullptr) { + int fadviseOptions(0); + if (strstr(fadvise, "SEQUENTIAL")) { fadviseOptions |= POSIX_FADV_SEQUENTIAL; } + if (strstr(fadvise, "RANDOM")) { fadviseOptions |= POSIX_FADV_RANDOM; } + if (strstr(fadvise, "WILLNEED")) { fadviseOptions |= POSIX_FADV_WILLNEED; } + if (strstr(fadvise, "DONTNEED")) { fadviseOptions |= POSIX_FADV_DONTNEED; } + if (strstr(fadvise, "NOREUSE")) { fadviseOptions |= POSIX_FADV_NOREUSE; } + FastOS_FileInterface::setDefaultFAdviseOptions(fadviseOptions); + } +#endif +} + Params App::parseParams() { @@ -247,6 +267,7 @@ App::Main() { try { setupSignals(); + setup_fadvise(); FastOS_ThreadPool threadPool(128_Ki); FNET_Transport transport(buildTransportConfig()); transport.Start(&threadPool); diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_thread.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_thread.cpp index ac408cfb2de..f1243665636 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_thread.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/match_thread.cpp @@ -18,7 +18,6 @@ LOG_SETUP(".proton.matching.match_thread"); namespace proton::matching { -using search::queryeval::OptimizedAndNotForBlackListing; using search::queryeval::SearchIterator; using search::fef::MatchData; using search::fef::RankProgram; @@ -48,17 +47,6 @@ struct SimpleStrategy { } }; -// seek_next maps to OptimizedAndNotForBlackListing::seekFast -struct FastBlackListingStrategy { - static bool can_use(bool do_rank, bool do_limit, SearchIterator &search) { - return (!do_rank && !do_limit && - (dynamic_cast<OptimizedAndNotForBlackListing *>(&search) != nullptr)); - } - static uint32_t seek_next(SearchIterator &search, uint32_t docid) { - return static_cast<OptimizedAndNotForBlackListing &>(search).seekFast(docid); - } -}; - LazyValue get_score_feature(const RankProgram &rankProgram) { FeatureResolver resolver(rankProgram.get_seeds()); assert(resolver.num_features() == 1u); @@ -222,11 +210,7 @@ template <bool do_rank, bool do_limit, bool do_share, bool use_rank_drop_limit> void MatchThread::match_loop_helper_rank_limit_share_drop(MatchTools &tools, HitCollector &hits) { - if (FastBlackListingStrategy::can_use(do_rank, do_limit, tools.search())) { - match_loop<FastBlackListingStrategy, do_rank, do_limit, do_share, use_rank_drop_limit>(tools, hits); - } else { - match_loop<SimpleStrategy, do_rank, do_limit, do_share, use_rank_drop_limit>(tools, hits); - } + match_loop<SimpleStrategy, do_rank, do_limit, do_share, use_rank_drop_limit>(tools, hits); } template <bool do_rank, bool do_limit, bool do_share> diff --git a/searchlib/src/vespa/searchlib/attribute/CMakeLists.txt b/searchlib/src/vespa/searchlib/attribute/CMakeLists.txt index e267eaab06e..b3aa3bd958b 100644 --- a/searchlib/src/vespa/searchlib/attribute/CMakeLists.txt +++ b/searchlib/src/vespa/searchlib/attribute/CMakeLists.txt @@ -66,6 +66,8 @@ vespa_add_library(searchlib_attribute OBJECT loadedvalue.cpp multi_numeric_enum_search_context.cpp multi_numeric_search_context.cpp + multi_string_enum_search_context.cpp + multi_string_enum_hint_search_context.cpp multi_value_mapping.cpp multi_value_mapping_base.cpp multienumattribute.cpp @@ -108,6 +110,7 @@ vespa_add_library(searchlib_attribute OBJECT singlestringpostattribute.cpp single_numeric_enum_search_context.cpp single_numeric_search_context.cpp + single_small_numeric_search_context.cpp single_string_enum_search_context.cpp single_string_enum_hint_search_context.cpp sourceselector.cpp diff --git a/searchlib/src/vespa/searchlib/attribute/multi_string_enum_hint_search_context.cpp b/searchlib/src/vespa/searchlib/attribute/multi_string_enum_hint_search_context.cpp new file mode 100644 index 00000000000..55886ac85fa --- /dev/null +++ b/searchlib/src/vespa/searchlib/attribute/multi_string_enum_hint_search_context.cpp @@ -0,0 +1,15 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "multi_string_enum_hint_search_context.hpp" +#include <vespa/searchcommon/attribute/multivalue.h> + +using ValueRef = search::multivalue::Value<vespalib::datastore::AtomicEntryRef>; +using WeightedValueRef = search::multivalue::WeightedValue<vespalib::datastore::AtomicEntryRef>; + +namespace search::attribute { + +template class MultiStringEnumHintSearchContext<ValueRef>; + +template class MultiStringEnumHintSearchContext<WeightedValueRef>; + +} diff --git a/searchlib/src/vespa/searchlib/attribute/multi_string_enum_hint_search_context.h b/searchlib/src/vespa/searchlib/attribute/multi_string_enum_hint_search_context.h new file mode 100644 index 00000000000..92650851116 --- /dev/null +++ b/searchlib/src/vespa/searchlib/attribute/multi_string_enum_hint_search_context.h @@ -0,0 +1,24 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include "multi_string_enum_search_context.h" +#include "enumhintsearchcontext.h" + +namespace search::attribute { + +/* + * MultiStringEnumHintSearchContext handles the creation of search iterators + * for a query term on a multi value string enumerated attribute vector using + * dictionary information to eliminate searches for nonexisting words. + */ +template <typename M> +class MultiStringEnumHintSearchContext : public MultiStringEnumSearchContext<M>, + public EnumHintSearchContext +{ +public: + MultiStringEnumHintSearchContext(std::unique_ptr<QueryTermSimple> qTerm, bool cased, const AttributeVector& toBeSearched, const MultiValueMapping<M>& mv_mapping, const EnumStoreT<const char*>& enum_store, uint32_t doc_id_limit, uint64_t num_values); + ~MultiStringEnumHintSearchContext() override; +}; + +} diff --git a/searchlib/src/vespa/searchlib/attribute/multi_string_enum_hint_search_context.hpp b/searchlib/src/vespa/searchlib/attribute/multi_string_enum_hint_search_context.hpp new file mode 100644 index 00000000000..a6b0f3f5eb9 --- /dev/null +++ b/searchlib/src/vespa/searchlib/attribute/multi_string_enum_hint_search_context.hpp @@ -0,0 +1,20 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "multi_string_enum_hint_search_context.h" +#include <vespa/searchlib/query/query_term_ucs4.h> + +namespace search::attribute { + +template <typename M> +MultiStringEnumHintSearchContext<M>::MultiStringEnumHintSearchContext(std::unique_ptr<QueryTermSimple> qTerm, bool cased, const AttributeVector& toBeSearched, const MultiValueMapping<M>& mv_mapping, const EnumStoreT<const char*>& enum_store, uint32_t doc_id_limit, uint64_t num_values) + : MultiStringEnumSearchContext<M>(std::move(qTerm), cased, toBeSearched, mv_mapping, enum_store), + EnumHintSearchContext(enum_store.get_dictionary(), + doc_id_limit, num_values) +{ + this->setup_enum_hint_sc(enum_store, *this); +} + +template <typename M> +MultiStringEnumHintSearchContext<M>::~MultiStringEnumHintSearchContext() = default; + +} diff --git a/searchlib/src/vespa/searchlib/attribute/multi_string_enum_search_context.cpp b/searchlib/src/vespa/searchlib/attribute/multi_string_enum_search_context.cpp new file mode 100644 index 00000000000..4abaf02e2e8 --- /dev/null +++ b/searchlib/src/vespa/searchlib/attribute/multi_string_enum_search_context.cpp @@ -0,0 +1,15 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "multi_string_enum_search_context.hpp" +#include <vespa/searchcommon/attribute/multivalue.h> + +using ValueRef = search::multivalue::Value<vespalib::datastore::AtomicEntryRef>; +using WeightedValueRef = search::multivalue::WeightedValue<vespalib::datastore::AtomicEntryRef>; + +namespace search::attribute { + +template class MultiStringEnumSearchContext<ValueRef>; + +template class MultiStringEnumSearchContext<WeightedValueRef>; + +} diff --git a/searchlib/src/vespa/searchlib/attribute/multi_string_enum_search_context.h b/searchlib/src/vespa/searchlib/attribute/multi_string_enum_search_context.h new file mode 100644 index 00000000000..a4f05a5c9cc --- /dev/null +++ b/searchlib/src/vespa/searchlib/attribute/multi_string_enum_search_context.h @@ -0,0 +1,21 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include "multi_enum_search_context.h" +#include "string_search_context.h" + +namespace search::attribute { + +/* + * MultiStringEnumSearchContext handles the creation of search iterators for + * a query term on a multi value string enumerated attribute vector. + */ +template <typename M> +class MultiStringEnumSearchContext : public MultiEnumSearchContext<const char*, StringSearchContext, M> +{ +public: + MultiStringEnumSearchContext(std::unique_ptr<QueryTermSimple> qTerm, bool cased, const AttributeVector& toBeSearched, const MultiValueMapping<M>& mv_mapping, const EnumStoreT<const char*>& enum_store); +}; + +} diff --git a/searchlib/src/vespa/searchlib/attribute/multi_string_enum_search_context.hpp b/searchlib/src/vespa/searchlib/attribute/multi_string_enum_search_context.hpp new file mode 100644 index 00000000000..02a740b06dc --- /dev/null +++ b/searchlib/src/vespa/searchlib/attribute/multi_string_enum_search_context.hpp @@ -0,0 +1,17 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include "multi_string_enum_search_context.h" +#include "multi_enum_search_context.hpp" +#include <vespa/searchlib/query/query_term_simple.h> + +namespace search::attribute { + +template <typename M> +MultiStringEnumSearchContext<M>::MultiStringEnumSearchContext(std::unique_ptr<QueryTermSimple> qTerm, bool cased, const AttributeVector& toBeSearched, const MultiValueMapping<M>& mv_mapping, const EnumStoreT<const char*>& enum_store) + : MultiEnumSearchContext<const char*, StringSearchContext, M>(StringMatcher(std::move(qTerm), cased), toBeSearched, mv_mapping, enum_store) +{ +} + +} diff --git a/searchlib/src/vespa/searchlib/attribute/multistringattribute.h b/searchlib/src/vespa/searchlib/attribute/multistringattribute.h index e832f53777b..cf4169138fe 100644 --- a/searchlib/src/vespa/searchlib/attribute/multistringattribute.h +++ b/searchlib/src/vespa/searchlib/attribute/multistringattribute.h @@ -7,7 +7,6 @@ #include "enumstore.h" #include "multienumattribute.h" #include "multi_value_mapping.h" -#include "enumhintsearchcontext.h" #include <vespa/searchcommon/attribute/multivalue.h> namespace search { @@ -104,58 +103,6 @@ public: return getWeightedHelper(doc, v, sz); } - /* - * Specialization of SearchContext for weighted set type - */ - class StringImplSearchContext : public StringAttribute::StringSearchContext { - public: - StringImplSearchContext(QueryTermSimpleUP qTerm, const StringAttribute & toBeSearched) : - StringAttribute::StringSearchContext(std::move(qTerm), toBeSearched) - { } - protected: - const MultiValueStringAttributeT<B, M> & myAttribute() const { - return static_cast< const MultiValueStringAttributeT<B, M> & > (attribute()); - } - int32_t onFind(DocId docId, int32_t elemId) const override; - - template <typename Collector> - int32_t findNextWeight(DocId doc, int32_t elemId, int32_t & weight, Collector & collector) const; - }; - - /* - * Specialization of SearchContext for weighted set type - */ - class StringSetImplSearchContext : public StringImplSearchContext { - public: - StringSetImplSearchContext(attribute::SearchContext::QueryTermSimpleUP qTerm, const StringAttribute & toBeSearched) : - StringImplSearchContext(std::move(qTerm), toBeSearched) - { } - protected: - int32_t onFind(DocId docId, int32_t elemId, int32_t &weight) const override; - }; - - /* - * Specialization of SearchContext for array type - */ - class StringArrayImplSearchContext : public StringImplSearchContext { - public: - StringArrayImplSearchContext(attribute::SearchContext::QueryTermSimpleUP qTerm, const StringAttribute & toBeSearched) : - StringImplSearchContext(std::move(qTerm), toBeSearched) - { } - protected: - int32_t onFind(DocId docId, int32_t elemId, int32_t &weight) const override; - }; - - template <typename BT> - class StringTemplSearchContext : public BT, - public attribute::EnumHintSearchContext - { - using BT::queryTerm; - using AttrType = MultiValueStringAttributeT<B, M>; - public: - StringTemplSearchContext(attribute::SearchContext::QueryTermSimpleUP qTerm, const AttrType & toBeSearched); - }; - std::unique_ptr<attribute::SearchContext> getSearch(QueryTermSimpleUP term, const attribute::SearchContextParams & params) const override; }; diff --git a/searchlib/src/vespa/searchlib/attribute/multistringattribute.hpp b/searchlib/src/vespa/searchlib/attribute/multistringattribute.hpp index a6825cfb9bd..212a71dad74 100644 --- a/searchlib/src/vespa/searchlib/attribute/multistringattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/multistringattribute.hpp @@ -6,6 +6,7 @@ #include "multistringattribute.h" #include "enumattribute.hpp" #include "multienumattribute.hpp" +#include "multi_string_enum_hint_search_context.h" #include <vespa/vespalib/text/utf8.h> #include <vespa/vespalib/text/lowercase.h> #include <vespa/searchlib/util/bufferwriter.h> @@ -41,80 +42,8 @@ std::unique_ptr<attribute::SearchContext> MultiValueStringAttributeT<B, M>::getSearch(QueryTermSimpleUP qTerm, const attribute::SearchContextParams &) const { - if (this->getCollectionType() == attribute::CollectionType::WSET) { - return std::make_unique<StringTemplSearchContext<StringSetImplSearchContext>>(std::move(qTerm), *this); - } else { - return std::make_unique<StringTemplSearchContext<StringArrayImplSearchContext>>(std::move(qTerm), *this); - } -} - -namespace { - -template <typename E> -class EnumAccessor { -public: - EnumAccessor(const E & enumStore) : _enumStore(enumStore) { } - const char * get(typename E::Index index) const { return _enumStore.get_value(index); } -private: - const E & _enumStore; -}; - -} - -template <typename B, typename M> -int32_t -MultiValueStringAttributeT<B, M>::StringSetImplSearchContext::onFind(DocId doc, int32_t elemId, int32_t &weight) const -{ - StringAttribute::StringSearchContext::CollectWeight collector; - return this->findNextWeight(doc, elemId, weight, collector); -} - -template <typename B, typename M> -int32_t -MultiValueStringAttributeT<B, M>::StringArrayImplSearchContext::onFind(DocId doc, int32_t elemId, int32_t &weight) const -{ - StringAttribute::StringSearchContext::CollectHitCount collector; - return this->findNextWeight(doc, elemId, weight, collector); -} - -template <typename B, typename M> -template <typename Collector> -int32_t -MultiValueStringAttributeT<B, M>::StringImplSearchContext::findNextWeight(DocId doc, int32_t elemId, int32_t & weight, Collector & collector) const -{ - WeightedIndexArrayRef indices(myAttribute()._mvMapping.get(doc)); - - EnumAccessor<typename B::EnumStore> accessor(myAttribute()._enumStore); - int32_t foundElem = findNextMatch(indices, elemId, accessor, collector); - weight = collector.getWeight(); - return foundElem; -} - -template <typename B, typename M> -int32_t -MultiValueStringAttributeT<B, M>::StringImplSearchContext::onFind(DocId doc, int32_t elemId) const -{ - const auto& attr = static_cast<const MultiValueStringAttributeT<B, M>&>(attribute()); - WeightedIndexArrayRef indices(attr._mvMapping.get(doc)); - for (uint32_t i(elemId); i < indices.size(); i++) { - if (isMatch(attr._enumStore.get_value(indices[i].value_ref().load_acquire()))) { - return i; - } - } - - return -1; -} - -template <typename B, typename M> -template <typename BT> -MultiValueStringAttributeT<B, M>::StringTemplSearchContext<BT>:: -StringTemplSearchContext(QueryTermSimpleUP qTerm, const AttrType & toBeSearched) : - BT(std::move(qTerm), toBeSearched), - EnumHintSearchContext(toBeSearched.getEnumStore().get_dictionary(), - toBeSearched.getCommittedDocIdLimit(), - toBeSearched.getStatus().getNumValues()) -{ - this->setup_enum_hint_sc(toBeSearched.getEnumStore(), *this); + bool cased = this->get_match_is_cased(); + return std::make_unique<attribute::MultiStringEnumHintSearchContext<M>>(std::move(qTerm), cased, *this, this->_mvMapping, this->_enumStore, this->getCommittedDocIdLimit(), this->getStatus().getNumValues()); } } // namespace search diff --git a/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.h b/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.h index 9f8827028cc..17a67a67ddf 100644 --- a/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.h +++ b/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.h @@ -54,10 +54,6 @@ private: using PostingMap = typename PostingParent::PostingMap; using QueryTermSimpleUP = AttributeVector::QueryTermSimpleUP; using SelfType = MultiValueStringPostingAttributeT<B, T>; - using StringArrayImplSearchContext = typename MultiValueStringAttributeT<B, T>::StringArrayImplSearchContext; - using StringArrayPostingSearchContext = attribute::StringPostingSearchContext<StringArrayImplSearchContext, SelfType, int32_t>; - using StringSetImplSearchContext = typename MultiValueStringAttributeT<B, T>::StringSetImplSearchContext; - using StringSetPostingSearchContext = attribute::StringPostingSearchContext<StringSetImplSearchContext, SelfType, int32_t>; using WeightedIndex = typename MultiValueStringAttributeT<B, T>::WeightedIndex; using generation_t = typename MultiValueStringAttributeT<B, T>::generation_t; diff --git a/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.hpp b/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.hpp index 13de3bc6493..2c2ac48979d 100644 --- a/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.hpp @@ -5,6 +5,7 @@ #include "stringattribute.h" #include "multistringpostattribute.h" #include "multistringattribute.hpp" +#include "multi_string_enum_search_context.h" #include <vespa/searchlib/query/query_term_simple.h> namespace search { @@ -89,9 +90,10 @@ std::unique_ptr<attribute::SearchContext> MultiValueStringPostingAttributeT<B, T>::getSearch(QueryTermSimpleUP qTerm, const attribute::SearchContextParams & params) const { - using BaseSC = std::conditional_t<T::_hasWeight, StringSetImplSearchContext, StringArrayImplSearchContext>; - using SC = std::conditional_t<T::_hasWeight, StringSetPostingSearchContext, StringArrayPostingSearchContext>; - BaseSC base_sc(std::move(qTerm), *this); + using BaseSC = attribute::MultiStringEnumSearchContext<T>; + using SC = attribute::StringPostingSearchContext<BaseSC, SelfType, int32_t>; + bool cased = this->get_match_is_cased(); + BaseSC base_sc(std::move(qTerm), cased, *this, this->_mvMapping, this->_enumStore); return std::make_unique<SC>(std::move(base_sc), params.useBitVector(), *this); } diff --git a/searchlib/src/vespa/searchlib/attribute/single_small_numeric_search_context.cpp b/searchlib/src/vespa/searchlib/attribute/single_small_numeric_search_context.cpp new file mode 100644 index 00000000000..5eeef7cd61a --- /dev/null +++ b/searchlib/src/vespa/searchlib/attribute/single_small_numeric_search_context.cpp @@ -0,0 +1,35 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "single_small_numeric_search_context.h" +#include "attributeiterators.hpp" +#include <vespa/searchlib/queryeval/emptysearch.h> + +namespace search::attribute { + +SingleSmallNumericSearchContext::SingleSmallNumericSearchContext(std::unique_ptr<QueryTermSimple> qTerm, const AttributeVector& toBeSearched, const Word* word_data, Word value_mask, uint32_t value_shift_shift, uint32_t value_shift_mask, uint32_t word_shift) + : NumericSearchContext<NumericRangeMatcher<T>>(toBeSearched, *qTerm, false), + _wordData(word_data), + _valueMask(value_mask), + _valueShiftShift(value_shift_shift), + _valueShiftMask(value_shift_mask), + _wordShift(word_shift) +{ +} + +std::unique_ptr<queryeval::SearchIterator> +SingleSmallNumericSearchContext::createFilterIterator(fef::TermFieldMatchData* matchData, bool strict) +{ + if (!valid()) { + return std::make_unique<queryeval::EmptySearch>(); + } + if (getIsFilter()) { + return strict + ? std::make_unique<FilterAttributeIteratorStrict<SingleSmallNumericSearchContext>>(*this, matchData) + : std::make_unique<FilterAttributeIteratorT<SingleSmallNumericSearchContext>>(*this, matchData); + } + return strict + ? std::make_unique<AttributeIteratorStrict<SingleSmallNumericSearchContext>>(*this, matchData) + : std::make_unique<AttributeIteratorT<SingleSmallNumericSearchContext>>(*this, matchData); +} + +} diff --git a/searchlib/src/vespa/searchlib/attribute/single_small_numeric_search_context.h b/searchlib/src/vespa/searchlib/attribute/single_small_numeric_search_context.h new file mode 100644 index 00000000000..46ed02b3eca --- /dev/null +++ b/searchlib/src/vespa/searchlib/attribute/single_small_numeric_search_context.h @@ -0,0 +1,58 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include "numeric_search_context.h" +#include "numeric_range_matcher.h" +#include <vespa/vespalib/util/atomic.h> + +namespace search::attribute { + +/* + * SingleSmallNumericSearchContext handles the creation of search iterators for + * a query term on a single value small numeric attribute vector. + */ +class SingleSmallNumericSearchContext : public NumericSearchContext<NumericRangeMatcher<int8_t>> +{ +private: + using Word = uint32_t; + using T = int8_t; + const Word *_wordData; + Word _valueMask; + uint32_t _valueShiftShift; + uint32_t _valueShiftMask; + uint32_t _wordShift; + + int32_t onFind(DocId docId, int32_t elementId, int32_t & weight) const override { + return find(docId, elementId, weight); + } + + int32_t onFind(DocId docId, int32_t elementId) const override { + return find(docId, elementId); + } + +public: + SingleSmallNumericSearchContext(std::unique_ptr<QueryTermSimple> qTerm, const AttributeVector& toBeSearched, const Word* word_data, Word value_mask, uint32_t value_shift_shift, uint32_t value_shift_mask, uint32_t word_shift); + + int32_t find(DocId docId, int32_t elemId, int32_t & weight) const { + if ( elemId != 0) return -1; + const Word &word = _wordData[docId >> _wordShift]; + uint32_t valueShift = (docId & _valueShiftMask) << _valueShiftShift; + T v = (vespalib::atomic::load_ref_relaxed(word) >> valueShift) & _valueMask; + weight = 1; + return match(v) ? 0 : -1; + } + + int32_t find(DocId docId, int32_t elemId) const { + if ( elemId != 0) return -1; + const Word &word = _wordData[docId >> _wordShift]; + uint32_t valueShift = (docId & _valueShiftMask) << _valueShiftShift; + T v = (vespalib::atomic::load_ref_relaxed(word) >> valueShift) & _valueMask; + return match(v) ? 0 : -1; + } + + std::unique_ptr<queryeval::SearchIterator> + createFilterIterator(fef::TermFieldMatchData* matchData, bool strict) override; +}; + +} diff --git a/searchlib/src/vespa/searchlib/attribute/single_string_enum_hint_search_context.cpp b/searchlib/src/vespa/searchlib/attribute/single_string_enum_hint_search_context.cpp index 56c35d4b0b2..70023b27802 100644 --- a/searchlib/src/vespa/searchlib/attribute/single_string_enum_hint_search_context.cpp +++ b/searchlib/src/vespa/searchlib/attribute/single_string_enum_hint_search_context.cpp @@ -2,7 +2,6 @@ #include "single_string_enum_hint_search_context.h" #include <vespa/searchlib/query/query_term_ucs4.h> -#include <vespa/vespalib/util/regexp.h> namespace search::attribute { diff --git a/searchlib/src/vespa/searchlib/attribute/singlesmallnumericattribute.cpp b/searchlib/src/vespa/searchlib/attribute/singlesmallnumericattribute.cpp index 009078447dc..eca74255026 100644 --- a/searchlib/src/vespa/searchlib/attribute/singlesmallnumericattribute.cpp +++ b/searchlib/src/vespa/searchlib/attribute/singlesmallnumericattribute.cpp @@ -1,12 +1,11 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "singlesmallnumericattribute.h" -#include "attributeiterators.hpp" #include "attributevector.hpp" #include "iattributesavetarget.h" #include "primitivereader.h" +#include "single_small_numeric_search_context.h" #include <vespa/searchlib/query/query_term_simple.h> -#include <vespa/searchlib/queryeval/emptysearch.h> #include <vespa/searchlib/util/file_settings.h> #include <vespa/vespalib/data/databuffer.h> #include <vespa/vespalib/util/size_literals.h> @@ -172,7 +171,7 @@ std::unique_ptr<attribute::SearchContext> SingleValueSmallNumericAttribute::getSearch(std::unique_ptr<QueryTermSimple> qTerm, const attribute::SearchContextParams &) const { - return std::make_unique<SingleSearchContext>(std::move(qTerm), *this); + return std::make_unique<attribute::SingleSmallNumericSearchContext>(std::move(qTerm), *this, &_wordData.acquire_elem_ref(0), _valueMask, _valueShiftShift, _valueShiftMask, _wordShift); } void @@ -208,40 +207,6 @@ SingleValueSmallNumericAttribute::getEstimatedSaveByteSize() const return headerSize + sz; } -bool SingleValueSmallNumericAttribute::SingleSearchContext::valid() const { return this->isValid(); } - - -SingleValueSmallNumericAttribute::SingleSearchContext::SingleSearchContext(std::unique_ptr<QueryTermSimple> qTerm, - const SingleValueSmallNumericAttribute & toBeSearched) - : attribute::NumericRangeMatcher<T>(*qTerm), - SearchContext(toBeSearched), _wordData(&toBeSearched._wordData.acquire_elem_ref(0)), - _valueMask(toBeSearched._valueMask), - _valueShiftShift(toBeSearched._valueShiftShift), - _valueShiftMask(toBeSearched._valueShiftMask), - _wordShift(toBeSearched._wordShift) -{ } - -Int64Range -SingleValueSmallNumericAttribute::SingleSearchContext::getAsIntegerTerm() const { - return this->getRange(); -} - -std::unique_ptr<queryeval::SearchIterator> -SingleValueSmallNumericAttribute::SingleSearchContext::createFilterIterator(fef::TermFieldMatchData * matchData, bool strict) -{ - if (!valid()) { - return std::make_unique<queryeval::EmptySearch>(); - } - if (getIsFilter()) { - return strict - ? std::make_unique<FilterAttributeIteratorStrict<SingleSearchContext>>(*this, matchData) - : std::make_unique<FilterAttributeIteratorT<SingleSearchContext>>(*this, matchData); - } - return strict - ? std::make_unique<AttributeIteratorStrict<SingleSearchContext>>(*this, matchData) - : std::make_unique<AttributeIteratorT<SingleSearchContext>>(*this, matchData); -} - namespace { template <typename TT> diff --git a/searchlib/src/vespa/searchlib/attribute/singlesmallnumericattribute.h b/searchlib/src/vespa/searchlib/attribute/singlesmallnumericattribute.h index 77c4133817c..f6059d3d510 100644 --- a/searchlib/src/vespa/searchlib/attribute/singlesmallnumericattribute.h +++ b/searchlib/src/vespa/searchlib/attribute/singlesmallnumericattribute.h @@ -3,8 +3,6 @@ #pragma once #include "integerbase.h" -#include "floatbase.h" -#include "numeric_range_matcher.h" #include "search_context.h" #include <vespa/vespalib/util/atomic.h> #include <vespa/vespalib/util/rcuvector.h> @@ -15,7 +13,6 @@ namespace search { class SingleValueSmallNumericAttribute : public IntegerAttributeTemplate<int8_t> { private: -// friend class attribute::SearchContext; typedef IntegerAttributeTemplate<int8_t> B; typedef B::BaseType T; typedef B::DocId DocId; @@ -58,53 +55,6 @@ protected: public: - /* - * Specialization of SearchContext - */ - class SingleSearchContext : public attribute::NumericRangeMatcher<T>, public attribute::SearchContext - { - private: - const Word *_wordData; - Word _valueMask; - uint32_t _valueShiftShift; - uint32_t _valueShiftMask; - uint32_t _wordShift; - - int32_t onFind(DocId docId, int32_t elementId, int32_t & weight) const override { - return find(docId, elementId, weight); - } - - int32_t onFind(DocId docId, int32_t elementId) const override { - return find(docId, elementId); - } - - bool valid() const override; - - public: - SingleSearchContext(std::unique_ptr<QueryTermSimple> qTerm, const SingleValueSmallNumericAttribute & toBeSearched); - - int32_t find(DocId docId, int32_t elemId, int32_t & weight) const { - if ( elemId != 0) return -1; - const Word &word = _wordData[docId >> _wordShift]; - uint32_t valueShift = (docId & _valueShiftMask) << _valueShiftShift; - T v = (vespalib::atomic::load_ref_relaxed(word) >> valueShift) & _valueMask; - weight = 1; - return match(v) ? 0 : -1; - } - - int32_t find(DocId docId, int32_t elemId) const { - if ( elemId != 0) return -1; - const Word &word = _wordData[docId >> _wordShift]; - uint32_t valueShift = (docId & _valueShiftMask) << _valueShiftShift; - T v = (vespalib::atomic::load_ref_relaxed(word) >> valueShift) & _valueMask; - return match(v) ? 0 : -1; - } - - Int64Range getAsIntegerTerm() const override; - - std::unique_ptr<queryeval::SearchIterator> - createFilterIterator(fef::TermFieldMatchData * matchData, bool strict) override; - }; SingleValueSmallNumericAttribute(const vespalib::string & baseFileName, const Config &c, Word valueMask, uint32_t valueShiftShift, uint32_t valueShiftMask, uint32_t wordShift); diff --git a/searchlib/src/vespa/searchlib/attribute/stringbase.cpp b/searchlib/src/vespa/searchlib/attribute/stringbase.cpp index f30792099f8..b60ec269383 100644 --- a/searchlib/src/vespa/searchlib/attribute/stringbase.cpp +++ b/searchlib/src/vespa/searchlib/attribute/stringbase.cpp @@ -152,16 +152,6 @@ StringAttribute::onSerializeForDescendingSort(DocId doc, void * serTo, long avai return buf.size(); } -StringAttribute::StringSearchContext::StringSearchContext(QueryTermSimple::UP qTerm, - const StringAttribute & toBeSearched) - : attribute::StringSearchContext(toBeSearched, std::move(qTerm), toBeSearched.getConfig().get_match() == Config::Match::CASED) -{ -} - -StringAttribute::StringSearchContext::StringSearchContext(StringSearchContext&&) noexcept = default; - -StringAttribute::StringSearchContext::~StringSearchContext() = default; - uint32_t StringAttribute::clearDoc(DocId doc) { diff --git a/searchlib/src/vespa/searchlib/attribute/stringbase.h b/searchlib/src/vespa/searchlib/attribute/stringbase.h index c29626c13ea..e5e14829118 100644 --- a/searchlib/src/vespa/searchlib/attribute/stringbase.h +++ b/searchlib/src/vespa/searchlib/attribute/stringbase.h @@ -89,53 +89,6 @@ private: long onSerializeForAscendingSort(DocId doc, void * serTo, long available, const common::BlobConverter * bc) const override; long onSerializeForDescendingSort(DocId doc, void * serTo, long available, const common::BlobConverter * bc) const override; - -protected: - class StringSearchContext : public attribute::StringSearchContext { - public: - StringSearchContext(QueryTermSimpleUP qTerm, const StringAttribute & toBeSearched); - StringSearchContext(StringSearchContext&&) noexcept; - ~StringSearchContext() override; - protected: - bool isMatch(const char *src) const { return match(src); } - - class CollectHitCount { - public: - CollectHitCount() : _hitCount(0) { } - void addWeight(int32_t w) { - (void) w; - _hitCount++; - } - int32_t getWeight() const { return _hitCount; } - bool hasMatch() const { return _hitCount != 0; } - private: - uint32_t _hitCount; - }; - class CollectWeight { - public: - CollectWeight() : _hitCount(0), _weight(0) { } - void addWeight(int32_t w) { - _weight += w; - _hitCount++; - } - int32_t getWeight() const { return _weight; } - bool hasMatch() const { return _hitCount != 0; } - private: - uint32_t _hitCount; - int32_t _weight; - }; - - template<typename WeightedT, typename Accessor, typename Collector> - int32_t findNextMatch(vespalib::ConstArrayRef<WeightedT> w, int32_t elemId, const Accessor & ac, Collector & collector) const { - for (uint32_t i(elemId); i < w.size(); i++) { - if (isMatch(ac.get(w[i].value_ref().load_acquire()))) { - collector.addWeight(w[i].weight()); - return i; - } - } - return -1; - } - }; }; } diff --git a/searchlib/src/vespa/searchlib/queryeval/andnotsearch.cpp b/searchlib/src/vespa/searchlib/queryeval/andnotsearch.cpp index 2307b778381..c7b81bc9da7 100644 --- a/searchlib/src/vespa/searchlib/queryeval/andnotsearch.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/andnotsearch.cpp @@ -105,41 +105,11 @@ AndNotSearchStrict::internalSeek(uint32_t docid) } // namespace -OptimizedAndNotForBlackListing::OptimizedAndNotForBlackListing(MultiSearch::Children children) : - AndNotSearchStrictBase(std::move(children)) -{ -} - -void OptimizedAndNotForBlackListing::initRange(uint32_t beginid, uint32_t endid) -{ - AndNotSearch::initRange(beginid, endid); - setDocId(internalSeek<false>(beginid)); -} - -bool OptimizedAndNotForBlackListing::isBlackListIterator(const SearchIterator * iterator) -{ - return dynamic_cast<const BlackListIterator *>(iterator) != 0; -} - -void OptimizedAndNotForBlackListing::doSeek(uint32_t docid) -{ - setDocId(internalSeek<true>(docid)); -} - -void OptimizedAndNotForBlackListing::doUnpack(uint32_t docid) -{ - positive()->doUnpack(docid); -} - std::unique_ptr<SearchIterator> AndNotSearch::create(ChildrenIterators children_in, bool strict) { MultiSearch::Children children = std::move(children_in); if (strict) { - if ((children.size() == 2) && OptimizedAndNotForBlackListing::isBlackListIterator(children[1].get())) { - return std::make_unique<OptimizedAndNotForBlackListing>(std::move(children)); - } else { - return std::make_unique<AndNotSearchStrict>(std::move(children)); - } + return std::make_unique<AndNotSearchStrict>(std::move(children)); } else { return SearchIterator::UP(new AndNotSearch(std::move(children))); } diff --git a/searchlib/src/vespa/searchlib/queryeval/andnotsearch.h b/searchlib/src/vespa/searchlib/queryeval/andnotsearch.h index d65a3d9c72e..e474ab7c90c 100644 --- a/searchlib/src/vespa/searchlib/queryeval/andnotsearch.h +++ b/searchlib/src/vespa/searchlib/queryeval/andnotsearch.h @@ -48,50 +48,4 @@ private: UP andWith(UP filter, uint32_t estimate) override; }; -/** - * This is a specialized andnot iterator you get when you have no andnot's in you query and only get the blacklist blueprint. - * This one is now constructed at getSearch() phase. However this should be better handled in the AndNotBlueprint. - */ -class OptimizedAndNotForBlackListing : public AndNotSearchStrictBase -{ -private: - // This is the actual iterator that should be produced by the documentmetastore in searchcore, but that - // will probably be changed later on. An ordinary bitvector could be even better as that would open up for more optimizations. - //typedef FilterAttributeIteratorT<SingleValueSmallNumericAttribute::SingleSearchContext> BlackListIterator; - typedef AttributeIteratorT<SingleValueSmallNumericAttribute::SingleSearchContext> BlackListIterator; -public: - OptimizedAndNotForBlackListing(MultiSearch::Children children); - static bool isBlackListIterator(const SearchIterator * iterator); - - uint32_t seekFast(uint32_t docid) { - return internalSeek<true>(docid); - } - void initRange(uint32_t beginid, uint32_t endid) override; -private: - SearchIterator * positive() { return getChildren()[0].get(); } - BlackListIterator * blackList() { return static_cast<BlackListIterator *>(getChildren()[1].get()); } - template<bool doSeekOnly> - uint32_t internalSeek(uint32_t docid) { - uint32_t curr(docid); - while (true) { - if (doSeekOnly) { - positive()->doSeek(curr); - } else { - positive()->seek(curr); - } - if ( ! positive()->isAtEnd() ) { - curr = positive()->getDocId(); - if (! blackList()->seekFast(curr)) { - return curr; - } - curr++; - } else { - return search::endDocId; - } - } - } - void doSeek(uint32_t docid) override; - void doUnpack(uint32_t docid) override; -}; - } diff --git a/testutil/src/main/java/com/yahoo/test/OrderTester.java b/testutil/src/main/java/com/yahoo/test/OrderTester.java index 4acba4ee7fe..cc28ca9f469 100644 --- a/testutil/src/main/java/com/yahoo/test/OrderTester.java +++ b/testutil/src/main/java/com/yahoo/test/OrderTester.java @@ -15,8 +15,8 @@ import java.util.List; * */ -public abstract class OrderTester<T extends Comparable<T>> { - private ArrayList<List<T>> groups = new ArrayList<>(); +public abstract class OrderTester<T extends Comparable<? super T>> { + private final ArrayList<List<T>> groups = new ArrayList<>(); abstract protected void lessTest(T a, T b); abstract protected void greaterTest(T a, T b); @@ -24,7 +24,7 @@ public abstract class OrderTester<T extends Comparable<T>> { @SafeVarargs @SuppressWarnings("varargs") - private final OrderTester<T> addGroup(T... group) { + private OrderTester<T> addGroup(T... group) { groups.add(Arrays.asList(group)); return this; } diff --git a/testutil/src/main/java/com/yahoo/test/TotalOrderTester.java b/testutil/src/main/java/com/yahoo/test/TotalOrderTester.java index 850369fbc2e..e95bc056ba8 100644 --- a/testutil/src/main/java/com/yahoo/test/TotalOrderTester.java +++ b/testutil/src/main/java/com/yahoo/test/TotalOrderTester.java @@ -20,7 +20,7 @@ import static org.junit.Assert.assertTrue; * @author Vegard Sjonfjell */ -public class TotalOrderTester<T extends Comparable<T>> extends OrderTester<T> { +public class TotalOrderTester<T extends Comparable<? super T>> extends OrderTester<T> { protected void lessTest(T a, T b) throws AssertionError { assertTrue(a + " must be less than " + b, a.compareTo(b) <= -1); } diff --git a/vespa-hadoop/pom.xml b/vespa-hadoop/pom.xml index 2530e461354..6fe87541448 100644 --- a/vespa-hadoop/pom.xml +++ b/vespa-hadoop/pom.xml @@ -126,7 +126,21 @@ <groupId>com.yahoo.vespa</groupId> <artifactId>vespa-feed-client</artifactId> <version>${project.version}</version> - <scope>compile</scope> + </dependency> + <dependency> + <groupId>com.yahoo.vespa</groupId> + <artifactId>vespa-feed-client-api</artifactId> + <version>${project.version}</version> + </dependency> + + <!-- Jackson dependencies used in this module --> + <dependency> + <groupId>com.fasterxml.jackson.core</groupId> + <artifactId>jackson-core</artifactId> + </dependency> + <dependency> + <groupId>com.fasterxml.jackson.core</groupId> + <artifactId>jackson-databind</artifactId> </dependency> </dependencies> diff --git a/vespaclient-core/src/main/java/com/yahoo/feedapi/MessageBusSessionFactory.java b/vespaclient-core/src/main/java/com/yahoo/feedapi/MessageBusSessionFactory.java index 514669fe0ac..d9b1190aaed 100755 --- a/vespaclient-core/src/main/java/com/yahoo/feedapi/MessageBusSessionFactory.java +++ b/vespaclient-core/src/main/java/com/yahoo/feedapi/MessageBusSessionFactory.java @@ -18,7 +18,8 @@ public class MessageBusSessionFactory implements SessionFactory { public MessageBusSessionFactory(MessagePropertyProcessor processor) { this(processor, null, null); } - + + @SuppressWarnings("removal") // TODO: Remove on Vespa 8 private MessageBusSessionFactory(MessagePropertyProcessor processor, DocumentmanagerConfig documentmanagerConfig, SlobroksConfig slobroksConfig) { diff --git a/vespaclient-core/src/main/java/com/yahoo/feedapi/MessagePropertyProcessor.java b/vespaclient-core/src/main/java/com/yahoo/feedapi/MessagePropertyProcessor.java index e5da51f0918..84fbe63a576 100644 --- a/vespaclient-core/src/main/java/com/yahoo/feedapi/MessagePropertyProcessor.java +++ b/vespaclient-core/src/main/java/com/yahoo/feedapi/MessagePropertyProcessor.java @@ -33,10 +33,18 @@ public class MessagePropertyProcessor implements ConfigSubscriber.SingleSubscrib private String defaultDocprocChain = null; private boolean defaultAbortOnDocumentError = true; private boolean defaultAbortOnSendError = true; - private final LoadTypeSet loadTypes; + private final LoadTypeSet loadTypes; // TODO remove on Vespa 8 private boolean configChanged = false; + public MessagePropertyProcessor(FeederConfig config) { + loadTypes = new LoadTypeSet(); + configure(config); + } + /** + * @deprecated load types are deprecated. Use constructor without LoadTypeConfig instead. + */ + @Deprecated(forRemoval = true) // TODO: Remove on Vespa 8 public MessagePropertyProcessor(FeederConfig config, LoadTypeConfig loadTypeCfg) { loadTypes = new LoadTypeSet(); configure(config, loadTypeCfg); @@ -127,11 +135,19 @@ public class MessagePropertyProcessor implements ConfigSubscriber.SingleSubscrib return feederOptions; } + /** + * @deprecated load types are deprecated. configure without LoadTypeConfig instead. + */ + @Deprecated(forRemoval = true) // TODO: Remove on Vespa 8 public synchronized void configure(FeederConfig config, LoadTypeConfig loadTypeConfig) { loadTypes.configure(loadTypeConfig); configure(config); } + /** + * @deprecated load types are deprecated + */ + @Deprecated(forRemoval = true) // TODO: Remove on Vespa 8 LoadTypeSet getLoadTypes() { return loadTypes; } @@ -175,7 +191,7 @@ public class MessagePropertyProcessor implements ConfigSubscriber.SingleSubscrib private boolean abortOnDocumentError; private boolean abortOnFeedError; private boolean createIfNonExistent; - private LoadType loadType; + private LoadType loadType; // TODO remove on Vespa 8 private int traceLevel; PropertySetter(Route route, long timeout, long totalTimeout, DocumentProtocol.Priority priority, LoadType loadType, diff --git a/vespaclient-java/src/main/java/com/yahoo/dummyreceiver/DummyReceiver.java b/vespaclient-java/src/main/java/com/yahoo/dummyreceiver/DummyReceiver.java index cd524c07f73..c2985996bd0 100755 --- a/vespaclient-java/src/main/java/com/yahoo/dummyreceiver/DummyReceiver.java +++ b/vespaclient-java/src/main/java/com/yahoo/dummyreceiver/DummyReceiver.java @@ -4,7 +4,6 @@ package com.yahoo.dummyreceiver; import com.yahoo.concurrent.DaemonThreadFactory; import com.yahoo.documentapi.messagebus.MessageBusDocumentAccess; import com.yahoo.documentapi.messagebus.MessageBusParams; -import com.yahoo.documentapi.messagebus.loadtypes.LoadTypeSet; import com.yahoo.documentapi.messagebus.protocol.PutDocumentMessage; import com.yahoo.documentapi.messagebus.protocol.RemoveDocumentMessage; import com.yahoo.documentapi.messagebus.protocol.UpdateDocumentMessage; @@ -68,7 +67,7 @@ public class DummyReceiver implements MessageHandler { } private void init() { - MessageBusParams params = new MessageBusParams(new LoadTypeSet()); + MessageBusParams params = new MessageBusParams(); params.setRPCNetworkParams(new RPCNetworkParams().setIdentity(new Identity(name))); params.setDocumentManagerConfigId("client"); params.getMessageBusParams().setMaxPendingCount(0); diff --git a/vespaclient-java/src/main/java/com/yahoo/vespafeeder/Arguments.java b/vespaclient-java/src/main/java/com/yahoo/vespafeeder/Arguments.java index 8f5db4adf97..52f2857c7e5 100644 --- a/vespaclient-java/src/main/java/com/yahoo/vespafeeder/Arguments.java +++ b/vespaclient-java/src/main/java/com/yahoo/vespafeeder/Arguments.java @@ -153,7 +153,7 @@ public class Arguments { } } - propertyProcessor = new MessagePropertyProcessor(getFeederConfig(), new LoadTypeConfig(new LoadTypeConfig.Builder())); + propertyProcessor = new MessagePropertyProcessor(getFeederConfig()); } private String getParam(List<String> args, String arg) throws IllegalArgumentException { diff --git a/vespaclient-java/src/main/java/com/yahoo/vespaget/DocumentRetriever.java b/vespaclient-java/src/main/java/com/yahoo/vespaget/DocumentRetriever.java index b9917533a62..2454f5c8627 100644 --- a/vespaclient-java/src/main/java/com/yahoo/vespaget/DocumentRetriever.java +++ b/vespaclient-java/src/main/java/com/yahoo/vespaget/DocumentRetriever.java @@ -27,18 +27,32 @@ import java.util.Map; * * @author bjorncs */ +@SuppressWarnings("removal") // TODO: Remove on Vespa 8 public class DocumentRetriever { private final ClusterList clusterList; private final DocumentAccessFactory documentAccessFactory; private final ClientParameters params; - private final LoadTypeSet loadTypeSet; + private final LoadTypeSet loadTypeSet; // TODO remove on Vespa 8 private MessageBusSyncSession session; private MessageBusDocumentAccess documentAccess; public DocumentRetriever(ClusterList clusterList, DocumentAccessFactory documentAccessFactory, + ClientParameters params) { + this.clusterList = clusterList; + this.documentAccessFactory = documentAccessFactory; + this.loadTypeSet = new LoadTypeSet(); // TODO remove on Vespa 8 + this.params = params; + } + + /** + * @deprecated load types are deprecated. Use constructor without LoadTypeSet instead. + */ + @Deprecated(forRemoval = true) // TODO: Remove on Vespa 8 + public DocumentRetriever(ClusterList clusterList, + DocumentAccessFactory documentAccessFactory, LoadTypeSet loadTypeSet, ClientParameters params) { this.clusterList = clusterList; diff --git a/vespaclient-java/src/main/java/com/yahoo/vespaget/Main.java b/vespaclient-java/src/main/java/com/yahoo/vespaget/Main.java index fd2c9e964f7..7596246d16e 100644 --- a/vespaclient-java/src/main/java/com/yahoo/vespaget/Main.java +++ b/vespaclient-java/src/main/java/com/yahoo/vespaget/Main.java @@ -38,11 +38,12 @@ public class Main { Runtime.getRuntime().addShutdownHook(new Thread(documentRetriever::shutdown)); } + @SuppressWarnings("removal") // TODO: Remove on Vespa 8 private static DocumentRetriever createDocumentRetriever(ClientParameters params) { return new DocumentRetriever( new ClusterList("client"), new DocumentAccessFactory(), - new LoadTypeSet(params.configId), + new LoadTypeSet(params.configId), // TODO: Remove on Vespa 8 params ); } diff --git a/vespaclient-java/src/main/java/com/yahoo/vespavisit/VdsVisit.java b/vespaclient-java/src/main/java/com/yahoo/vespavisit/VdsVisit.java index 4d79f2f2e1d..0e64f824b63 100644 --- a/vespaclient-java/src/main/java/com/yahoo/vespavisit/VdsVisit.java +++ b/vespaclient-java/src/main/java/com/yahoo/vespavisit/VdsVisit.java @@ -10,7 +10,6 @@ import com.yahoo.documentapi.VisitorParameters; import com.yahoo.documentapi.VisitorSession; import com.yahoo.documentapi.messagebus.MessageBusDocumentAccess; import com.yahoo.documentapi.messagebus.MessageBusParams; -import com.yahoo.documentapi.messagebus.loadtypes.LoadTypeSet; import com.yahoo.documentapi.messagebus.protocol.DocumentProtocol; import com.yahoo.log.LogSetup; import com.yahoo.messagebus.StaticThrottlePolicy; @@ -36,7 +35,7 @@ import java.util.stream.Collectors; public class VdsVisit { private VdsVisitParameters params; - private MessageBusParams mbparams = new MessageBusParams(new LoadTypeSet()); + private MessageBusParams mbparams = new MessageBusParams(); private VisitorSession session; private final VisitorSessionAccessorFactory sessionAccessorFactory; diff --git a/vespaclient-java/src/main/java/com/yahoo/vespavisit/VdsVisitTarget.java b/vespaclient-java/src/main/java/com/yahoo/vespavisit/VdsVisitTarget.java index d1fbde7dd42..7dfa3a2cf2e 100644 --- a/vespaclient-java/src/main/java/com/yahoo/vespavisit/VdsVisitTarget.java +++ b/vespaclient-java/src/main/java/com/yahoo/vespavisit/VdsVisitTarget.java @@ -8,7 +8,6 @@ import com.yahoo.documentapi.VisitorDestinationParameters; import com.yahoo.documentapi.VisitorDestinationSession; import com.yahoo.documentapi.messagebus.MessageBusDocumentAccess; import com.yahoo.documentapi.messagebus.MessageBusParams; -import com.yahoo.documentapi.messagebus.loadtypes.LoadTypeSet; import java.util.logging.Level; import com.yahoo.log.LogSetup; import com.yahoo.messagebus.network.Identity; @@ -209,7 +208,7 @@ public class VdsVisitTarget { public void run() throws Exception { initShutdownHook(); log.log(Level.FINE, "Starting VdsVisitTarget"); - MessageBusParams mbusParams = new MessageBusParams(new LoadTypeSet()); + MessageBusParams mbusParams = new MessageBusParams(); mbusParams.getRPCNetworkParams().setIdentity(new Identity(slobrokAddress)); if (port > 0) { diff --git a/vespaclient-java/src/test/java/com/yahoo/vespaget/DocumentRetrieverTest.java b/vespaclient-java/src/test/java/com/yahoo/vespaget/DocumentRetrieverTest.java index 8d7483c2196..30d117ab105 100644 --- a/vespaclient-java/src/test/java/com/yahoo/vespaget/DocumentRetrieverTest.java +++ b/vespaclient-java/src/test/java/com/yahoo/vespaget/DocumentRetrieverTest.java @@ -129,7 +129,6 @@ public class DocumentRetrieverTest { return new DocumentRetriever( clusterList, mockedFactory, - new LoadTypeSet(), params); } @@ -145,7 +144,7 @@ public class DocumentRetrieverTest { when(mockedSession.syncSend(any())).thenReturn(createDocumentReply(DOC_ID_1)); - LoadTypeSet loadTypeSet = new LoadTypeSet(); + LoadTypeSet loadTypeSet = new LoadTypeSet(); // TODO remove on Vespa 8 loadTypeSet.addLoadType(1, "loadtype", DocumentProtocol.Priority.HIGH_1); DocumentRetriever documentRetriever = new DocumentRetriever( new ClusterList(), diff --git a/vespajlib/abi-spec.json b/vespajlib/abi-spec.json index 20c7d435964..e69631e8375 100644 --- a/vespajlib/abi-spec.json +++ b/vespajlib/abi-spec.json @@ -3232,6 +3232,7 @@ "methods": [ "public static boolean isTextCharacter(int)", "public static java.util.OptionalInt validateTextString(java.lang.String)", + "public static boolean isValidTextString(java.lang.String)", "public static boolean isDisplayable(int)", "public static java.lang.String stripInvalidCharacters(java.lang.String)", "public static java.lang.String truncate(java.lang.String, int)", diff --git a/vespajlib/pom.xml b/vespajlib/pom.xml index b4898b14b86..ed6ae3678f4 100644 --- a/vespajlib/pom.xml +++ b/vespajlib/pom.xml @@ -36,6 +36,11 @@ <artifactId>aircompressor</artifactId> <scope>compile</scope> </dependency> + <dependency> + <groupId>org.apache.commons</groupId> + <artifactId>commons-compress</artifactId> + <scope>compile</scope> + </dependency> <!-- provided scope --> <dependency> diff --git a/vespajlib/src/main/java/ai/vespa/validation/Name.java b/vespajlib/src/main/java/ai/vespa/validation/Name.java index b275917dcff..a6ab456c285 100644 --- a/vespajlib/src/main/java/ai/vespa/validation/Name.java +++ b/vespajlib/src/main/java/ai/vespa/validation/Name.java @@ -3,8 +3,6 @@ package ai.vespa.validation; import java.util.regex.Pattern; -import static ai.vespa.validation.Validation.requireMatch; - /** * A name has from 1 to 64 {@link String} characters which may be letters, numbers, * dashes or underscores, and must start with a letter. diff --git a/vespajlib/src/main/java/ai/vespa/validation/package-info.java b/vespajlib/src/main/java/ai/vespa/validation/package-info.java new file mode 100644 index 00000000000..edbab3a6fd1 --- /dev/null +++ b/vespajlib/src/main/java/ai/vespa/validation/package-info.java @@ -0,0 +1,5 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +@ExportPackage +package ai.vespa.validation; + +import com.yahoo.osgi.annotation.ExportPackage; diff --git a/vespajlib/src/main/java/com/yahoo/compress/ArchiveStreamReader.java b/vespajlib/src/main/java/com/yahoo/compress/ArchiveStreamReader.java new file mode 100644 index 00000000000..e65a645f5be --- /dev/null +++ b/vespajlib/src/main/java/com/yahoo/compress/ArchiveStreamReader.java @@ -0,0 +1,216 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.compress; + +import com.yahoo.path.Path; +import com.yahoo.yolean.Exceptions; +import org.apache.commons.compress.archivers.ArchiveEntry; +import org.apache.commons.compress.archivers.ArchiveInputStream; +import org.apache.commons.compress.archivers.tar.TarArchiveEntry; +import org.apache.commons.compress.archivers.tar.TarArchiveInputStream; +import org.apache.commons.compress.archivers.zip.ZipArchiveEntry; +import org.apache.commons.compress.archivers.zip.ZipArchiveInputStream; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.io.UncheckedIOException; +import java.util.Objects; +import java.util.OptionalLong; +import java.util.function.Predicate; +import java.util.zip.GZIPInputStream; + +/** + * Helper class for safely reading files from a compressed archive. + * + * @author mpolden + */ +public class ArchiveStreamReader implements AutoCloseable { + + private final ArchiveInputStream archiveInputStream; + private final Options options; + + private long totalRead = 0; + private long entriesRead = 0; + + private ArchiveStreamReader(ArchiveInputStream archiveInputStream, Options options) { + this.archiveInputStream = Objects.requireNonNull(archiveInputStream); + this.options = Objects.requireNonNull(options); + } + + /** Create reader for an inputStream containing a tar.gz file */ + public static ArchiveStreamReader ofTarGzip(InputStream inputStream, Options options) { + return new ArchiveStreamReader(new TarArchiveInputStream(Exceptions.uncheck(() -> new GZIPInputStream(inputStream))), options); + } + + /** Create reader for an inputStream containing a ZIP file */ + public static ArchiveStreamReader ofZip(InputStream inputStream, Options options) { + return new ArchiveStreamReader(new ZipArchiveInputStream(inputStream), options); + } + + /** + * Read the next file in this archive and write it to given outputStream. Returns information about the read archive + * file, or null if there are no more files to read. + */ + public ArchiveFile readNextTo(OutputStream outputStream) { + ArchiveEntry entry; + try { + while ((entry = archiveInputStream.getNextEntry()) != null) { + Path path = Path.fromString(requireNormalized(entry.getName(), options.allowDotSegment)); + if (isSymlink(entry)) throw new IllegalArgumentException("Archive entry " + path + " is a symbolic link, which is unsupported"); + if (entry.isDirectory()) continue; + if (!options.pathPredicate.test(path.toString())) continue; + if (++entriesRead > options.maxEntries) throw new IllegalArgumentException("Attempted to read more entries than entry limit of " + options.maxEntries); + + long size = 0; + byte[] buffer = new byte[2048]; + int read; + while ((read = archiveInputStream.read(buffer)) != -1) { + totalRead += read; + size += read; + if (totalRead > options.maxSize) throw new IllegalArgumentException("Total size of archive exceeds size limit of " + options.maxSize + " bytes"); + if (read > options.maxEntrySize) { + if (!options.truncateEntry) throw new IllegalArgumentException("Size of entry " + path + " exceeded entry size limit of " + options.maxEntrySize + " bytes"); + } else { + outputStream.write(buffer, 0, read); + } + } + return new ArchiveFile(path, crc32(entry), size); + } + } catch (IOException e) { + throw new UncheckedIOException(e); + } + return null; + } + + @Override + public void close() { + Exceptions.uncheck(archiveInputStream::close); + } + + /** Information about a file extracted from a compressed archive */ + public static class ArchiveFile { + + private final Path path; + private final OptionalLong crc32; + private final long size; + + public ArchiveFile(Path name, OptionalLong crc32, long size) { + this.path = Objects.requireNonNull(name); + this.crc32 = Objects.requireNonNull(crc32); + if (crc32.isPresent()) { + requireNonNegative("crc32", crc32.getAsLong()); + } + this.size = requireNonNegative("size", size); + } + + /** The path of this file inside its containing archive */ + public Path path() { + return path; + } + + /** The CRC-32 checksum of this file, if any */ + public OptionalLong crc32() { + return crc32; + } + + /** The decompressed size of this file */ + public long size() { + return size; + } + + } + + /** Get the CRC-32 checksum of given archive entry, if any */ + private static OptionalLong crc32(ArchiveEntry entry) { + long crc32 = -1; + if (entry instanceof ZipArchiveEntry) { + crc32 = ((ZipArchiveEntry) entry).getCrc(); + } + return crc32 > -1 ? OptionalLong.of(crc32) : OptionalLong.empty(); + } + + private static boolean isSymlink(ArchiveEntry entry) { + // Symlinks inside ZIP files are not part of the ZIP spec and are only supported by some implementations, such + // as Info-ZIP. + // + // Commons Compress only has limited support for symlinks as they are only detected when the ZIP file is read + // through org.apache.commons.compress.archivers.zip.ZipFile. This is not the case in this class, because it must + // support reading ZIP files from generic input streams. The check below thus always returns false. + if (entry instanceof ZipArchiveEntry) return ((ZipArchiveEntry) entry).isUnixSymlink(); + if (entry instanceof TarArchiveEntry) return ((TarArchiveEntry) entry).isSymbolicLink(); + throw new IllegalArgumentException("Unsupported archive entry " + entry.getClass().getSimpleName() + ", cannot check for symbolic link"); + } + + private static String requireNormalized(String name, boolean allowDotSegment) { + for (var part : name.split("/")) { + if (part.isEmpty() || (!allowDotSegment && part.equals(".")) || part.equals("..")) { + throw new IllegalArgumentException("Unexpected non-normalized path found in zip content: '" + name + "'"); + } + } + return name; + } + + private static long requireNonNegative(String field, long n) { + if (n < 0) throw new IllegalArgumentException(field + " cannot be negative, got " + n); + return n; + } + + /** Options for reading entries of an archive */ + public static class Options { + + private long maxSize = 8 * (long) Math.pow(1024, 3); // 8 GB + private long maxEntrySize = Long.MAX_VALUE; + private long maxEntries = Long.MAX_VALUE; + private boolean truncateEntry = false; + private boolean allowDotSegment = false; + private Predicate<String> pathPredicate = (path) -> true; + + private Options() {} + + /** Returns the standard set of read options */ + public static Options standard() { + return new Options(); + } + + /** Set the maximum total size of decompressed entries. Default is 8 GB */ + public Options maxSize(long size) { + this.maxSize = requireNonNegative("size", size); + return this; + } + + /** Set the maximum size a decompressed entry. Default is no limit */ + public Options maxEntrySize(long size) { + this.maxEntrySize = requireNonNegative("size", size); + return this; + } + + /** Set the maximum number of entries to decompress. Default is no limit */ + public Options maxEntries(long count) { + this.maxEntries = requireNonNegative("count", count); + return this; + } + + /** + * Set whether to truncate the content of an entry exceeding the configured size limit, instead of throwing. + * Default is to throw. + */ + public Options truncateEntry(boolean truncate) { + this.truncateEntry = truncate; + return this; + } + + /** Set a predicate that an entry path must match in order to be extracted. Default is to extract all entries */ + public Options pathPredicate(Predicate<String> predicate) { + this.pathPredicate = predicate; + return this; + } + + /** Set whether to allow single-dot segments in entry paths. Default is false */ + public Options allowDotSegment(boolean allow) { + this.allowDotSegment = allow; + return this; + } + + } + +} diff --git a/vespajlib/src/main/java/com/yahoo/compress/ZstdOuputStream.java b/vespajlib/src/main/java/com/yahoo/compress/ZstdOutputStream.java index c423f4c76bf..f439ee03ea6 100644 --- a/vespajlib/src/main/java/com/yahoo/compress/ZstdOuputStream.java +++ b/vespajlib/src/main/java/com/yahoo/compress/ZstdOutputStream.java @@ -7,7 +7,7 @@ import java.io.OutputStream; /** * @author bjorncs */ -public class ZstdOuputStream extends OutputStream { +public class ZstdOutputStream extends OutputStream { private final ZstdCompressor compressor = new ZstdCompressor(); @@ -19,13 +19,13 @@ public class ZstdOuputStream extends OutputStream { private int inputPosition = 0; private boolean isClosed = false; - public ZstdOuputStream(OutputStream out, int inputBufferSize) { + public ZstdOutputStream(OutputStream out, int inputBufferSize) { this.out = out; this.inputBuffer = new byte[inputBufferSize]; this.outputBuffer = new byte[ZstdCompressor.getMaxCompressedLength(inputBufferSize)]; } - public ZstdOuputStream(OutputStream out) { + public ZstdOutputStream(OutputStream out) { this(out, DEFAULT_INPUT_BUFFER_SIZE); } diff --git a/vespajlib/src/main/java/com/yahoo/text/Text.java b/vespajlib/src/main/java/com/yahoo/text/Text.java index 662100aa8ea..30eba3ebd65 100644 --- a/vespajlib/src/main/java/com/yahoo/text/Text.java +++ b/vespajlib/src/main/java/com/yahoo/text/Text.java @@ -48,7 +48,11 @@ public final class Text { // The link above notes that 0x7F-0x84 and 0x86-0x9F are discouraged, but they are still allowed - // see http://www.w3.org/International/questions/qa-controls - if (codepoint < 0x80) return allowedAsciiChars[codepoint]; + return (codepoint < 0x80) + ? allowedAsciiChars[codepoint] + : isTextCharAboveUsAscii(codepoint); + } + private static boolean isTextCharAboveUsAscii(int codepoint) { if (codepoint < 0xFDD0) return true; if (codepoint <= 0xFDDF) return false; if (codepoint < 0x1FFFE) return true; @@ -110,6 +114,23 @@ public final class Text { return OptionalInt.empty(); } + /** + * Validates that the given string value only contains text characters. + */ + public static boolean isValidTextString(String string) { + for (int i = 0; i < string.length(); ) { + int codePoint = string.codePointAt(i); + if ( ! Text.isTextCharacter(codePoint)) return false; + + int charCount = Character.charCount(codePoint); + if (Character.isHighSurrogate(string.charAt(i))) { + if ( (charCount == 1) || !Character.isLowSurrogate(string.charAt(i+1))) return false; + } + i += charCount; + } + return true; + } + /** Returns whether the given code point is displayable. */ public static boolean isDisplayable(int codePoint) { switch (Character.getType(codePoint)) { diff --git a/vespajlib/src/test/java/com/yahoo/compress/ArchiveStreamReaderTest.java b/vespajlib/src/test/java/com/yahoo/compress/ArchiveStreamReaderTest.java new file mode 100644 index 00000000000..b7f019282b7 --- /dev/null +++ b/vespajlib/src/test/java/com/yahoo/compress/ArchiveStreamReaderTest.java @@ -0,0 +1,131 @@ +package com.yahoo.compress; + +import com.yahoo.compress.ArchiveStreamReader.Options; +import com.yahoo.yolean.Exceptions; +import org.apache.commons.compress.archivers.zip.ZipArchiveEntry; +import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream; +import org.junit.jupiter.api.Test; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; +import java.util.HashMap; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +/** + * @author mpolden + */ +class ArchiveStreamReaderTest { + + @Test + void reading() { + Map<String, String> zipFiles = Map.of("foo", "contents of foo", + "bar", "contents of bar", + "baz", "0".repeat(2049)); + Map<String, String> zipContents = new HashMap<>(zipFiles); + zipContents.put("dir/", ""); // Directories are always ignored + Map<String, String> extracted = readAll(zip(zipContents), Options.standard()); + assertEquals(zipFiles, extracted); + } + + @Test + void entry_size_limit() { + Map<String, String> entries = Map.of("foo.xml", "foobar"); + Options options = Options.standard().pathPredicate("foo.xml"::equals).maxEntrySize(1); + try { + readAll(zip(entries), options); + fail("Expected exception"); + } catch (IllegalArgumentException ignored) {} + + entries = Map.of("foo.xml", "foobar", + "foo.jar", "0".repeat(100) // File not extracted and thus not subject to size limit + ); + Map<String, String> extracted = readAll(zip(entries), options.maxEntrySize(10)); + assertEquals(Map.of("foo.xml", "foobar"), extracted); + } + + @Test + void size_limit() { + Map<String, String> entries = Map.of("foo.xml", "foo", "bar.xml", "bar"); + try { + readAll(zip(entries), Options.standard().maxSize(4)); + fail("Expected exception"); + } catch (IllegalArgumentException ignored) {} + } + + @Test + void entry_limit() { + Map<String, String> entries = Map.of("foo.xml", "foo", "bar.xml", "bar"); + try { + readAll(zip(entries), Options.standard().maxEntries(1)); + fail("Expected exception"); + } catch (IllegalArgumentException ignored) {} + } + + @Test + void paths() { + Map<String, Boolean> tests = Map.of( + "../../services.xml", true, + "/../.././services.xml", true, + "./application/././services.xml", true, + "application//services.xml", true, + "artifacts/", false, // empty dir + "services..xml", false, + "application/services.xml", false, + "components/foo-bar-deploy.jar", false, + "services.xml", false + ); + + Options options = Options.standard().maxEntrySize(1024); + tests.forEach((name, expectException) -> { + try { + readAll(zip(Map.of(name, "foo")), options.pathPredicate(name::equals)); + assertFalse(expectException, "Expected exception for '" + name + "'"); + } catch (IllegalArgumentException ignored) { + assertTrue(expectException, "Unexpected exception for '" + name + "'"); + } + }); + } + + private static Map<String, String> readAll(InputStream inputStream, Options options) { + ArchiveStreamReader reader = ArchiveStreamReader.ofZip(inputStream, options); + ArchiveStreamReader.ArchiveFile file; + Map<String, String> entries = new HashMap<>(); + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + while ((file = reader.readNextTo(baos)) != null) { + entries.put(file.path().toString(), baos.toString(StandardCharsets.UTF_8)); + baos.reset(); + } + return entries; + } + + private static InputStream zip(Map<String, String> entries) { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + ZipArchiveOutputStream archiveOutputStream = null; + try { + archiveOutputStream = new ZipArchiveOutputStream(baos); + for (var kv : entries.entrySet()) { + String entryName = kv.getKey(); + String contents = kv.getValue(); + ZipArchiveEntry entry = new ZipArchiveEntry(entryName); + archiveOutputStream.putArchiveEntry(entry); + archiveOutputStream.write(contents.getBytes(StandardCharsets.UTF_8)); + archiveOutputStream.closeArchiveEntry(); + } + } catch (IOException e) { + throw new UncheckedIOException(e); + } finally { + if (archiveOutputStream != null) Exceptions.uncheck(archiveOutputStream::close); + } + return new ByteArrayInputStream(baos.toByteArray()); + } + +} diff --git a/vespajlib/src/test/java/com/yahoo/compress/ZstdOuputStreamTest.java b/vespajlib/src/test/java/com/yahoo/compress/ZstdOutputStreamTest.java index 5f6140271ad..c766c6e0c19 100644 --- a/vespajlib/src/test/java/com/yahoo/compress/ZstdOuputStreamTest.java +++ b/vespajlib/src/test/java/com/yahoo/compress/ZstdOutputStreamTest.java @@ -12,13 +12,13 @@ import static org.junit.jupiter.api.Assertions.assertTrue; /** * @author bjorncs */ -class ZstdOuputStreamTest { +class ZstdOutputStreamTest { @Test void output_stream_compresses_input() throws IOException { byte[] inputData = "The quick brown fox jumps over the lazy dog".getBytes(); ByteArrayOutputStream arrayOut = new ByteArrayOutputStream(); - try (ZstdOuputStream zstdOut = new ZstdOuputStream(arrayOut, 12)) { + try (ZstdOutputStream zstdOut = new ZstdOutputStream(arrayOut, 12)) { zstdOut.write(inputData[0]); zstdOut.write(inputData, 1, inputData.length - 1); } @@ -37,7 +37,7 @@ class ZstdOuputStreamTest { } byte[] inputData = builder.toString().getBytes(); ByteArrayOutputStream arrayOut = new ByteArrayOutputStream(); - try (ZstdOuputStream zstdOut = new ZstdOuputStream(arrayOut)) { + try (ZstdOutputStream zstdOut = new ZstdOutputStream(arrayOut)) { zstdOut.write(inputData); } int compressedSize = arrayOut.toByteArray().length; diff --git a/vespajlib/src/test/java/com/yahoo/text/TextTestCase.java b/vespajlib/src/test/java/com/yahoo/text/TextTestCase.java index a2cb2158278..33274380aad 100644 --- a/vespajlib/src/test/java/com/yahoo/text/TextTestCase.java +++ b/vespajlib/src/test/java/com/yahoo/text/TextTestCase.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.text; +import org.junit.Ignore; import org.junit.Test; import java.util.OptionalInt; @@ -76,4 +77,47 @@ public class TextTestCase { public void testFormat() { assertEquals("foo 3.14", Text.format("%s %.2f", "foo", 3.1415926536)); } + + private static long isValid(String [] strings, int num) { + long sum = 0; + for (int i=0; i < num; i++) { + if (Text.isValidTextString(strings[i%strings.length])) { + sum++; + } + } + return sum; + } + private static long validate(String [] strings, int num) { + long sum = 0; + for (int i=0; i < num; i++) { + if (Text.validateTextString(strings[i%strings.length]).isEmpty()) { + sum++; + } + } + return sum; + } + + @Ignore + @Test + public void benchmarkValidate() { + String [] strings = new String[100]; + for (int i=0; i < strings.length; i++) { + strings[i] = new StringBuilder("some text ").append(i).append("of mine.").appendCodePoint(0xDFFFC).append("foo").toString(); + } + long sum = validate(strings, 1000000); + System.out.println("Warmup num validate = " + sum); + sum = isValid(strings, 1000000); + System.out.println("Warmup num isValid = " + sum); + + long start = System.nanoTime(); + sum = validate(strings, 100000000); + long diff = System.nanoTime() - start; + System.out.println("Validation num validate = " + sum + ". Took " + diff + "ns"); + + start = System.nanoTime(); + sum = isValid(strings, 100000000); + diff = System.nanoTime() - start; + System.out.println("Validation num isValid = " + sum + ". Took " + diff + "ns"); + + } } diff --git a/vespalib/src/tests/slime/slime_binary_format_test.cpp b/vespalib/src/tests/slime/slime_binary_format_test.cpp index 459826d691e..ba2aacb88b9 100644 --- a/vespalib/src/tests/slime/slime_binary_format_test.cpp +++ b/vespalib/src/tests/slime/slime_binary_format_test.cpp @@ -248,9 +248,9 @@ TEST("testCmprUlong") { for (uint32_t n = 1; n <= MAX_CMPR_SIZE; ++n) { TEST_STATE(vespalib::make_string("n = %d", n).c_str()); uint64_t min = (n == 1) ? 0x00 - : (1L << ((n - 1) * 7)); + : (1ULL << ((n - 1) * 7)); uint64_t max = (n == MAX_CMPR_SIZE) ? 0xffffffffffffffff - : (1L << (n * 7)) - 1; + : (1ULL << (n * 7)) - 1; SimpleBuffer expect_min; SimpleBuffer expect_max; for (uint32_t i = 0; i < n; ++i) { diff --git a/vespalib/src/vespa/vespalib/util/array.hpp b/vespalib/src/vespa/vespalib/util/array.hpp index 72178f0391b..24136e544b8 100644 --- a/vespalib/src/vespa/vespalib/util/array.hpp +++ b/vespalib/src/vespa/vespalib/util/array.hpp @@ -60,7 +60,9 @@ Array<T>::Array(const Array & rhs) : _array(rhs._array.create(rhs.size() * sizeof(T))), _sz(rhs.size()) { - construct(array(0), rhs.array(0), _sz, std::is_trivially_copyable<T>()); + if (_sz > 0) [[likely]] { + construct(array(0), rhs.array(0), _sz, std::is_trivially_copyable<T>()); + } } template <typename T> diff --git a/vespalib/src/vespa/vespalib/util/fiddle.h b/vespalib/src/vespa/vespalib/util/fiddle.h index 20a13ff4654..f4d2ac33695 100644 --- a/vespalib/src/vespa/vespalib/util/fiddle.h +++ b/vespalib/src/vespa/vespalib/util/fiddle.h @@ -24,8 +24,8 @@ uint32_t mix(uint32_t prefix, uint32_t suffix, uint32_t prefix_bits) { if (prefix_bits >= 32) { return prefix; } - uint32_t suffix_mask = (1 << (32 - prefix_bits)) - 1; - uint32_t prefix_mask = (0 - 1) - suffix_mask; + uint32_t suffix_mask = (1u << (32u - prefix_bits)) - 1u; + uint32_t prefix_mask = (0u - 1u) - suffix_mask; return (prefix & prefix_mask) | (suffix & suffix_mask); } diff --git a/zkfacade/pom.xml b/zkfacade/pom.xml index fd02410d03f..86a60702c26 100644 --- a/zkfacade/pom.xml +++ b/zkfacade/pom.xml @@ -86,13 +86,12 @@ </exclusions> </dependency> <!-- snappy-java and metrics-core are included here - to be able to work with ZooKeeper 3.6.2 due to + to be able to work with ZooKeeper >= 3.6.2 due to class loading issues --> <dependency> <groupId>io.dropwizard.metrics</groupId> <artifactId>metrics-core</artifactId> <scope>compile</scope> - <version>3.2.5</version> <exclusions> <exclusion> <groupId>org.slf4j</groupId> @@ -104,7 +103,6 @@ <groupId>org.xerial.snappy</groupId> <artifactId>snappy-java</artifactId> <scope>compile</scope> - <version>1.1.7</version> </dependency> <dependency> <groupId>org.mockito</groupId> diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCurator.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCurator.java index bd3389b8d4d..7c91e54dd4b 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCurator.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCurator.java @@ -36,7 +36,7 @@ public class MockCurator extends Curator { /** * Creates a mock curator * - * @param stableOrdering if true children of a node are returned in the same order each time they are queries. + * @param stableOrdering if true children of a node are returned in the same order each time they are queried. * This is not what ZooKeeper does. */ public MockCurator(boolean stableOrdering) { diff --git a/zookeeper-command-line-client/pom.xml b/zookeeper-command-line-client/pom.xml index a8105c78881..236bd5245a9 100644 --- a/zookeeper-command-line-client/pom.xml +++ b/zookeeper-command-line-client/pom.xml @@ -61,6 +61,11 @@ <artifactId>log4j-over-slf4j</artifactId> <scope>compile</scope> </dependency> + <dependency> + <groupId>org.xerial.snappy</groupId> + <artifactId>snappy-java</artifactId> + <scope>compile</scope> + </dependency> </dependencies> <build> diff --git a/zookeeper-server/zookeeper-server-3.7.0/pom.xml b/zookeeper-server/zookeeper-server-3.7.0/pom.xml index 01fd83a496b..8daa6003f1e 100644 --- a/zookeeper-server/zookeeper-server-3.7.0/pom.xml +++ b/zookeeper-server/zookeeper-server-3.7.0/pom.xml @@ -62,7 +62,6 @@ <groupId>io.dropwizard.metrics</groupId> <artifactId>metrics-core</artifactId> <scope>compile</scope> - <version>3.2.5</version> <exclusions> <exclusion> <groupId>org.slf4j</groupId> @@ -74,7 +73,6 @@ <groupId>org.xerial.snappy</groupId> <artifactId>snappy-java</artifactId> <scope>compile</scope> - <version>1.1.7</version> </dependency> <dependency> <groupId>junit</groupId> |