diff options
22 files changed, 281 insertions, 191 deletions
diff --git a/config-model-api/abi-spec.json b/config-model-api/abi-spec.json index 2c5be906633..b28401f1873 100644 --- a/config-model-api/abi-spec.json +++ b/config-model-api/abi-spec.json @@ -1289,7 +1289,8 @@ "public boolean useReconfigurableDispatcher()", "public int contentLayerMetadataFeatureLevel()", "public boolean dynamicHeapSize()", - "public java.lang.String unknownConfigDefinition()" + "public java.lang.String unknownConfigDefinition()", + "public int searchHandlerThreadpool()" ], "fields" : [ ] }, 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 57d013ebd01..024a4c233e5 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,6 +120,7 @@ public interface ModelContext { @ModelFeatureFlag(owners = {"vekterli"}) default int contentLayerMetadataFeatureLevel() { return 0; } @ModelFeatureFlag(owners = {"bjorncs"}) default boolean dynamicHeapSize() { return false; } @ModelFeatureFlag(owners = {"hmusum"}) default String unknownConfigDefinition() { return "log"; } + @ModelFeatureFlag(owners = {"hmusum"}) default int searchHandlerThreadpool() { return 2; } } /** Warning: As elsewhere in this package, do not make backwards incompatible changes that will break old config models! */ diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/SearchHandler.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/SearchHandler.java index 3cd296c1469..7bdd2ce51a4 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/SearchHandler.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/SearchHandler.java @@ -49,16 +49,19 @@ class SearchHandler extends ProcessingHandler<SearchChains> { private static class Threadpool extends ContainerThreadpool { + private final int threads; + Threadpool(DeployState ds, Element options) { super(ds, "search-handler", options); + threads = ds.featureFlags().searchHandlerThreadpool(); } @Override public void setDefaultConfigValues(ContainerThreadpoolConfig.Builder builder) { builder.maxThreadExecutionTimeSeconds(190) .keepAliveTime(5.0) - .maxThreads(-2) - .minThreads(-2) + .maxThreads(-threads) + .minThreads(-threads) .queueSize(-40); } 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 96b0b03c832..029158056b8 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 @@ -29,7 +29,6 @@ import com.yahoo.config.provision.Zone; import com.yahoo.container.jdisc.secretstore.SecretStore; import com.yahoo.vespa.config.server.tenant.SecretStoreExternalIdRetriever; import com.yahoo.vespa.flags.FetchVector; -import com.yahoo.vespa.flags.Flag; import com.yahoo.vespa.flags.FlagSource; import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.flags.PermanentFlags; @@ -210,6 +209,7 @@ public class ModelContextImpl implements ModelContext { private final int contentLayerMetadataFeatureLevel; private final boolean dynamicHeapSize; private final String unknownConfigDefinition; + private final int searchHandlerThreadpool; public FeatureFlags(FlagSource source, ApplicationId appId, Version version) { this.defaultTermwiseLimit = flagValue(source, appId, version, Flags.DEFAULT_TERM_WISE_LIMIT); @@ -254,6 +254,7 @@ public class ModelContextImpl implements ModelContext { this.contentLayerMetadataFeatureLevel = flagValue(source, appId, version, Flags.CONTENT_LAYER_METADATA_FEATURE_LEVEL); this.dynamicHeapSize = flagValue(source, appId, version, Flags.DYNAMIC_HEAP_SIZE); this.unknownConfigDefinition = flagValue(source, appId, version, Flags.UNKNOWN_CONFIG_DEFINITION); + this.searchHandlerThreadpool = flagValue(source, appId, version, Flags.SEARCH_HANDLER_THREADPOOL); } @Override public int heapSizePercentage() { return heapPercentage; } @@ -306,6 +307,7 @@ public class ModelContextImpl implements ModelContext { @Override public int contentLayerMetadataFeatureLevel() { return contentLayerMetadataFeatureLevel; } @Override public boolean dynamicHeapSize() { return dynamicHeapSize; } @Override public String unknownConfigDefinition() { return unknownConfigDefinition; } + @Override public int searchHandlerThreadpool() { return searchHandlerThreadpool; } private static <V> V flagValue(FlagSource source, ApplicationId appId, Version vespaVersion, UnboundFlag<? extends V, ?, ?> flag) { return flag.bindTo(source) diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificate.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificate.java index 53d807b0139..6f056edd226 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificate.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificate.java @@ -13,9 +13,9 @@ public record EndpointCertificate(String keyName, String certName, int version, String rootRequestId, // The id of the first request made for this certificate. Should not change. Optional<String> leafRequestId, // The id of the last known request made for this certificate. Changes on refresh, may be outdated! List<String> requestedDnsSans, String issuer, Optional<Long> expiry, - Optional<Long> lastRefreshed, Optional<String> randomizedId) { + Optional<Long> lastRefreshed, Optional<String> generatedId) { - public EndpointCertificate withRandomizedId(String randomizedId) { + public EndpointCertificate withGeneratedId(String generatedId) { return new EndpointCertificate( this.keyName, this.certName, @@ -27,7 +27,7 @@ public record EndpointCertificate(String keyName, String certName, int version, this.issuer, this.expiry, this.lastRefreshed, - Optional.of(randomizedId)); + Optional.of(generatedId)); } public EndpointCertificate withKeyName(String keyName) { @@ -42,7 +42,7 @@ public record EndpointCertificate(String keyName, String certName, int version, this.issuer, this.expiry, this.lastRefreshed, - this.randomizedId); + this.generatedId); } public EndpointCertificate withCertName(String certName) { @@ -57,7 +57,7 @@ public record EndpointCertificate(String keyName, String certName, int version, this.issuer, this.expiry, this.lastRefreshed, - this.randomizedId); + this.generatedId); } public EndpointCertificate withVersion(int version) { @@ -72,7 +72,7 @@ public record EndpointCertificate(String keyName, String certName, int version, this.issuer, this.expiry, this.lastRefreshed, - this.randomizedId); + this.generatedId); } public EndpointCertificate withLastRequested(long lastRequested) { @@ -87,7 +87,7 @@ public record EndpointCertificate(String keyName, String certName, int version, this.issuer, this.expiry, this.lastRefreshed, - this.randomizedId); + this.generatedId); } public EndpointCertificate withLastRefreshed(long lastRefreshed) { @@ -102,7 +102,7 @@ public record EndpointCertificate(String keyName, String certName, int version, this.issuer, this.expiry, Optional.of(lastRefreshed), - this.randomizedId); + this.generatedId); } public EndpointCertificate withRootRequestId(String rootRequestId) { @@ -117,7 +117,7 @@ public record EndpointCertificate(String keyName, String certName, int version, this.issuer, this.expiry, this.lastRefreshed, - this.randomizedId); + this.generatedId); } public EndpointCertificate withLeafRequestId(Optional<String> leafRequestId) { @@ -132,7 +132,7 @@ public record EndpointCertificate(String keyName, String certName, int version, this.issuer, this.expiry, this.lastRefreshed, - this.randomizedId); + this.generatedId); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java index 90c4a506f10..b763af1af9d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java @@ -51,7 +51,6 @@ import com.yahoo.vespa.hosted.rotation.config.RotationsConfig; import java.nio.charset.StandardCharsets; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Comparator; @@ -135,7 +134,7 @@ public class RoutingController { } // Add zone-scoped endpoints - Map<EndpointId, GeneratedEndpointList> generatedForDeclaredEndpoints = new HashMap<>(); + Map<EndpointId, List<GeneratedEndpoint>> generatedForDeclaredEndpoints = new HashMap<>(); Set<ClusterSpec.Id> clustersWithToken = new HashSet<>(); boolean generatedEndpointsEnabled = generatedEndpointsEnabled(deployment.applicationId()); RoutingPolicyList applicationPolicies = policies().read(TenantAndApplicationId.from(deployment.applicationId())); @@ -149,9 +148,10 @@ public class RoutingController { Optional<RoutingPolicy> clusterPolicy = deploymentPolicies.cluster(clusterId).first(); List<GeneratedEndpoint> generatedForCluster = clusterPolicy.map(policy -> policy.generatedEndpoints().cluster().asList()) .orElseGet(List::of); - // Generate endpoints if cluster does not have any - if (generatedForCluster.isEmpty()) { - generatedForCluster = generateEndpoints(tokenSupported, certificate, Optional.empty()); + // Generate endpoint for each auth method, if not present + generatedForCluster = generateEndpoints(AuthMethod.mtls, certificate, Optional.empty(), generatedForCluster); + if (tokenSupported) { + generatedForCluster = generateEndpoints(AuthMethod.token, certificate, Optional.empty(), generatedForCluster); } GeneratedEndpointList generatedEndpoints = generatedEndpointsEnabled ? GeneratedEndpointList.copyOf(generatedForCluster) : GeneratedEndpointList.EMPTY; endpoints = endpoints.and(endpointsOf(deployment, clusterId, generatedEndpoints).scope(Scope.zone)); @@ -162,18 +162,34 @@ public class RoutingController { ClusterSpec.Id clusterId = ClusterSpec.Id.from(container.id()); applicationPolicies.cluster(clusterId).asList().stream() .flatMap(policy -> policy.generatedEndpoints().declared().asList().stream()) - .forEach(ge -> generatedForDeclaredEndpoints.computeIfAbsent(ge.endpoint().get(), (k) -> GeneratedEndpointList.of(ge))); + .forEach(ge -> { + List<GeneratedEndpoint> generated = generatedForDeclaredEndpoints.computeIfAbsent(ge.endpoint().get(), (k) -> new ArrayList<>()); + if (!generated.contains(ge)) { + generated.add(ge); + } + }); } // Generate endpoints if declared endpoint does not have any Stream.concat(spec.endpoints().stream(), spec.instances().stream().flatMap(i -> i.endpoints().stream())) .forEach(endpoint -> { EndpointId endpointId = EndpointId.of(endpoint.endpointId()); - generatedForDeclaredEndpoints.computeIfAbsent(endpointId, (k) -> { + generatedForDeclaredEndpoints.compute(endpointId, (k, old) -> { + if (old == null) { + old = List.of(); + } + List<GeneratedEndpoint> generatedEndpoints = generateEndpoints(AuthMethod.mtls, certificate, Optional.of(endpointId), old); boolean tokenSupported = clustersWithToken.contains(ClusterSpec.Id.from(endpoint.containerId())); - return generatedEndpointsEnabled ? GeneratedEndpointList.copyOf(generateEndpoints(tokenSupported, certificate, Optional.of(endpointId))) : null; + if (tokenSupported){ + generatedEndpoints = generateEndpoints(AuthMethod.token, certificate, Optional.of(endpointId), generatedEndpoints); + } + return generatedEndpoints; }); }); - Map<EndpointId, GeneratedEndpointList> generatedEndpoints = generatedEndpointsEnabled ? generatedForDeclaredEndpoints : Map.of(); + Map<EndpointId, GeneratedEndpointList> generatedEndpoints = generatedEndpointsEnabled + ? generatedForDeclaredEndpoints.entrySet() + .stream() + .collect(Collectors.toMap(Map.Entry::getKey, kv -> GeneratedEndpointList.copyOf(kv.getValue()))) + : Map.of(); endpoints = endpoints.and(declaredEndpointsOf(application.get().id(), spec, generatedEndpoints).targets(deployment)); PreparedEndpoints prepared = new PreparedEndpoints(deployment, endpoints, @@ -186,12 +202,6 @@ public class RoutingController { return prepared; } - private List<GeneratedEndpoint> generateEndpoints(boolean tokenSupported, Optional<EndpointCertificate> certificate, Optional<EndpointId> endpoint) { - return certificate.flatMap(EndpointCertificate::randomizedId) - .map(id -> generateEndpoints(id, tokenSupported, endpoint)) - .orElseGet(List::of); - } - // -------------- Implicit endpoints (scopes 'zone' and 'weighted') -------------- /** Returns the zone- and region-scoped endpoints of given deployment */ @@ -480,19 +490,22 @@ public class RoutingController { } } - /** Generate endpoints for all authentication methods, using given application part */ - private List<GeneratedEndpoint> generateEndpoints(String applicationPart, boolean token, Optional<EndpointId> endpoint) { - return Arrays.stream(AuthMethod.values()) - .filter(method -> switch (method) { - case token -> token; - case mtls -> true; - case none -> false; - }) - .map(method -> new GeneratedEndpoint(GeneratedEndpoint.createPart(controller.random(true)), - applicationPart, - method, - endpoint)) - .toList(); + /** Returns generated endpoints. A new endpoint is generated if no matching endpoint already exists */ + private List<GeneratedEndpoint> generateEndpoints(AuthMethod authMethod, Optional<EndpointCertificate> certificate, + Optional<EndpointId> declaredEndpoint, + List<GeneratedEndpoint> current) { + if (current.stream().anyMatch(e -> e.authMethod() == authMethod && e.endpoint().equals(declaredEndpoint))) { + return current; + } + Optional<String> applicationPart = certificate.flatMap(EndpointCertificate::generatedId); + if (applicationPart.isPresent()) { + current = new ArrayList<>(current); + current.add(new GeneratedEndpoint(GeneratedEndpoint.createPart(controller.random(true)), + applicationPart.get(), + authMethod, + declaredEndpoint)); + } + return current; } /** Generate the cluster part of a {@link GeneratedEndpoint} for use in a {@link Endpoint.Scope#weighted} endpoint */ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/BasicServicesXml.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/BasicServicesXml.java index 8f3a02528fe..6a634cb8f53 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/BasicServicesXml.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/BasicServicesXml.java @@ -9,7 +9,6 @@ import org.w3c.dom.Element; import java.util.ArrayList; import java.util.List; import java.util.Objects; -import java.util.TreeSet; /** * A partially parsed variant of services.xml, for use by the {@link com.yahoo.vespa.hosted.controller.Controller}. @@ -40,7 +39,9 @@ public record BasicServicesXml(List<Container> containers) { for (var childNode : XML.getChildren(root)) { if (childNode.getTagName().equals(CONTAINER_TAG)) { String id = childNode.getAttribute("id"); - if (id.isEmpty()) throw new IllegalArgumentException(CONTAINER_TAG + " tag requires 'id' attribute"); + if (id.isEmpty()) { + id = CONTAINER_TAG; // ID defaults to tag name when unset. See ConfigModelBuilder::getIdString + } List<Container.AuthMethod> methods = new ArrayList<>(); List<TokenId> tokens = new ArrayList<>(); parseAuthMethods(childNode, methods, tokens); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificates.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificates.java index b5e012253c7..ec2ef4b7ff8 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificates.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificates.java @@ -119,11 +119,11 @@ public class EndpointCertificates { TenantAndApplicationId application = TenantAndApplicationId.from(instance.id()); Optional<AssignedCertificate> perInstanceAssignedCertificate = curator.readAssignedCertificate(application, Optional.of(instance.name())); - if (perInstanceAssignedCertificate.isPresent() && perInstanceAssignedCertificate.get().certificate().randomizedId().isPresent()) { + if (perInstanceAssignedCertificate.isPresent() && perInstanceAssignedCertificate.get().certificate().generatedId().isPresent()) { return updateLastRequested(perInstanceAssignedCertificate.get()).certificate(); } else if (! zone.environment().isManuallyDeployed()) { Optional<AssignedCertificate> perApplicationAssignedCertificate = curator.readAssignedCertificate(application, Optional.empty()); - if (perApplicationAssignedCertificate.isPresent() && perApplicationAssignedCertificate.get().certificate().randomizedId().isPresent()) { + if (perApplicationAssignedCertificate.isPresent() && perApplicationAssignedCertificate.get().certificate().generatedId().isPresent()) { return updateLastRequested(perApplicationAssignedCertificate.get()).certificate(); } } @@ -181,7 +181,7 @@ public class EndpointCertificates { boolean legacyNames = assignLegacyNames.with(FetchVector.Dimension.INSTANCE_ID, instance.id().serializedForm()) .with(FetchVector.Dimension.APPLICATION_ID, instance.id().toSerializedFormWithoutInstance()).value(); - var requiredSansForZone = legacyNames || currentCertificate.get().randomizedId().isEmpty() ? + var requiredSansForZone = legacyNames || currentCertificate.get().generatedId().isEmpty() ? controller.routing().certificateDnsNames(deployment, deploymentSpec) : List.<String>of(); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/UnassignedCertificate.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/UnassignedCertificate.java index 3a8580b7eb5..1566949664b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/UnassignedCertificate.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/UnassignedCertificate.java @@ -14,13 +14,13 @@ import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCe public record UnassignedCertificate(EndpointCertificate certificate, UnassignedCertificate.State state) { public UnassignedCertificate { - if (certificate.randomizedId().isEmpty()) { - throw new IllegalArgumentException("randomizedId must be set for a pooled certificate"); + if (certificate.generatedId().isEmpty()) { + throw new IllegalArgumentException("generatedId must be set for a pooled certificate"); } } public String id() { - return certificate.randomizedId().get(); + return certificate.generatedId().get(); } public UnassignedCertificate withState(State state) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CertificatePoolMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CertificatePoolMaintainer.java index ed383175cc3..a07b6e14625 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CertificatePoolMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/CertificatePoolMaintainer.java @@ -106,7 +106,7 @@ public class CertificatePoolMaintainer extends ControllerMaintainer { curator.readAssignedCertificates().stream() .map(AssignedCertificate::certificate) - .map(EndpointCertificate::randomizedId) + .map(EndpointCertificate::generatedId) .forEach(id -> id.ifPresent(existingNames::add)); String id = generateRandomId(); @@ -122,7 +122,7 @@ public class CertificatePoolMaintainer extends ControllerMaintainer { Optional.empty(), endpointCertificateAlgo.value(), useAlternateCertProvider.value()) - .withRandomizedId(id); + .withGeneratedId(id); UnassignedCertificate certificate = new UnassignedCertificate(f, UnassignedCertificate.State.requested); curator.writeUnassignedCertificate(certificate); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java index 805bf3d7ada..f4936dcfa8b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java @@ -11,7 +11,6 @@ import com.yahoo.container.jdisc.secretstore.SecretStore; import com.yahoo.transaction.Mutex; import com.yahoo.transaction.NestedTransaction; import com.yahoo.vespa.flags.BooleanFlag; -import com.yahoo.vespa.flags.FetchVector; import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.flags.IntFlag; import com.yahoo.vespa.flags.PermanentFlags; @@ -280,7 +279,7 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { */ assignedCertificates.stream() .filter(c -> c.instance().isPresent()) - .filter(c -> c.certificate().randomizedId().isEmpty()) + .filter(c -> c.certificate().generatedId().isEmpty()) .filter(c -> controller().applications().getApplication(c.application()).isPresent()) // In case application has been deleted, but certificate is pending deletion .limit(assignRandomizedIdRate.value()) .forEach(c -> assignRandomizedId(c.application(), c.instance().get())); @@ -300,7 +299,7 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { log.log(Level.INFO, "Assigned certificate missing for " + tenantAndApplicationId.instance(instanceName).toFullString() + " when assigning randomized id"); } // Verify that the assigned certificate still does not have randomized id assigned - if (assignedCertificate.get().certificate().randomizedId().isPresent()) return; + if (assignedCertificate.get().certificate().generatedId().isPresent()) return; controller().applications().lockApplicationOrThrow(tenantAndApplicationId, application -> { DeploymentSpec deploymentSpec = application.get().deploymentSpec(); @@ -320,7 +319,7 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { EndpointCertificate withRandomNames = requestRandomNames( tenantAndApplicationId, instanceLevelAssignedCertificate.instance(), - applicationLevelAssignedCertificate.get().certificate().randomizedId() + applicationLevelAssignedCertificate.get().certificate().generatedId() .orElseThrow(() -> new IllegalArgumentException("Application certificate already assigned to " + tenantAndApplicationId.toString() + ", but random id is missing")), Optional.of(instanceLevelAssignedCertificate.certificate())); AssignedCertificate assignedCertWithRandomNames = instanceLevelAssignedCertificate.with(withRandomNames); @@ -365,12 +364,12 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { previousRequest, endpointCertificateAlgo.value(), useAlternateCertProvider.value()) - .withRandomizedId(randomId); + .withGeneratedId(randomId); } private String generateRandomId() { List<String> unassignedIds = curator.readUnassignedCertificates().stream().map(UnassignedCertificate::id).toList(); - List<String> assignedIds = curator.readAssignedCertificates().stream().map(AssignedCertificate::certificate).map(EndpointCertificate::randomizedId).filter(Optional::isPresent).map(Optional::get).toList(); + List<String> assignedIds = curator.readAssignedCertificates().stream().map(AssignedCertificate::certificate).map(EndpointCertificate::generatedId).filter(Optional::isPresent).map(Optional::get).toList(); Set<String> allIds = Stream.concat(unassignedIds.stream(), assignedIds.stream()).collect(Collectors.toSet()); String randomId; do { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/EndpointCertificateSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/EndpointCertificateSerializer.java index fae9ea1e0e3..2ff4f1fb194 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/EndpointCertificateSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/EndpointCertificateSerializer.java @@ -35,7 +35,7 @@ public class EndpointCertificateSerializer { private final static String issuerField = "issuer"; private final static String expiryField = "expiry"; private final static String lastRefreshedField = "lastRefreshed"; - private final static String randomizedIdField = "randomizedId"; + private final static String generatedIdField = "randomizedId"; public static Slime toSlime(EndpointCertificate cert) { Slime slime = new Slime(); @@ -56,7 +56,7 @@ public class EndpointCertificateSerializer { object.setString(issuerField, cert.issuer()); cert.expiry().ifPresent(expiry -> object.setLong(expiryField, expiry)); cert.lastRefreshed().ifPresent(refreshTime -> object.setLong(lastRefreshedField, refreshTime)); - cert.randomizedId().ifPresent(randomizedId -> object.setString(randomizedIdField, randomizedId)); + cert.generatedId().ifPresent(id -> object.setString(generatedIdField, id)); } public static EndpointCertificate fromSlime(Inspector inspector) { @@ -79,8 +79,8 @@ public class EndpointCertificateSerializer { inspector.field(lastRefreshedField).valid() ? Optional.of(inspector.field(lastRefreshedField).asLong()) : Optional.empty(), - inspector.field(randomizedIdField).valid() ? - Optional.of(inspector.field(randomizedIdField).asString()) : + inspector.field(generatedIdField).valid() ? + Optional.of(inspector.field(generatedIdField).asString()) : Optional.empty()); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/BasicServicesXmlTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/BasicServicesXmlTest.java index c28185466b0..f7bd1e06e68 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/BasicServicesXmlTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/BasicServicesXmlTest.java @@ -18,11 +18,13 @@ class BasicServicesXmlTest { public void parse() { assertServices(new BasicServicesXml(List.of()), "<services/>"); assertServices(new BasicServicesXml(List.of(new Container("foo", List.of(Container.AuthMethod.mtls), List.of()), - new Container("bar", List.of(Container.AuthMethod.mtls), List.of()))), + new Container("bar", List.of(Container.AuthMethod.mtls), List.of()), + new Container("container", List.of(Container.AuthMethod.mtls), List.of()))), """ <services> <container id="foo"/> <container id="bar"/> + <container/> </services> """); assertServices(new BasicServicesXml(List.of( diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesTest.java index a6d3b435dcb..2bc11adddf7 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesTest.java @@ -28,7 +28,6 @@ import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.integration.SecretStoreMock; import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock; -import com.yahoo.vespa.hosted.controller.maintenance.EndpointCertificateMaintainer; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -296,7 +295,7 @@ public class EndpointCertificatesTest { // Initial certificate is requested directly from provider Optional<EndpointCertificate> certFromProvider = endpointCertificates.get(instance, prodZone, DeploymentSpec.empty); assertTrue(certFromProvider.isPresent()); - assertFalse(certFromProvider.get().randomizedId().isPresent()); + assertFalse(certFromProvider.get().generatedId().isPresent()); // Pooled certificates become available tester.flagSource().withBooleanFlag(Flags.RANDOMIZED_ENDPOINT_NAMES.id(), true); @@ -315,8 +314,8 @@ public class EndpointCertificatesTest { String certId = "pool-cert-1"; addCertificateToPool(certId, UnassignedCertificate.State.ready); Optional<EndpointCertificate> cert = endpointCertificates.get(instance, prodZone, DeploymentSpec.empty); - assertEquals(certId, cert.get().randomizedId().get()); - assertEquals(certId, tester.curator().readAssignedCertificate(TenantAndApplicationId.from(instance.id()), Optional.empty()).get().certificate().randomizedId().get(), "Certificate is assigned at application-level"); + assertEquals(certId, cert.get().generatedId().get()); + assertEquals(certId, tester.curator().readAssignedCertificate(TenantAndApplicationId.from(instance.id()), Optional.empty()).get().certificate().generatedId().get(), "Certificate is assigned at application-level"); assertTrue(tester.controller().curator().readUnassignedCertificate(certId).isEmpty(), "Certificate is removed from pool"); assertEquals(clock.instant().getEpochSecond(), cert.get().lastRequested()); } @@ -326,8 +325,8 @@ public class EndpointCertificatesTest { addCertificateToPool(certId, UnassignedCertificate.State.ready); ZoneId devZone = tester.zoneRegistry().zones().all().routingMethod(RoutingMethod.exclusive).in(Environment.dev).zones().stream().findFirst().orElseThrow().getId(); Optional<EndpointCertificate> cert = endpointCertificates.get(instance, devZone, DeploymentSpec.empty); - assertEquals(certId, cert.get().randomizedId().get()); - assertEquals(certId, tester.curator().readAssignedCertificate(instance.id()).get().certificate().randomizedId().get(), "Certificate is assigned at instance-level"); + assertEquals(certId, cert.get().generatedId().get()); + assertEquals(certId, tester.curator().readAssignedCertificate(instance.id()).get().certificate().generatedId().get(), "Certificate is assigned at instance-level"); assertTrue(tester.controller().curator().readUnassignedCertificate(certId).isEmpty(), "Certificate is removed from pool"); assertEquals(clock.instant().getEpochSecond(), cert.get().lastRequested()); } @@ -338,7 +337,7 @@ public class EndpointCertificatesTest { // Initial certificate is requested directly from provider Optional<EndpointCertificate> certFromProvider = endpointCertificates.get(instance, prodZone, DeploymentSpec.empty); assertTrue(certFromProvider.isPresent()); - assertFalse(certFromProvider.get().randomizedId().isPresent()); + assertFalse(certFromProvider.get().generatedId().isPresent()); // Simulate endpoint certificate maintainer to assign random id TenantAndApplicationId tenantAndApplicationId = TenantAndApplicationId.from(instance.id()); @@ -346,7 +345,7 @@ public class EndpointCertificatesTest { Optional<AssignedCertificate> assignedCertificate = tester.controller().curator().readAssignedCertificate(tenantAndApplicationId, instanceName); assertTrue(assignedCertificate.isPresent()); String assignedRandomId = "randomid"; - AssignedCertificate updated = assignedCertificate.get().with(assignedCertificate.get().certificate().withRandomizedId(assignedRandomId)); + AssignedCertificate updated = assignedCertificate.get().with(assignedCertificate.get().certificate().withGeneratedId(assignedRandomId)); tester.controller().curator().writeAssignedCertificate(updated); // Pooled certificates become available @@ -358,12 +357,12 @@ public class EndpointCertificatesTest { // Request cert for app Optional<EndpointCertificate> cert = endpointCertificates.get(instance, prodZone, DeploymentSpec.empty); - assertEquals(assignedRandomId, cert.get().randomizedId().get()); + assertEquals(assignedRandomId, cert.get().generatedId().get()); // Pooled cert remains unassigned List<String> unassignedCertificateIds = tester.curator().readUnassignedCertificates().stream() .map(UnassignedCertificate::certificate) - .map(EndpointCertificate::randomizedId) + .map(EndpointCertificate::generatedId) .map(Optional::get) .toList(); assertEquals(List.of(certId), unassignedCertificateIds); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainerTest.java index 647c809231e..2f996bac897 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainerTest.java @@ -41,7 +41,6 @@ import java.util.stream.Stream; import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.devUsEast1; import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.perfUsEast3; import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.productionUsCentral1; -import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.productionUsEast3; import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.productionUsWest1; import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.stagingTest; import static com.yahoo.vespa.hosted.controller.deployment.DeploymentContext.systemTest; @@ -138,7 +137,7 @@ public class EndpointCertificateMaintainerTest { tester.clock().advance(Duration.ofDays(3)); secretStore.setSecret(assignedCertificate.certificate().keyName(), "foo", 1); secretStore.setSecret(assignedCertificate.certificate().certName(), "bar", 1); - tester.controller().serviceRegistry().endpointCertificateProvider().requestCaSignedCertificate("preprovisioned." + assignedCertificate.certificate().randomizedId().get(), assignedCertificate.certificate().requestedDnsSans(), Optional.of(assignedCertificate.certificate()), "rsa_2048", false); + tester.controller().serviceRegistry().endpointCertificateProvider().requestCaSignedCertificate("preprovisioned." + assignedCertificate.certificate().generatedId().get(), assignedCertificate.certificate().requestedDnsSans(), Optional.of(assignedCertificate.certificate()), "rsa_2048", false); // We should now pick up the new key and cert version + uuid, but not force trigger deployment yet assertEquals(0.0, maintainer.maintain(), 0.0000001); @@ -206,7 +205,7 @@ public class EndpointCertificateMaintainerTest { assertTrue(applicationCertificate.isPresent()); Optional<AssignedCertificate> instanceCertificate = tester.curator().readAssignedCertificate(TenantAndApplicationId.from(app), Optional.of(app.instance())); assertTrue(instanceCertificate.isPresent()); - assertEquals(instanceCertificate.get().certificate().randomizedId(), applicationCertificate.get().certificate().randomizedId()); + assertEquals(instanceCertificate.get().certificate().generatedId(), applicationCertificate.get().certificate().generatedId()); // Verify the 3 wildcard random names are same in all certs List<String> appWildcardSans = applicationCertificate.get().certificate().requestedDnsSans(); @@ -226,13 +225,13 @@ public class EndpointCertificateMaintainerTest { assertEquals(1, tester.curator().readAssignedCertificates().size()); maintainer.maintain(); - String randomId = tester.curator().readAssignedCertificate(instance1).get().certificate().randomizedId().get(); + String randomId = tester.curator().readAssignedCertificate(instance1).get().certificate().generatedId().get(); deployToAssignCert(deploymentTester, instance2, List.of(productionUsWest1), Optional.of("instance1,instance2")); maintainer.maintain(); assertEquals(3, tester.curator().readAssignedCertificates().size()); - assertEquals(randomId, tester.curator().readAssignedCertificate(instance1).get().certificate().randomizedId().get()); + assertEquals(randomId, tester.curator().readAssignedCertificate(instance1).get().certificate().generatedId().get()); } @Test @@ -247,7 +246,7 @@ public class EndpointCertificateMaintainerTest { // Verify certificate is assigned random id and 3 new names Optional<AssignedCertificate> assignedCertificate = tester.curator().readAssignedCertificate(devApp); - assertTrue(assignedCertificate.get().certificate().randomizedId().isPresent()); + assertTrue(assignedCertificate.get().certificate().generatedId().isPresent()); List<String> newRequestedSans = assignedCertificate.get().certificate().requestedDnsSans(); List<String> randomizedNames = newRequestedSans.stream().filter(san -> !originalRequestedSans.contains(san)).toList(); assertEquals(3, randomizedNames.size()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java index 22523103208..3405009714d 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java @@ -1206,6 +1206,70 @@ public class RoutingPoliciesTest { } @Test + public void generated_endpoints_enable_token() { + var tester = new RoutingPoliciesTester(SystemName.Public); + var context = tester.newDeploymentContext("tenant1", "app1", "default"); + tester.controllerTester().flagSource().withBooleanFlag(Flags.RANDOMIZED_ENDPOINT_NAMES.id(), true); + tester.controllerTester().flagSource().withBooleanFlag(Flags.LEGACY_ENDPOINTS.id(), false); + addCertificateToPool("cafed00d", UnassignedCertificate.State.ready, tester); + + // Deploy application without token + var zone1 = ZoneId.from("prod", "aws-us-east-1c"); + ApplicationPackage applicationPackage = applicationPackageBuilder().region(zone1.region()) + .container("c0", AuthMethod.mtls) + .endpoint("foo", "c0") + .build(); + tester.provisionLoadBalancers(1, context.instanceId(), ZoneId.from("test", "aws-us-east-2c")); + tester.provisionLoadBalancers(1, context.instanceId(), ZoneId.from("staging", "aws-us-east-3c")); + tester.provisionLoadBalancers(1, context.instanceId(), zone1); + context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.test, Environment.staging, Environment.prod).deploy(); + assertEquals(List.of("a9c8c045.cafed00d.g.vespa-app.cloud", + "ebd395b6.cafed00d.z.vespa-app.cloud", + "fcf1bd63.cafed00d.aws-us-east-1.w.vespa-app.cloud"), + tester.recordNames()); + + // Re-deploy with token enabled + applicationPackage = applicationPackageBuilder().region(zone1.region()) + .container("c0", AuthMethod.mtls, AuthMethod.token) + .endpoint("foo", "c0") + .build(); + tester.provisionLoadBalancers(1, context.instanceId(), ZoneId.from("test", "aws-us-east-2c")); + tester.provisionLoadBalancers(1, context.instanceId(), ZoneId.from("staging", "aws-us-east-3c")); + context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.test, Environment.staging, Environment.prod).deploy(); + // Additional zone- and global-scoped endpoints are added (token) + assertEquals(List.of("a9c8c045.cafed00d.g.vespa-app.cloud", + "b7e79800.cafed00d.z.vespa-app.cloud", + "c60d3149.cafed00d.g.vespa-app.cloud", + "ebd395b6.cafed00d.z.vespa-app.cloud", + "fcf1bd63.cafed00d.aws-us-east-1.w.vespa-app.cloud"), + tester.recordNames()); + + // Add new endpoint is generated for an additional global endpoint + applicationPackage = applicationPackageBuilder().region(zone1.region()) + .container("c0", AuthMethod.mtls, AuthMethod.token) + .endpoint("foo", "c0") + .endpoint("bar", "c0") + .build(); + tester.provisionLoadBalancers(1, context.instanceId(), ZoneId.from("test", "aws-us-east-2c")); + tester.provisionLoadBalancers(1, context.instanceId(), ZoneId.from("staging", "aws-us-east-3c")); + context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.test, Environment.staging, Environment.prod).deploy(); + List<String> expectedRecords = List.of("a9c8c045.cafed00d.g.vespa-app.cloud", + "aa7591aa.cafed00d.g.vespa-app.cloud", + "b7e79800.cafed00d.z.vespa-app.cloud", + "c60d3149.cafed00d.g.vespa-app.cloud", + "d467800f.cafed00d.g.vespa-app.cloud", + "ebd395b6.cafed00d.z.vespa-app.cloud", + "fcf1bd63.cafed00d.aws-us-east-1.w.vespa-app.cloud"); + assertEquals(expectedRecords, tester.recordNames()); + + // No change on redeployment + tester.provisionLoadBalancers(1, context.instanceId(), ZoneId.from("test", "aws-us-east-2c")); + tester.provisionLoadBalancers(1, context.instanceId(), ZoneId.from("staging", "aws-us-east-3c")); + context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.test, Environment.staging, Environment.prod).deploy(); + assertEquals(expectedRecords, tester.recordNames()); + } + + @Test public void generated_endpoints_only() { var tester = new RoutingPoliciesTester(SystemName.Public); var context = tester.newDeploymentContext("tenant1", "app1", "default"); @@ -1216,6 +1280,7 @@ public class RoutingPoliciesTest { // Deploy application var zone1 = ZoneId.from("prod", "aws-us-east-1c"); + var zone2 = ZoneId.from("prod", "aws-eu-west-1a"); ApplicationPackage applicationPackage = applicationPackageBuilder().region(zone1.region()) .container("c0", AuthMethod.mtls) .endpoint("foo", "c0") @@ -1232,6 +1297,23 @@ public class RoutingPoliciesTest { "ebd395b6.cafed00d.z.vespa-app.cloud", "fcf1bd63.cafed00d.aws-us-east-1.w.vespa-app.cloud"), tester.recordNames()); + + // Another zone is added to global endpoint + applicationPackage = applicationPackageBuilder().region(zone1.region()) + .region(zone2.region()) + .container("c0", AuthMethod.mtls) + .endpoint("foo", "c0") + .build(); + tester.provisionLoadBalancers(1, context.instanceId(), ZoneId.from("test", "aws-us-east-2c")); + tester.provisionLoadBalancers(1, context.instanceId(), ZoneId.from("staging", "aws-us-east-3c")); + tester.provisionLoadBalancers(1, context.instanceId(), zone2); + context.submit(applicationPackage).deferLoadBalancerProvisioningIn(Environment.test, Environment.staging, Environment.prod).deploy(); + assertEquals(List.of("a6414896.cafed00d.aws-eu-west-1.w.vespa-app.cloud", + "a9c8c045.cafed00d.g.vespa-app.cloud", + "cbff1506.cafed00d.z.vespa-app.cloud", + "ebd395b6.cafed00d.z.vespa-app.cloud", + "fcf1bd63.cafed00d.aws-us-east-1.w.vespa-app.cloud"), + tester.recordNames()); } @Test diff --git a/dependency-versions/pom.xml b/dependency-versions/pom.xml index 0cb0a3a9659..9e262ce45e8 100644 --- a/dependency-versions/pom.xml +++ b/dependency-versions/pom.xml @@ -137,7 +137,7 @@ <junit.platform.vespa.tenant.version>1.8.1</junit.platform.vespa.tenant.version> <!-- Maven plugins --> - <clover-maven-plugin.vespa.version>4.4.1</clover-maven-plugin.vespa.version> + <clover-maven-plugin.vespa.version>4.5.0</clover-maven-plugin.vespa.version> <maven-antrun-plugin.vespa.version>3.1.0</maven-antrun-plugin.vespa.version> <maven-assembly-plugin.vespa.version>3.6.0</maven-assembly-plugin.vespa.version> <maven-bundle-plugin.vespa.version>5.1.9</maven-bundle-plugin.vespa.version> 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 0d187514e53..27c9e9ee7da 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -422,6 +422,13 @@ public class Flags { "Takes effect on redeployment through controller", INSTANCE_ID, APPLICATION_ID, TENANT_ID); + public static final UnboundIntFlag SEARCH_HANDLER_THREADPOOL = defineIntFlag( + "search-handler-threadpool", 2, + List.of("bjorncs", "baldersheim"), "2023-10-01", "2024-01-01", + "Adjust search handler threadpool size", + "Takes effect at redeployment", + APPLICATION_ID); + /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, List<String> owners, String createdAt, String expiresAt, String description, diff --git a/searchlib/src/vespa/searchlib/docstore/filechunk.cpp b/searchlib/src/vespa/searchlib/docstore/filechunk.cpp index 71dfed86fdb..6d0c025038a 100644 --- a/searchlib/src/vespa/searchlib/docstore/filechunk.cpp +++ b/searchlib/src/vespa/searchlib/docstore/filechunk.cpp @@ -6,14 +6,12 @@ #include "randreaders.h" #include <vespa/searchlib/util/filekit.h> #include <vespa/vespalib/util/lambdatask.h> -#include <vespa/vespalib/util/size_literals.h> #include <vespa/vespalib/data/fileheader.h> #include <vespa/vespalib/data/databuffer.h> #include <vespa/vespalib/stllike/asciistream.h> #include <vespa/vespalib/objects/nbostream.h> #include <vespa/vespalib/util/executor.h> #include <vespa/vespalib/util/arrayqueue.hpp> -#include <vespa/vespalib/util/array.hpp> #include <vespa/fastos/file.h> #include <filesystem> #include <future> @@ -117,33 +115,14 @@ FileChunk::addNumBuckets(size_t numBucketsInChunk) } } -class TmpChunkMeta : public ChunkMeta, - public std::vector<LidMeta> -{ -public: - void fill(vespalib::nbostream & is) { - resize(getNumEntries()); - for (LidMeta & lm : *this) { - lm.deserialize(is); - } - } -}; - -using TmpChunkMetaV = std::vector<TmpChunkMeta>; - -namespace { - void -verifyOrAssert(const TmpChunkMetaV & v) -{ - for (auto prev(v.begin()), it(prev); it != v.end(); ++it) { - assert(prev->getLastSerial() <= it->getLastSerial()); - prev = it; +FileChunk::TmpChunkMeta::fill(vespalib::nbostream & is) { + resize(getNumEntries()); + for (LidMeta & lm : *this) { + lm.deserialize(is); } } -} - void FileChunk::erase() { @@ -152,98 +131,92 @@ FileChunk::erase() std::filesystem::remove(std::filesystem::path(_dataFileName)); } -size_t +void FileChunk::updateLidMap(const unique_lock &guard, ISetLid &ds, uint64_t serialNum, uint32_t docIdLimit) { - size_t sz(0); assert(_chunkInfo.empty()); FastOS_File idxFile(_idxFileName.c_str()); idxFile.enableMemoryMap(0); - if (idxFile.OpenReadOnly()) { - if (idxFile.IsMemoryMapped()) { - const int64_t fileSize = idxFile.getSize(); - if (_idxHeaderLen == 0) { - _idxHeaderLen = readIdxHeader(idxFile, _docIdLimit); + if ( ! idxFile.OpenReadOnly()) { + LOG_ABORT("should not reach here"); + } + if ( ! idxFile.IsMemoryMapped()) { + assert(idxFile.getSize() == 0); + return; + } + const int64_t fileSize = idxFile.getSize(); + if (_idxHeaderLen == 0) { + _idxHeaderLen = readIdxHeader(idxFile, _docIdLimit); + } + BucketDensityComputer globalBucketMap(_bucketizer); + // Guard comes from the same bucketizer so the same guard can be used + // for both local and global BucketDensityComputer + vespalib::GenerationHandler::Guard bucketizerGuard = globalBucketMap.getGuard(); + vespalib::nbostream is(static_cast<const char *>(idxFile.MemoryMapPtr(0)) + _idxHeaderLen, + fileSize - _idxHeaderLen); + for (size_t count=0; ! is.empty() && is.good(); count++) { + const int64_t lastKnownGoodPos = _idxHeaderLen + is.rp(); + TmpChunkMeta chunkMeta; + try { + chunkMeta.deserialize(is); + chunkMeta.fill(is); + if ((count == 0) && (chunkMeta.getLastSerial() < serialNum)) { + LOG(warning, "last serial num(%" PRIu64 ") from previous file is bigger than my first(%" PRIu64 + "). That is odd.Current filename is '%s'", + serialNum, chunkMeta.getLastSerial(), _idxFileName.c_str()); + serialNum = chunkMeta.getLastSerial(); } - vespalib::nbostream is(static_cast<const char *>(idxFile.MemoryMapPtr(0)) + _idxHeaderLen, - fileSize - _idxHeaderLen); - TmpChunkMetaV tempVector; - tempVector.reserve(fileSize/(sizeof(ChunkMeta)+sizeof(LidMeta))); - while ( ! is.empty() && is.good()) { - const int64_t lastKnownGoodPos = _idxHeaderLen + is.rp(); - tempVector.emplace_back(); - TmpChunkMeta & chunkMeta(tempVector.back()); - try { - chunkMeta.deserialize(is); - chunkMeta.fill(is); - } catch (const vespalib::IllegalStateException & e) { - LOG(warning, "Exception deserializing idx file : %s", e.what()); - LOG(warning, "File '%s' seems to be partially truncated. Will truncate from size=%" PRId64 " to %" PRId64, - _idxFileName.c_str(), fileSize, lastKnownGoodPos); - FastOS_File toTruncate(_idxFileName.c_str()); - if ( toTruncate.OpenReadWrite()) { - if (toTruncate.SetSize(lastKnownGoodPos)) { - tempVector.resize(tempVector.size() - 1); - } else { - throw SummaryException("SetSize() failed.", toTruncate, VESPA_STRLOC); - } - } else { - throw SummaryException("Open for truncation failed.", toTruncate, VESPA_STRLOC); - } - break; + assert(serialNum <= chunkMeta.getLastSerial()); + serialNum = handleChunk(guard, ds, docIdLimit, bucketizerGuard, globalBucketMap, chunkMeta); + assert(serialNum >= _lastPersistedSerialNum.load(std::memory_order_relaxed)); + _lastPersistedSerialNum.store(serialNum, std::memory_order_relaxed); + } catch (const vespalib::IllegalStateException & e) { + LOG(warning, "Exception deserializing idx file : %s", e.what()); + LOG(warning, "File '%s' seems to be partially truncated. Will truncate from size=%" PRId64 " to %" PRId64, + _idxFileName.c_str(), fileSize, lastKnownGoodPos); + FastOS_File toTruncate(_idxFileName.c_str()); + if ( toTruncate.OpenReadWrite()) { + if (toTruncate.SetSize(lastKnownGoodPos)) { + } else { + throw SummaryException("SetSize() failed.", toTruncate, VESPA_STRLOC); } + } else { + throw SummaryException("Open for truncation failed.", toTruncate, VESPA_STRLOC); } - if ( ! tempVector.empty()) { - verifyOrAssert(tempVector); - if (tempVector[0].getLastSerial() < serialNum) { - LOG(warning, - "last serial num(%" PRIu64 ") from previous file is " - "bigger than my first(%" PRIu64 "). That is odd." - "Current filename is '%s'", - serialNum, tempVector[0].getLastSerial(), - _idxFileName.c_str()); - serialNum = tempVector[0].getLastSerial(); - } - BucketDensityComputer globalBucketMap(_bucketizer); - // Guard comes from the same bucketizer so the same guard can be used - // for both local and global BucketDensityComputer - vespalib::GenerationHandler::Guard bucketizerGuard = globalBucketMap.getGuard(); - for (const TmpChunkMeta & chunkMeta : tempVector) { - assert(serialNum <= chunkMeta.getLastSerial()); - BucketDensityComputer bucketMap(_bucketizer); - for (size_t i(0), m(chunkMeta.getNumEntries()); i < m; i++) { - const LidMeta & lidMeta(chunkMeta[i]); - if (lidMeta.getLid() < docIdLimit) { - if (_bucketizer && (lidMeta.size() > 0)) { - document::BucketId bucketId = _bucketizer->getBucketOf(bucketizerGuard, lidMeta.getLid()); - bucketMap.recordLid(bucketId); - globalBucketMap.recordLid(bucketId); - } - ds.setLid(guard, lidMeta.getLid(), LidInfo(getFileId().getId(), _chunkInfo.size(), lidMeta.size())); - _numLids++; - } else { - remove(lidMeta.getLid(), lidMeta.size()); - } - _addedBytes += adjustSize(lidMeta.size()); - } - serialNum = chunkMeta.getLastSerial(); - addNumBuckets(bucketMap.getNumBuckets()); - _chunkInfo.emplace_back(chunkMeta.getOffset(), chunkMeta.getSize(), chunkMeta.getLastSerial()); - assert(serialNum >= _lastPersistedSerialNum.load(std::memory_order_relaxed)); - _lastPersistedSerialNum.store(serialNum, std::memory_order_relaxed); - } - _numUniqueBuckets = globalBucketMap.getNumBuckets(); + break; + } + } + _numUniqueBuckets = globalBucketMap.getNumBuckets(); +} + +uint64_t +FileChunk::handleChunk(const unique_lock &guard, ISetLid &ds, uint32_t docIdLimit, + const vespalib::GenerationHandler::Guard & bucketizerGuard, BucketDensityComputer &globalBucketMap, + const TmpChunkMeta & chunkMeta) { + BucketDensityComputer bucketMap(_bucketizer); + for (size_t i(0), m(chunkMeta.getNumEntries()); i < m; i++) { + const LidMeta & lidMeta(chunkMeta[i]); + if (lidMeta.getLid() < docIdLimit) { + if (_bucketizer && (lidMeta.size() > 0)) { + document::BucketId bucketId = _bucketizer->getBucketOf(bucketizerGuard, lidMeta.getLid()); + bucketMap.recordLid(bucketId); + globalBucketMap.recordLid(bucketId); } + ds.setLid(guard, lidMeta.getLid(), LidInfo(getFileId().getId(), _chunkInfo.size(), lidMeta.size())); + _numLids++; } else { - assert(idxFile.getSize() == 0); + remove(lidMeta.getLid(), lidMeta.size()); } - } else { - LOG_ABORT("should not reach here"); + _addedBytes += adjustSize(lidMeta.size()); } - return sz; + uint64_t serialNum = chunkMeta.getLastSerial(); + addNumBuckets(bucketMap.getNumBuckets()); + _chunkInfo.emplace_back(chunkMeta.getOffset(), chunkMeta.getSize(), chunkMeta.getLastSerial()); + return serialNum; } + void FileChunk::enableRead() { @@ -581,8 +554,7 @@ FileChunk::getStats() const uint64_t serialNum = getLastPersistedSerialNum(); uint32_t docIdLimit = getDocIdLimit(); uint64_t nameId = getNameId().getId(); - return DataStoreFileChunkStats(diskFootprint, diskBloat, bucketSpread, - serialNum, serialNum, docIdLimit, nameId); + return {diskFootprint, diskBloat, bucketSpread, serialNum, serialNum, docIdLimit, nameId}; } } // namespace search diff --git a/searchlib/src/vespa/searchlib/docstore/filechunk.h b/searchlib/src/vespa/searchlib/docstore/filechunk.h index 3664da3dfd9..446a53de446 100644 --- a/searchlib/src/vespa/searchlib/docstore/filechunk.h +++ b/searchlib/src/vespa/searchlib/docstore/filechunk.h @@ -47,7 +47,7 @@ public: class BucketDensityComputer { public: - BucketDensityComputer(const IBucketizer * bucketizer) : _bucketizer(bucketizer), _count(0) { } + explicit BucketDensityComputer(const IBucketizer * bucketizer) : _bucketizer(bucketizer), _count(0) { } void recordLid(const vespalib::GenerationHandler::Guard & guard, uint32_t lid, uint32_t dataSize) { if (_bucketizer && (dataSize > 0)) { recordLid(_bucketizer->getBucketOf(guard, lid)); @@ -109,7 +109,7 @@ public: const IBucketizer *bucketizer); virtual ~FileChunk(); - virtual size_t updateLidMap(const unique_lock &guard, ISetLid &lidMap, uint64_t serialNum, uint32_t docIdLimit); + virtual void updateLidMap(const unique_lock &guard, ISetLid &lidMap, uint64_t serialNum, uint32_t docIdLimit); virtual ssize_t read(uint32_t lid, SubChunkId chunk, vespalib::DataBuffer & buffer) const; virtual void read(LidInfoWithLidV::const_iterator begin, size_t count, IBufferVisitor & visitor) const; void remove(uint32_t lid, uint32_t size); @@ -118,7 +118,7 @@ public: virtual size_t getMemoryMetaFootprint() const; virtual vespalib::MemoryUsage getMemoryUsage() const; - virtual size_t getDiskHeaderFootprint(void) const { return _dataHeaderLen + _idxHeaderLen; } + virtual size_t getDiskHeaderFootprint() const { return _dataHeaderLen + _idxHeaderLen; } size_t getDiskBloat() const { return (_addedBytes == 0) ? getDiskFootprint() @@ -199,6 +199,16 @@ public: static vespalib::string createIdxFileName(const vespalib::string & name); static vespalib::string createDatFileName(const vespalib::string & name); private: + class TmpChunkMeta : public ChunkMeta, + public std::vector<LidMeta> + { + public: + void fill(vespalib::nbostream & is); + }; + using BucketizerGuard = vespalib::GenerationHandler::Guard; + uint64_t handleChunk(const unique_lock &guard, ISetLid &lidMap, uint32_t docIdLimit, + const BucketizerGuard & bucketizerGuard, BucketDensityComputer & global, + const TmpChunkMeta & chunkMeta); using File = std::unique_ptr<FileRandRead>; const FileId _fileId; const NameId _nameId; diff --git a/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.cpp b/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.cpp index 7102b80d7d0..973287fc7bd 100644 --- a/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.cpp +++ b/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.cpp @@ -174,16 +174,15 @@ WriteableFileChunk::~WriteableFileChunk() } } -size_t +void WriteableFileChunk::updateLidMap(const unique_lock &guard, ISetLid &ds, uint64_t serialNum, uint32_t docIdLimit) { - size_t sz = FileChunk::updateLidMap(guard, ds, serialNum, docIdLimit); + FileChunk::updateLidMap(guard, ds, serialNum, docIdLimit); _nextChunkId = _chunkInfo.size(); _active = std::make_unique<Chunk>(_nextChunkId++, Chunk::Config(_config.getMaxChunkBytes())); _serialNum = getLastPersistedSerialNum(); _firstChunkIdToBeWritten = _active->getId(); setDiskFootprint(0); - return sz; } void diff --git a/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.h b/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.h index b5a52dc83f7..028915d28e0 100644 --- a/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.h +++ b/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.h @@ -64,7 +64,7 @@ public: size_t getMemoryFootprint() const override; size_t getMemoryMetaFootprint() const override; vespalib::MemoryUsage getMemoryUsage() const override; - size_t updateLidMap(const unique_lock &guard, ISetLid &lidMap, uint64_t serialNum, uint32_t docIdLimit) override; + void updateLidMap(const unique_lock &guard, ISetLid &lidMap, uint64_t serialNum, uint32_t docIdLimit) override; void waitForDiskToCatchUpToNow() const; void flushPendingChunks(uint64_t serialNum); DataStoreFileChunkStats getStats() const override; |