diff options
54 files changed, 344 insertions, 355 deletions
diff --git a/cloud-tenant-base-dependencies-enforcer/pom.xml b/cloud-tenant-base-dependencies-enforcer/pom.xml index 481211390e0..11da20ad635 100644 --- a/cloud-tenant-base-dependencies-enforcer/pom.xml +++ b/cloud-tenant-base-dependencies-enforcer/pom.xml @@ -120,8 +120,6 @@ <include>org.glassfish.hk2:hk2-utils:[${hk2.version}]:jar:provided</include> <include>org.glassfish.hk2:osgi-resource-locator:[${hk2.osgi-resource-locator.version}]:jar:provided</include> <include>org.glassfish.jersey.bundles.repackaged:jersey-guava:[${jersey2.version}]:jar:provided</include> - <include>org.glassfish.jersey.containers:jersey-container-servlet-core:[${jersey2.version}]:jar:provided</include> - <include>org.glassfish.jersey.containers:jersey-container-servlet:[${jersey2.version}]:jar:provided</include> <include>org.glassfish.jersey.core:jersey-client:[${jersey2.version}]:jar:provided</include> <include>org.glassfish.jersey.core:jersey-common:[${jersey2.version}]:jar:provided</include> <include>org.glassfish.jersey.core:jersey-server:[${jersey2.version}]:jar:provided</include> diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/Model.java b/config-model-api/src/main/java/com/yahoo/config/model/api/Model.java index f98d72374bf..c709b5dad85 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/Model.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/Model.java @@ -6,7 +6,6 @@ import com.yahoo.config.ConfigInstance; import com.yahoo.config.FileReference; import com.yahoo.config.provision.AllocatedHosts; import com.yahoo.vespa.config.ConfigKey; -import com.yahoo.vespa.config.ConfigPayload; import com.yahoo.vespa.config.buildergen.ConfigDefinition; import java.time.Instant; @@ -44,7 +43,8 @@ public interface Model { * * @param fileDistribution instance that can be called to distribute files */ - void distributeFiles(FileDistribution fileDistribution); + // TODO: Remove when Vespa < 7.453 is gone from production + default void distributeFiles(FileDistribution fileDistribution) {}; /** The set of files that should be distributed to the hosts in this model. */ Set<FileReference> fileReferences(); diff --git a/config-model/src/main/java/com/yahoo/config/model/test/MockRoot.java b/config-model/src/main/java/com/yahoo/config/model/test/MockRoot.java index 87d6554f691..d95da6e9a7c 100644 --- a/config-model/src/main/java/com/yahoo/config/model/test/MockRoot.java +++ b/config-model/src/main/java/com/yahoo/config/model/test/MockRoot.java @@ -26,7 +26,6 @@ import java.io.IOException; import java.io.StringReader; import java.util.ArrayList; import java.util.Collections; -import java.util.List; import java.util.Set; @@ -41,10 +40,10 @@ public class MockRoot extends AbstractConfigProducerRoot { private static final long serialVersionUID = 1L; - private HostSystem hostSystem; + private final HostSystem hostSystem; private final DeployState deployState; - private FileDistributor fileDistributor; + private final FileDistributor fileDistributor; private Admin admin; public MockRoot() { @@ -67,7 +66,7 @@ public class MockRoot extends AbstractConfigProducerRoot { super(rootConfigId); hostSystem = new HostSystem(this, "hostsystem", deployState.getProvisioner(), deployState.getDeployLogger()); this.deployState = deployState; - fileDistributor = new FileDistributor(deployState.getFileRegistry(), List.of(), deployState.isHosted()); + fileDistributor = new FileDistributor(deployState.getFileRegistry()); } public FileDistributionConfigProducer getFileDistributionConfigProducer() { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/InstanceResolver.java b/config-model/src/main/java/com/yahoo/vespa/model/InstanceResolver.java index 11e97bc8a95..2b3e4515740 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/InstanceResolver.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/InstanceResolver.java @@ -2,8 +2,6 @@ package com.yahoo.vespa.model; import com.yahoo.config.ConfigBuilder; -import com.yahoo.config.ConfigInstance; -import com.yahoo.config.ConfigurationRuntimeException; import com.yahoo.config.codegen.CNode; import com.yahoo.config.codegen.InnerCNode; import com.yahoo.config.codegen.LeafCNode; @@ -38,24 +36,6 @@ class InstanceResolver { private static final java.util.logging.Logger log = java.util.logging.Logger.getLogger(InstanceResolver.class.getName()); /** - * Resolves this config key into a correctly typed ConfigInstance using the given config builder. - * FIXME: Make private once config overrides are deprecated.? - * - * @param builder a ConfigBuilder to create the instance from. - * @param targetDef the def to use - * @return the config instance or null of no producer for this found in model - */ - static ConfigInstance resolveToInstance(ConfigInstance.Builder builder, InnerCNode targetDef) { - try { - if (targetDef != null) applyDef(builder, targetDef); - Class<? extends ConfigInstance> clazz = getConfigClass(builder.getClass()); - return clazz.getConstructor(builder.getClass()).newInstance(builder); - } catch (Exception e) { - throw new ConfigurationRuntimeException(e); - } - } - - /** * If some fields on the builder are null now, set them from the def. Do recursively. * <p> * If the targetDef has some schema incompatibilities, they are not handled here @@ -126,15 +106,6 @@ class InstanceResolver { } } - @SuppressWarnings("unchecked") - private static Class<? extends ConfigInstance> getConfigClass(Class<? extends ConfigInstance.Builder> builderClass) { - Class<?> configClass = builderClass.getEnclosingClass(); - if (configClass == null || ! ConfigInstance.class.isAssignableFrom(configClass)) { - throw new ConfigurationRuntimeException("Builder class " + builderClass + " has enclosing class " + configClass + ", which is not a ConfigInstance"); - } - return (Class<? extends ConfigInstance>) configClass; - } - static String packageName(ConfigDefinitionKey cKey, PackagePrefix packagePrefix) { return packagePrefix.value + cKey.getNamespace(); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java b/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java index 93a9600e134..1c87fced55f 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java @@ -14,12 +14,10 @@ import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.application.api.ValidationOverrides; -import com.yahoo.config.codegen.InnerCNode; import com.yahoo.config.model.ApplicationConfigProducerRoot; import com.yahoo.config.model.ConfigModelRegistry; import com.yahoo.config.model.ConfigModelRepo; import com.yahoo.config.model.NullConfigModelRegistry; -import com.yahoo.config.model.api.FileDistribution; import com.yahoo.config.model.api.HostInfo; import com.yahoo.config.model.api.Model; import com.yahoo.config.model.api.Provisioned; @@ -31,9 +29,9 @@ import com.yahoo.config.provision.AllocatedHosts; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.container.QrConfig; import com.yahoo.path.Path; +import com.yahoo.searchdefinition.LargeRankExpressions; import com.yahoo.searchdefinition.OnnxModel; import com.yahoo.searchdefinition.OnnxModels; -import com.yahoo.searchdefinition.LargeRankExpressions; import com.yahoo.searchdefinition.RankProfile; import com.yahoo.searchdefinition.RankProfileRegistry; import com.yahoo.searchdefinition.RankingConstants; @@ -43,7 +41,6 @@ import com.yahoo.searchdefinition.document.SDField; import com.yahoo.searchdefinition.processing.Processing; import com.yahoo.vespa.config.ConfigDefinitionKey; import com.yahoo.vespa.config.ConfigKey; -import com.yahoo.vespa.config.ConfigPayload; import com.yahoo.vespa.config.ConfigPayloadBuilder; import com.yahoo.vespa.config.GenericConfig.GenericConfigBuilder; import com.yahoo.vespa.model.InstanceResolver.PackagePrefix; @@ -65,7 +62,6 @@ import com.yahoo.vespa.model.ml.OnnxModelInfo; import com.yahoo.vespa.model.routing.Routing; import com.yahoo.vespa.model.search.AbstractSearchCluster; import com.yahoo.vespa.model.utils.internal.ReflectionUtil; -import com.yahoo.yolean.Exceptions; import org.xml.sax.SAXException; import java.io.IOException; @@ -271,7 +267,7 @@ public final class VespaModel extends AbstractConfigProducerRoot implements Seri /** Creates a mutable model with no services instantiated */ public static VespaModel createIncomplete(DeployState deployState) throws IOException, SAXException { return new VespaModel(new NullConfigModelRegistry(), deployState, false, - new FileDistributor(deployState.getFileRegistry(), List.of(), deployState.isHosted())); + new FileDistributor(deployState.getFileRegistry())); } private void validateWrapExceptions() { @@ -484,7 +480,6 @@ public final class VespaModel extends AbstractConfigProducerRoot implements Seri log.log(Level.FINE, () -> "Trying to get config for " + builder.getClass().getDeclaringClass().getName() + " for config id " + quote(configProducer.getConfigId()) + ", found=" + found + ", foundOverride=" + foundOverride); - } /** @@ -512,23 +507,6 @@ public final class VespaModel extends AbstractConfigProducerRoot implements Seri return builder; } - private ConfigPayload getConfigFromBuilder(ConfigInstance.Builder builder, InnerCNode targetDef) { - if (builder instanceof GenericConfigBuilder) return ((GenericConfigBuilder) builder).getPayload(); - - try { - ConfigInstance instance = InstanceResolver.resolveToInstance(builder, targetDef); - log.log(Level.FINE, () -> "getConfigFromBuilder for builder " + builder.getClass().getName() + ", instance=" + instance); - return ConfigPayload.fromInstance(instance); - } catch (ConfigurationRuntimeException e) { - // This can happen in cases where services ask for config that no longer exist before they have been able - // to reconfigure themselves. This happens for instance whenever jdisc reconfigures itself until - // ticket 6599572 is fixed. When that happens, consider propagating a full error rather than empty payload - // back to the client. - log.log(Level.INFO, "Error resolving instance for builder '" + builder.getClass().getName() + "', returning empty config: " + Exceptions.toMessageString(e)); - return ConfigPayload.fromBuilder(new ConfigPayloadBuilder()); - } - } - @Override public Set<ConfigKey<?>> allConfigsProduced() { Set<ConfigKey<?>> keySet = new LinkedHashSet<>(); @@ -599,11 +577,6 @@ public final class VespaModel extends AbstractConfigProducerRoot implements Seri } @Override - public void distributeFiles(FileDistribution fileDistribution) { - getFileDistributor().sendDeployedFiles(fileDistribution); - } - - @Override public AllocatedHosts allocatedHosts() { return allocatedHosts; } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java index d894891c9b0..912efbd9a24 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java @@ -322,7 +322,7 @@ public class VespaMetricSet { metrics.add(new Metric("documents_total.count")); metrics.add(new Metric("dispatch_internal.rate")); metrics.add(new Metric("dispatch_fdispatch.rate")); - addMetric(metrics, "jdisc.render.latency", Set.of("min", "max", "count", "sum", "last")); + addMetric(metrics, "jdisc.render.latency", Set.of("min", "max", "count", "sum", "last", "average")); metrics.add(new Metric("totalhits_per_query.max")); metrics.add(new Metric("totalhits_per_query.sum")); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminBuilderBase.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminBuilderBase.java index 963d2dde7fc..2bb8b6c9d1d 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminBuilderBase.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminBuilderBase.java @@ -68,11 +68,11 @@ public abstract class DomAdminBuilderBase extends VespaDomBuilder.DomConfigProdu } @Override - protected Admin doBuild(DeployState deployState, AbstractConfigProducer parent, Element adminElement) { + protected Admin doBuild(DeployState deployState, AbstractConfigProducer<?> parent, Element adminElement) { Monitoring monitoring = getMonitoring(XML.getChild(adminElement,"monitoring"), deployState.isHosted()); Metrics metrics = new MetricsBuilder(applicationType, PredefinedMetricSets.get()) .buildMetrics(XML.getChild(adminElement, "metrics")); - FileDistributionConfigProducer fileDistributionConfigProducer = getFileDistributionConfigProducer(parent, deployState.isHosted()); + FileDistributionConfigProducer fileDistributionConfigProducer = getFileDistributionConfigProducer(parent); Admin admin = new Admin(parent, monitoring, metrics, multitenant, fileDistributionConfigProducer, deployState.isHosted()); admin.setApplicationType(applicationType); @@ -82,8 +82,8 @@ public abstract class DomAdminBuilderBase extends VespaDomBuilder.DomConfigProdu return admin; } - private FileDistributionConfigProducer getFileDistributionConfigProducer(AbstractConfigProducer parent, boolean isHosted) { - return new FileDistributionConfigProducer(parent, fileRegistry, configServerSpecs, isHosted); + private FileDistributionConfigProducer getFileDistributionConfigProducer(AbstractConfigProducer<?> parent) { + return new FileDistributionConfigProducer(parent, fileRegistry); } protected abstract void doBuildAdmin(DeployState deployState, Admin admin, Element adminE); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributionConfigProducer.java b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributionConfigProducer.java index 2c6808b5773..8fa4d796a68 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributionConfigProducer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributionConfigProducer.java @@ -2,12 +2,10 @@ package com.yahoo.vespa.model.filedistribution; import com.yahoo.config.application.api.FileRegistry; -import com.yahoo.config.model.api.ConfigServerSpec; import com.yahoo.config.model.producer.AbstractConfigProducer; import com.yahoo.vespa.model.Host; import java.util.IdentityHashMap; -import java.util.List; import java.util.Map; /** @@ -15,17 +13,16 @@ import java.util.Map; * <p> * File distribution config producer, delegates getting config to {@link FileDistributionConfigProvider} (one per host) */ -public class FileDistributionConfigProducer extends AbstractConfigProducer { +public class FileDistributionConfigProducer extends AbstractConfigProducer<AbstractConfigProducer<?>> { private final Map<Host, FileDistributionConfigProvider> fileDistributionConfigProviders = new IdentityHashMap<>(); private final FileDistributor fileDistributor; - public FileDistributionConfigProducer(AbstractConfigProducer ancestor, FileRegistry fileRegistry, - List<ConfigServerSpec> configServerSpec, boolean isHosted) { - this(ancestor, new FileDistributor(fileRegistry, configServerSpec, isHosted)); + public FileDistributionConfigProducer(AbstractConfigProducer<?> ancestor, FileRegistry fileRegistry) { + this(ancestor, new FileDistributor(fileRegistry)); } - private FileDistributionConfigProducer(AbstractConfigProducer parent, FileDistributor fileDistributor) { + private FileDistributionConfigProducer(AbstractConfigProducer<?> parent, FileDistributor fileDistributor) { super(parent, "filedistribution"); this.fileDistributor = fileDistributor; } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributor.java b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributor.java index d8da911e32f..cf2ff2f5c3b 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributor.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributor.java @@ -2,16 +2,12 @@ package com.yahoo.vespa.model.filedistribution; import com.yahoo.config.FileReference; -import com.yahoo.config.model.api.ConfigServerSpec; -import com.yahoo.config.model.api.FileDistribution; import com.yahoo.config.application.api.FileRegistry; -import com.yahoo.vespa.model.ConfigProxy; import com.yahoo.vespa.model.Host; import java.nio.ByteBuffer; import java.util.HashSet; import java.util.LinkedHashMap; -import java.util.List; import java.util.Map; import java.util.Set; @@ -27,16 +23,12 @@ import java.util.Set; public class FileDistributor { private final FileRegistry fileRegistry; - private final List<ConfigServerSpec> configServerSpecs; - private final boolean isHosted; /** A map from file reference to the hosts to which that file reference should be distributed */ private final Map<FileReference, Set<Host>> filesToHosts = new LinkedHashMap<>(); - public FileDistributor(FileRegistry fileRegistry, List<ConfigServerSpec> configServerSpecs, boolean isHosted) { + public FileDistributor(FileRegistry fileRegistry) { this.fileRegistry = fileRegistry; - this.configServerSpecs = configServerSpecs; - this.isHosted = isHosted; } /** @@ -102,23 +94,4 @@ public class FileDistributor { return Set.copyOf(filesToHosts.keySet()); } - // should only be called during deploy - public void sendDeployedFiles(FileDistribution dbHandler) { - String fileSourceHost = fileSourceHost(); - - // Ask other config servers to download, for redundancy - configServerSpecs.stream() - .filter(spec -> !spec.getHostName().equals(fileSourceHost)) - .forEach(spec -> dbHandler.startDownload(spec.getHostName(), spec.getConfigServerPort(), allFilesToSend())); - - // Skip starting download for application hosts when on hosted, since this is just a hint and requests for files - // will fail until the application is activated (this call is done when preparing an application deployment) - // due to authorization of RPC requests on config servers only considering files belonging to active applications - if (isHosted) return; - - getTargetHosts().stream() - .filter(host -> ! host.getHostname().equals(fileSourceHost)) - .forEach(host -> dbHandler.startDownload(host.getHostname(), ConfigProxy.BASEPORT, filesToSendToHost(host))); - } - } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/utils/ContentClusterUtils.java b/config-model/src/test/java/com/yahoo/vespa/model/content/utils/ContentClusterUtils.java index b816d803276..d89fa4498f2 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/utils/ContentClusterUtils.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/utils/ContentClusterUtils.java @@ -60,7 +60,7 @@ public class ContentClusterUtils { public static ContentCluster createCluster(String clusterXml, MockRoot root) { Document doc = XML.getDocument(clusterXml); Admin admin = new Admin(root, new DefaultMonitoring("vespa", 60), new Metrics(), false, - new FileDistributionConfigProducer(root, new MockFileRegistry(), List.of(), false), + new FileDistributionConfigProducer(root, new MockFileRegistry()), root.getDeployState().isHosted()); ConfigModelContext context = ConfigModelContext.create(null, root.getDeployState(), null,null, root, null); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/filedistribution/FileDistributorTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/filedistribution/FileDistributorTestCase.java index 55672c5aa16..0acfdf35279 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/filedistribution/FileDistributorTestCase.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/filedistribution/FileDistributorTestCase.java @@ -2,16 +2,12 @@ package com.yahoo.vespa.model.filedistribution; import com.yahoo.config.FileReference; -import com.yahoo.config.model.api.FileDistribution; import com.yahoo.config.model.application.provider.MockFileRegistry; import com.yahoo.config.model.test.MockHosts; import org.junit.Test; -import java.io.File; import java.util.Arrays; import java.util.HashSet; -import java.util.List; -import java.util.Set; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -24,8 +20,7 @@ public class FileDistributorTestCase { @Test public void fileDistributor() { MockHosts hosts = new MockHosts(); - - FileDistributor fileDistributor = new FileDistributor(new MockFileRegistry(), List.of(), false); + FileDistributor fileDistributor = new FileDistributor(new MockFileRegistry()); String file1 = "component/path1"; String file2 = "component/path2"; @@ -38,24 +33,6 @@ public class FileDistributorTestCase { assertNotNull(ref1); assertNotNull(ref2); - - MockFileDistribution dbHandler = new MockFileDistribution(); - fileDistributor.sendDeployedFiles(dbHandler); - assertEquals(3, dbHandler.filesToDownloadCalled); // One time for each host - } - - private static class MockFileDistribution implements FileDistribution { - int filesToDownloadCalled = 0; - - @Override - public void startDownload(String hostName, int port, Set<FileReference> fileReferences) { - filesToDownloadCalled++; - } - - @Override - public File getFileReferencesDir() { - return null; - } } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java index dedd9e08655..61debbd055c 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java @@ -127,7 +127,6 @@ public class SessionPreparer { preparation.writeEndpointCertificateMetadataZK(); preparation.writeContainerEndpointsZK(); preparation.writeApplicationRoles(); - preparation.distribute(); } log.log(Level.FINE, () -> "time used " + params.getTimeoutBudget().timesUsed() + " : " + applicationId); return preparation.result(); @@ -298,12 +297,6 @@ public class SessionPreparer { checkTimeout("write application roles to zookeeper"); } - void distribute() { - prepareResult.asList().forEach(modelResult -> modelResult.model - .distributeFiles(modelResult.fileDistributionProvider.getFileDistribution())); - checkTimeout("distribute files"); - } - PrepareResult result() { return prepareResult; } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java b/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java index cc9b799f2d9..c88b0cf7099 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java @@ -29,9 +29,11 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import static com.yahoo.vespa.config.server.zookeeper.ZKApplication.DEFCONFIGS_ZK_SUBPATH; import static com.yahoo.vespa.config.server.zookeeper.ZKApplication.USERAPP_ZK_SUBPATH; @@ -45,7 +47,7 @@ public class ZKApplicationPackage implements ApplicationPackage { private final ZKApplication zkApplication; - private final Map<Version, PreGeneratedFileRegistry> fileRegistryMap = new HashMap<>(); + private final Map<Version, FileRegistry> fileRegistryMap = new HashMap<>(); private final Optional<AllocatedHosts> allocatedHosts; public static final String fileRegistryNode = "fileregistry"; @@ -91,7 +93,7 @@ public class ZKApplicationPackage implements ApplicationPackage { importFileRegistry(Joiner.on("/").join(fileRegistryNode, version)))); } - private PreGeneratedFileRegistry importFileRegistry(String fileRegistryNode) { + private FileRegistry importFileRegistry(String fileRegistryNode) { try { return PreGeneratedFileRegistry.importRegistry(zkApplication.getDataReader(Path.fromString(fileRegistryNode))); } catch (Exception e) { @@ -159,9 +161,9 @@ public class ZKApplicationPackage implements ApplicationPackage { return Collections.unmodifiableMap(fileRegistryMap); } - private Optional<PreGeneratedFileRegistry> getPreGeneratedFileRegistry(Version vespaVersion) { + private Optional<FileRegistry> getFileRegistry(Version vespaVersion) { // Assumes at least one file registry, which we always have. - Optional<PreGeneratedFileRegistry> fileRegistry = Optional.ofNullable(fileRegistryMap.get(vespaVersion)); + Optional<FileRegistry> fileRegistry = Optional.ofNullable(fileRegistryMap.get(vespaVersion)); if (fileRegistry.isEmpty()) { fileRegistry = Optional.of(fileRegistryMap.values().iterator().next()); } @@ -241,11 +243,21 @@ public class ZKApplicationPackage implements ApplicationPackage { return Optional.empty(); } + private static Set<String> getPaths(FileRegistry fileRegistry) { + Set<String> paths = new LinkedHashSet<>(); + synchronized (fileRegistry) { + for (FileRegistry.Entry e : fileRegistry.export()) { + paths.add(e.relativePath); + } + } + return paths; + } + @Override public List<ComponentInfo> getComponentsInfo(Version vespaVersion) { List<ComponentInfo> components = new ArrayList<>(); - PreGeneratedFileRegistry fileRegistry = getPreGeneratedFileRegistry(vespaVersion).get(); - for (String path : fileRegistry.getPaths()) { + FileRegistry fileRegistry = getFileRegistry(vespaVersion).get(); + for (String path : getPaths(fileRegistry)) { if (path.startsWith(COMPONENT_DIR + File.separator) && path.endsWith(".jar")) { ComponentInfo component = new ComponentInfo(path); components.add(component); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ModelStub.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ModelStub.java index ed12f4dbbe1..e0610d119db 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/ModelStub.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ModelStub.java @@ -3,12 +3,10 @@ package com.yahoo.vespa.config.server; import com.yahoo.config.ConfigInstance; import com.yahoo.config.FileReference; -import com.yahoo.config.model.api.FileDistribution; import com.yahoo.config.model.api.HostInfo; import com.yahoo.config.model.api.Model; import com.yahoo.config.provision.AllocatedHosts; import com.yahoo.vespa.config.ConfigKey; -import com.yahoo.vespa.config.ConfigPayload; import com.yahoo.vespa.config.buildergen.ConfigDefinition; import java.util.Collection; @@ -41,9 +39,6 @@ public class ModelStub implements Model { } @Override - public void distributeFiles(FileDistribution fileDistribution) { } - - @Override public Set<FileReference> fileReferences() { return new HashSet<>(); } @Override diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/MockModel.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/MockModel.java index 7e57eea74d6..2093a5bbb7c 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/MockModel.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/MockModel.java @@ -3,15 +3,13 @@ package com.yahoo.vespa.config.server.application; import com.yahoo.config.ConfigInstance; import com.yahoo.config.FileReference; -import com.yahoo.config.model.api.FileDistribution; import com.yahoo.config.model.api.HostInfo; import com.yahoo.config.model.api.Model; import com.yahoo.config.model.api.PortInfo; import com.yahoo.config.model.api.ServiceInfo; -import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.AllocatedHosts; +import com.yahoo.config.provision.ClusterSpec; import com.yahoo.vespa.config.ConfigKey; -import com.yahoo.vespa.config.ConfigPayload; import com.yahoo.vespa.config.buildergen.ConfigDefinition; import java.util.Arrays; @@ -94,11 +92,6 @@ public class MockModel implements Model { } @Override - public void distributeFiles(FileDistribution fileDistribution) { - throw new UnsupportedOperationException(); - } - - @Override public Set<FileReference> fileReferences() { return new HashSet<>(); } @Override diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/DeploymentMetricsRetrieverTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/DeploymentMetricsRetrieverTest.java index 865d3b71b6e..938b43dfa78 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/DeploymentMetricsRetrieverTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/DeploymentMetricsRetrieverTest.java @@ -3,14 +3,12 @@ package com.yahoo.vespa.config.server.metrics; import com.yahoo.config.ConfigInstance; import com.yahoo.config.FileReference; -import com.yahoo.config.model.api.FileDistribution; import com.yahoo.config.model.api.HostInfo; import com.yahoo.config.model.api.Model; import com.yahoo.config.model.api.ServiceInfo; import com.yahoo.config.provision.AllocatedHosts; import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.config.ConfigKey; -import com.yahoo.vespa.config.ConfigPayload; import com.yahoo.vespa.config.buildergen.ConfigDefinition; import com.yahoo.vespa.config.server.application.Application; import org.junit.Test; @@ -104,11 +102,6 @@ public class DeploymentMetricsRetrieverTest { } @Override - public void distributeFiles(FileDistribution fileDistribution) { - throw new UnsupportedOperationException(); - } - - @Override public Set<FileReference> fileReferences() { return new HashSet<>(); } @Override diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/ProtonMetricsRetrieverTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/ProtonMetricsRetrieverTest.java index 0aad2f1ee26..3b39d9162ef 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/ProtonMetricsRetrieverTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/ProtonMetricsRetrieverTest.java @@ -3,16 +3,16 @@ package com.yahoo.vespa.config.server.metrics; import com.yahoo.config.ConfigInstance; import com.yahoo.config.FileReference; -import com.yahoo.config.model.api.FileDistribution; import com.yahoo.config.model.api.HostInfo; import com.yahoo.config.model.api.Model; import com.yahoo.config.model.api.ServiceInfo; import com.yahoo.config.provision.AllocatedHosts; import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.config.ConfigKey; -import com.yahoo.vespa.config.ConfigPayload; import com.yahoo.vespa.config.buildergen.ConfigDefinition; import com.yahoo.vespa.config.server.application.Application; +import org.junit.Test; + import java.net.URI; import java.util.ArrayList; import java.util.Collection; @@ -20,7 +20,6 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; -import org.junit.Test; import static org.junit.Assert.assertEquals; @@ -98,11 +97,6 @@ public class ProtonMetricsRetrieverTest { } @Override - public void distributeFiles(FileDistribution fileDistribution) { - throw new UnsupportedOperationException(); - } - - @Override public Set<FileReference> fileReferences() { return new HashSet<>(); } @Override diff --git a/container-dependencies-enforcer/pom.xml b/container-dependencies-enforcer/pom.xml index d692adac3c3..ee0db90a49e 100644 --- a/container-dependencies-enforcer/pom.xml +++ b/container-dependencies-enforcer/pom.xml @@ -113,8 +113,6 @@ <include>org.glassfish.hk2:hk2-utils:[${hk2.version}]:jar:provided</include> <include>org.glassfish.hk2:osgi-resource-locator:[${hk2.osgi-resource-locator.version}]:jar:provided</include> <include>org.glassfish.jersey.bundles.repackaged:jersey-guava:[${jersey2.version}]:jar:provided</include> - <include>org.glassfish.jersey.containers:jersey-container-servlet-core:[${jersey2.version}]:jar:provided</include> - <include>org.glassfish.jersey.containers:jersey-container-servlet:[${jersey2.version}]:jar:provided</include> <include>org.glassfish.jersey.core:jersey-client:[${jersey2.version}]:jar:provided</include> <include>org.glassfish.jersey.core:jersey-common:[${jersey2.version}]:jar:provided</include> <include>org.glassfish.jersey.core:jersey-server:[${jersey2.version}]:jar:provided</include> diff --git a/container-dependency-versions/pom.xml b/container-dependency-versions/pom.xml index 29e9f85256d..b66def32df8 100644 --- a/container-dependency-versions/pom.xml +++ b/container-dependency-versions/pom.xml @@ -253,16 +253,6 @@ <version>${jersey2.version}</version> </dependency> <dependency> - <groupId>org.glassfish.jersey.containers</groupId> - <artifactId>jersey-container-servlet-core</artifactId> - <version>${jersey2.version}</version> - </dependency> - <dependency> - <groupId>org.glassfish.jersey.containers</groupId> - <artifactId>jersey-container-servlet</artifactId> - <version>${jersey2.version}</version> - </dependency> - <dependency> <groupId>org.glassfish.jersey.core</groupId> <artifactId>jersey-client</artifactId> <version>${jersey2.version}</version> diff --git a/container-disc/pom.xml b/container-disc/pom.xml index b255b6af02a..f437f513276 100644 --- a/container-disc/pom.xml +++ b/container-disc/pom.xml @@ -229,9 +229,7 @@ javax.ws.rs-api-${javax.ws.rs-api.version}.jar, jersey-client-${jersey2.version}.jar, jersey-common-${jersey2.version}.jar, - jersey-container-servlet-${jersey2.version}.jar, - jersey-container-servlet-core-${jersey2.version}.jar, - jersey-entity-filtering-${jersey2.version}.jar, <!-- new feature from 2.16, provided for convenience --> + jersey-entity-filtering-${jersey2.version}.jar, <!-- needed by jersey-media-json-jackson --> jersey-guava-${jersey2.version}.jar, jersey-media-jaxb-${jersey2.version}.jar, jersey-media-json-jackson-${jersey2.version}.jar, diff --git a/eval/CMakeLists.txt b/eval/CMakeLists.txt index d5a0b2a14ae..4af337dcb67 100644 --- a/eval/CMakeLists.txt +++ b/eval/CMakeLists.txt @@ -18,6 +18,7 @@ vespa_define_module( src/tests/eval/compile_cache src/tests/eval/compiled_function src/tests/eval/fast_value + src/tests/eval/feature_name_extractor src/tests/eval/function src/tests/eval/function_speed src/tests/eval/gbdt diff --git a/searchlib/src/tests/rankingexpression/feature_name_extractor/.gitignore b/eval/src/tests/eval/feature_name_extractor/.gitignore index 88c86c1720e..88c86c1720e 100644 --- a/searchlib/src/tests/rankingexpression/feature_name_extractor/.gitignore +++ b/eval/src/tests/eval/feature_name_extractor/.gitignore diff --git a/eval/src/tests/eval/feature_name_extractor/CMakeLists.txt b/eval/src/tests/eval/feature_name_extractor/CMakeLists.txt new file mode 100644 index 00000000000..7126060e974 --- /dev/null +++ b/eval/src/tests/eval/feature_name_extractor/CMakeLists.txt @@ -0,0 +1,8 @@ +# Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +vespa_add_executable(eval_name_extractor_test_app TEST + SOURCES + feature_name_extractor_test.cpp + DEPENDS + vespaeval +) +vespa_add_test(NAME eval_name_extractor_test_app COMMAND eval_name_extractor_test_app) diff --git a/searchlib/src/tests/rankingexpression/feature_name_extractor/feature_name_extractor_test.cpp b/eval/src/tests/eval/feature_name_extractor/feature_name_extractor_test.cpp index 7b3683f75d5..3acf1ee2142 100644 --- a/searchlib/src/tests/rankingexpression/feature_name_extractor/feature_name_extractor_test.cpp +++ b/eval/src/tests/eval/feature_name_extractor/feature_name_extractor_test.cpp @@ -1,8 +1,8 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include <vespa/vespalib/testkit/test_kit.h> -#include <vespa/searchlib/features/rankingexpression/feature_name_extractor.h> +#include <vespa/eval/eval/feature_name_extractor.h> -using search::features::rankingexpression::FeatureNameExtractor; +using vespalib::eval::FeatureNameExtractor; void verify_extract(const vespalib::string &input, const vespalib::string &expect_symbol, diff --git a/eval/src/vespa/eval/eval/CMakeLists.txt b/eval/src/vespa/eval/eval/CMakeLists.txt index 639ac3b5864..8271c7a4bed 100644 --- a/eval/src/vespa/eval/eval/CMakeLists.txt +++ b/eval/src/vespa/eval/eval/CMakeLists.txt @@ -13,6 +13,7 @@ vespa_add_library(eval_eval OBJECT fast_addr_map.cpp fast_forest.cpp fast_value.cpp + feature_name_extractor.cpp function.cpp gbdt.cpp int8float.cpp diff --git a/searchlib/src/vespa/searchlib/features/rankingexpression/feature_name_extractor.cpp b/eval/src/vespa/eval/eval/feature_name_extractor.cpp index 1987f476780..f613d026d03 100644 --- a/searchlib/src/vespa/searchlib/features/rankingexpression/feature_name_extractor.cpp +++ b/eval/src/vespa/eval/eval/feature_name_extractor.cpp @@ -2,9 +2,7 @@ #include "feature_name_extractor.h" -namespace search { -namespace features { -namespace rankingexpression { +namespace vespalib::eval { namespace { @@ -81,6 +79,4 @@ FeatureNameExtractor::extract_symbol(const char *pos_in, const char *end_in, pos_out = pos_in; } -} // namespace rankingexpression -} // namespace features -} // namespace search +} diff --git a/eval/src/vespa/eval/eval/feature_name_extractor.h b/eval/src/vespa/eval/eval/feature_name_extractor.h new file mode 100644 index 00000000000..b3a9e0567a3 --- /dev/null +++ b/eval/src/vespa/eval/eval/feature_name_extractor.h @@ -0,0 +1,18 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include "function.h" + +namespace vespalib::eval { + +/** + * Custom symbol extractor used to extract ranking feature names when + * parsing ranking expressions. + **/ +struct FeatureNameExtractor : public vespalib::eval::SymbolExtractor { + void extract_symbol(const char *pos_in, const char *end_in, + const char *&pos_out, vespalib::string &symbol_out) const override; +}; + +} diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_vector_explorer.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_vector_explorer.cpp index ddf240ad90c..51ac20dd4bb 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_vector_explorer.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_vector_explorer.cpp @@ -158,6 +158,8 @@ AttributeVectorExplorer::get_state(const vespalib::slime::Inserter &inserter, bo object.setLong("numDocs", status.getNumDocs()); object.setLong("lastSerialNum", status.getLastSyncToken()); object.setLong("allocatedMemory", status.getAllocated()); + object.setLong("usedMemory", status.getUsed()); + object.setLong("onHoldMemory", status.getOnHold()); object.setLong("committedDocIdLimit", attr.getCommittedDocIdLimit()); } } diff --git a/searchlib/CMakeLists.txt b/searchlib/CMakeLists.txt index ea47dddb99b..16a35a9ce9f 100644 --- a/searchlib/CMakeLists.txt +++ b/searchlib/CMakeLists.txt @@ -209,7 +209,6 @@ vespa_define_module( src/tests/queryeval/weak_and_scorers src/tests/queryeval/weighted_set_term src/tests/queryeval/wrappers - src/tests/rankingexpression/feature_name_extractor src/tests/rankingexpression/intrinsic_blueprint_adapter src/tests/ranksetup src/tests/ranksetup/verify_feature diff --git a/searchlib/src/tests/rankingexpression/feature_name_extractor/CMakeLists.txt b/searchlib/src/tests/rankingexpression/feature_name_extractor/CMakeLists.txt deleted file mode 100644 index a2e153c7527..00000000000 --- a/searchlib/src/tests/rankingexpression/feature_name_extractor/CMakeLists.txt +++ /dev/null @@ -1,8 +0,0 @@ -# Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -vespa_add_executable(searchlib_feature_name_extractor_test_app TEST - SOURCES - feature_name_extractor_test.cpp - DEPENDS - searchlib -) -vespa_add_test(NAME searchlib_feature_name_extractor_test_app COMMAND searchlib_feature_name_extractor_test_app) diff --git a/searchlib/src/vespa/searchlib/features/rankingexpression/CMakeLists.txt b/searchlib/src/vespa/searchlib/features/rankingexpression/CMakeLists.txt index 68b4c4bb043..715fbb4446e 100644 --- a/searchlib/src/vespa/searchlib/features/rankingexpression/CMakeLists.txt +++ b/searchlib/src/vespa/searchlib/features/rankingexpression/CMakeLists.txt @@ -2,7 +2,6 @@ vespa_add_library(searchlib_features_rankingexpression OBJECT SOURCES expression_replacer.cpp - feature_name_extractor.cpp intrinsic_blueprint_adapter.cpp intrinsic_expression.cpp DEPENDS diff --git a/searchlib/src/vespa/searchlib/features/rankingexpression/feature_name_extractor.h b/searchlib/src/vespa/searchlib/features/rankingexpression/feature_name_extractor.h index b7d82744953..4d59e95e7a3 100644 --- a/searchlib/src/vespa/searchlib/features/rankingexpression/feature_name_extractor.h +++ b/searchlib/src/vespa/searchlib/features/rankingexpression/feature_name_extractor.h @@ -2,22 +2,10 @@ #pragma once -#include <vespa/eval/eval/function.h> +#include <vespa/eval/eval/feature_name_extractor.h> -namespace search { -namespace features { -namespace rankingexpression { +namespace search::features::rankingexpression { -/** - * Custom symbol extractor used to extract ranking feature names when - * parsing ranking expressions. - **/ -struct FeatureNameExtractor : public vespalib::eval::SymbolExtractor { - void extract_symbol(const char *pos_in, const char *end_in, - const char *&pos_out, vespalib::string &symbol_out) const override; -}; - -} // namespace rankingexpression -} // namespace features -} // namespace search +using FeatureNameExtractor = vespalib::eval::FeatureNameExtractor; +} diff --git a/searchlib/src/vespa/searchlib/tensor/hnsw_graph.h b/searchlib/src/vespa/searchlib/tensor/hnsw_graph.h index ce2bc172752..4d07f74f8e3 100644 --- a/searchlib/src/vespa/searchlib/tensor/hnsw_graph.h +++ b/searchlib/src/vespa/searchlib/tensor/hnsw_graph.h @@ -18,7 +18,10 @@ struct HnswGraph { // This uses 10 bits for buffer id -> 1024 buffers. // As we have very short arrays we get less fragmentation with fewer and larger buffers. - using EntryRefType = vespalib::datastore::EntryRefT<22>; + using LevelArrayEntryRefType = vespalib::datastore::EntryRefT<22>; + + // This uses 12 bits for buffer id -> 4096 buffers. + using LinkArrayEntryRefType = vespalib::datastore::EntryRefT<20>; // Provides mapping from document id -> node reference. // The reference is used to lookup the node data in NodeStore. @@ -27,13 +30,13 @@ struct HnswGraph { // This stores the level arrays for all nodes. // Each node consists of an array of levels (from level 0 to n) where each entry is a reference to the link array at that level. - using NodeStore = vespalib::datastore::ArrayStore<AtomicEntryRef, EntryRefType>; + using NodeStore = vespalib::datastore::ArrayStore<AtomicEntryRef, LevelArrayEntryRefType>; using StoreConfig = vespalib::datastore::ArrayStoreConfig; using LevelArrayRef = NodeStore::ConstArrayRef; // This stores all link arrays. // A link array consists of the document ids of the nodes a particular node is linked to. - using LinkStore = vespalib::datastore::ArrayStore<uint32_t, EntryRefType>; + using LinkStore = vespalib::datastore::ArrayStore<uint32_t, LinkArrayEntryRefType>; using LinkArrayRef = LinkStore::ConstArrayRef; NodeRefVector node_refs; diff --git a/searchlib/src/vespa/searchlib/tensor/hnsw_index.cpp b/searchlib/src/vespa/searchlib/tensor/hnsw_index.cpp index 545ee7cfa96..8183a7caf3d 100644 --- a/searchlib/src/vespa/searchlib/tensor/hnsw_index.cpp +++ b/searchlib/src/vespa/searchlib/tensor/hnsw_index.cpp @@ -12,6 +12,7 @@ #include <vespa/vespalib/util/memory_allocator.h> #include <vespa/vespalib/util/rcuvector.hpp> #include <vespa/vespalib/util/size_literals.h> +#include <vespa/vespalib/util/time.h> #include <vespa/log/log.h> LOG_SETUP(".searchlib.tensor.hnsw_index"); @@ -29,7 +30,8 @@ constexpr size_t min_num_arrays_for_new_buffer = 512_Ki; constexpr float alloc_grow_factor = 0.3; // TODO: Adjust these numbers to what we accept as max in config. constexpr size_t max_level_array_size = 16; -constexpr size_t max_link_array_size = 64; +constexpr size_t max_link_array_size = 193; +constexpr vespalib::duration MAX_COUNT_DURATION(100ms); bool has_link_to(vespalib::ConstArrayRef<uint32_t> links, uint32_t id) { for (uint32_t link : links) { @@ -276,8 +278,7 @@ HnswIndex::search_layer(const TypedCells& input, uint32_t neighbors_to_find, HnswIndex::HnswIndex(const DocVectorAccess& vectors, DistanceFunction::UP distance_func, RandomLevelGenerator::UP level_generator, const Config& cfg) - : - _graph(), + : _graph(), _vectors(vectors), _distance_func(std::move(distance_func)), _level_generator(std::move(level_generator)), @@ -486,7 +487,15 @@ void HnswIndex::get_state(const vespalib::slime::Inserter& inserter) const { auto& object = inserter.insertObject(); - StateExplorerUtils::memory_usage_to_slime(memory_usage(), object.setObject("memory_usage")); + auto& memUsageObj = object.setObject("memory_usage"); + StateExplorerUtils::memory_usage_to_slime(memory_usage(), memUsageObj.setObject("all")); + StateExplorerUtils::memory_usage_to_slime(_graph.node_refs.getMemoryUsage(), memUsageObj.setObject("node_refs")); + StateExplorerUtils::memory_usage_to_slime(_graph.nodes.getMemoryUsage(), memUsageObj.setObject("nodes")); + StateExplorerUtils::memory_usage_to_slime(_graph.links.getMemoryUsage(), memUsageObj.setObject("links")); + StateExplorerUtils::memory_usage_to_slime(_visited_set_pool.memory_usage(), memUsageObj.setObject("visited_set_pool")); + auto& visitedObj = object.setObject("visited_set"); + visitedObj.setLong("create_count", _visited_set_pool.create_count()); + visitedObj.setLong("reuse_count", _visited_set_pool.reuse_count()); object.setLong("nodes", _graph.size()); auto& histogram_array = object.setArray("level_histogram"); auto& links_hst_array = object.setArray("level_0_links_histogram"); @@ -500,9 +509,13 @@ HnswIndex::get_state(const vespalib::slime::Inserter& inserter) const for (uint32_t hist_val : histograms.links_histogram) { links_hst_array.addLong(hist_val); } - uint32_t reachable = count_reachable_nodes(); - uint32_t unreachable = valid_nodes - reachable; - object.setLong("unreachable_nodes", unreachable); + auto count_result = count_reachable_nodes(); + uint32_t unreachable = valid_nodes - count_result.first; + if (count_result.second) { + object.setLong("unreachable_nodes", unreachable); + } else { + object.setLong("unreachable_nodes_incomplete_count", unreachable); + } auto entry_node = _graph.get_entry_node(); object.setLong("entry_docid", entry_node.docid); object.setLong("entry_level", entry_node.level); @@ -650,31 +663,35 @@ HnswIndex::check_link_symmetry() const return all_sym; } -uint32_t +std::pair<uint32_t, bool> HnswIndex::count_reachable_nodes() const { auto entry = _graph.get_entry_node(); int search_level = entry.level; if (search_level < 0) { - return 0; + return {0, true}; } - auto visited = _visited_set_pool.get(_graph.size()); + std::vector<bool> visited(_graph.size()); LinkArray found_links; found_links.push_back(entry.docid); - visited.mark(entry.docid); + visited[entry.docid] = true; + vespalib::steady_time doom = vespalib::steady_clock::now() + MAX_COUNT_DURATION; while (search_level >= 0) { for (uint32_t idx = 0; idx < found_links.size(); ++idx) { + if (vespalib::steady_clock::now() > doom) { + return {found_links.size(), false}; + } uint32_t docid = found_links[idx]; auto neighbors = _graph.get_link_array(docid, search_level); for (uint32_t neighbor : neighbors) { - if (visited.is_marked(neighbor)) continue; - visited.mark(neighbor); + if (visited[neighbor]) continue; + visited[neighbor] = true; found_links.push_back(neighbor); } } --search_level; } - return found_links.size(); + return {found_links.size(), true}; } } // namespace diff --git a/searchlib/src/vespa/searchlib/tensor/hnsw_index.h b/searchlib/src/vespa/searchlib/tensor/hnsw_index.h index 5bd9d17adc3..ef0f38c2263 100644 --- a/searchlib/src/vespa/searchlib/tensor/hnsw_index.h +++ b/searchlib/src/vespa/searchlib/tensor/hnsw_index.h @@ -183,7 +183,7 @@ public: HnswNode get_node(uint32_t docid) const; void set_node(uint32_t docid, const HnswNode &node); bool check_link_symmetry() const; - uint32_t count_reachable_nodes() const; + std::pair<uint32_t, bool> count_reachable_nodes() const; static vespalib::datastore::ArrayStoreConfig make_default_node_store_config(); static vespalib::datastore::ArrayStoreConfig make_default_link_store_config(); diff --git a/searchlib/src/vespa/searchlib/tensor/hnsw_index_saver.cpp b/searchlib/src/vespa/searchlib/tensor/hnsw_index_saver.cpp index 6593a60d6b5..8ac613c0976 100644 --- a/searchlib/src/vespa/searchlib/tensor/hnsw_index_saver.cpp +++ b/searchlib/src/vespa/searchlib/tensor/hnsw_index_saver.cpp @@ -6,7 +6,31 @@ namespace search::tensor { -HnswIndexSaver::~HnswIndexSaver() {} +namespace { + +size_t +count_valid_link_arrays(const HnswGraph & graph) { + size_t count(0); + size_t num_nodes = graph.node_refs.size(); + for (size_t i = 0; i < num_nodes; ++i) { + auto node_ref = graph.node_refs[i].load_acquire(); + if (node_ref.valid()) { + count += graph.nodes.get(node_ref).size(); + } + } + return count; +} + +} + +HnswIndexSaver::MetaData::MetaData() + : entry_docid(0), + entry_level(-1), + refs(), + nodes() +{} +HnswIndexSaver::MetaData::~MetaData() = default; +HnswIndexSaver::~HnswIndexSaver() = default; HnswIndexSaver::HnswIndexSaver(const HnswGraph &graph) : _graph_links(graph.links), _meta_data() @@ -15,19 +39,22 @@ HnswIndexSaver::HnswIndexSaver(const HnswGraph &graph) _meta_data.entry_docid = entry.docid; _meta_data.entry_level = entry.level; size_t num_nodes = graph.node_refs.size(); - _meta_data.nodes.reserve(num_nodes); + assert (num_nodes <= (std::numeric_limits<uint32_t>::max() - 1)); + size_t link_array_count = count_valid_link_arrays(graph); + assert (link_array_count <= std::numeric_limits<uint32_t>::max()); + _meta_data.refs.reserve(link_array_count); + _meta_data.nodes.reserve(num_nodes+1); for (size_t i = 0; i < num_nodes; ++i) { - LevelVector node; + _meta_data.nodes.push_back(_meta_data.refs.size()); auto node_ref = graph.node_refs[i].load_acquire(); if (node_ref.valid()) { auto levels = graph.nodes.get(node_ref); for (const auto& links_ref : levels) { - auto level = links_ref.load_acquire(); - node.push_back(level); + _meta_data.refs.push_back(links_ref.load_acquire()); } } - _meta_data.nodes.emplace_back(std::move(node)); } + _meta_data.nodes.push_back(_meta_data.refs.size()); } void @@ -35,12 +62,15 @@ HnswIndexSaver::save(BufferWriter& writer) const { writer.write(&_meta_data.entry_docid, sizeof(uint32_t)); writer.write(&_meta_data.entry_level, sizeof(int32_t)); - uint32_t num_nodes = _meta_data.nodes.size(); + uint32_t num_nodes = _meta_data.nodes.size() - 1; writer.write(&num_nodes, sizeof(uint32_t)); - for (const auto &node : _meta_data.nodes) { - uint32_t num_levels = node.size(); + for (uint32_t i(0); i < num_nodes; i++) { + uint32_t offset = _meta_data.nodes[i]; + uint32_t next_offset = _meta_data.nodes[i+1]; + uint32_t num_levels = next_offset - offset; writer.write(&num_levels, sizeof(uint32_t)); - for (auto links_ref : node) { + for (; offset < next_offset; offset++) { + auto links_ref = _meta_data.refs[offset]; if (links_ref.valid()) { vespalib::ConstArrayRef<uint32_t> link_array = _graph_links.get(links_ref); uint32_t num_links = link_array.size(); diff --git a/searchlib/src/vespa/searchlib/tensor/hnsw_index_saver.h b/searchlib/src/vespa/searchlib/tensor/hnsw_index_saver.h index 387e35efd8b..9052c3f1909 100644 --- a/searchlib/src/vespa/searchlib/tensor/hnsw_index_saver.h +++ b/searchlib/src/vespa/searchlib/tensor/hnsw_index_saver.h @@ -5,6 +5,7 @@ #include "nearest_neighbor_index_saver.h" #include "hnsw_graph.h" #include <vespa/vespalib/datastore/entryref.h> +#include <vespa/vespalib/stllike/allocator.h> #include <vector> namespace search::tensor { @@ -17,18 +18,19 @@ namespace search::tensor { **/ class HnswIndexSaver : public NearestNeighborIndexSaver { public: - using LevelVector = std::vector<vespalib::datastore::EntryRef>; - HnswIndexSaver(const HnswGraph &graph); - ~HnswIndexSaver(); + ~HnswIndexSaver() override; void save(BufferWriter& writer) const override; private: struct MetaData { + using EntryRef = vespalib::datastore::EntryRef; uint32_t entry_docid; int32_t entry_level; - std::vector<LevelVector> nodes; - MetaData() : entry_docid(0), entry_level(-1), nodes() {} + std::vector<EntryRef, vespalib::allocator_large<EntryRef>> refs; + std::vector<uint32_t, vespalib::allocator_large<uint32_t>> nodes; + MetaData(); + ~MetaData(); }; const HnswGraph::LinkStore &_graph_links; MetaData _meta_data; diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/duper/HostsModel.java b/service-monitor/src/main/java/com/yahoo/vespa/service/duper/HostsModel.java index 10820ccc508..2f63fae2198 100644 --- a/service-monitor/src/main/java/com/yahoo/vespa/service/duper/HostsModel.java +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/duper/HostsModel.java @@ -3,12 +3,10 @@ package com.yahoo.vespa.service.duper; import com.yahoo.config.ConfigInstance; import com.yahoo.config.FileReference; -import com.yahoo.config.model.api.FileDistribution; import com.yahoo.config.model.api.HostInfo; import com.yahoo.config.model.api.Model; import com.yahoo.config.provision.AllocatedHosts; import com.yahoo.vespa.config.ConfigKey; -import com.yahoo.vespa.config.ConfigPayload; import com.yahoo.vespa.config.buildergen.ConfigDefinition; import java.time.Instant; @@ -51,11 +49,6 @@ public class HostsModel implements Model { } @Override - public void distributeFiles(FileDistribution fileDistribution) { - throw new UnsupportedOperationException(); - } - - @Override public Set<FileReference> fileReferences() { throw new UnsupportedOperationException(); } diff --git a/slobrok/src/vespa/slobrok/server/service_map_history.cpp b/slobrok/src/vespa/slobrok/server/service_map_history.cpp index 5a2f8c6ff43..551ea367bc9 100644 --- a/slobrok/src/vespa/slobrok/server/service_map_history.cpp +++ b/slobrok/src/vespa/slobrok/server/service_map_history.cpp @@ -43,8 +43,7 @@ ServiceMapHistory::UpdateLog::updatedSince(const Generation &gen) const { //----------------------------------------------------------------------------- ServiceMapHistory::ServiceMapHistory() - : _lock(), - _map(), + : _map(), _waitList(), _log() {} @@ -56,63 +55,49 @@ ServiceMapHistory::~ServiceMapHistory() { void ServiceMapHistory::notify_updated() { WaitList waitList; - { - std::lock_guard guard(_lock); - std::swap(waitList, _waitList); - } + std::swap(waitList, _waitList); for (auto & [ handler, gen ] : waitList) { handler->handle(makeDiffFrom(gen)); } } void ServiceMapHistory::asyncGenerationDiff(DiffCompletionHandler *handler, const Generation &fromGen) { - { - std::lock_guard guard(_lock); - if (fromGen == myGen()) { - _waitList.emplace_back(handler, fromGen); - return; - } + if (fromGen == myGen()) { + _waitList.emplace_back(handler, fromGen); + return; } handler->handle(makeDiffFrom(fromGen)); } bool ServiceMapHistory::cancel(DiffCompletionHandler *handler) { - std::lock_guard guard(_lock); size_t removed = std::erase_if(_waitList, [=](const Waiter &elem){ return elem.first == handler; }); return (removed > 0); } void ServiceMapHistory::remove(const ServiceMapping &mapping) { - { - std::lock_guard guard(_lock); - auto iter = _map.find(mapping.name); - if (iter == _map.end()) { - LOG(debug, "already removed: %s", mapping.name.c_str()); - return; // already removed - } - LOG_ASSERT(iter->second == mapping.spec); - _map.erase(iter); - _log.add(mapping.name); + auto iter = _map.find(mapping.name); + if (iter == _map.end()) { + LOG(debug, "already removed: %s", mapping.name.c_str()); + return; // already removed } + LOG_ASSERT(iter->second == mapping.spec); + _map.erase(iter); + _log.add(mapping.name); notify_updated(); } void ServiceMapHistory::add(const ServiceMapping &mapping) { - { - std::lock_guard guard(_lock); - auto iter = _map.find(mapping.name); - if (iter != _map.end() && iter->second == mapping.spec) { - // already ok - return; - } - _map.insert_or_assign(mapping.name, mapping.spec); - _log.add(mapping.name); + auto iter = _map.find(mapping.name); + if (iter != _map.end() && iter->second == mapping.spec) { + // already ok + return; } + _map.insert_or_assign(mapping.name, mapping.spec); + _log.add(mapping.name); notify_updated(); } MapDiff ServiceMapHistory::makeDiffFrom(const Generation &fromGen) const { - std::lock_guard guard(_lock); if (_log.isInRange(fromGen)) { std::vector<vespalib::string> removes; ServiceMappingList updates; diff --git a/slobrok/src/vespa/slobrok/server/service_map_history.h b/slobrok/src/vespa/slobrok/server/service_map_history.h index 9026586b945..726ba778ca2 100644 --- a/slobrok/src/vespa/slobrok/server/service_map_history.h +++ b/slobrok/src/vespa/slobrok/server/service_map_history.h @@ -6,7 +6,6 @@ #include <vespa/vespalib/util/gencnt.h> #include <vespa/vespalib/util/arrayqueue.hpp> #include <map> -#include <mutex> #include "map_listener.h" #include "service_mapping.h" #include "map_diff.h" @@ -52,7 +51,6 @@ private: using Waiter = std::pair<DiffCompletionHandler *, Generation>; using WaitList = std::vector<Waiter>; - mutable std::mutex _lock; Map _map; WaitList _waitList; UpdateLog _log; diff --git a/slobrok/src/vespa/slobrok/server/service_map_mirror.cpp b/slobrok/src/vespa/slobrok/server/service_map_mirror.cpp index 51db84e24a4..90deaa0f09c 100644 --- a/slobrok/src/vespa/slobrok/server/service_map_mirror.cpp +++ b/slobrok/src/vespa/slobrok/server/service_map_mirror.cpp @@ -8,8 +8,7 @@ namespace slobrok { ServiceMapMirror::ServiceMapMirror() : _map(), - _currGen(0), - _lock() + _currGen(0) {} ServiceMapMirror::~ServiceMapMirror() { @@ -17,7 +16,6 @@ ServiceMapMirror::~ServiceMapMirror() { } void ServiceMapMirror::apply(const MapDiff &diff) { - std::lock_guard guard(_lock); LOG(debug, "Applying diff from gen %u", diff.fromGen.getAsInt()); LOG_ASSERT(diff.fromGen == _currGen); for (const auto & name : diff.removed) { @@ -54,7 +52,6 @@ void ServiceMapMirror::apply(const MapDiff &diff) { } void ServiceMapMirror::clear() { - std::lock_guard guard(_lock); for (const auto & [ k, v ] : _map) { ServiceMapping mapping{k, v}; for (auto * listener : _listeners) { @@ -66,7 +63,6 @@ void ServiceMapMirror::clear() { } ServiceMappingList ServiceMapMirror::allMappings() const { - std::lock_guard guard(_lock); ServiceMappingList result; result.reserve(_map.size()); for (const auto & [ k, v ] : _map) { @@ -76,12 +72,10 @@ ServiceMappingList ServiceMapMirror::allMappings() const { } void ServiceMapMirror::registerListener(MapListener &listener) { - std::lock_guard guard(_lock); _listeners.insert(&listener); } void ServiceMapMirror::unregisterListener(MapListener &listener) { - std::lock_guard guard(_lock); _listeners.erase(&listener); } diff --git a/slobrok/src/vespa/slobrok/server/service_map_mirror.h b/slobrok/src/vespa/slobrok/server/service_map_mirror.h index 4cd1c91b7f1..7418aeaec92 100644 --- a/slobrok/src/vespa/slobrok/server/service_map_mirror.h +++ b/slobrok/src/vespa/slobrok/server/service_map_mirror.h @@ -7,7 +7,6 @@ #include "map_source.h" #include <vespa/vespalib/util/gencnt.h> #include <map> -#include <mutex> #include <set> namespace slobrok { @@ -41,7 +40,6 @@ private: using Map = std::map<vespalib::string, vespalib::string>; Map _map; Generation _currGen; - mutable std::mutex _lock; std::set<MapListener *> _listeners; }; diff --git a/vespa_jersey2/pom.xml b/vespa_jersey2/pom.xml index c39b92cd371..5524323244e 100644 --- a/vespa_jersey2/pom.xml +++ b/vespa_jersey2/pom.xml @@ -20,14 +20,6 @@ <artifactId>javax.ws.rs-api</artifactId> </dependency> <dependency> - <groupId>org.glassfish.jersey.containers</groupId> - <artifactId>jersey-container-servlet-core</artifactId> - </dependency> - <dependency> - <groupId>org.glassfish.jersey.containers</groupId> - <artifactId>jersey-container-servlet</artifactId> - </dependency> - <dependency> <groupId>org.glassfish.jersey.media</groupId> <artifactId>jersey-media-json-jackson</artifactId> <exclusions> @@ -63,6 +55,21 @@ <groupId>com.fasterxml.jackson.datatype</groupId> <artifactId>jackson-datatype-jsr310</artifactId> </dependency> + + <dependency> + <!-- Previously pulled in by jersey-container-servlet-core. Contains packages imported by + jersey-entity-filtering, which is used by jersey-media-json-jackson, which is used by hosted Vespa + framework bundles, July 2021. --> + <groupId>org.glassfish.jersey.core</groupId> + <artifactId>jersey-server</artifactId> + </dependency> + + <dependency> + <!-- Previously pulled in by jersey-container-servlet-core. + Contains packages imported by hosted user applications, July 2021. --> + <groupId>javax.validation</groupId> + <artifactId>validation-api</artifactId> + </dependency> </dependencies> <build> diff --git a/vespalib/src/tests/datastore/array_store/array_store_test.cpp b/vespalib/src/tests/datastore/array_store/array_store_test.cpp index 417d8b80d87..562ecaaecfa 100644 --- a/vespalib/src/tests/datastore/array_store/array_store_test.cpp +++ b/vespalib/src/tests/datastore/array_store/array_store_test.cpp @@ -150,13 +150,13 @@ TEST("require that we test with trivial and non-trivial types") TEST_F("control static sizes", NumberFixture(3)) { #ifdef _LIBCPP_VERSION - EXPECT_EQUAL(400u, sizeof(f.store)); + EXPECT_EQUAL(424u, sizeof(f.store)); EXPECT_EQUAL(296u, sizeof(NumberFixture::ArrayStoreType::DataStoreType)); #else - EXPECT_EQUAL(432u, sizeof(f.store)); + EXPECT_EQUAL(456u, sizeof(f.store)); EXPECT_EQUAL(328u, sizeof(NumberFixture::ArrayStoreType::DataStoreType)); #endif - EXPECT_EQUAL(72u, sizeof(NumberFixture::ArrayStoreType::SmallArrayType)); + EXPECT_EQUAL(96u, sizeof(NumberFixture::ArrayStoreType::SmallArrayType)); MemoryUsage usage = f.store.getMemoryUsage(); EXPECT_EQUAL(960u, usage.allocatedBytes()); EXPECT_EQUAL(32u, usage.usedBytes()); diff --git a/vespalib/src/tests/datastore/buffer_type/buffer_type_test.cpp b/vespalib/src/tests/datastore/buffer_type/buffer_type_test.cpp index 414c35864ac..4cd192f602f 100644 --- a/vespalib/src/tests/datastore/buffer_type/buffer_type_test.cpp +++ b/vespalib/src/tests/datastore/buffer_type/buffer_type_test.cpp @@ -49,7 +49,7 @@ struct Fixture { } ~Fixture() { for (auto& setup : setups) { - bufferType.onHold(&setup._usedElems, &setup._deadElems); + bufferType.onHold(setup._bufferId, &setup._usedElems, &setup._deadElems); bufferType.onFree(setup._usedElems); } } @@ -134,9 +134,9 @@ TEST("arrays to alloc considers used elements across all active buffers of same { Fixture f(Setup().used(6 * 4)); f.assertArraysToAlloc(6 * 0.5); - f.add_setup(Setup().used(8 * 4)); + f.add_setup(Setup().used(8 * 4).bufferId(2)); f.assertArraysToAlloc((6 + 8) * 0.5); - f.add_setup(Setup().used(10 * 4)); + f.add_setup(Setup().used(10 * 4).bufferId(3)); f.assertArraysToAlloc((6 + 8 + 10) * 0.5); } @@ -144,7 +144,7 @@ TEST("arrays to alloc considers used elements across all active buffers of same { Fixture f(Setup().used(6 * 4)); f.assertArraysToAlloc(6 * 0.5); - f.add_setup(Setup().used(8 * 4).resizing(true)); + f.add_setup(Setup().used(8 * 4).resizing(true).bufferId(2)); f.assertArraysToAlloc(8 + (6 + 8) * 0.5); } @@ -152,9 +152,9 @@ TEST("arrays to alloc considers (and subtracts) dead elements across all active { Fixture f(Setup().used(6 * 4).dead(2 * 4)); f.assertArraysToAlloc((6 - 2) * 0.5); - f.add_setup(Setup().used(12 * 4).dead(4 * 4)); + f.add_setup(Setup().used(12 * 4).dead(4 * 4).bufferId(2)); f.assertArraysToAlloc((6 - 2 + 12 - 4) * 0.5); - f.add_setup(Setup().used(20 * 4).dead(6 * 4)); + f.add_setup(Setup().used(20 * 4).dead(6 * 4).bufferId(3)); f.assertArraysToAlloc((6 - 2 + 12 - 4 + 20 - 6) * 0.5); } @@ -162,7 +162,7 @@ TEST("arrays to alloc considers (and subtracts) dead elements across all active { Fixture f(Setup().used(6 * 4).dead(2 * 4)); f.assertArraysToAlloc((6 - 2) * 0.5); - f.add_setup(Setup().used(12 * 4).dead(4 * 4).resizing(true)); + f.add_setup(Setup().used(12 * 4).dead(4 * 4).resizing(true).bufferId(2)); f.assertArraysToAlloc(12 + (6 - 2 + 12 - 4) * 0.5); } diff --git a/vespalib/src/tests/datastore/datastore/datastore_test.cpp b/vespalib/src/tests/datastore/datastore/datastore_test.cpp index 548ab9199da..1c4817ea35f 100644 --- a/vespalib/src/tests/datastore/datastore/datastore_test.cpp +++ b/vespalib/src/tests/datastore/datastore/datastore_test.cpp @@ -55,6 +55,7 @@ public: using GrowthStats = std::vector<int>; +using BufferStats = std::vector<int>; constexpr float ALLOC_GROW_FACTOR = 0.4; constexpr size_t HUGE_PAGE_ARRAY_SIZE = (MemoryAllocator::HUGEPAGE_SIZE / sizeof(int)); @@ -123,6 +124,19 @@ public: ++i; } } + BufferStats getBuffers(size_t bufs) { + BufferStats buffers; + while (buffers.size() < bufs) { + RefType iRef = (_type.getArraySize() == 1) ? + (_store.template allocator<DataType>(_typeId).alloc().ref) : + (_store.template allocator<DataType>(_typeId).allocArray(_type.getArraySize()).ref); + int buffer_id = iRef.bufferId(); + if (buffers.empty() || buffers.back() != buffer_id) { + buffers.push_back(buffer_id); + } + } + return buffers; + } vespalib::MemoryUsage getMemoryUsage() const { return _store.getMemoryUsage(); } }; @@ -563,7 +577,7 @@ TEST(DataStoreTest, require_that_buffer_growth_works) assertGrowStats({ 4, 4, 4, 4, 8, 16, 16, 32, 64, 64 }, { 4 }, 20, 4, 0); // Resize if buffer size is less than 4, min size 0 - assertGrowStats({ 4, 4, 8, 32, 32, 32, 64, 128, 128, 128 }, + assertGrowStats({ 4, 4, 8, 32, 32, 64, 64, 128, 128, 128 }, { 0, 1, 2, 4 }, 4, 0, 4); // Always switch to new buffer, min size 16 assertGrowStats({ 16, 16, 16, 32, 32, 64, 128, 128, 128 }, @@ -580,7 +594,7 @@ TEST(DataStoreTest, require_that_buffer_growth_works) // Buffers with sizes larger than the huge page size of the mmap allocator. ASSERT_EQ(524288u, HUGE_PAGE_ARRAY_SIZE); - assertGrowStats({ 262144, 524288, 524288, 524288 * 3, 524288 * 3, 524288 * 4, 524288 * 5, 524288 * 5, 524288 * 5, 524288 * 5 }, + assertGrowStats({ 262144, 524288, 524288, 524288 * 3, 524288 * 3, 524288 * 5, 524288 * 5, 524288 * 5, 524288 * 5, 524288 * 5 }, { 0, 1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, 8192, 16384, 32768, 65536, 131072, 262144 }, 4, 0, HUGE_PAGE_ARRAY_SIZE / 2, HUGE_PAGE_ARRAY_SIZE * 5); } @@ -615,10 +629,10 @@ TEST(DataStoreTest, require_that_offset_in_EntryRefT_is_within_bounds_when_alloc * The max bytes to alloc is: maxArrays * arraySize * elementSize. */ assertGrowStats<uint8_t>({8192,16384,16384,65536,65536,98304,98304,98304,98304,98304,98304,98304}, 3); - assertGrowStats<uint8_t>({16384,16384,65536,65536,65536,131072,163840,163840,163840,163840,163840,163840}, 5); + assertGrowStats<uint8_t>({16384,16384,65536,65536,131072,131072,163840,163840,163840,163840,163840,163840}, 5); assertGrowStats<uint8_t>({16384,32768,32768,131072,131072,229376,229376,229376,229376,229376,229376,229376}, 7); assertGrowStats<uint32_t>({8192,16384,16384,65536,65536,98304,98304,98304,98304,98304,98304,98304}, 3); - assertGrowStats<uint32_t>({16384,16384,65536,65536,65536,131072,163840,163840,163840,163840,163840,163840}, 5); + assertGrowStats<uint32_t>({16384,16384,65536,65536,131072,131072,163840,163840,163840,163840,163840,163840}, 5); assertGrowStats<uint32_t>({16384,32768,32768,131072,131072,229376,229376,229376,229376,229376,229376,229376}, 7); } @@ -666,8 +680,24 @@ TEST(DataStoreTest, can_set_memory_allocator) EXPECT_EQ(AllocStats(3, 3), stats); } +namespace { + +void +assertBuffers(BufferStats exp_buffers, size_t num_arrays_for_new_buffer) +{ + EXPECT_EQ(exp_buffers, IntGrowStore(1, 1, 1024, num_arrays_for_new_buffer).getBuffers(exp_buffers.size())); +} + +} + +TEST(DataStoreTest, can_reuse_active_buffer_as_primary_buffer) +{ + assertBuffers({ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}, 0); + assertBuffers({ 1, 2, 3, 4, 1, 2, 3, 4, 1, 2, 3}, 16); +} + TEST(DataStoreTest, control_static_sizes) { - EXPECT_EQ(72, sizeof(BufferTypeBase)); + EXPECT_EQ(96, sizeof(BufferTypeBase)); EXPECT_EQ(32, sizeof(BufferState::FreeList)); EXPECT_EQ(1, sizeof(BufferState::State)); EXPECT_EQ(144, sizeof(BufferState)); diff --git a/vespalib/src/vespa/vespalib/datastore/buffer_type.cpp b/vespalib/src/vespa/vespalib/datastore/buffer_type.cpp index eb5865cd68c..068c58a163c 100644 --- a/vespalib/src/vespa/vespalib/datastore/buffer_type.cpp +++ b/vespalib/src/vespa/vespalib/datastore/buffer_type.cpp @@ -32,10 +32,10 @@ BufferTypeBase::BufferTypeBase(uint32_t arraySize, _maxArrays(maxArrays), _numArraysForNewBuffer(std::min(numArraysForNewBuffer, maxArrays)), _allocGrowFactor(allocGrowFactor), - _activeBuffers(0), _holdBuffers(0), _holdUsedElems(0), - _aggr_counts() + _aggr_counts(), + _active_buffers() { } @@ -48,10 +48,10 @@ BufferTypeBase::BufferTypeBase(uint32_t arraySize, BufferTypeBase::~BufferTypeBase() { - assert(_activeBuffers == 0); assert(_holdBuffers == 0); assert(_holdUsedElems == 0); assert(_aggr_counts.empty()); + assert(_active_buffers.empty()); } ElemCount @@ -63,8 +63,9 @@ BufferTypeBase::getReservedElements(uint32_t bufferId) const void BufferTypeBase::onActive(uint32_t bufferId, ElemCount* usedElems, ElemCount* deadElems, void* buffer) { - ++_activeBuffers; _aggr_counts.add_buffer(usedElems, deadElems); + assert(std::find(_active_buffers.begin(), _active_buffers.end(), bufferId) == _active_buffers.end()); + _active_buffers.emplace_back(bufferId); size_t reservedElems = getReservedElements(bufferId); if (reservedElems != 0u) { initializeReservedElements(buffer, reservedElems); @@ -74,10 +75,12 @@ BufferTypeBase::onActive(uint32_t bufferId, ElemCount* usedElems, ElemCount* dea } void -BufferTypeBase::onHold(const ElemCount* usedElems, const ElemCount* deadElems) +BufferTypeBase::onHold(uint32_t buffer_id, const ElemCount* usedElems, const ElemCount* deadElems) { - --_activeBuffers; ++_holdBuffers; + auto itr = std::find(_active_buffers.begin(), _active_buffers.end(), buffer_id); + assert(itr != _active_buffers.end()); + _active_buffers.erase(itr); _aggr_counts.remove_buffer(usedElems, deadElems); _holdUsedElems += *usedElems; } @@ -90,6 +93,17 @@ BufferTypeBase::onFree(ElemCount usedElems) _holdUsedElems -= usedElems; } +void +BufferTypeBase::resume_primary_buffer(uint32_t buffer_id, ElemCount* used_elems, ElemCount* dead_elems) +{ + auto itr = std::find(_active_buffers.begin(), _active_buffers.end(), buffer_id); + assert(itr != _active_buffers.end()); + _active_buffers.erase(itr); + _active_buffers.emplace_back(buffer_id); + _aggr_counts.remove_buffer(used_elems, dead_elems); + _aggr_counts.add_buffer(used_elems, dead_elems); +} + const alloc::MemoryAllocator* BufferTypeBase::get_memory_allocator() const { @@ -141,10 +155,11 @@ BufferTypeBase::calcArraysToAlloc(uint32_t bufferId, ElemCount elemsNeeded, bool uint32_t BufferTypeBase::get_scaled_num_arrays_for_new_buffer() const { - if (_activeBuffers <= 1u || _numArraysForNewBuffer == 0u) { + uint32_t active_buffers_count = get_active_buffers_count(); + if (active_buffers_count <= 1u || _numArraysForNewBuffer == 0u) { return _numArraysForNewBuffer; } - double scale_factor = std::pow(1.0 + _allocGrowFactor, _activeBuffers - 1); + double scale_factor = std::pow(1.0 + _allocGrowFactor, active_buffers_count - 1); double scaled_result = _numArraysForNewBuffer * scale_factor; if (scaled_result >= _maxArrays) { return _maxArrays; diff --git a/vespalib/src/vespa/vespalib/datastore/buffer_type.h b/vespalib/src/vespa/vespalib/datastore/buffer_type.h index 1f024743c8b..62d9282aae0 100644 --- a/vespalib/src/vespa/vespalib/datastore/buffer_type.h +++ b/vespalib/src/vespa/vespalib/datastore/buffer_type.h @@ -61,8 +61,9 @@ public: virtual void cleanHold(void *buffer, size_t offset, ElemCount numElems, CleanContext cleanCtx) = 0; size_t getArraySize() const { return _arraySize; } virtual void onActive(uint32_t bufferId, ElemCount* usedElems, ElemCount* deadElems, void* buffer); - void onHold(const ElemCount* usedElems, const ElemCount* deadElems); + void onHold(uint32_t buffer_id, const ElemCount* usedElems, const ElemCount* deadElems); virtual void onFree(ElemCount usedElems); + void resume_primary_buffer(uint32_t buffer_id, ElemCount* used_elems, ElemCount* dead_elems); virtual const alloc::MemoryAllocator* get_memory_allocator() const; /** @@ -72,9 +73,11 @@ public: void clampMaxArrays(uint32_t maxArrays); - uint32_t getActiveBuffers() const { return _activeBuffers; } + uint32_t get_active_buffers_count() const { return _active_buffers.size(); } + const std::vector<uint32_t>& get_active_buffers() const noexcept { return _active_buffers; } size_t getMaxArrays() const { return _maxArrays; } uint32_t get_scaled_num_arrays_for_new_buffer() const; + uint32_t get_num_arrays_for_new_buffer() const noexcept { return _numArraysForNewBuffer; } protected: struct BufferCounts { @@ -120,6 +123,7 @@ protected: uint32_t _holdBuffers; size_t _holdUsedElems; // Number of used elements in all held buffers for this type. AggregatedBufferCounts _aggr_counts; + std::vector<uint32_t> _active_buffers; }; /** diff --git a/vespalib/src/vespa/vespalib/datastore/bufferstate.cpp b/vespalib/src/vespa/vespalib/datastore/bufferstate.cpp index 68606c5ea19..704fabffe25 100644 --- a/vespalib/src/vespa/vespalib/datastore/bufferstate.cpp +++ b/vespalib/src/vespa/vespalib/datastore/bufferstate.cpp @@ -138,7 +138,7 @@ BufferState::onActive(uint32_t bufferId, uint32_t typeId, void -BufferState::onHold() +BufferState::onHold(uint32_t buffer_id) { assert(_state == ACTIVE); assert(_typeHandler != nullptr); @@ -148,7 +148,7 @@ BufferState::onHold() assert(_holdElems <= (_usedElems - _deadElems)); _deadElems = 0; _holdElems = _usedElems; // Put everyting on hold - _typeHandler->onHold(&_usedElems, &_deadElems); + _typeHandler->onHold(buffer_id, &_usedElems, &_deadElems); if ( ! isFreeListEmpty()) { removeFromFreeListList(); FreeList().swap(_freeList); @@ -191,7 +191,7 @@ BufferState::onFree(void *&buffer) void -BufferState::dropBuffer(void *&buffer) +BufferState::dropBuffer(uint32_t buffer_id, void *&buffer) { if (_state == FREE) { assert(buffer == nullptr); @@ -199,7 +199,7 @@ BufferState::dropBuffer(void *&buffer) } assert(buffer != nullptr || _allocElems == 0); if (_state == ACTIVE) { - onHold(); + onHold(buffer_id); } if (_state == HOLD) { onFree(buffer); @@ -301,5 +301,11 @@ BufferState::fallbackResize(uint32_t bufferId, std::atomic_thread_fence(std::memory_order_release); } +void +BufferState::resume_primary_buffer(uint32_t buffer_id) +{ + _typeHandler->resume_primary_buffer(buffer_id, &_usedElems, &_deadElems); +} + } diff --git a/vespalib/src/vespa/vespalib/datastore/bufferstate.h b/vespalib/src/vespa/vespalib/datastore/bufferstate.h index f8738f17daa..e95bef6f459 100644 --- a/vespalib/src/vespa/vespalib/datastore/bufferstate.h +++ b/vespalib/src/vespa/vespalib/datastore/bufferstate.h @@ -99,7 +99,7 @@ public: /** * Transition from ACTIVE to HOLD state. */ - void onHold(); + void onHold(uint32_t buffer_id); /** * Transition from HOLD to FREE state. @@ -157,7 +157,7 @@ public: void cleanHold(void *buffer, size_t offset, ElemCount numElems) { _typeHandler->cleanHold(buffer, offset, numElems, BufferTypeBase::CleanContext(_extraUsedBytes, _extraHoldBytes)); } - void dropBuffer(void *&buffer); + void dropBuffer(uint32_t buffer_id, void *&buffer); uint32_t getTypeId() const { return _typeId; } uint32_t getArraySize() const { return _arraySize; } size_t getDeadElems() const { return _deadElems; } @@ -192,6 +192,7 @@ public: FreeList &freeList() { return _freeList; } const FreeListList *freeListList() const { return _freeListList; } FreeListList *freeListList() { return _freeListList; } + void resume_primary_buffer(uint32_t buffer_id); }; diff --git a/vespalib/src/vespa/vespalib/datastore/datastorebase.cpp b/vespalib/src/vespa/vespalib/datastore/datastorebase.cpp index aff26fc7b84..8cc0edd98c0 100644 --- a/vespalib/src/vespa/vespalib/datastore/datastorebase.cpp +++ b/vespalib/src/vespa/vespalib/datastore/datastorebase.cpp @@ -120,6 +120,47 @@ DataStoreBase::switch_primary_buffer(uint32_t typeId, size_t elemsNeeded) _primary_buffer_ids[typeId] = buffer_id; } +bool +DataStoreBase::consider_grow_active_buffer(uint32_t type_id, size_t elems_needed) +{ + auto type_handler = _typeHandlers[type_id]; + uint32_t buffer_id = _primary_buffer_ids[type_id]; + uint32_t active_buffers_count = type_handler->get_active_buffers_count(); + constexpr uint32_t min_active_buffers = 4u; + if (active_buffers_count < min_active_buffers) { + return false; + } + if (type_handler->get_num_arrays_for_new_buffer() == 0u) { + return false; + } + assert(!_states[buffer_id].getCompacting()); + uint32_t min_buffer_id = buffer_id; + size_t min_used = _states[buffer_id].size(); + uint32_t checked_active_buffers = 1u; + for (auto &alt_buffer_id : type_handler->get_active_buffers()) { + if (alt_buffer_id != buffer_id && !_states[alt_buffer_id].getCompacting()) { + ++checked_active_buffers; + if (_states[alt_buffer_id].size() < min_used) { + min_buffer_id = alt_buffer_id; + min_used = _states[alt_buffer_id].size(); + } + } + } + if (checked_active_buffers < min_active_buffers) { + return false; + } + auto array_size = type_handler->getArraySize(); + if (elems_needed + min_used > type_handler->getMaxArrays() * array_size) { + return false; + } + if (min_buffer_id != buffer_id) { + // Resume another active buffer for same type as primary buffer + _primary_buffer_ids[type_id] = min_buffer_id; + _states[min_buffer_id].resume_primary_buffer(min_buffer_id); + } + return true; +} + void DataStoreBase::switch_or_grow_primary_buffer(uint32_t typeId, size_t elemsNeeded) { @@ -129,8 +170,14 @@ DataStoreBase::switch_or_grow_primary_buffer(uint32_t typeId, size_t elemsNeeded size_t numEntriesForNewBuffer = numArraysForNewBuffer * arraySize; uint32_t bufferId = _primary_buffer_ids[typeId]; if (elemsNeeded + _states[bufferId].size() >= numEntriesForNewBuffer) { - // Don't try to resize existing buffer, new buffer will be large enough - switch_primary_buffer(typeId, elemsNeeded); + if (consider_grow_active_buffer(typeId, elemsNeeded)) { + bufferId = _primary_buffer_ids[typeId]; + if (elemsNeeded > _states[bufferId].remaining()) { + fallbackResize(bufferId, elemsNeeded); + } + } else { + switch_primary_buffer(typeId, elemsNeeded); + } } else { fallbackResize(bufferId, elemsNeeded); } @@ -214,7 +261,7 @@ DataStoreBase::dropBuffers() { uint32_t numBuffers = _buffers.size(); for (uint32_t bufferId = 0; bufferId < numBuffers; ++bufferId) { - _states[bufferId].dropBuffer(_buffers[bufferId].getBuffer()); + _states[bufferId].dropBuffer(bufferId, _buffers[bufferId].getBuffer()); } _genHolder.clearHoldLists(); } @@ -234,7 +281,7 @@ DataStoreBase::getMemoryUsage() const void DataStoreBase::holdBuffer(uint32_t bufferId) { - _states[bufferId].onHold(); + _states[bufferId].onHold(bufferId); size_t holdBytes = 0u; // getMemStats() still accounts held buffers GenerationHeldBase::UP hold(new BufferHold(holdBytes, *this, bufferId)); _genHolder.hold(std::move(hold)); @@ -398,6 +445,7 @@ void DataStoreBase::finishCompact(const std::vector<uint32_t> &toHold) { for (uint32_t bufferId : toHold) { + assert(_states[bufferId].getCompacting()); holdBuffer(bufferId); } } @@ -429,13 +477,10 @@ DataStoreBase::startCompactWorstBuffer(uint32_t typeId) { uint32_t buffer_id = get_primary_buffer_id(typeId); const BufferTypeBase *typeHandler = _typeHandlers[typeId]; - assert(typeHandler->getActiveBuffers() >= 1u); - if (typeHandler->getActiveBuffers() == 1u) { + assert(typeHandler->get_active_buffers_count() >= 1u); + if (typeHandler->get_active_buffers_count() == 1u) { // Single active buffer for type, no need for scan - _states[buffer_id].setCompacting(); - _states[buffer_id].disableElemHoldList(); - disableFreeList(buffer_id); - switch_primary_buffer(typeId, 0u); + markCompacting(buffer_id); return buffer_id; } // Multiple active buffers for type, must perform full scan @@ -452,6 +497,7 @@ DataStoreBase::startCompactWorstBuffer(uint32_t initWorstBufferId, BufferStateAc for (uint32_t bufferId = 0; bufferId < _numBuffers; ++bufferId) { const auto &state = getBufferState(bufferId); if (filterFunc(state)) { + assert(!state.getCompacting()); size_t deadElems = state.getDeadElems() - state.getTypeHandler()->getReservedElements(bufferId); if (deadElems > worstDeadElems) { worstBufferId = bufferId; @@ -472,6 +518,7 @@ DataStoreBase::markCompacting(uint32_t bufferId) if ((bufferId == buffer_id) || primary_buffer_too_dead(getBufferState(buffer_id))) { switch_primary_buffer(typeId, 0u); } + assert(!state.getCompacting()); state.setCompacting(); state.disableElemHoldList(); state.setFreeListList(nullptr); diff --git a/vespalib/src/vespa/vespalib/datastore/datastorebase.h b/vespalib/src/vespa/vespalib/datastore/datastorebase.h index 5a50dba6a3c..97ead502408 100644 --- a/vespalib/src/vespa/vespalib/datastore/datastorebase.h +++ b/vespalib/src/vespa/vespalib/datastore/datastorebase.h @@ -232,6 +232,7 @@ public: */ void switch_primary_buffer(uint32_t typeId, size_t elemsNeeded); + bool consider_grow_active_buffer(uint32_t type_id, size_t elems_needed); void switch_or_grow_primary_buffer(uint32_t typeId, size_t elemsNeeded); vespalib::MemoryUsage getMemoryUsage() const; diff --git a/vespalib/src/vespa/vespalib/datastore/entryref.cpp b/vespalib/src/vespa/vespalib/datastore/entryref.cpp index 0551e617283..83501e49ed3 100644 --- a/vespalib/src/vespa/vespalib/datastore/entryref.cpp +++ b/vespalib/src/vespa/vespalib/datastore/entryref.cpp @@ -8,6 +8,7 @@ namespace vespalib::datastore { template EntryRefT<24u, 8u>::EntryRefT(size_t, uint32_t); template EntryRefT<31u, 1u>::EntryRefT(size_t, uint32_t); template EntryRefT<22u,10u>::EntryRefT(size_t, uint32_t); +template EntryRefT<20u,12u>::EntryRefT(size_t, uint32_t); template EntryRefT<19u,13u>::EntryRefT(size_t, uint32_t); template EntryRefT<18u, 6u>::EntryRefT(size_t, uint32_t); template EntryRefT<15u,17u>::EntryRefT(size_t, uint32_t); diff --git a/vespalib/src/vespa/vespalib/stllike/allocator.h b/vespalib/src/vespa/vespalib/stllike/allocator.h index 0a890973e09..70d8f435286 100644 --- a/vespalib/src/vespa/vespalib/stllike/allocator.h +++ b/vespalib/src/vespa/vespalib/stllike/allocator.h @@ -13,7 +13,6 @@ namespace vespalib { */ template <typename T> class allocator_large { - using PtrAndSize = alloc::MemoryAllocator::PtrAndSize; public: allocator_large() noexcept : _allocator(alloc::MemoryAllocator::select_allocator()) {} using value_type = T; |