diff options
50 files changed, 647 insertions, 1048 deletions
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainer.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainer.java index a93ae926b52..405cfb47c72 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainer.java @@ -38,11 +38,13 @@ public final class ApplicationContainer extends Container { JettyHttpServer server = Optional.ofNullable(getHttp()) .map(Http::getHttpServer) .orElse(getDefaultHttpServer()); - String serverName = server.getComponentId().getName(); - var connectorFactory = tlsCa - .map(caCert -> new HostedSslConnectorFactory(serverName, tlsSecrets.get(), caCert)) - .orElseGet(() -> new HostedSslConnectorFactory(serverName, tlsSecrets.get())); - server.addConnector(connectorFactory); + if (server.getConnectorFactories().stream().noneMatch(connectorFactory -> connectorFactory instanceof HostedSslConnectorFactory)) { + String serverName = server.getComponentId().getName(); + var connectorFactory = tlsCa + .map(caCert -> new HostedSslConnectorFactory(serverName, tlsSecrets.get(), caCert)) + .orElseGet(() -> new HostedSslConnectorFactory(serverName, tlsSecrets.get())); + server.addConnector(connectorFactory); + } } addComponent(getFS4ResourcePool()); // TODO Remove when FS4 based search protocol is gone } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTestBase.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTestBase.java index 41997925666..9e02572737e 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTestBase.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTestBase.java @@ -43,6 +43,11 @@ public abstract class ContainerModelBuilderTestBase { " <nodes>" + " <node hostalias='mockhost' />" + " </nodes>"; + public static final String multiNode = + " <nodes>" + + " <node hostalias='mockhost1' />" + + " <node hostalias='mockhost2' />" + + " </nodes>"; protected MockRoot root; diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/JettyContainerModelBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/JettyContainerModelBuilderTest.java index f0fcb239521..929e520f984 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/JettyContainerModelBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/JettyContainerModelBuilderTest.java @@ -5,6 +5,8 @@ import com.yahoo.config.model.api.TlsSecrets; import com.yahoo.config.model.builder.xml.test.DomBuilderTest; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.deploy.TestProperties; +import com.yahoo.config.model.provision.HostsXmlProvisioner; +import com.yahoo.config.model.test.MockRoot; import com.yahoo.container.ComponentsConfig; import com.yahoo.container.jdisc.FilterBindingsProvider; import com.yahoo.jdisc.http.ConnectorConfig; @@ -17,6 +19,7 @@ import com.yahoo.vespa.model.container.http.ssl.ConfiguredFilebasedSslProvider; import org.junit.Test; import org.w3c.dom.Element; +import java.io.StringReader; import java.util.List; import java.util.Optional; @@ -238,11 +241,27 @@ public class JettyContainerModelBuilderTest extends ContainerModelBuilderTestBas " </ssl>", " </server>", " </http>", - nodesXml, + multiNode, "", "</container>"); - DeployState deployState = new DeployState.Builder().properties(new TestProperties().setHostedVespa(true).setTlsSecrets(Optional.of(new TlsSecrets("CERT", "KEY")))).build(); + String hostsxml = "<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n" + + "<hosts>\n" + + " <host name=\"mockhost-1\">\n" + + " <alias>mockhost1</alias>\n" + + " </host>\n" + + " <host name=\"mockhost-2\">\n" + + " <alias>mockhost2</alias>\n" + + " </host>\n" + + "</hosts>\n"; + DeployState deployState = new DeployState.Builder() + .properties( + new TestProperties() + .setHostedVespa(true) + .setTlsSecrets(Optional.of(new TlsSecrets("CERT", "KEY")))) + .modelHostProvisioner(new HostsXmlProvisioner(new StringReader(hostsxml))) + .build(); + MockRoot root = new MockRoot("root", deployState); createModel(root, deployState, null, clusterElem); ConnectorConfig sslProvider = root.getConfig(ConnectorConfig.class, "default/http/jdisc-jetty/ssl"); assertTrue(sslProvider.ssl().enabled()); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TlsSecretsKeys.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TlsSecretsKeys.java index eaa4916d8fc..78d39ef996b 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TlsSecretsKeys.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TlsSecretsKeys.java @@ -59,17 +59,15 @@ public class TlsSecretsKeys { } private Optional<TlsSecrets> readFromSecretStore(Optional<String> secretKeyname) { - if(secretKeyname.isEmpty()) return Optional.empty(); - TlsSecrets tlsSecretParameters = TlsSecrets.MISSING; + if (secretKeyname.isEmpty()) return Optional.empty(); try { String cert = secretStore.getSecret(secretKeyname.get() + "-cert"); String key = secretStore.getSecret(secretKeyname.get() + "-key"); - tlsSecretParameters = new TlsSecrets(cert, key); + return Optional.of(new TlsSecrets(cert, key)); } catch (RuntimeException e) { // Assume not ready yet -// log.log(LogLevel.DEBUG, "Could not fetch certificate/key with prefix: " + secretKeyname.get(), e); + return Optional.of(TlsSecrets.MISSING); } - return Optional.of(tlsSecretParameters); } /** Returns a transaction which deletes these tls secrets key if they exist */ diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/NodeRepositoryNode.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/NodeRepositoryNode.java index 75d4e2036ab..57c71720962 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/NodeRepositoryNode.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/noderepository/NodeRepositoryNode.java @@ -82,6 +82,10 @@ public class NodeRepositoryNode { private Integer cost; @JsonProperty("minCpuCores") private Double minCpuCores; + @JsonProperty("bandwidth") + private Double bandwidth; + @JsonProperty("fastDisk") + private Boolean fastDisk; @JsonProperty("description") private String description; @JsonProperty("history") @@ -347,6 +351,22 @@ public class NodeRepositoryNode { this.minCpuCores = minCpuCores; } + public Double getBandwidth() { + return bandwidth; + } + + public void setBandwidth(Double bandwidth) { + this.bandwidth = bandwidth; + } + + public Boolean getFastDisk() { + return fastDisk; + } + + public void setFastDisk(Boolean fastDisk) { + this.fastDisk = fastDisk; + } + public String getDescription() { return description; } @@ -436,6 +456,8 @@ public class NodeRepositoryNode { ", minMainMemoryAvailableGb=" + minMainMemoryAvailableGb + ", cost=" + cost + ", minCpuCores=" + minCpuCores + + ", bandwidth=" + bandwidth + + ", fastDisk=" + fastDisk + ", description='" + description + '\'' + ", history=" + Arrays.toString(history) + ", allowedToBeDown=" + allowedToBeDown + diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index 64f9d042121..d7a0465a8df 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -9,8 +9,8 @@ import com.yahoo.config.application.api.ValidationOverrides; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; -import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.TenantName; +import com.yahoo.config.provision.zone.ZoneApi; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.vespa.athenz.api.AthenzDomain; import com.yahoo.vespa.athenz.api.AthenzIdentity; @@ -49,13 +49,10 @@ import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName; import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingEndpoint; import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingGenerator; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; -import com.yahoo.vespa.hosted.controller.application.AssignedRotation; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; import com.yahoo.vespa.hosted.controller.application.DeploymentSpecValidator; import com.yahoo.vespa.hosted.controller.application.Endpoint; -import com.yahoo.vespa.hosted.controller.application.EndpointId; -import com.yahoo.vespa.hosted.controller.application.EndpointList; import com.yahoo.vespa.hosted.controller.application.JobList; import com.yahoo.vespa.hosted.controller.application.JobStatus; import com.yahoo.vespa.hosted.controller.application.JobStatus.JobRun; @@ -66,8 +63,6 @@ import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger; import com.yahoo.vespa.hosted.controller.dns.NameServiceQueue.Priority; import com.yahoo.vespa.hosted.controller.maintenance.RoutingPolicies; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; -import com.yahoo.vespa.hosted.controller.rotation.Rotation; -import com.yahoo.vespa.hosted.controller.rotation.RotationId; import com.yahoo.vespa.hosted.controller.rotation.RotationLock; import com.yahoo.vespa.hosted.controller.rotation.RotationRepository; import com.yahoo.vespa.hosted.controller.security.AccessControl; @@ -83,12 +78,13 @@ import java.security.Principal; import java.time.Clock; import java.time.Duration; import java.time.Instant; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.EnumSet; import java.util.HashMap; -import java.util.LinkedHashSet; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -99,7 +95,6 @@ import java.util.function.Consumer; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; -import java.util.stream.Stream; import static com.yahoo.vespa.hosted.controller.api.integration.configserver.Node.State.active; import static com.yahoo.vespa.hosted.controller.api.integration.configserver.Node.State.reserved; @@ -128,7 +123,6 @@ public class ApplicationController { private final RoutingGenerator routingGenerator; private final RoutingPolicies routingPolicies; private final Clock clock; - private final BooleanFlag useMultipleEndpoints; private final DeploymentTrigger deploymentTrigger; private final BooleanFlag provisionApplicationCertificate; private final ApplicationCertificateProvider applicationCertificateProvider; @@ -146,7 +140,6 @@ public class ApplicationController { this.routingGenerator = routingGenerator; this.routingPolicies = new RoutingPolicies(controller); this.clock = clock; - this.useMultipleEndpoints = Flags.MULTIPLE_GLOBAL_ENDPOINTS.bindTo(controller.flagSource()); this.artifactRepository = artifactRepository; this.applicationStore = applicationStore; @@ -233,7 +226,7 @@ public class ApplicationController { } /** Find the global endpoint of given deployment, if any */ - public Optional<RoutingEndpoint> findGlobalEndpoint(DeploymentId deployment) { + private Optional<RoutingEndpoint> findGlobalEndpoint(DeploymentId deployment) { return routingGenerator.endpoints(deployment).stream() .filter(RoutingEndpoint::isGlobal) .findFirst(); @@ -298,8 +291,8 @@ public class ApplicationController { Version platformVersion; ApplicationVersion applicationVersion; ApplicationPackage applicationPackage; - Set<String> legacyRotations = new LinkedHashSet<>(); - Set<ContainerEndpoint> endpoints = new LinkedHashSet<>(); + Set<ContainerEndpoint> endpoints; + Set<String> legacyRotations; Optional<ApplicationCertificate> applicationCertificate; try (Lock lock = lock(applicationId)) { @@ -338,41 +331,22 @@ public class ApplicationController { // TODO(jvenstad): Remove this when all packages are validated upon submission, as in ApplicationApiHandler.submit(...). verifyApplicationIdentityConfiguration(applicationId.tenant(), applicationPackage, deployingIdentity); - // Assign global rotation - // TODO(ogronnesby): Remove feature flag and replace calls to withRotationLegacy with withRotation - // TODO(mpolden): Remove all handling of legacy endpoints once withRotationLegacy disappears - if (useMultipleEndpoints.with(FetchVector.Dimension.APPLICATION_ID, application.get().id().serializedForm()).value()) { - application = withRotation(application, zone); - - // Include global DNS names - Application app = application.get(); - app.rotations().stream() - .filter(assignedRotation -> assignedRotation.regions().contains(zone.region())) - .map(assignedRotation -> { - return new ContainerEndpoint( - assignedRotation.clusterId().value(), - Stream.concat( - app.endpointsIn(controller.system(), assignedRotation.endpointId()).legacy(false).asList().stream().map(Endpoint::dnsName), - Stream.of(assignedRotation.rotationId().asString()) - ).collect(Collectors.toList()) - ); - }) - .forEach(endpoints::add); + // Assign and register endpoints + application = withRotation(application, zone); + endpoints = registerEndpointsInDns(application.get(), zone); + legacyRotations = endpoints.stream() + .map(ContainerEndpoint::names) + .flatMap(Collection::stream) + .collect(Collectors.toSet()); + + if (controller.zoneRegistry().zones().directlyRouted().ids().contains(zone)) { + // Get application certificate (provisions a new certificate if missing) + List<? extends ZoneApi> zones = controller.zoneRegistry().zones().all().zones(); + applicationCertificate = getApplicationCertificate(application.get()); } else { - application = withRotationLegacy(application, zone); - - // Add both the names we have in DNS for each endpoint as well as name of the rotation so healthchecks works - Application app = application.get(); - app.endpointsIn(controller.system()).asList().stream().map(Endpoint::dnsName).forEach(legacyRotations::add); - app.rotations().stream() - .map(AssignedRotation::rotationId) - .map(RotationId::asString) - .forEach(legacyRotations::add); + applicationCertificate = Optional.empty(); } - // Get application certificate (provisions a new certificate if missing) - applicationCertificate = getApplicationCertificate(application.get()); - // Update application with information from application package if ( ! preferOldestVersion && ! application.get().deploymentJobs().deployedInternally() @@ -479,77 +453,70 @@ public class ApplicationController { } /** Makes sure the application has a global rotation, if eligible. */ - private LockedApplication withRotationLegacy(LockedApplication application, ZoneId zone) { - if (zone.environment() == Environment.prod && application.get().deploymentSpec().globalServiceId().isPresent()) { - try (RotationLock rotationLock = rotationRepository.lock()) { - Rotation rotation = rotationRepository.getOrAssignRotation(application.get(), rotationLock); - application = application.with(createDefaultGlobalIdRotation(application.get(), rotation)); - store(application); // store assigned rotation even if deployment fails - - EndpointList globalEndpoints = application.get() - .endpointsIn(controller.system()) - .scope(Endpoint.Scope.global); - - globalEndpoints.main().ifPresent(mainEndpoint -> { - registerCname(mainEndpoint.dnsName(), rotation.name()); - globalEndpoints.legacy(true).asList().forEach(endpoint -> registerCname(endpoint.dnsName(), rotation.name())); - }); - } - } - return application; - } - - private List<AssignedRotation> createDefaultGlobalIdRotation(Application application, Rotation rotation) { - Set<RegionName> regions = application.deploymentSpec().zones().stream() - .filter(zone -> zone.environment().isProduction()) - .flatMap(zone -> zone.region().stream()) - .collect(Collectors.toSet()); - var assignment = new AssignedRotation( - ClusterSpec.Id.from(application.deploymentSpec().globalServiceId().get()), - EndpointId.default_(), - rotation.id(), - regions - ); - return List.of(assignment); - } - - /** Makes sure the application has a global rotation, if eligible. */ private LockedApplication withRotation(LockedApplication application, ZoneId zone) { if (zone.environment() == Environment.prod) { try (RotationLock rotationLock = rotationRepository.lock()) { var rotations = rotationRepository.getOrAssignRotations(application.get(), rotationLock); application = application.with(rotations); store(application); // store assigned rotation even if deployment fails - registerAssignedRotationCnames(application.get()); } } return application; } - private void registerAssignedRotationCnames(Application application) { - application.rotations().forEach(assignedRotation -> { + /** + * Register endpoints for rotations assigned to given application and zone in DNS. + * + * @return the registered endpoints + */ + private Set<ContainerEndpoint> registerEndpointsInDns(Application application, ZoneId zone) { + var containerEndpoints = new HashSet<ContainerEndpoint>(); + var registerLegacyNames = application.deploymentSpec().globalServiceId().isPresent(); + for (var assignedRotation : application.rotations()) { + var names = new ArrayList<String>(); var endpoints = application.endpointsIn(controller.system(), assignedRotation.endpointId()) .scope(Endpoint.Scope.global); - var maybeRotation = rotationRepository.getRotation(assignedRotation.rotationId()); - maybeRotation.ifPresent(rotation -> { - // For rotations assigned using <endpoints/> syntax, we only register the non-legacy name in DNS. - endpoints.main().ifPresent(mainEndpoint -> registerCname(mainEndpoint.dnsName(), rotation.name())); - }); - }); + + // Skip rotations which do not apply to this zone. Legacy names always point to all zones + if (!registerLegacyNames && !assignedRotation.regions().contains(zone.region())) { + continue; + } + + // Omit legacy DNS names when assigning rotations using <endpoints/> syntax + if (!registerLegacyNames) { + endpoints = endpoints.legacy(false); + } + + // Register names in DNS + var rotation = rotationRepository.getRotation(assignedRotation.rotationId()); + if (rotation.isPresent()) { + endpoints.asList().forEach(endpoint -> { + controller.nameServiceForwarder().createCname(RecordName.from(endpoint.dnsName()), + RecordData.fqdn(rotation.get().name()), + Priority.normal); + names.add(endpoint.dnsName()); + }); + } + + // Include rotation ID as a valid name of this container endpoint (required by global routing health checks) + names.add(assignedRotation.rotationId().asString()); + containerEndpoints.add(new ContainerEndpoint(assignedRotation.clusterId().value(), names)); + } + return Collections.unmodifiableSet(containerEndpoints); } private Optional<ApplicationCertificate> getApplicationCertificate(Application application) { + boolean provisionCertificate = provisionApplicationCertificate.with(FetchVector.Dimension.APPLICATION_ID, + application.id().serializedForm()).value(); + if (!provisionCertificate) { + return Optional.empty(); + } + // Re-use certificate if already provisioned Optional<ApplicationCertificate> applicationCertificate = curator.readApplicationCertificate(application.id()); if(applicationCertificate.isPresent()) return applicationCertificate; - // TODO(tokle): Verify that the application is deploying to a zone where certificate provisioning is enabled - boolean provisionCertificate = provisionApplicationCertificate.with(FetchVector.Dimension.APPLICATION_ID, - application.id().serializedForm()).value(); - if (!provisionCertificate) { - return Optional.empty(); - } ApplicationCertificate newCertificate = applicationCertificateProvider.requestCaSignedCertificate(application.id()); curator.writeApplicationCertificate(application.id(), newCertificate); @@ -610,11 +577,6 @@ public class ApplicationController { options.deployCurrentVersion); } - /** Register a CNAME record in DNS */ - private void registerCname(String name, String targetName) { - controller.nameServiceForwarder().createCname(RecordName.from(name), RecordData.fqdn(targetName), Priority.normal); - } - /** Returns the endpoints of the deployment, or empty if the request fails */ public List<URI> getDeploymentEndpoints(DeploymentId deploymentId) { if ( ! get(deploymentId.applicationId()) @@ -676,7 +638,7 @@ public class ApplicationController { */ public void deleteApplication(ApplicationId applicationId, Optional<Credentials> credentials) { Tenant tenant = controller.tenants().require(applicationId.tenant()); - if (tenant.type() != Tenant.Type.user && ! credentials.isPresent()) + if (tenant.type() != Tenant.Type.user && credentials.isEmpty()) throw new IllegalArgumentException("Could not delete application '" + applicationId + "': No credentials provided"); // Find all instances of the application diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java index a6775e19dd6..ef1ce19b167 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java @@ -309,5 +309,7 @@ public class Endpoint { } return new Endpoint(name, application, zone, system, port, legacy, directRouting, wildcard); } + } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java index 965cc1a9b16..c16cf1e1997 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java @@ -242,21 +242,29 @@ public class VersionStatus { boolean isReleased, Collection<HostName> configServerHostnames, Controller controller) { - boolean isSystemVersion = statistics.version().equals(systemVersion); - boolean isControllerVersion = statistics.version().equals(controllerVersion.version()); - VespaVersion.Confidence confidence = controller.curator().readConfidenceOverrides().get(statistics.version()); + var isSystemVersion = statistics.version().equals(systemVersion); + var isControllerVersion = statistics.version().equals(controllerVersion.version()); + var confidence = controller.curator().readConfidenceOverrides().get(statistics.version()); // Compute confidence if there's no override if (confidence == null) { if (isSystemVersion || isControllerVersion) { // Always compute confidence for system and controller confidence = VespaVersion.confidenceFrom(statistics, controller); - } else { // This is an older version so we keep the existing confidence, if any + } else { // This is an older version so we preserve the existing confidence, if any confidence = confidenceFor(statistics.version(), controller) .orElseGet(() -> VespaVersion.confidenceFrom(statistics, controller)); } } + // Preserve existing commit details if we've previously computed status for this version + var commitSha = controllerVersion.commitSha(); + var commitDate = controllerVersion.commitDate(); + var previousStatus = controller.versionStatus().version(statistics.version()); + if (previousStatus != null) { + commitSha = previousStatus.releaseCommit(); + commitDate = previousStatus.committedAt(); + } return new VespaVersion(statistics, - controllerVersion.commitSha(), - controllerVersion.commitDate(), + commitSha, + commitDate, isControllerVersion, isSystemVersion, isReleased, @@ -268,9 +276,9 @@ public class VersionStatus { /** Returns the current confidence for the given version */ private static Optional<VespaVersion.Confidence> confidenceFor(Version version, Controller controller) { return controller.versionStatus().versions().stream() - .filter(v -> version.equals(v.versionNumber())) - .map(VespaVersion::confidence) - .findFirst(); + .filter(v -> version.equals(v.versionNumber())) + .map(VespaVersion::confidence) + .findFirst(); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java index 6f8a10543e7..210b94737b0 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java @@ -279,13 +279,11 @@ public class ControllerTest { @Test public void testDnsAliasRegistration() { - ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.MULTIPLE_GLOBAL_ENDPOINTS.id(), true); - Application application = tester.createApplication("app1", "tenant1", 1, 1L); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .environment(Environment.prod) - .globalServiceId("foo") + .endpoint("default", "foo") .region("us-west-1") .region("us-central-1") // Two deployments should result in each DNS alias being registered once .build(); @@ -352,8 +350,6 @@ public class ControllerTest { @Test public void testDnsAliasRegistrationWithEndpoints() { - ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.MULTIPLE_GLOBAL_ENDPOINTS.id(), true); - Application application = tester.createApplication("app1", "tenant1", 1, 1L); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() @@ -409,8 +405,6 @@ public class ControllerTest { @Test public void testDnsAliasRegistrationWithChangingZones() { - ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.MULTIPLE_GLOBAL_ENDPOINTS.id(), true); - Application application = tester.createApplication("app1", "tenant1", 1, 1L); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() @@ -457,8 +451,6 @@ public class ControllerTest { @Test public void testUnassignRotations() { - ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.MULTIPLE_GLOBAL_ENDPOINTS.id(), true); - Application application = tester.createApplication("app1", "tenant1", 1, 1L); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() @@ -490,16 +482,14 @@ public class ControllerTest { ); } - @Test + @Test public void testUpdatesExistingDnsAlias() { - ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.MULTIPLE_GLOBAL_ENDPOINTS.id(), true); - // Application 1 is deployed and deleted { Application app1 = tester.createApplication("app1", "tenant1", 1, 1L); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .environment(Environment.prod) - .globalServiceId("foo") + .endpoint("default", "foo") .region("us-west-1") .region("us-central-1") // Two deployments should result in each DNS alias being registered once .build(); @@ -545,7 +535,7 @@ public class ControllerTest { Application app2 = tester.createApplication("app2", "tenant2", 2, 1L); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .environment(Environment.prod) - .globalServiceId("foo") + .endpoint("default", "foo") .region("us-west-1") .region("us-central-1") .build(); @@ -564,7 +554,7 @@ public class ControllerTest { Application app1 = tester.createApplication("app1", "tenant1", 1, 1L); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() .environment(Environment.prod) - .globalServiceId("foo") + .endpoint("default", "foo") .region("us-west-1") .region("us-central-1") .build(); @@ -706,7 +696,7 @@ public class ControllerTest { } @Test - public void testDeployProvisionsCertificate() { + public void testDeploySelectivelyProvisionsCertificate() { ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.PROVISION_APPLICATION_CERTIFICATE.id(), true); Function<Application, Optional<ApplicationCertificate>> certificate = (application) -> tester.controller().curator().readApplicationCertificate(application.id()); @@ -732,7 +722,7 @@ public class ControllerTest { tester.controller().applications().deploy(app2.id(), zone, Optional.of(applicationPackage), DeployOptions.none()); assertTrue("Application deployed and activated", tester.controllerTester().configServer().application(app2.id()).get().activated()); - assertTrue("Provisions certificate in " + Environment.dev, certificate.apply(app2).isPresent()); + assertFalse("Does not provision certificate in " + Environment.dev, certificate.apply(app2).isPresent()); } @Test diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java index 11ed95c3f15..46422188a01 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java @@ -117,9 +117,13 @@ public class DeploymentTester { controller().updateVersionStatus(VersionStatus.compute(controller())); } - /** Upgrade controller to given version */ public void upgradeController(Version version) { - controller().curator().writeControllerVersion(controller().hostname(), new ControllerVersion(version, "badc0ffee", Instant.EPOCH)); + upgradeController(version, "badc0ffee", Instant.EPOCH); + } + + /** Upgrade controller to given version */ + public void upgradeController(Version version, String commitSha, Instant commitDate) { + controller().curator().writeControllerVersion(controller().hostname(), new ControllerVersion(version, commitSha, commitDate)); computeVersionStatus(); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RotationStatusUpdaterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RotationStatusUpdaterTest.java index 4ef77573f93..903b1378438 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RotationStatusUpdaterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RotationStatusUpdaterTest.java @@ -3,8 +3,6 @@ package com.yahoo.vespa.hosted.controller.maintenance; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.zone.ZoneId; -import com.yahoo.vespa.flags.Flags; -import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.api.integration.routing.RotationStatus; import com.yahoo.vespa.hosted.controller.application.Deployment; @@ -27,7 +25,6 @@ public class RotationStatusUpdaterTest { @Test public void updates_rotation_status() { var tester = new DeploymentTester(); - ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.MULTIPLE_GLOBAL_ENDPOINTS.id(), true); var globalRotationService = tester.controllerTester().globalRoutingService(); var updater = new RotationStatusUpdater(tester.controller(), Duration.ofDays(1), new JobControl(tester.controller().curator())); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index b7755d01f75..363522700d1 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -21,8 +21,6 @@ import com.yahoo.vespa.athenz.api.AthenzIdentity; import com.yahoo.vespa.athenz.api.AthenzUser; import com.yahoo.vespa.athenz.api.OktaAccessToken; import com.yahoo.vespa.config.SlimeUtils; -import com.yahoo.vespa.flags.Flags; -import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.LockedTenant; import com.yahoo.vespa.hosted.controller.api.application.v4.EnvironmentResource; @@ -763,7 +761,6 @@ public class ApplicationApiTest extends ControllerContainerTest { @Test public void multiple_endpoints() { // Setup - ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.MULTIPLE_GLOBAL_ENDPOINTS.id(), true); tester.computeVersionStatus(); createAthenzDomainWithAdmin(ATHENZ_TENANT_DOMAIN, USER_ID); ApplicationPackage applicationPackage = new ApplicationPackageBuilder() diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java index a9fdaac6ee7..7a57ebb65dc 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java @@ -331,6 +331,37 @@ public class VersionStatusTest { .keySet().contains(version0)); } + @Test + public void testCommitDetailsPreservation() { + var tester = new DeploymentTester(); + + // Commit details are set for initial version + var version0 = new Version("6.2"); + var commitSha0 = "badc0ffee"; + var commitDate0 = Instant.EPOCH; + tester.upgradeSystem(version0); + assertEquals(version0, tester.controller().versionStatus().systemVersion().get().versionNumber()); + assertEquals(commitSha0, tester.controller().versionStatus().systemVersion().get().releaseCommit()); + assertEquals(commitDate0, tester.controller().versionStatus().systemVersion().get().committedAt()); + + // Deploy app on version0 to keep computing statistics for that version + tester.createAndDeploy("app", 1, "canary"); + + // Commit details are updated for new version + var version1 = new Version("6.3"); + var commitSha1 = "deadbeef"; + var commitDate1 = Instant.ofEpochMilli(123); + tester.upgradeController(version1, commitSha1, commitDate1); + tester.upgradeSystemApplications(version1); + assertEquals(version1, tester.controller().versionStatus().systemVersion().get().versionNumber()); + assertEquals(commitSha1, tester.controller().versionStatus().systemVersion().get().releaseCommit()); + assertEquals(commitDate1, tester.controller().versionStatus().systemVersion().get().committedAt()); + + // Commit details for previous version are preserved + assertEquals(commitSha0, tester.controller().versionStatus().version(version0).releaseCommit()); + assertEquals(commitDate0, tester.controller().versionStatus().version(version0).committedAt()); + } + private static void writeControllerVersion(HostName hostname, Version version, CuratorDb db) { db.writeControllerVersion(hostname, new ControllerVersion(version, "badc0ffee", Instant.EPOCH)); } 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 cd5045054e2..7dcdda38765 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -145,12 +145,6 @@ public class Flags { "Takes effect on deployment through controller", APPLICATION_ID); - public static final UnboundBooleanFlag MULTIPLE_GLOBAL_ENDPOINTS = defineFeatureFlag( - "multiple-global-endpoints", false, - "Allow applications to use new endpoints syntax in deployment.xml", - "Takes effect on deployment through controller", - APPLICATION_ID); - public static final UnboundBooleanFlag ENABLE_GROUPING_SESSION_CACHE = defineFeatureFlag( "enable-grouping-session-cache", true, "Enable grouping session cache", diff --git a/node-admin/scripts/etc-hosts.sh b/node-admin/scripts/etc-hosts.sh deleted file mode 100755 index 68d882da22a..00000000000 --- a/node-admin/scripts/etc-hosts.sh +++ /dev/null @@ -1,106 +0,0 @@ -#!/bin/bash -# Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -set -e - -declare -r NETWORK_PREFIX=172.18 -declare -r CONFIG_SERVER_HOSTNAME=config-server -declare -r CONFIG_SERVER_IP="$NETWORK_PREFIX.1.1" - -declare -r APP_HOSTNAME_PREFIX=cnode- -declare -r APP_NETWORK_PREFIX="$NETWORK_PREFIX.2" -declare -r NUM_APP_CONTAINERS=20 # Statically allocated number of nodes. - -declare -r SYSTEM_TEST_HOSTNAME_PREFIX=stest- -declare -r SYSTEM_TEST_NETWORK_PREFIX="$NETWORK_PREFIX.3" -declare -r NUM_SYSTEM_TEST_CONTAINERS=5 # Statically allocated number of nodes. - -declare -r HOSTS_FILE=/etc/hosts -declare -r HOSTS_LINE_SUFFIX=" # Managed by etc-hosts.sh" - -function IsInHostsAlready { - local ip="$1" - local hostname="$2" - local file="$3" - - # TODO: Escape $ip to make sure it's matched as a literal in the regex. - local matching_ip_line - matching_ip_line=$(grep -E "^$ip[ \\t]" "$file") - - local -i num_ip_lines=0 - # This 'if' is needed because wc -l <<< "" is 1. - if [ -n "$matching_ip_line" ] - then - num_ip_lines=$(wc -l <<< "$matching_ip_line") - fi - - local matching_hostname_line - matching_hostname_line=$(grep -E "^[^#]*[ \\t]$hostname(\$|[ \\t])" "$file") - - local -i num_hostname_lines=0 - # This 'if' is needed because wc -l <<< "" is 1. - if [ -n "$matching_hostname_line" ] - then - num_hostname_lines=$(wc -l <<< "$matching_hostname_line") - fi - - if ((num_ip_lines == 1)) && ((num_hostname_lines == 1)) && - [ "$matching_ip_line" == "$matching_hostname_line" ] - then - return 0 - elif ((num_ip_lines == 0)) && ((num_hostname_lines == 0)) - then - return 1 - else - printf "$file contains a conflicting host specification for $hostname/$ip" - exit 1 - fi -} - -function AddHost { - local ip="$1" - local hostname="$2" - local file="$3" - - if IsInHostsAlready "$ip" "$hostname" "$file" - then - return - fi - - echo -n "Adding host $hostname ($ip) to $file... " - printf "%-11s %s%s\n" "$ip" "$hostname" "$HOSTS_LINE_SUFFIX" >> "$file" - echo done -} - -function Stop { - # TODO: Remove entries. - : -} - -function StartAsRoot { - # May need sudo - if [ ! -w "$HOSTS_FILE" ] - then - Fail "$HOSTS_FILE is not writeable (run script with sudo)" - fi - - AddHost "$CONFIG_SERVER_IP" "$CONFIG_SERVER_HOSTNAME" "$HOSTS_FILE" - - local -i index=1 - for ((; index <= NUM_APP_CONTAINERS; ++index)) - do - local ip="$APP_NETWORK_PREFIX.$index" - local container_name="$APP_HOSTNAME_PREFIX$index" - AddHost "$ip" "$container_name" "$HOSTS_FILE" - done - - local -i index=1 - for ((; index <= NUM_SYSTEM_TEST_CONTAINERS; ++index)) - do - local ip="$SYSTEM_TEST_NETWORK_PREFIX.$index" - local container_name="$SYSTEM_TEST_HOSTNAME_PREFIX$index" - AddHost "$ip" "$container_name" "$HOSTS_FILE" - done -} - -StartAsRoot "$@" diff --git a/node-admin/scripts/node-repo.sh b/node-admin/scripts/node-repo.sh deleted file mode 100755 index 82f99f28e72..00000000000 --- a/node-admin/scripts/node-repo.sh +++ /dev/null @@ -1,360 +0,0 @@ -#!/bin/bash -# Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -set -e - -declare -r VESPA_WEB_SERVICE_PORT=4080 - -# Output from InnerCurlNodeRepo, see there for details. -declare CURL_RESPONSE - -function Usage { - cat <<EOF -Usage: ${0##*/} <command> [<args>...] -Script for manipulating the Node Repository. - -Commands - add [-c <configserverhost>] -p <parent-hostname> [-f <parent-flavor>] - [-n <flavor> <hostname>...] - With -f, provision "host" node <parent-hostname> with flavor - <parent-flavor>. With -n, provision "tenant" nodes <hostname...> with - flavor <flavor> and parent host <parenthostname>. - reprovision [-c <configserverhost>] -p <parenthostname> <hostname>... - Fail node <hostname>, then rm and add. - rm [-c <configserverhost>] <hostname>... - Remove nodes from node repo. - set-state [-c <configserverhost>] <state> <hostname>... - Set the node repo state. - -By default, <configserverhost> is config-server. - -Example - To remove docker-1--1 through docker-1--5 from the node repo at configserver.com, - - ${0##*/} rm -c configserver.com \ - docker-1--{1,2,3,4,5}.dockerhosts.com -EOF - - exit 1 -} - -function Fail { - printf "%s\nRun '${0##*/} help' for usage\n" "$*" - exit 1 -} - -# Invoke curl such that: -# -# - Arguments to this function are passed to curl. -# -# - Additional arguments are passed to curl to filter noise (--silent -# --show-error). -# -# - The curl stdout (i.e. the response) is stored in the global CURL_RESPONSE -# variable on return of the function. -# -# - If curl returns 0 (i.e. a successful HTTP response) with a JSON response -# that contains an "error-code" key, this function will instead return 22. 22 -# does not conflict with a curl return code, because curl only ever returns -# 22 when --fail is specified. -# -# Except, if the JSON response contains a "message" of the form "Cannot add -# tmp-cnode-0: A node with this name already exists", InnerCurlNodeRepo -# returns 0 instead of 22, even if the HTTP response status code is an error. -# -# Note: Why not use --fail with curl? Because as of 2015-11-24, if the node -# exists when provisioning a node, the node repo returns a 400 Bad Request -# with a JSON body containing a "message" field as described above. With -# --fail, this would result in curl exiting with code 22, which is -# indistinguishable from other HTTP errors. Can the output from --show-error -# be used in combination with --fail? No, because that ends up saying "curl: -# (22) The requested URL returned error: 400 Bad Request" when the node -# exists, making it indistinguishable from other malformed request error -# messages. -# -# TODO: Make node repo return a unique HTTP error code when node already -# exists. It's also fragile to test for the error message in the response. -function InnerCurlNodeRepo { - # --show-error causes error message to be printed on error, even with - # --silent, which is useful when we print the error message to Fail. - local -a command=(curl --silent --show-error "$@") - - # We need the 'if' here, because a non-zero exit code of a command will - # exit the process, with 'set -e'. - if CURL_RESPONSE=$("${command[@]}" 2>&1) - then - # Match a JSON of the form: - # { - # "error-code": "BAD_REQUEST", - # "message": "Cannot add cnode-0: A node with this name already exists" - # } - if [[ "$CURL_RESPONSE" =~ '"error-code"' ]] - then - if [[ "$CURL_RESPONSE" =~ '"message"'[^\"]*\"(.*)\" ]] - then - local message="${BASH_REMATCH[1]}" - if [[ "$message" =~ 'already exists' ]] - then - return 0 - fi - fi - - return 22 - fi - - return 0 - else - # Do not move this statement outside of this else: $? gets cleared when - # the execution passes out of the else-fi block. - return $? - fi -} - -function CurlOrFail { - if InnerCurlNodeRepo "$@" - then - : # This form of if-else is used to preserve $?. - else - local error_code=$? - - # Terminate the current progress-bar-like line - printf ' failed\n' - - Fail "Error ($error_code) from the node repo at '$url': '$CURL_RESPONSE'" - fi -} - -function ProvisionDockerNode { - local config_server_hostname="$1" - local container_hostname="$2" - local parent_hostname="$3" - local flavor="$4" - - local json="[ - { - \"hostname\":\"$container_hostname\", - \"parentHostname\":\"$parent_hostname\", - \"openStackId\":\"fake-$container_hostname\", - \"flavor\":\"$flavor\", - \"type\":\"tenant\" - } - ]" - - ProvisionNode $config_server_hostname "$json" -} - - -# Docker host, the docker nodes points to this host in parentHostname in their node config -function ProvisionDockerHost { - local config_server_hostname="$1" - local docker_host_hostname="$2" - local flavor="$3" - - local json="[ - { - \"hostname\":\"$docker_host_hostname\", - \"openStackId\":\"$docker_host_hostname\", - \"flavor\":\"$flavor\", - \"type\":\"host\" - } - ]" - - ProvisionNode $config_server_hostname "$json" -} - -# Docker node in node repo (both docker hosts and docker nodes) -function ProvisionNode { - local config_server_hostname="$1" - local json="$2" - - local url="http://$config_server_hostname:$VESPA_WEB_SERVICE_PORT/nodes/v2/node" - - CurlOrFail -H "Content-Type: application/json" -X POST -d "$json" "$url" -} - -function SetNodeState { - local config_server_hostname="$1" - local hostname="$2" - local state="$3" - - local url="http://$config_server_hostname:$VESPA_WEB_SERVICE_PORT/nodes/v2/state/$state/$hostname" - CurlOrFail -X PUT "$url" -} - -function AddCommand { - local config_server_hostname=config-server - local parent_hostname= - - OPTIND=1 - local option - while getopts "c:p:f:n:" option - do - case "$option" in - c) config_server_hostname="$OPTARG" ;; - p) parent_hostname="$OPTARG" ;; - f) parent_host_flavor="$OPTARG" ;; - n) node_flavor="$OPTARG" ;; - ?) exit 1 ;; # E.g. option lacks argument, in case error has been - # already been printed - *) Fail "Unknown option '$option' with value '$OPTARG'" - esac - done - - if [ -z "$parent_hostname" ] - then - Fail "Parent hostname not specified (-p)" - fi - - shift $((OPTIND - 1)) - - if [ -n "$parent_host_flavor" ] - then - echo "Provisioning Docker host $parent_hostname with flavor $parent_host_flavor" - ProvisionDockerHost "$config_server_hostname" \ - "$parent_hostname" \ - "$parent_host_flavor" - fi - - if [ -n "$node_flavor" ] - then - echo -n "Provisioning $# nodes with parent host $parent_hostname" - local container_hostname - for container_hostname in "$@" - do - ProvisionDockerNode "$config_server_hostname" \ - "$container_hostname" \ - "$parent_hostname" \ - "$node_flavor" - echo -n . - done - - echo " done" - fi -} - -function ReprovisionCommand { - local config_server_hostname=config-server - local parent_hostname= - - OPTIND=1 - local option - while getopts "c:p:" option - do - case "$option" in - c) config_server_hostname="$OPTARG" ;; - p) parent_hostname="$OPTARG" ;; - ?) exit 1 ;; # E.g. option lacks argument, in case error has been - # already been printed - *) Fail "Unknown option '$option' with value '$OPTARG'" - esac - done - - if [ -z "$parent_hostname" ] - then - Fail "Parent hostname not specified (-p)" - fi - - shift $((OPTIND - 1)) - - if (($# == 0)) - then - Fail "No node hostnames were specified" - fi - - # Simulate calls to the following commands. - SetStateCommand -c "$config_server_hostname" failed "$@" - RemoveCommand -c "$config_server_hostname" "$@" - AddCommand -c "$config_server_hostname" -p "$parent_hostname" "$@" -} - -function RemoveCommand { - local config_server_hostname=config-server - - OPTIND=1 - local option - while getopts "c:" option - do - case "$option" in - c) config_server_hostname="$OPTARG" ;; - ?) exit 1 ;; # E.g. option lacks argument, in case error has been - # already been printed - *) Fail "Unknown option '$option' with value '$OPTARG'" - esac - done - - shift $((OPTIND - 1)) - - if (($# == 0)) - then - Fail "No nodes were specified" - fi - - echo -n "Removing $# nodes" - - local hostname - for hostname in "$@" - do - local url="http://$config_server_hostname:$VESPA_WEB_SERVICE_PORT/nodes/v2/node/$hostname" - CurlOrFail -X DELETE "$url" - echo -n . - done - - echo " done" -} - -function SetStateCommand { - local config_server_hostname=config-server - - OPTIND=1 - local option - while getopts "c:" option - do - case "$option" in - c) config_server_hostname="$OPTARG" ;; - ?) exit 1 ;; # E.g. option lacks argument, in case error has been - # already been printed - *) Fail "Unknown option '$option' with value '$OPTARG'" - esac - done - - shift $((OPTIND - 1)) - - if (($# <= 1)) - then - Fail "Too few arguments" - fi - - local state="$1" - shift - - echo -n "Setting $# nodes to $state" - - local hostname - for hostname in "$@" - do - SetNodeState "$config_server_hostname" "$hostname" "$state" - echo -n . - done - - echo " done" -} - -function Main { - if (($# == 0)) - then - Usage - fi - local command="$1" - shift - - case "$command" in - add) AddCommand "$@" ;; - reprovision) ReprovisionCommand "$@" ;; - rm) RemoveCommand "$@" ;; - set-state) SetStateCommand "$@" ;; - help) Usage "$@" ;; - *) Usage ;; - esac -} - -Main "$@" diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/AddNode.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/AddNode.java index b83bd9895fc..233fe8318ae 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/AddNode.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/AddNode.java @@ -1,9 +1,10 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.configserver.noderepository; +import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.host.FlavorOverrides; -import java.util.Collections; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -15,29 +16,29 @@ public class AddNode { public final String hostname; public final Optional<String> parentHostname; - public final String nodeFlavor; + public final Optional<String> nodeFlavor; + public final Optional<FlavorOverrides> flavorOverrides; + public final Optional<NodeResources> nodeResources; public final NodeType nodeType; public final Set<String> ipAddresses; public final Set<String> additionalIpAddresses; - /** - * Constructor for a host node (has no parent) - */ - public AddNode(String hostname, String nodeFlavor, NodeType nodeType, Set<String> ipAddresses, Set<String> additionalIpAddresses) { - this(hostname, Optional.empty(), nodeFlavor, nodeType, ipAddresses, additionalIpAddresses); + public static AddNode forHost(String hostname, String nodeFlavor, Optional<FlavorOverrides> flavorOverrides, NodeType nodeType, Set<String> ipAddresses, Set<String> additionalIpAddresses) { + return new AddNode(hostname, Optional.empty(), Optional.of(nodeFlavor), flavorOverrides, Optional.empty(), nodeType, ipAddresses, additionalIpAddresses); } - /** - * Constructor for a child node (Must set parentHostname, no additionalIpAddresses) - */ - public AddNode(String hostname, String parentHostname, String nodeFlavor, NodeType nodeType, Set<String> ipAddresses) { - this(hostname, Optional.of(parentHostname), nodeFlavor, nodeType, ipAddresses, Collections.emptySet()); + public static AddNode forNode(String hostname, String parentHostname, NodeResources nodeResources, NodeType nodeType, Set<String> ipAddresses) { + return new AddNode(hostname, Optional.of(parentHostname), Optional.empty(), Optional.empty(), Optional.of(nodeResources), nodeType, ipAddresses, Set.of()); } - public AddNode(String hostname, Optional<String> parentHostname, String nodeFlavor, NodeType nodeType, Set<String> ipAddresses, Set<String> additionalIpAddresses) { + private AddNode(String hostname, Optional<String> parentHostname, + Optional<String> nodeFlavor, Optional<FlavorOverrides> flavorOverrides, Optional<NodeResources> nodeResources, + NodeType nodeType, Set<String> ipAddresses, Set<String> additionalIpAddresses) { this.hostname = hostname; this.parentHostname = parentHostname; this.nodeFlavor = nodeFlavor; + this.flavorOverrides = flavorOverrides; + this.nodeResources = nodeResources; this.nodeType = nodeType; this.ipAddresses = ipAddresses; this.additionalIpAddresses = additionalIpAddresses; diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeOwner.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeOwner.java deleted file mode 100644 index c41e050d534..00000000000 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeOwner.java +++ /dev/null @@ -1,64 +0,0 @@ -// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.node.admin.configserver.noderepository; - -import com.yahoo.config.provision.ApplicationId; - -/** - * @author freva - */ -public class NodeOwner { - private final String tenant; - private final String application; - private final String instance; - - public NodeOwner(String tenant, String application, String instance) { - this.tenant = tenant; - this.application = application; - this.instance = instance; - } - - public String tenant() { - return tenant; - } - - public String application() { - return application; - } - - public String instance() { - return instance; - } - - public ApplicationId asApplicationId() { - return ApplicationId.from(tenant, application, instance); - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - - NodeOwner owner = (NodeOwner) o; - - if (!tenant.equals(owner.tenant)) return false; - if (!application.equals(owner.application)) return false; - return instance.equals(owner.instance); - - } - - @Override - public int hashCode() { - int result = tenant.hashCode(); - result = 31 * result + application.hashCode(); - result = 31 * result + instance.hashCode(); - return result; - } - - public String toString() { - return "Owner {" + - " tenant = " + tenant + - " application = " + application + - " instance = " + instance + - " }"; - } -} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java index 6fb6d44bd6f..c2e68bdb329 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java @@ -3,7 +3,9 @@ package com.yahoo.vespa.hosted.node.admin.configserver.noderepository; import com.fasterxml.jackson.databind.JsonNode; import com.yahoo.component.Version; +import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.DockerImage; +import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import java.time.Instant; @@ -11,6 +13,9 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; +import static com.yahoo.config.provision.NodeResources.DiskSpeed.fast; +import static com.yahoo.config.provision.NodeResources.DiskSpeed.slow; + /** * @author stiankri */ @@ -19,7 +24,6 @@ public class NodeSpec { private final NodeState state; private final NodeType type; private final String flavor; - private final String canonicalFlavor; private final Optional<DockerImage> wantedDockerImage; private final Optional<DockerImage> currentDockerImage; @@ -43,15 +47,10 @@ public class NodeSpec { private final Optional<Boolean> allowedToBeDown; private final Optional<Boolean> wantToDeprovision; - private final Optional<NodeOwner> owner; + private final Optional<ApplicationId> owner; private final Optional<NodeMembership> membership; - private final double vcpus; - private final double memoryGb; - private final double diskGb; - - private final boolean fastDisk; - private final double bandwidth; + private final NodeResources resources; private final Set<String> ipAddresses; private final Set<String> additionalIpAddresses; @@ -66,14 +65,13 @@ public class NodeSpec { NodeState state, NodeType type, String flavor, - String canonicalFlavor, Optional<Version> wantedVespaVersion, Optional<Version> currentVespaVersion, Optional<Version> wantedOsVersion, Optional<Version> currentOsVersion, Optional<Boolean> allowedToBeDown, Optional<Boolean> wantToDeprovision, - Optional<NodeOwner> owner, + Optional<ApplicationId> owner, Optional<NodeMembership> membership, Optional<Long> wantedRestartGeneration, Optional<Long> currentRestartGeneration, @@ -82,11 +80,7 @@ public class NodeSpec { Optional<Instant> wantedFirmwareCheck, Optional<Instant> currentFirmwareCheck, Optional<String> modelName, - double vcpus, - double memoryGb, - double diskGb, - boolean fastDisk, - double bandwidth, + NodeResources resources, Set<String> ipAddresses, Set<String> additionalIpAddresses, NodeReports reports, @@ -104,7 +98,6 @@ public class NodeSpec { this.state = Objects.requireNonNull(state); this.type = Objects.requireNonNull(type); this.flavor = Objects.requireNonNull(flavor); - this.canonicalFlavor = canonicalFlavor; this.modelName = modelName; this.wantedVespaVersion = Objects.requireNonNull(wantedVespaVersion); this.currentVespaVersion = Objects.requireNonNull(currentVespaVersion); @@ -120,11 +113,7 @@ public class NodeSpec { this.currentRebootGeneration = currentRebootGeneration; this.wantedFirmwareCheck = Objects.requireNonNull(wantedFirmwareCheck); this.currentFirmwareCheck = Objects.requireNonNull(currentFirmwareCheck); - this.vcpus = vcpus; - this.memoryGb = memoryGb; - this.diskGb = diskGb; - this.fastDisk = fastDisk; - this.bandwidth = bandwidth; + this.resources = Objects.requireNonNull(resources); this.ipAddresses = Objects.requireNonNull(ipAddresses); this.additionalIpAddresses = Objects.requireNonNull(additionalIpAddresses); this.reports = Objects.requireNonNull(reports); @@ -147,10 +136,6 @@ public class NodeSpec { return flavor; } - public String canonicalFlavor() { - return canonicalFlavor; - } - public Optional<DockerImage> wantedDockerImage() { return wantedDockerImage; } @@ -211,7 +196,7 @@ public class NodeSpec { return wantToDeprovision; } - public Optional<NodeOwner> owner() { + public Optional<ApplicationId> owner() { return owner; } @@ -219,24 +204,28 @@ public class NodeSpec { return membership; } + public NodeResources resources() { + return resources; + } + public double vcpus() { - return vcpus; + return resources.vcpu(); } public double memoryGb() { - return memoryGb; + return resources.memoryGb(); } public double diskGb() { - return diskGb; + return resources.diskGb(); } public boolean isFastDisk() { - return fastDisk; + return resources.diskSpeed() == fast; } - public double bandwidth() { - return bandwidth; + public double bandwidthGbps() { + return resources.bandwidthGbps(); } public Set<String> ipAddresses() { @@ -266,7 +255,6 @@ public class NodeSpec { Objects.equals(state, that.state) && Objects.equals(type, that.type) && Objects.equals(flavor, that.flavor) && - Objects.equals(canonicalFlavor, that.canonicalFlavor) && Objects.equals(wantedVespaVersion, that.wantedVespaVersion) && Objects.equals(currentVespaVersion, that.currentVespaVersion) && Objects.equals(wantedOsVersion, that.wantedOsVersion) && @@ -281,11 +269,7 @@ public class NodeSpec { Objects.equals(currentRebootGeneration, that.currentRebootGeneration) && Objects.equals(wantedFirmwareCheck, that.wantedFirmwareCheck) && Objects.equals(currentFirmwareCheck, that.currentFirmwareCheck) && - Objects.equals(vcpus, that.vcpus) && - Objects.equals(memoryGb, that.memoryGb) && - Objects.equals(diskGb, that.diskGb) && - Objects.equals(fastDisk, that.fastDisk) && - Objects.equals(bandwidth, that.bandwidth) && + Objects.equals(resources, that.resources) && Objects.equals(ipAddresses, that.ipAddresses) && Objects.equals(additionalIpAddresses, that.additionalIpAddresses) && Objects.equals(reports, that.reports) && @@ -301,7 +285,6 @@ public class NodeSpec { state, type, flavor, - canonicalFlavor, wantedVespaVersion, currentVespaVersion, wantedOsVersion, @@ -316,11 +299,7 @@ public class NodeSpec { currentRebootGeneration, wantedFirmwareCheck, currentFirmwareCheck, - vcpus, - memoryGb, - diskGb, - fastDisk, - bandwidth, + resources, ipAddresses, additionalIpAddresses, reports, @@ -336,7 +315,6 @@ public class NodeSpec { + " state=" + state + " type=" + type + " flavor=" + flavor - + " canonicalFlavor=" + canonicalFlavor + " wantedVespaVersion=" + wantedVespaVersion + " currentVespaVersion=" + currentVespaVersion + " wantedOsVersion=" + wantedOsVersion @@ -345,17 +323,13 @@ public class NodeSpec { + " wantToDeprovision=" + wantToDeprovision + " owner=" + owner + " membership=" + membership - + " vcpus=" + vcpus + " wantedRestartGeneration=" + wantedRestartGeneration + " currentRestartGeneration=" + currentRestartGeneration + " wantedRebootGeneration=" + wantedRebootGeneration + " currentRebootGeneration=" + currentRebootGeneration + " wantedFirmwareCheck=" + wantedFirmwareCheck + " currentFirmwareCheck=" + currentFirmwareCheck - + " memoryGb=" + memoryGb - + " diskGb=" + diskGb - + " fastDisk=" + fastDisk - + " bandwidth=" + bandwidth + + " resources=" + resources + " ipAddresses=" + ipAddresses + " additionalIpAddresses=" + additionalIpAddresses + " reports=" + reports @@ -368,7 +342,6 @@ public class NodeSpec { private NodeState state; private NodeType type; private String flavor; - private String canonicalFlavor; private Optional<DockerImage> wantedDockerImage = Optional.empty(); private Optional<DockerImage> currentDockerImage = Optional.empty(); private Optional<Version> wantedVespaVersion = Optional.empty(); @@ -377,7 +350,7 @@ public class NodeSpec { private Optional<Version> currentOsVersion = Optional.empty(); private Optional<Boolean> allowedToBeDown = Optional.empty(); private Optional<Boolean> wantToDeprovision = Optional.empty(); - private Optional<NodeOwner> owner = Optional.empty(); + private Optional<ApplicationId> owner = Optional.empty(); private Optional<NodeMembership> membership = Optional.empty(); private Optional<Long> wantedRestartGeneration = Optional.empty(); private Optional<Long> currentRestartGeneration = Optional.empty(); @@ -386,11 +359,7 @@ public class NodeSpec { private Optional<Instant> wantedFirmwareCheck = Optional.empty(); private Optional<Instant> currentFirmwareCheck = Optional.empty(); private Optional<String> modelName = Optional.empty(); - private double vcpus; - private double memoryGb; - private double diskGb; - private boolean fastDisk; - private double bandwidth; + private NodeResources resources = new NodeResources(0, 0, 0, 0, slow); private Set<String> ipAddresses = Set.of(); private Set<String> additionalIpAddresses = Set.of(); private NodeReports reports = new NodeReports(); @@ -403,12 +372,7 @@ public class NodeSpec { state(node.state); type(node.type); flavor(node.flavor); - canonicalFlavor(node.canonicalFlavor); - vcpus(node.vcpus); - memoryGb(node.memoryGb); - diskGb(node.diskGb); - fastDisk(node.fastDisk); - bandwidth(node.bandwidth); + resources(node.resources); ipAddresses(node.ipAddresses); additionalIpAddresses(node.additionalIpAddresses); wantedRebootGeneration(node.wantedRebootGeneration); @@ -462,11 +426,6 @@ public class NodeSpec { return this; } - public Builder canonicalFlavor(String canonicalFlavor) { - this.canonicalFlavor = canonicalFlavor; - return this; - } - public Builder wantedVespaVersion(Version wantedVespaVersion) { this.wantedVespaVersion = Optional.of(wantedVespaVersion); return this; @@ -497,7 +456,7 @@ public class NodeSpec { return this; } - public Builder owner(NodeOwner owner) { + public Builder owner(ApplicationId owner) { this.owner = Optional.of(owner); return this; } @@ -537,29 +496,29 @@ public class NodeSpec { return this; } - public Builder vcpus(double minCpuCores) { - this.vcpus = minCpuCores; + public Builder resources(NodeResources resources) { + this.resources = resources; return this; } - public Builder memoryGb(double minMainMemoryAvailableGb) { - this.memoryGb = minMainMemoryAvailableGb; - return this; + public Builder vcpus(double vcpus) { + return resources(resources.withVcpu(vcpus)); } - public Builder diskGb(double minDiskAvailableGb) { - this.diskGb = minDiskAvailableGb; - return this; + public Builder memoryGb(double memoryGb) { + return resources(resources.withMemoryGb(memoryGb)); + } + + public Builder diskGb(double diskGb) { + return resources(resources.withDiskGb(diskGb)); } public Builder fastDisk(boolean fastDisk) { - this.fastDisk = fastDisk; - return this; + return resources(resources.withDiskSpeed(fastDisk ? fast : slow)); } - public Builder bandwidth(double bandwidth) { - this.bandwidth = bandwidth; - return this; + public Builder bandwidthGbps(double bandwidthGbps) { + return resources(resources.withBandwidthGbps(bandwidthGbps)); } public Builder ipAddresses(Set<String> ipAddresses) { @@ -627,10 +586,6 @@ public class NodeSpec { return flavor; } - public String canonicalFlavor() { - return canonicalFlavor; - } - public Optional<Version> wantedVespaVersion() { return wantedVespaVersion; } @@ -655,7 +610,7 @@ public class NodeSpec { return wantToDeprovision; } - public Optional<NodeOwner> owner() { + public Optional<ApplicationId> owner() { return owner; } @@ -679,24 +634,8 @@ public class NodeSpec { return currentRebootGeneration; } - public double vcpus() { - return vcpus; - } - - public double memoryGb() { - return memoryGb; - } - - public double diskGb() { - return diskGb; - } - - public boolean isFastDisk() { - return fastDisk; - } - - public double bandwidth() { - return bandwidth; + public NodeResources resources() { + return resources; } public Set<String> ipAddresses() { @@ -716,15 +655,13 @@ public class NodeSpec { } public NodeSpec build() { - return new NodeSpec(hostname, wantedDockerImage, currentDockerImage, state, type, - flavor, canonicalFlavor, + return new NodeSpec(hostname, wantedDockerImage, currentDockerImage, state, type, flavor, wantedVespaVersion, currentVespaVersion, wantedOsVersion, currentOsVersion, allowedToBeDown, wantToDeprovision, owner, membership, wantedRestartGeneration, currentRestartGeneration, wantedRebootGeneration, currentRebootGeneration, wantedFirmwareCheck, currentFirmwareCheck, modelName, - vcpus, memoryGb, diskGb, - fastDisk, bandwidth, ipAddresses, additionalIpAddresses, + resources, ipAddresses, additionalIpAddresses, reports, parentHostname); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java index fe19b81614d..582ba3dd09c 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java @@ -4,8 +4,11 @@ package com.yahoo.vespa.hosted.node.admin.configserver.noderepository; import com.fasterxml.jackson.databind.JsonNode; import com.google.common.base.Strings; import com.yahoo.component.Version; +import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.DockerImage; +import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.host.FlavorOverrides; import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerApi; import com.yahoo.vespa.hosted.node.admin.configserver.HttpException; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.bindings.GetAclResponse; @@ -25,6 +28,9 @@ import java.util.logging.Logger; import java.util.stream.Collectors; import java.util.stream.Stream; +import static com.yahoo.config.provision.NodeResources.DiskSpeed.fast; +import static com.yahoo.config.provision.NodeResources.DiskSpeed.slow; + /** * @author stiankri, dybis */ @@ -154,14 +160,13 @@ public class RealNodeRepository implements NodeRepository { nodeState, nodeType, node.flavor, - node.canonicalFlavor, Optional.ofNullable(node.wantedVespaVersion).map(Version::fromString), Optional.ofNullable(node.vespaVersion).map(Version::fromString), Optional.ofNullable(node.wantedOsVersion).map(Version::fromString), Optional.ofNullable(node.currentOsVersion).map(Version::fromString), Optional.ofNullable(node.allowedToBeDown), Optional.ofNullable(node.wantToDeprovision), - Optional.ofNullable(node.owner).map(o -> new NodeOwner(o.tenant, o.application, o.instance)), + Optional.ofNullable(node.owner).map(o -> ApplicationId.from(o.tenant, o.application, o.instance)), membership, Optional.ofNullable(node.restartGeneration), Optional.ofNullable(node.currentRestartGeneration), @@ -170,11 +175,12 @@ public class RealNodeRepository implements NodeRepository { Optional.ofNullable(node.wantedFirmwareCheck).map(Instant::ofEpochMilli), Optional.ofNullable(node.currentFirmwareCheck).map(Instant::ofEpochMilli), Optional.ofNullable(node.modelName), - node.minCpuCores, - node.minMainMemoryAvailableGb, - node.minDiskAvailableGb, - node.fastDisk, - node.bandwidth, + new NodeResources( + node.minCpuCores, + node.minMainMemoryAvailableGb, + node.minDiskAvailableGb, + node.bandwidth / 1000, + node.fastDisk ? fast : slow), node.ipAddresses, node.additionalIpAddresses, reports, @@ -186,7 +192,15 @@ public class RealNodeRepository implements NodeRepository { node.openStackId = "fake-" + addNode.hostname; node.hostname = addNode.hostname; node.parentHostname = addNode.parentHostname.orElse(null); - node.flavor = addNode.nodeFlavor; + addNode.nodeFlavor.ifPresent(f -> node.flavor = f); + addNode.flavorOverrides.flatMap(FlavorOverrides::diskGb).ifPresent(d -> node.minDiskAvailableGb = d); + addNode.nodeResources.ifPresent(resources -> { + node.minCpuCores = resources.vcpu(); + node.minMainMemoryAvailableGb = resources.memoryGb(); + node.minDiskAvailableGb = resources.diskGb(); + node.bandwidth = resources.bandwidthGbps() * 1000; + node.fastDisk = resources.diskSpeed() == NodeResources.DiskSpeed.fast; + }); node.type = addNode.nodeType.name(); node.ipAddresses = addNode.ipAddresses; node.additionalIpAddresses = addNode.additionalIpAddresses; diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java index 2b4f9277fc7..90d70a94144 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java @@ -28,8 +28,6 @@ public class NodeRepositoryNode { public String openStackId; @JsonProperty("flavor") public String flavor; - @JsonProperty("canonicalFlavor") - public String canonicalFlavor; @JsonProperty("membership") public Membership membership; @JsonProperty("owner") @@ -98,7 +96,6 @@ public class NodeRepositoryNode { ", openStackId='" + openStackId + '\'' + ", modelName='" + modelName + '\'' + ", flavor='" + flavor + '\'' + - ", canonicalFlavor='" + canonicalFlavor + '\'' + ", membership=" + membership + ", owner=" + owner + ", restartGeneration=" + restartGeneration + diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java index f4355ed3afa..91f4924cefa 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java @@ -141,9 +141,9 @@ public class StorageMaintainer { context.node().parentHostname().ifPresent(parent -> attributes.put("parent_hostname", parent)); context.node().currentVespaVersion().ifPresent(version -> attributes.put("vespa_version", version.toFullString())); context.node().owner().ifPresent(owner -> { - attributes.put("tenant", owner.tenant()); - attributes.put("application", owner.application()); - attributes.put("instance", owner.instance()); + attributes.put("tenant", owner.tenant().value()); + attributes.put("application", owner.application().value()); + attributes.put("instance", owner.instance().value()); }); return Collections.unmodifiableMap(attributes); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java index b7e7b97cdd8..161775b0702 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java @@ -14,7 +14,6 @@ import com.yahoo.vespa.hosted.dockerapi.ContainerResources; import com.yahoo.vespa.hosted.dockerapi.exception.ContainerNotFoundException; import com.yahoo.vespa.hosted.dockerapi.exception.DockerException; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeAttributes; -import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeOwner; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeState; @@ -345,7 +344,6 @@ public class NodeAgentImpl implements NodeAgent { double cpuCap = noCpuCap(context.zone()) ? 0 : context.node().owner() - .map(NodeOwner::asApplicationId) .map(appId -> containerCpuCap.with(FetchVector.Dimension.APPLICATION_ID, appId.serializedForm())) .orElse(containerCpuCap) .with(FetchVector.Dimension.HOSTNAME, context.node().hostname()) diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java index 0938eb23b49..e30572fc63e 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java @@ -1,11 +1,12 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - package com.yahoo.vespa.hosted.node.admin.configserver.noderepository; import com.yahoo.application.Networking; import com.yahoo.application.container.JDisc; import com.yahoo.config.provision.DockerImage; +import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.host.FlavorOverrides; import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerApi; import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerApiImpl; import com.yahoo.vespa.hosted.provision.testutils.ContainerConfig; @@ -17,11 +18,10 @@ import java.io.IOException; import java.net.ServerSocket; import java.net.URI; import java.time.Instant; -import java.util.Arrays; import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.Optional; +import java.util.Set; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; @@ -162,23 +162,22 @@ public class RealNodeRepositoryTest { @Test public void testAddNodes() { - AddNode host = new AddNode("host123.domain.tld", "default", NodeType.confighost, - Collections.singleton("::1"), new HashSet<>(Arrays.asList("::2", "::3"))); - - AddNode node = new AddNode("host123-1.domain.tld", "host123.domain.tld", "docker", NodeType.config, - new HashSet<>(Arrays.asList("::2", "::3"))); + AddNode host = AddNode.forHost("host123.domain.tld", "default", Optional.of(FlavorOverrides.ofDisk(123)), NodeType.confighost, + Set.of("::1"), Set.of("::2", "::3")); - List<AddNode> nodesToAdd = Arrays.asList(host, node); + NodeResources nodeResources = new NodeResources(1, 2, 3, 4, NodeResources.DiskSpeed.slow); + AddNode node = AddNode.forNode("host123-1.domain.tld", "host123.domain.tld", nodeResources, NodeType.config, Set.of("::2", "::3")); assertFalse(nodeRepositoryApi.getOptionalNode("host123.domain.tld").isPresent()); - nodeRepositoryApi.addNodes(nodesToAdd); - - NodeSpec hostSpecInNodeRepo = nodeRepositoryApi.getOptionalNode("host123.domain.tld") - .orElseThrow(RuntimeException::new); + nodeRepositoryApi.addNodes(List.of(host, node)); - assertEquals(host.nodeFlavor, hostSpecInNodeRepo.flavor()); - assertEquals(host.nodeType, hostSpecInNodeRepo.type()); + NodeSpec hostSpec = nodeRepositoryApi.getOptionalNode("host123.domain.tld").orElseThrow(); + assertEquals("default", hostSpec.flavor()); + assertEquals(123, hostSpec.diskGb(), 0); + assertEquals(NodeType.confighost, hostSpec.type()); - assertTrue(nodeRepositoryApi.getOptionalNode("host123-1.domain.tld").isPresent()); + NodeSpec nodeSpec = nodeRepositoryApi.getOptionalNode("host123-1.domain.tld").orElseThrow(); + assertEquals(nodeResources, nodeSpec.resources()); + assertEquals(NodeType.config, nodeSpec.type()); } } diff --git a/searchcore/src/tests/proton/attribute/attributeflush_test.cpp b/searchcore/src/tests/proton/attribute/attributeflush_test.cpp index 81ab6142006..36b1d64a7a9 100644 --- a/searchcore/src/tests/proton/attribute/attributeflush_test.cpp +++ b/searchcore/src/tests/proton/attribute/attributeflush_test.cpp @@ -192,6 +192,10 @@ getInt32Config() return AVConfig(AVBasicType::INT32); } +AVConfig getInt32ArrayConfig() +{ + return AVConfig(AVBasicType::INT32, AVCollectionType::ARRAY); +} class Test : public vespalib::TestApp { @@ -225,6 +229,8 @@ private: void requireThatFlushedAttributeCanBeLoaded(const HwInfo &hwInfo); void requireThatFlushedAttributeCanBeLoaded(); + + void requireThatFlushFailurePreventsSyncTokenUpdate(); public: int Main() override; @@ -272,6 +278,11 @@ struct AttributeManagerFixture cfg.setFastSearch(true); return _m.addAttribute({name, cfg}, createSerialNum); } + AttributeVector::SP addIntArrayPostingAttribute(const vespalib::string &name) { + AVConfig cfg(getInt32ArrayConfig()); + cfg.setFastSearch(true); + return _m.addAttribute({name, cfg}, createSerialNum); + } }; AttributeManagerFixture::AttributeManagerFixture(BaseFixture &bf) @@ -612,6 +623,23 @@ Test::requireThatFlushedAttributeCanBeLoaded() TEST_DO(requireThatFlushedAttributeCanBeLoaded(HwInfo(HwInfo::Disk(0, true, false), HwInfo::Memory(0), HwInfo::Cpu(0)))); } +void +Test::requireThatFlushFailurePreventsSyncTokenUpdate() +{ + BaseFixture f; + AttributeManagerFixture amf(f); + auto &am = amf._m; + auto av = amf.addIntArrayPostingAttribute("a12"); + EXPECT_EQUAL(1u, av->getNumDocs()); + auto flush_target = am.getFlushable("a12"); + EXPECT_EQUAL(0u, flush_target->getFlushedSerialNum()); + auto flush_task = flush_target->initFlush(200); + // Trigger flush failure + av->getEnumStoreBase()->get_data_store_base().inc_compaction_count(); + flush_task->run(); + EXPECT_EQUAL(0u, flush_target->getFlushedSerialNum()); +} + int Test::Main() { @@ -631,6 +659,7 @@ Test::Main() TEST_DO(requireThatLastFlushTimeIsReported()); TEST_DO(requireThatShrinkWorks()); TEST_DO(requireThatFlushedAttributeCanBeLoaded()); + TEST_DO(requireThatFlushFailurePreventsSyncTokenUpdate()); TEST_DONE(); } diff --git a/searchlib/src/tests/attribute/enumeratedsave/enumeratedsave_test.cpp b/searchlib/src/tests/attribute/enumeratedsave/enumeratedsave_test.cpp index 9972fa785eb..785f8f03737 100644 --- a/searchlib/src/tests/attribute/enumeratedsave/enumeratedsave_test.cpp +++ b/searchlib/src/tests/attribute/enumeratedsave/enumeratedsave_test.cpp @@ -8,6 +8,7 @@ #include <vespa/searchlib/attribute/attributeguard.h> #include <vespa/searchlib/attribute/attributememoryfilebufferwriter.h> #include <vespa/searchlib/attribute/attributememorysavetarget.h> +#include <vespa/searchlib/attribute/attributesaver.h> #include <vespa/searchlib/attribute/attrvector.h> #include <vespa/searchlib/attribute/multinumericattribute.h> #include <vespa/searchlib/attribute/multistringattribute.h> @@ -142,6 +143,7 @@ private: SearchContextPtr getSearch(const V & vec); MemAttr::SP saveMem(AttributeVector &v); + void saveMemDuringCompaction(AttributeVector &v); void checkMem(AttributeVector &v, const MemAttr &e); MemAttr::SP saveBoth(AttributePtr v); AttributePtr make(Config cfg, const vespalib::string &pref, bool fastSearch = false); @@ -503,6 +505,19 @@ EnumeratedSaveTest::saveMem(AttributeVector &v) return res; } +void +EnumeratedSaveTest::saveMemDuringCompaction(AttributeVector &v) +{ + MemAttr::SP res(new MemAttr); + auto *enum_store_base = v.getEnumStoreBase(); + if (enum_store_base != nullptr) { + auto saver = v.onInitSave(v.getBaseFileName()); + // Simulate compaction + enum_store_base->get_data_store_base().inc_compaction_count(); + auto save_result = saver->save(*res); + EXPECT_EQUAL(!v.hasMultiValue(), save_result); + } +} void EnumeratedSaveTest::checkMem(AttributeVector &v, const MemAttr &e) @@ -608,6 +623,8 @@ EnumeratedSaveTest::testReload(AttributePtr v0, TEST_DO((checkLoad<VectorType, BufferType>(v, pref + "2_e", v2))); TEST_DO(checkMem(*v, supportsEnumerated ? *emv2 : *mv2)); + saveMemDuringCompaction(*v); + TermFieldMatchData md; SearchContextPtr sc = getSearch<VectorType>(as<VectorType>(v)); sc->fetchPostings(true); diff --git a/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp b/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp index 5476f0f8e66..f91e5b01bcb 100644 --- a/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp +++ b/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp @@ -54,7 +54,7 @@ private: void testCompaction(); template <typename EnumStoreType> - void testCompaction(bool hasPostings, bool disableReEnumerate); + void testCompaction(bool hasPostings); void testReset(); template <typename EnumStoreType> @@ -396,15 +396,13 @@ EnumStoreTest::testUniques void EnumStoreTest::testCompaction() { - testCompaction<StringEnumStore>(false, false); - testCompaction<StringEnumStore>(true, false); - testCompaction<StringEnumStore>(false, true); - testCompaction<StringEnumStore>(true, true); + testCompaction<StringEnumStore>(false); + testCompaction<StringEnumStore>(true); } template <typename EnumStoreType> void -EnumStoreTest::testCompaction(bool hasPostings, bool disableReEnumerate) +EnumStoreTest::testCompaction(bool hasPostings) { // entrySize = 15 before alignment uint32_t entrySize = EnumStoreType::alignEntrySize(15); @@ -461,31 +459,30 @@ EnumStoreTest::testCompaction(bool hasPostings, bool disableReEnumerate) EXPECT_TRUE(!ses.findIndex("enum00", idx)); EXPECT_EQUAL(entrySize + RESERVED_BYTES, ses.getBuffer(0).getDeadElems()); + auto &data_store_base = ses.get_data_store_base(); + auto old_compaction_count = data_store_base.get_compaction_count(); + // perform compaction - if (disableReEnumerate) { - ses.disableReEnumerate(); - } EnumStoreBase::EnumIndexMap old2New; EXPECT_TRUE(ses.performCompaction(3 * entrySize, old2New)); - if (disableReEnumerate) { - ses.enableReEnumerate(); - } EXPECT_TRUE(ses.getRemaining() >= 3 * entrySize); EXPECT_TRUE(ses.getBuffer(1).remaining() >= 3 * entrySize); EXPECT_TRUE(ses.getBuffer(1).size() == entrySize * 4); EXPECT_TRUE(ses.getBuffer(1).getDeadElems() == 0); - EXPECT_EQUAL((disableReEnumerate ? 4u : 3u), ses.getLastEnum()); + EXPECT_NOT_EQUAL(old_compaction_count, data_store_base.get_compaction_count()); + + EXPECT_EQUAL(3u, ses.getLastEnum()); // add new unique strings ses.addEnum("enum05", idx); - EXPECT_EQUAL((disableReEnumerate ? 5u : 4u), ses.getEnum(idx)); + EXPECT_EQUAL(4u, ses.getEnum(idx)); ses.addEnum("enum06", idx); - EXPECT_EQUAL((disableReEnumerate ? 6u : 5u), ses.getEnum(idx)); + EXPECT_EQUAL(5u, ses.getEnum(idx)); ses.addEnum("enum00", idx); - EXPECT_EQUAL((disableReEnumerate ? 7u : 6u), ses.getEnum(idx)); + EXPECT_EQUAL(6u, ses.getEnum(idx)); - EXPECT_EQUAL((disableReEnumerate ? 7u : 6u), ses.getLastEnum()); + EXPECT_EQUAL(6u, ses.getLastEnum()); // compare old and new indices for (uint32_t i = 0; i < indices.size(); ++i) { diff --git a/searchlib/src/vespa/searchlib/attribute/attributesaver.h b/searchlib/src/vespa/searchlib/attribute/attributesaver.h index 113efc30dbf..f90f9492487 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributesaver.h +++ b/searchlib/src/vespa/searchlib/attribute/attributesaver.h @@ -34,6 +34,8 @@ public: bool save(IAttributeSaveTarget &saveTarget); bool hasGenerationGuard() const; + + const vespalib::string &get_file_name() const { return _header.getFileName(); } }; } // namespace search diff --git a/searchlib/src/vespa/searchlib/attribute/enumattributesaver.cpp b/searchlib/src/vespa/searchlib/attribute/enumattributesaver.cpp index 7f0422ce78d..a1c7a343ac8 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumattributesaver.cpp +++ b/searchlib/src/vespa/searchlib/attribute/enumattributesaver.cpp @@ -3,36 +3,19 @@ #include "enumattributesaver.h" #include "iattributesavetarget.h" #include <vespa/vespalib/util/bufferwriter.h> +#include <vespa/vespalib/datastore/unique_store_enumerator.hpp> namespace search { EnumAttributeSaver:: -EnumAttributeSaver(const EnumStoreBase &enumStore, bool disableReEnumerate) +EnumAttributeSaver(const EnumStoreBase &enumStore) : _enumStore(enumStore), - _disableReEnumerate(disableReEnumerate), - _rootRef() + _enumerator(_enumStore.getEnumStoreDict(), _enumStore.get_data_store_base()) { - if (_disableReEnumerate) { - // Prevent enum store from re-enumerating enum values during compaction - _enumStore.disableReEnumerate(); - } - const EnumStoreDictBase &enumDict = enumStore.getEnumStoreDict(); - _rootRef = enumDict.getFrozenRootRef(); } EnumAttributeSaver::~EnumAttributeSaver() { - enableReEnumerate(); -} - -void -EnumAttributeSaver::enableReEnumerate() -{ - if (_disableReEnumerate) { - // compaction of enumstore can now re-enumerate enum values - _enumStore.enableReEnumerate(); - _disableReEnumerate = false; - } } void @@ -42,9 +25,15 @@ EnumAttributeSaver::writeUdat(IAttributeSaveTarget &saveTarget) std::unique_ptr<BufferWriter> udatWriter(saveTarget.udatWriter().allocBufferWriter()); const EnumStoreDictBase &enumDict = _enumStore.getEnumStoreDict(); - enumDict.writeAllValues(*udatWriter, _rootRef); + enumDict.writeAllValues(*udatWriter, _enumerator.get_frozen_root()); udatWriter->flush(); } } } // namespace search + +namespace search::datastore { + +template class UniqueStoreEnumerator<EnumStoreIndex>; + +} diff --git a/searchlib/src/vespa/searchlib/attribute/enumattributesaver.h b/searchlib/src/vespa/searchlib/attribute/enumattributesaver.h index 9d5dd52be4a..9c703c4f72c 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumattributesaver.h +++ b/searchlib/src/vespa/searchlib/attribute/enumattributesaver.h @@ -3,6 +3,7 @@ #pragma once #include "enumstorebase.h" +#include <vespa/vespalib/datastore/unique_store_enumerator.h> namespace search { @@ -15,17 +16,21 @@ class IAttributeSaveTarget; */ class EnumAttributeSaver { +public: + using Enumerator = datastore::UniqueStoreEnumerator<EnumStoreIndex>; + +private: const EnumStoreBase &_enumStore; - bool _disableReEnumerate; - btree::BTreeNode::Ref _rootRef; + Enumerator _enumerator; public: - EnumAttributeSaver(const EnumStoreBase &enumStore, bool disableReEnumerate); + EnumAttributeSaver(const EnumStoreBase &enumStore); ~EnumAttributeSaver(); - void enableReEnumerate(); void writeUdat(IAttributeSaveTarget &saveTarget); const EnumStoreBase &getEnumStore() const { return _enumStore; } + Enumerator &get_enumerator() { return _enumerator; } + void clear() { _enumerator.clear(); } }; } // namespace search diff --git a/searchlib/src/vespa/searchlib/attribute/enumhintsearchcontext.cpp b/searchlib/src/vespa/searchlib/attribute/enumhintsearchcontext.cpp index db63ddbb3ff..4fec660468b 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumhintsearchcontext.cpp +++ b/searchlib/src/vespa/searchlib/attribute/enumhintsearchcontext.cpp @@ -13,8 +13,7 @@ EnumHintSearchContext:: EnumHintSearchContext(const EnumStoreDictBase &dictionary, uint32_t docIdLimit, uint64_t numValues) - : _dictionary(dictionary), - _frozenRootRef(dictionary.get_frozen_root()), + : _dict_snapshot(dictionary.get_read_snapshot()), _uniqueValues(0u), _docIdLimit(docIdLimit), _numValues(numValues) @@ -28,7 +27,7 @@ EnumHintSearchContext::~EnumHintSearchContext() = default; void EnumHintSearchContext::lookupTerm(const EnumStoreComparator &comp) { - _uniqueValues = _dictionary.lookupFrozenTerm(_frozenRootRef, comp); + _uniqueValues = _dict_snapshot->count(comp); } @@ -36,7 +35,7 @@ void EnumHintSearchContext::lookupRange(const EnumStoreComparator &low, const EnumStoreComparator &high) { - _uniqueValues = _dictionary.lookupFrozenRange(_frozenRootRef, low, high); + _uniqueValues = _dict_snapshot->count_in_range(low, high); } void diff --git a/searchlib/src/vespa/searchlib/attribute/enumhintsearchcontext.h b/searchlib/src/vespa/searchlib/attribute/enumhintsearchcontext.h index 7d6d31fce50..1844c228014 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumhintsearchcontext.h +++ b/searchlib/src/vespa/searchlib/attribute/enumhintsearchcontext.h @@ -15,8 +15,7 @@ namespace search::attribute { class EnumHintSearchContext : public IPostingListSearchContext { - const EnumStoreDictBase &_dictionary; - const btree::BTreeNode::Ref _frozenRootRef; + const EnumStoreDictBase::ReadSnapshot::UP _dict_snapshot; uint32_t _uniqueValues; uint32_t _docIdLimit; uint64_t _numValues; // attr.getStatus().getNumValues(); diff --git a/searchlib/src/vespa/searchlib/attribute/enumstore.hpp b/searchlib/src/vespa/searchlib/attribute/enumstore.hpp index 3527414801c..153ee2ec7bd 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumstore.hpp +++ b/searchlib/src/vespa/searchlib/attribute/enumstore.hpp @@ -349,7 +349,6 @@ EnumStoreT<EntryType>::performCompaction(Dictionary &dict, EnumIndexMap & old2Ne typedef typename Dictionary::Iterator DictionaryIterator; uint32_t freeBufferIdx = _store.getActiveBufferId(TYPE_ID); datastore::BufferState & freeBuf = _store.getBufferState(freeBufferIdx); - bool disabledReEnumerate = _disabledReEnumerate; uint32_t newEnum = 0; // copy entries from active buffer to free buffer for (DictionaryIterator iter = dict.begin(); iter.valid(); ++iter) { @@ -365,11 +364,7 @@ EnumStoreT<EntryType>::performCompaction(Dictionary &dict, EnumIndexMap & old2Ne #endif Type value = e.getValue(); uint32_t refCount = e.getRefCount(); - uint32_t oldEnum = e.getEnum(); uint32_t entrySize = this->getEntrySize(value); - if (disabledReEnumerate) { - newEnum = oldEnum; // use old enum value - } uint64_t offset = freeBuf.size(); Index newIdx = Index(offset, freeBufferIdx); @@ -379,9 +374,7 @@ EnumStoreT<EntryType>::performCompaction(Dictionary &dict, EnumIndexMap & old2Ne #ifdef LOG_ENUM_STORE LOG(info, "performCompaction(): new entry: enum = %u, refCount = %u, value = %s", newEnum, 0, value); #endif - if (!disabledReEnumerate) { - ++newEnum; - } + ++newEnum; freeBuf.pushed_back(entrySize); assert(Index::pad(offset) == 0); #ifdef LOG_ENUM_STORE @@ -397,9 +390,6 @@ EnumStoreT<EntryType>::performCompaction(Dictionary &dict, EnumIndexMap & old2Ne old2New[activeIdx] = newIdx; } - if (disabledReEnumerate) { - newEnum = this->_nextEnum; // use old range of enum values - } this->postCompact(newEnum); } diff --git a/searchlib/src/vespa/searchlib/attribute/enumstorebase.cpp b/searchlib/src/vespa/searchlib/attribute/enumstorebase.cpp index 38a42ebd12e..196a1ee056a 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumstorebase.cpp +++ b/searchlib/src/vespa/searchlib/attribute/enumstorebase.cpp @@ -72,8 +72,7 @@ EnumStoreBase::EnumStoreBase(uint64_t initBufferSize, bool hasPostings) _store(), _type(), _nextEnum(0), - _toHoldBuffers(), - _disabledReEnumerate(false) + _toHoldBuffers() { if (hasPostings) _enumDict = new EnumStoreDict<EnumPostingTree>(*this); @@ -167,22 +166,6 @@ EnumStoreBase::fallbackResize(uint64_t bytesNeeded) void -EnumStoreBase::disableReEnumerate() const -{ - assert(!_disabledReEnumerate); - _disabledReEnumerate = true; -} - - -void -EnumStoreBase::enableReEnumerate() const -{ - assert(_disabledReEnumerate); - _disabledReEnumerate = false; -} - - -void EnumStoreBase::postCompact(uint32_t newEnum) { _store.finishCompact(_toHoldBuffers); @@ -195,24 +178,6 @@ EnumStoreBase::failNewSize(uint64_t minNewSize, uint64_t maxSize) throw vespalib::IllegalStateException(vespalib::make_string("EnumStoreBase::failNewSize: Minimum new size (%" PRIu64 ") exceeds max size (%" PRIu64 ")", minNewSize, maxSize)); } -template <class Tree> -void -EnumStoreBase::reEnumerate(const Tree &tree) -{ - typedef typename Tree::Iterator Iterator; - Iterator it(tree.begin()); - uint32_t enumValue = 0; - while (it.valid()) { - EntryBase eb(getEntryBase(it.getKey())); - eb.setEnum(enumValue); - ++enumValue; - ++it; - } - _nextEnum = enumValue; - std::atomic_thread_fence(std::memory_order_release); -} - - ssize_t EnumStoreBase::deserialize0(const void *src, size_t available, @@ -324,13 +289,6 @@ EnumStoreDict<Dictionary>::getNumUniques() const template <typename Dictionary> void -EnumStoreDict<Dictionary>::reEnumerate() -{ - _enumStore.reEnumerate(this->_dict); -} - -template <typename Dictionary> -void EnumStoreDict<Dictionary>:: writeAllValues(BufferWriter &writer, btree::BTreeNode::Ref rootRef) const @@ -477,37 +435,6 @@ EnumStoreDict<Dictionary>::onReset() this->_dict.clear(); } -template <typename Dictionary> -uint32_t -EnumStoreDict<Dictionary>::lookupFrozenTerm(BTreeNode::Ref frozenRootRef, - const datastore::EntryComparator &comp) const -{ - typename Dictionary::ConstIterator itr(BTreeNode::Ref(), - this->_dict.getAllocator()); - itr.lower_bound(frozenRootRef, Index(), comp); - if (itr.valid() && !comp(Index(), itr.getKey())) { - return 1u; - } - return 0u; -} - -template <typename Dictionary> -uint32_t -EnumStoreDict<Dictionary>:: -lookupFrozenRange(BTreeNode::Ref frozenRootRef, - const datastore::EntryComparator &low, - const datastore::EntryComparator &high) const -{ - typename Dictionary::ConstIterator lowerDictItr(BTreeNode::Ref(), - this->_dict.getAllocator()); - lowerDictItr.lower_bound(frozenRootRef, Index(), low); - auto upperDictItr = lowerDictItr; - if (upperDictItr.valid() && !high(Index(), upperDictItr.getKey())) { - upperDictItr.seekPast(Index(), high); - } - return upperDictItr - lowerDictItr; -} - template <> EnumPostingTree & @@ -552,14 +479,6 @@ EnumStoreDict<Dictionary>::hasData() const template class datastore::DataStoreT<datastore::AlignedEntryRefT<31, 4> >; template -void -EnumStoreBase::reEnumerate<EnumTree>(const EnumTree &tree); - -template -void -EnumStoreBase::reEnumerate<EnumPostingTree>(const EnumPostingTree &tree); - -template ssize_t EnumStoreBase::deserialize<EnumTree>(const void *src, size_t available, IndexVector &idx, EnumTree &tree); diff --git a/searchlib/src/vespa/searchlib/attribute/enumstorebase.h b/searchlib/src/vespa/searchlib/attribute/enumstorebase.h index b9499659657..02fd97d7321 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumstorebase.h +++ b/searchlib/src/vespa/searchlib/attribute/enumstorebase.h @@ -64,7 +64,6 @@ public: virtual ~EnumStoreDictBase(); virtual uint32_t getNumUniques() const = 0; - virtual void reEnumerate() = 0; virtual void writeAllValues(BufferWriter &writer, btree::BTreeNode::Ref rootRef) const = 0; virtual ssize_t deserialize(const void *src, size_t available, IndexVector &idx) = 0; @@ -82,13 +81,6 @@ public: virtual void onReset() = 0; virtual btree::BTreeNode::Ref getFrozenRootRef() const = 0; - virtual uint32_t lookupFrozenTerm(btree::BTreeNode::Ref frozenRootRef, - const datastore::EntryComparator &comp) const = 0; - - virtual uint32_t lookupFrozenRange(btree::BTreeNode::Ref frozenRootRef, - const datastore::EntryComparator &low, - const datastore::EntryComparator &high) const = 0; - virtual EnumPostingTree &getPostingDictionary() = 0; virtual const EnumPostingTree &getPostingDictionary() const = 0; virtual bool hasData() const = 0; @@ -117,7 +109,6 @@ public: Dictionary &getDictionary() { return this->_dict; } uint32_t getNumUniques() const override; - void reEnumerate() override; void writeAllValues(BufferWriter &writer, btree::BTreeNode::Ref rootRef) const override; ssize_t deserialize(const void *src, size_t available, IndexVector &idx) override; void fixupRefCounts(const EnumVector &hist) override; @@ -141,13 +132,6 @@ public: void onReset() override; btree::BTreeNode::Ref getFrozenRootRef() const override { return this->get_frozen_root(); } - uint32_t lookupFrozenTerm(btree::BTreeNode::Ref frozenRootRef, - const datastore::EntryComparator &comp) const override; - - uint32_t lookupFrozenRange(btree::BTreeNode::Ref frozenRootRef, - const datastore::EntryComparator &low, - const datastore::EntryComparator &high) const override; - EnumPostingTree & getPostingDictionary() override; const EnumPostingTree & getPostingDictionary() const override; @@ -241,8 +225,6 @@ protected: EnumBufferType _type; uint32_t _nextEnum; std::vector<uint32_t> _toHoldBuffers; // used during compaction - // set before backgound flush, cleared during background flush - mutable std::atomic<bool> _disabledReEnumerate; static const uint32_t TYPE_ID = 0; @@ -310,17 +292,6 @@ public: bool getPendingCompact() const { return _type.getPendingCompact(); } void clearPendingCompact() { _type.clearPendingCompact(); } - template <typename Tree> - void reEnumerate(const Tree &tree); - - void reEnumerate() { _enumDict->reEnumerate(); } - - // Disable reenumeration during compaction. - void disableReEnumerate() const; - - // Allow reenumeration during compaction. - void enableReEnumerate() const; - virtual void writeValues(BufferWriter &writer, const Index *idxs, size_t count) const = 0; void writeEnumValues(BufferWriter &writer, const Index *idxs, size_t count) const; @@ -354,6 +325,7 @@ public: const EnumPostingTree &getPostingDictionary() const { return _enumDict->getPostingDictionary(); } + const datastore::DataStoreBase &get_data_store_base() const { return _store; } }; vespalib::asciistream & operator << (vespalib::asciistream & os, const EnumStoreBase::Index & idx); diff --git a/searchlib/src/vespa/searchlib/attribute/multienumattribute.hpp b/searchlib/src/vespa/searchlib/attribute/multienumattribute.hpp index d1364854e41..e6dfeddf993 100644 --- a/searchlib/src/vespa/searchlib/attribute/multienumattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/multienumattribute.hpp @@ -218,7 +218,6 @@ template <typename B, typename M> std::unique_ptr<AttributeSaver> MultiValueEnumAttribute<B, M>::onInitSave(vespalib::stringref fileName) { - this->_enumStore.reEnumerate(); auto guard = this->getGenerationHandler().takeGuard(); return std::make_unique<MultiValueEnumAttributeSaver<WeightedIndex>> (std::move(guard), this->createAttributeHeader(fileName), this->_mvMapping, this->_enumStore); diff --git a/searchlib/src/vespa/searchlib/attribute/multienumattributesaver.cpp b/searchlib/src/vespa/searchlib/attribute/multienumattributesaver.cpp index c220d144b53..32f46d859bc 100644 --- a/searchlib/src/vespa/searchlib/attribute/multienumattributesaver.cpp +++ b/searchlib/src/vespa/searchlib/attribute/multienumattributesaver.cpp @@ -4,6 +4,9 @@ #include "multivalueattributesaverutils.h" #include "multivalue.h" +#include <vespa/log/log.h> +LOG_SETUP(".searchlib.attribute.multi_enum_attribute_saver"); + using vespalib::GenerationHandler; using search::multivalueattributesaver::CountWriter; using search::multivalueattributesaver::WeightWriter; @@ -19,15 +22,18 @@ namespace { class DatWriter { std::vector<EnumStoreIndex> _indexes; - const EnumStoreBase &_enumStore; + const EnumAttributeSaver::Enumerator &_enumerator; std::unique_ptr<search::BufferWriter> _datWriter; + std::function<bool()> _compaction_interferred; public: DatWriter(IAttributeSaveTarget &saveTarget, - const EnumStoreBase &enumStore) + const EnumAttributeSaver::Enumerator &enumerator, + std::function<bool()> compaction_interferred) : _indexes(), - _enumStore(enumStore), - _datWriter(saveTarget.datWriter().allocBufferWriter()) + _enumerator(enumerator), + _datWriter(saveTarget.datWriter().allocBufferWriter()), + _compaction_interferred(compaction_interferred) { assert(saveTarget.getEnumerated()); _indexes.reserve(1000); @@ -42,8 +48,15 @@ public: void flush() { if (!_indexes.empty()) { - _enumStore.writeEnumValues(*_datWriter, - &_indexes[0], _indexes.size()); + for (auto ref : _indexes) { + uint32_t enumValue = _enumerator.map_entry_ref_to_enum_value_or_zero(ref); + assert(enumValue != 0u || _compaction_interferred()); + // Enumerator enumerates known entry refs (based on + // dictionary tree) to values >= 1, but file format + // starts enumeration at 0. + --enumValue; + _datWriter->write(&enumValue, sizeof(uint32_t)); + } _indexes.clear(); } } @@ -70,11 +83,19 @@ MultiValueEnumAttributeSaver(GenerationHandler::Guard &&guard, const EnumStoreBase &enumStore) : Parent(std::move(guard), header, mvMapping), _mvMapping(mvMapping), - _enumSaver(enumStore, true) + _enumSaver(enumStore), + _enum_store_data_store_base(enumStore.get_data_store_base()), + _compaction_count(_enum_store_data_store_base.get_compaction_count()) { } +template <typename MultiValueT> +bool +MultiValueEnumAttributeSaver<MultiValueT>::compaction_interferred() const +{ + return _compaction_count != _enum_store_data_store_base.get_compaction_count(); +} template <typename MultiValueT> MultiValueEnumAttributeSaver<MultiValueT>::~MultiValueEnumAttributeSaver() = default; @@ -84,20 +105,33 @@ bool MultiValueEnumAttributeSaver<MultiValueT>:: onSave(IAttributeSaveTarget &saveTarget) { + bool compaction_broke_save = false; CountWriter countWriter(saveTarget); WeightWriter<MultiValueType::_hasWeight> weightWriter(saveTarget); - DatWriter datWriter(saveTarget, _enumSaver.getEnumStore()); + DatWriter datWriter(saveTarget, _enumSaver.get_enumerator(), + [this]() { return compaction_interferred(); }); _enumSaver.writeUdat(saveTarget); + _enumSaver.get_enumerator().enumerateValues(); for (uint32_t docId = 0; docId < _frozenIndices.size(); ++docId) { datastore::EntryRef idx = _frozenIndices[docId]; vespalib::ConstArrayRef<MultiValueType> handle(_mvMapping.getDataForIdx(idx)); countWriter.writeCount(handle.size()); weightWriter.writeWeights(handle); datWriter.writeValues(handle); + if (((docId % 0x1000) == 0) && compaction_interferred()) { + compaction_broke_save = true; + break; + } } datWriter.flush(); - _enumSaver.enableReEnumerate(); - return true; + _enumSaver.clear(); + if (compaction_interferred()) { + compaction_broke_save = true; + } + if (compaction_broke_save) { + LOG(warning, "Aborted save of attribute vector to '%s' due to compaction of unique values", get_file_name().c_str()); + } + return !compaction_broke_save; } using EnumIdxArray = multivalue::Value<EnumStoreIndex>; diff --git a/searchlib/src/vespa/searchlib/attribute/multienumattributesaver.h b/searchlib/src/vespa/searchlib/attribute/multienumattributesaver.h index 3bc646057f7..d5d0db7acdc 100644 --- a/searchlib/src/vespa/searchlib/attribute/multienumattributesaver.h +++ b/searchlib/src/vespa/searchlib/attribute/multienumattributesaver.h @@ -25,6 +25,9 @@ class MultiValueEnumAttributeSaver : public MultiValueAttributeSaver const MultiValueMapping &_mvMapping; EnumAttributeSaver _enumSaver; + const datastore::DataStoreBase &_enum_store_data_store_base; + uint64_t _compaction_count; + bool compaction_interferred() const; public: bool onSave(IAttributeSaveTarget &saveTarget) override; MultiValueEnumAttributeSaver(GenerationHandler::Guard &&guard, diff --git a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.hpp b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.hpp index 7b9ba6f61d2..8b29b8c09df 100644 --- a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.hpp @@ -308,7 +308,6 @@ template <typename B> std::unique_ptr<AttributeSaver> SingleValueEnumAttribute<B>::onInitSave(vespalib::stringref fileName) { - this->_enumStore.reEnumerate(); auto guard = this->getGenerationHandler().takeGuard(); return std::make_unique<SingleValueEnumAttributeSaver> (std::move(guard), diff --git a/searchlib/src/vespa/searchlib/attribute/singleenumattributesaver.cpp b/searchlib/src/vespa/searchlib/attribute/singleenumattributesaver.cpp index 24df3438570..31569cc9535 100644 --- a/searchlib/src/vespa/searchlib/attribute/singleenumattributesaver.cpp +++ b/searchlib/src/vespa/searchlib/attribute/singleenumattributesaver.cpp @@ -17,7 +17,7 @@ SingleValueEnumAttributeSaver(GenerationHandler::Guard &&guard, const EnumStoreBase &enumStore) : AttributeSaver(std::move(guard), header), _indices(std::move(indices)), - _enumSaver(enumStore, false) + _enumSaver(enumStore) { } @@ -31,14 +31,21 @@ bool SingleValueEnumAttributeSaver::onSave(IAttributeSaveTarget &saveTarget) { _enumSaver.writeUdat(saveTarget); - const EnumStoreBase &enumStore = _enumSaver.getEnumStore(); std::unique_ptr<search::BufferWriter> datWriter(saveTarget.datWriter(). allocBufferWriter()); assert(saveTarget.getEnumerated()); - enumStore.writeEnumValues(*datWriter, - &_indices[0], _indices.size()); + auto &enumerator = _enumSaver.get_enumerator(); + enumerator.enumerateValues(); + for (auto ref : _indices) { + uint32_t enumValue = enumerator.mapEntryRefToEnumValue(ref); + assert(enumValue != 0u); + // Enumerator enumerates known entry refs (based on dictionary tree) + // to values >= 1, but file format starts enumeration at 0. + --enumValue; + datWriter->write(&enumValue, sizeof(uint32_t)); + } datWriter->flush(); - _enumSaver.enableReEnumerate(); + _enumSaver.clear(); return true; } diff --git a/vespalib/CMakeLists.txt b/vespalib/CMakeLists.txt index 041b2ccaba0..1a111ed49ef 100644 --- a/vespalib/CMakeLists.txt +++ b/vespalib/CMakeLists.txt @@ -42,6 +42,7 @@ vespa_define_module( src/tests/datastore/buffer_type src/tests/datastore/datastore src/tests/datastore/unique_store + src/tests/datastore/unique_store_dictionary src/tests/datastore/unique_store_string_allocator src/tests/delegatelist src/tests/dotproduct diff --git a/vespalib/src/tests/datastore/unique_store_dictionary/CMakeLists.txt b/vespalib/src/tests/datastore/unique_store_dictionary/CMakeLists.txt new file mode 100644 index 00000000000..b1478dea22c --- /dev/null +++ b/vespalib/src/tests/datastore/unique_store_dictionary/CMakeLists.txt @@ -0,0 +1,9 @@ +# Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +vespa_add_executable(vespalib_unique_store_dictionary_test_app TEST + SOURCES + unique_store_dictionary_test.cpp + DEPENDS + vespalib + gtest +) +vespa_add_test(NAME vespalib_unique_store_dictionary_test_app COMMAND vespalib_unique_store_dictionary_test_app) diff --git a/vespalib/src/tests/datastore/unique_store_dictionary/unique_store_dictionary_test.cpp b/vespalib/src/tests/datastore/unique_store_dictionary/unique_store_dictionary_test.cpp new file mode 100644 index 00000000000..8b963dbf007 --- /dev/null +++ b/vespalib/src/tests/datastore/unique_store_dictionary/unique_store_dictionary_test.cpp @@ -0,0 +1,84 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include <vespa/vespalib/datastore/unique_store.hpp> +#include <vespa/vespalib/datastore/unique_store_dictionary.hpp> +#include <vespa/vespalib/gtest/gtest.h> + +#include <vespa/log/log.h> +LOG_SETUP("unique_store_dictionary_test"); + +using namespace search::datastore; +using namespace search::datastore::uniquestore; + +class Comparator : public EntryComparator { +private: + EntryRef _to_find; + + EntryRef resolve(EntryRef ref) const { + if (ref == EntryRef()) { + return _to_find; + } + return ref; + } + +public: + Comparator(uint32_t to_find) + : _to_find(to_find) + {} + bool operator()(const EntryRef lhs, const EntryRef rhs) const override { + return resolve(lhs).ref() < resolve(rhs).ref(); + } +}; + +struct DictionaryReadTest : public ::testing::Test { + DefaultUniqueStoreDictionary dict; + UniqueStoreDictionaryBase::ReadSnapshot::UP snapshot; + + DictionaryReadTest() + : dict(), + snapshot() + { + } + DictionaryReadTest& add(uint32_t value) { + auto result = dict.add(Comparator(value), [=]() { return EntryRef(value); }); + assert(result.inserted()); + return *this; + } + void take_snapshot() { + dict.freeze(); + snapshot = dict.get_read_snapshot(); + } +}; + +TEST_F(DictionaryReadTest, can_count_occurrences_of_a_key) +{ + add(3).add(5).take_snapshot(); + EXPECT_EQ(0, snapshot->count(Comparator(2))); + EXPECT_EQ(1, snapshot->count(Comparator(3))); + EXPECT_EQ(0, snapshot->count(Comparator(4))); + EXPECT_EQ(1, snapshot->count(Comparator(5))); +} + +TEST_F(DictionaryReadTest, can_count_occurrences_of_keys_in_a_range) +{ + add(3).add(5).add(7).add(9).take_snapshot(); + EXPECT_EQ(1, snapshot->count_in_range(Comparator(3), Comparator(3))); + EXPECT_EQ(1, snapshot->count_in_range(Comparator(3), Comparator(4))); + EXPECT_EQ(2, snapshot->count_in_range(Comparator(3), Comparator(5))); + EXPECT_EQ(3, snapshot->count_in_range(Comparator(3), Comparator(7))); + EXPECT_EQ(4, snapshot->count_in_range(Comparator(3), Comparator(9))); + EXPECT_EQ(4, snapshot->count_in_range(Comparator(3), Comparator(10))); + + EXPECT_EQ(0, snapshot->count_in_range(Comparator(5), Comparator(3))); +} + +TEST_F(DictionaryReadTest, can_iterate_all_keys) +{ + using EntryRefVector = std::vector<EntryRef>; + add(3).add(5).add(7).take_snapshot(); + EntryRefVector refs; + snapshot->foreach_key([&](EntryRef ref){ refs.emplace_back(ref); }); + EXPECT_EQ(EntryRefVector({EntryRef(3), EntryRef(5), EntryRef(7)}), refs); +} + +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/vespalib/src/vespa/vespalib/datastore/datastorebase.cpp b/vespalib/src/vespa/vespalib/datastore/datastorebase.cpp index 222e820546e..2f9dc1dc84a 100644 --- a/vespalib/src/vespa/vespalib/datastore/datastorebase.cpp +++ b/vespalib/src/vespa/vespalib/datastore/datastorebase.cpp @@ -84,6 +84,7 @@ DataStoreBase::DataStoreBase(uint32_t numBuffers, size_t maxArrays) _elemHold2List(), _numBuffers(numBuffers), _maxArrays(maxArrays), + _compaction_count(0u), _genHolder() { } @@ -369,6 +370,7 @@ DataStoreBase::startCompact(uint32_t typeId) } } switchActiveBuffer(typeId, 0u); + inc_compaction_count(); return toHold; } @@ -453,6 +455,7 @@ DataStoreBase::markCompacting(uint32_t bufferId) state.setCompacting(); state.disableElemHoldList(); state.setFreeListList(nullptr); + inc_compaction_count(); } std::vector<uint32_t> diff --git a/vespalib/src/vespa/vespalib/datastore/datastorebase.h b/vespalib/src/vespa/vespalib/datastore/datastorebase.h index 4886237194f..59e0d76b638 100644 --- a/vespalib/src/vespa/vespalib/datastore/datastorebase.h +++ b/vespalib/src/vespa/vespalib/datastore/datastorebase.h @@ -8,6 +8,7 @@ #include <vespa/vespalib/util/memoryusage.h> #include <vector> #include <deque> +#include <atomic> namespace search::datastore { @@ -148,6 +149,7 @@ protected: const uint32_t _numBuffers; const size_t _maxArrays; + mutable std::atomic<uint64_t> _compaction_count; vespalib::GenerationHolder _genHolder; @@ -355,6 +357,8 @@ public: uint32_t startCompactWorstBuffer(uint32_t typeId); std::vector<uint32_t> startCompactWorstBuffers(bool compactMemory, bool compactAddressSpace); + uint64_t get_compaction_count() const { return _compaction_count.load(std::memory_order_relaxed); } + void inc_compaction_count() const { ++_compaction_count; } }; } diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.h b/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.h index cbc681b96e3..cd13e88c77e 100644 --- a/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.h +++ b/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.h @@ -13,13 +13,26 @@ class EntryComparatorWrapper; * A dictionary for unique store. Mostly accessed via base class. */ template <typename DictionaryT, typename ParentT = UniqueStoreDictionaryBase> -class UniqueStoreDictionary : public ParentT -{ +class UniqueStoreDictionary : public ParentT { protected: using DictionaryType = DictionaryT; using DataType = typename DictionaryType::DataType; + using FrozenView = typename DictionaryType::FrozenView; + using ReadSnapshot = typename ParentT::ReadSnapshot; using generation_t = typename ParentT::generation_t; + class ReadSnapshotImpl : public ReadSnapshot { + private: + FrozenView _frozen_view; + + public: + ReadSnapshotImpl(FrozenView frozen_view); + EntryRef get_frozen_root() const override { return _frozen_view.getRoot(); } + size_t count(const EntryComparator& comp) const override; + size_t count_in_range(const EntryComparator& low, const EntryComparator& high) const override; + void foreach_key(std::function<void(EntryRef)> callback) const override; + }; + DictionaryType _dict; public: @@ -35,8 +48,8 @@ public: uint32_t get_num_uniques() const override; vespalib::MemoryUsage get_memory_usage() const override; void build(const std::vector<EntryRef> &refs, const std::vector<uint32_t> &ref_counts, std::function<void(EntryRef)> hold) override; + std::unique_ptr<ReadSnapshot> get_read_snapshot() const override; EntryRef get_frozen_root() const override; - void foreach_key(EntryRef root, std::function<void(EntryRef)> callback) const override; }; } diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.hpp b/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.hpp index 6e40b0b7c97..f3087bc5610 100644 --- a/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.hpp +++ b/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.hpp @@ -17,6 +17,47 @@ namespace search::datastore { template <typename DictionaryT, typename ParentT> +UniqueStoreDictionary<DictionaryT, ParentT>:: +ReadSnapshotImpl::ReadSnapshotImpl(FrozenView frozen_view) + : _frozen_view(frozen_view) +{ +} + +template <typename DictionaryT, typename ParentT> +size_t +UniqueStoreDictionary<DictionaryT, ParentT>:: +ReadSnapshotImpl::count(const EntryComparator& comp) const +{ + auto itr = _frozen_view.lowerBound(EntryRef(), comp); + if (itr.valid() && !comp(EntryRef(), itr.getKey())) { + return 1u; + } + return 0u; +} + +template <typename DictionaryT, typename ParentT> +size_t +UniqueStoreDictionary<DictionaryT, ParentT>:: +ReadSnapshotImpl::count_in_range(const EntryComparator& low, + const EntryComparator& high) const +{ + auto low_itr = _frozen_view.lowerBound(EntryRef(), low); + auto high_itr = low_itr; + if (high_itr.valid() && !high(EntryRef(), high_itr.getKey())) { + high_itr.seekPast(EntryRef(), high); + } + return high_itr - low_itr; +} + +template <typename DictionaryT, typename ParentT> +void +UniqueStoreDictionary<DictionaryT, ParentT>:: +ReadSnapshotImpl::foreach_key(std::function<void(EntryRef)> callback) const +{ + _frozen_view.foreach_key(callback); +} + +template <typename DictionaryT, typename ParentT> UniqueStoreDictionary<DictionaryT, ParentT>::UniqueStoreDictionary() : ParentT(), _dict() @@ -135,18 +176,17 @@ UniqueStoreDictionary<DictionaryT, ParentT>::build(const std::vector<EntryRef> & } template <typename DictionaryT, typename ParentT> -EntryRef -UniqueStoreDictionary<DictionaryT, ParentT>::get_frozen_root() const +std::unique_ptr<typename ParentT::ReadSnapshot> +UniqueStoreDictionary<DictionaryT, ParentT>::get_read_snapshot() const { - return _dict.getFrozenView().getRoot(); + return std::make_unique<ReadSnapshotImpl>(_dict.getFrozenView()); } template <typename DictionaryT, typename ParentT> -void -UniqueStoreDictionary<DictionaryT, ParentT>::foreach_key(EntryRef root, - std::function<void(EntryRef)> callback) const +EntryRef +UniqueStoreDictionary<DictionaryT, ParentT>::get_frozen_root() const { - _dict.getAllocator().getNodeStore().foreach_key(root, callback); + return _dict.getFrozenView().getRoot(); } } diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary_base.h b/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary_base.h index 82c8687513a..e09fdc6093c 100644 --- a/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary_base.h +++ b/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary_base.h @@ -12,12 +12,27 @@ class EntryComparator; struct ICompactable; class UniqueStoreAddResult; -/* - * Interface class for unique store dictonary. +/** + * Interface class for unique store dictionary. */ -class UniqueStoreDictionaryBase -{ +class UniqueStoreDictionaryBase { public: + /** + * Class that provides a read snapshot of the dictionary. + * + * A generation guard that must be taken and held while the snapshot is considered valid. + */ + class ReadSnapshot { + public: + using UP = std::unique_ptr<ReadSnapshot>; + virtual ~ReadSnapshot() = default; + // TODO: Remove when all relevant functions have been migrated to this API. + virtual EntryRef get_frozen_root() const = 0; + virtual size_t count(const EntryComparator& comp) const = 0; + virtual size_t count_in_range(const EntryComparator& low, const EntryComparator& high) const = 0; + virtual void foreach_key(std::function<void(EntryRef)> callback) const = 0; + }; + using generation_t = vespalib::GenerationHandler::generation_t; virtual ~UniqueStoreDictionaryBase() = default; virtual void freeze() = 0; @@ -30,8 +45,8 @@ public: virtual uint32_t get_num_uniques() const = 0; virtual vespalib::MemoryUsage get_memory_usage() const = 0; virtual void build(const std::vector<EntryRef> &refs, const std::vector<uint32_t> &ref_counts, std::function<void(EntryRef)> hold) = 0; + virtual std::unique_ptr<ReadSnapshot> get_read_snapshot() const = 0; virtual EntryRef get_frozen_root() const = 0; - virtual void foreach_key(EntryRef root, std::function<void(EntryRef)> callback) const = 0; }; } diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_enumerator.h b/vespalib/src/vespa/vespalib/datastore/unique_store_enumerator.h index 94c217bf836..3f260bfbc15 100644 --- a/vespalib/src/vespa/vespalib/datastore/unique_store_enumerator.h +++ b/vespalib/src/vespa/vespalib/datastore/unique_store_enumerator.h @@ -14,24 +14,27 @@ namespace search::datastore { */ template <typename RefT> class UniqueStoreEnumerator { +public: using RefType = RefT; + using EnumValues = std::vector<std::vector<uint32_t>>; - const UniqueStoreDictionaryBase &_dict; - EntryRef _root; +private: + UniqueStoreDictionaryBase::ReadSnapshot::UP _dict_snapshot; const DataStoreBase &_store; - std::vector<std::vector<uint32_t>> _enumValues; + EnumValues _enumValues; uint32_t _next_enum_val; public: UniqueStoreEnumerator(const UniqueStoreDictionaryBase &dict, const DataStoreBase &store); ~UniqueStoreEnumerator(); + EntryRef get_frozen_root() const { return _dict_snapshot->get_frozen_root(); } void enumerateValue(EntryRef ref); void enumerateValues(); + void clear(); template <typename Function> void - foreach_key(Function &&func) const - { - _dict.foreach_key(_root, func); + foreach_key(Function &&func) const { + _dict_snapshot->foreach_key(func); } uint32_t mapEntryRefToEnumValue(EntryRef ref) const { @@ -45,6 +48,19 @@ public: return 0u; } } + + uint32_t map_entry_ref_to_enum_value_or_zero(EntryRef ref) const { + if (ref.valid()) { + RefType iRef(ref); + if (iRef.unscaled_offset() < _enumValues[iRef.bufferId()].size()) { + return _enumValues[iRef.bufferId()][iRef.unscaled_offset()]; + } else { + return 0u; + } + } else { + return 0u; + } + } }; } diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_enumerator.hpp b/vespalib/src/vespa/vespalib/datastore/unique_store_enumerator.hpp index b364c2a2ba3..10ca0944519 100644 --- a/vespalib/src/vespa/vespalib/datastore/unique_store_enumerator.hpp +++ b/vespalib/src/vespa/vespalib/datastore/unique_store_enumerator.hpp @@ -8,9 +8,9 @@ namespace search::datastore { template <typename RefT> UniqueStoreEnumerator<RefT>::UniqueStoreEnumerator(const UniqueStoreDictionaryBase &dict, const DataStoreBase &store) - : _dict(dict), - _root(_dict.get_frozen_root()), + : _dict_snapshot(dict.get_read_snapshot()), _store(store), + _enumValues(), _next_enum_val(1) { } @@ -45,7 +45,14 @@ UniqueStoreEnumerator<RefT>::enumerateValues() } } _next_enum_val = 1; - _dict.foreach_key(_root, [this](EntryRef ref) { enumerateValue(ref); }); + _dict_snapshot->foreach_key([this](EntryRef ref) { enumerateValue(ref); }); +} + +template <typename RefT> +void +UniqueStoreEnumerator<RefT>::clear() +{ + EnumValues().swap(_enumValues); } } |