diff options
76 files changed, 1187 insertions, 340 deletions
diff --git a/CMakeLists.txt b/CMakeLists.txt index 906a00ad843..044af8ff499 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -142,8 +142,8 @@ add_subdirectory(vespaclient-java) add_subdirectory(vespajlib) add_subdirectory(vespalib) add_subdirectory(vespalog) -if(CMAKE_HOST_SYSTEM_NAME STREQUAL "Darwin") -else() +if(NOT CMAKE_HOST_SYSTEM_NAME STREQUAL "Darwin" AND + NOT DEFINED VESPA_USE_SANITIZER) add_subdirectory(vespamalloc) endif() add_subdirectory(vsm) diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/Roles.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/Roles.java index 578f516f01e..a0c73fa7ff8 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/Roles.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/Roles.java @@ -36,6 +36,7 @@ public class Roles { String[] parts = value.split("\\."); if (parts.length == 1 && parts[0].equals("hostedOperator")) return Role.hostedOperator(); if (parts.length == 1 && parts[0].equals("hostedSupporter")) return Role.hostedSupporter(); + if (parts.length == 1 && parts[0].equals("hostedAccountant")) return Role.hostedAccountant(); if (parts.length == 2) return toRole(TenantName.from(parts[0]), parts[1]); if (parts.length == 3) return toRole(TenantName.from(parts[0]), ApplicationName.from(parts[1]), parts[2]); throw new IllegalArgumentException("Malformed or illegal role value '" + value + "'."); diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/User.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/User.java index 7ad8a8a4197..dcce25bda95 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/User.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/User.java @@ -8,7 +8,7 @@ import java.util.Objects; */ public class User { - public static final String ATTRIBUTE_NAME = User.class.getName(); + public static final String ATTRIBUTE_NAME = "vespa.user.attributes"; private final String email; private final String name; diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java index 0316803558b..baa5a093eed 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java @@ -219,8 +219,8 @@ enum PathGroup { /** Paths used for receiving payment callbacks */ paymentProcessor(PathPrefix.none, "/payment/notification"), - /** Invoice management */ - invoiceManagement(PathPrefix.none, "/billing/v1/invoice/{*}"); + /** Paths used for invoice management */ + hostedAccountant(PathPrefix.api, "/billing/v1/invoice/{*}"); final List<String> pathSpecs; diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java index 0afa0668a00..00550387db5 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java @@ -22,8 +22,11 @@ enum Policy { /** Full access to everything. */ operator(Privilege.grant(Action.all()) - .on(PathGroup.all()) - .in(SystemName.all())), + .on(PathGroup.allExcept(PathGroup.hostedAccountant)) + .in(SystemName.all()), + Privilege.grant(Action.read) + .on(PathGroup.hostedAccountant) + .in(SystemName.PublicCd)), /** Full access to everything. */ supporter(Privilege.grant(Action.read) @@ -167,6 +170,11 @@ enum Policy { /** Read the generated bills */ billingInformationRead(Privilege.grant(Action.read) .on(PathGroup.billingList) + .in(SystemName.PublicCd)), + + /** Invoice management */ + hostedAccountant(Privilege.grant(Action.all()) + .on(PathGroup.hostedAccountant) .in(SystemName.PublicCd)); private final Set<Privilege> privileges; diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Role.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Role.java index d3c5e412215..90350de5dbd 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Role.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Role.java @@ -76,6 +76,9 @@ public abstract class Role { /** Returns the role of the payment processor */ public static UnboundRole paymentProcessor() { return new UnboundRole(RoleDefinition.paymentProcessor); } + /** Returns the role of the invoice manager */ + public static UnboundRole hostedAccountant() { return new UnboundRole(RoleDefinition.hostedAccountant); } + /** Returns the role definition of this bound role. */ public RoleDefinition definition() { return roleDefinition; } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java index 438e79bcc4f..6467050d3f3 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java @@ -89,7 +89,9 @@ public enum RoleDefinition { systemFlagsDryrunner(Policy.systemFlagsDryrun), - paymentProcessor(Policy.paymentProcessor); + paymentProcessor(Policy.paymentProcessor), + + hostedAccountant(Policy.hostedAccountant); private final Set<RoleDefinition> parents; private final Set<Policy> policies; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java index 817ec9c08e8..ecb9a872676 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java @@ -42,7 +42,8 @@ public class OsUpgrader extends InfrastructureUpgrader<OsVersionTarget> { protected void upgrade(OsVersionTarget target, SystemApplication application, ZoneApi zone) { Optional<Duration> zoneUpgradeBudget = target.upgradeBudget() .map(totalBudget -> zoneBudgetOf(totalBudget, zone)); - log.info(String.format("Upgrading OS of %s to version %s in %s in cloud %s%s", application.id(), target, + log.info(String.format("Upgrading OS of %s to version %s in %s in cloud %s%s", application.id(), + target.osVersion().version().toFullString(), zone.getId(), zone.getCloudName(), zoneUpgradeBudget.map(d -> " with time budget " + d).orElse(""))); controller().serviceRegistry().configServer().nodeRepository().upgradeOs(zone.getId(), application.nodeType(), diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java index 0e3295b1143..51a10755605 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java @@ -120,8 +120,14 @@ public class UserApiHandler extends LoggingRequestHandler { return response; } + private HttpResponse userMetadata(HttpRequest request) { - User user = getAttribute(request, User.ATTRIBUTE_NAME, User.class); + @SuppressWarnings("unchecked") + Map<String, String> userAttributes = (Map<String, String>) getAttribute(request, User.ATTRIBUTE_NAME, Map.class); + User user = new User(userAttributes.get("email"), + userAttributes.get("name"), + userAttributes.get("nickname"), + userAttributes.get("picture")); Set<Role> roles = getAttribute(request, SecurityContext.ATTRIBUTE_NAME, SecurityContext.class).roles(); Map<TenantName, List<TenantRole>> tenantRolesByTenantName = roles.stream() diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerCloudTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerCloudTest.java index 8dd1f9ad10a..b935f8cbbe4 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerCloudTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerCloudTest.java @@ -12,6 +12,8 @@ import com.yahoo.yolean.Exceptions; import java.nio.charset.StandardCharsets; import java.security.Principal; +import java.util.HashMap; +import java.util.Map; import java.util.Set; import java.util.function.Supplier; @@ -91,7 +93,17 @@ public class ControllerContainerCloudTest extends ControllerContainerTest { public Request get() { Request request = new Request("http://localhost:8080" + path, data, method, principal); request.getAttributes().put(SecurityContext.ATTRIBUTE_NAME, new SecurityContext(principal, roles)); - if (user != null) request.getAttributes().put(User.ATTRIBUTE_NAME, user); + if (user != null) { + Map<String, String> userAttributes = new HashMap<>(); + userAttributes.put("email", user.email()); + if (user.name() != null) + userAttributes.put("name", user.name()); + if (user.nickname() != null) + userAttributes.put("nickname", user.nickname()); + if (user.picture() != null) + userAttributes.put("picture", user.picture()); + request.getAttributes().put(User.ATTRIBUTE_NAME, Map.copyOf(userAttributes)); + } request.getHeaders().put("Content-Type", contentType); return request; } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiOnPremTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiOnPremTest.java index 343a1f07a21..3a63caf52cd 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiOnPremTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiOnPremTest.java @@ -12,6 +12,8 @@ import com.yahoo.vespa.hosted.controller.restapi.ControllerContainerTest; import org.junit.Test; import java.io.File; +import java.util.HashMap; +import java.util.Map; import java.util.stream.Stream; /** @@ -61,7 +63,15 @@ public class UserApiOnPremTest extends ControllerContainerTest { private Request createUserRequest(User user, AthenzIdentity identity) { Request request = new Request("http://localhost:8080/api/user/v1/user"); - request.getAttributes().put(User.ATTRIBUTE_NAME, user); + Map<String, String> userAttributes = new HashMap<>(); + userAttributes.put("email", user.email()); + if (user.name() != null) + userAttributes.put("name", user.name()); + if (user.nickname() != null) + userAttributes.put("nickname", user.nickname()); + if (user.picture() != null) + userAttributes.put("picture", user.picture()); + request.getAttributes().put(User.ATTRIBUTE_NAME, Map.copyOf(userAttributes)); return addIdentityToRequest(request, identity); } } diff --git a/default_build_settings.cmake b/default_build_settings.cmake index c9908380f9c..64c918370aa 100644 --- a/default_build_settings.cmake +++ b/default_build_settings.cmake @@ -159,10 +159,13 @@ function(vespa_use_default_build_settings) endif() if(VESPA_OS_DISTRO_COMBINED STREQUAL "rhel 6.10") setup_vespa_default_build_settings_rhel_6_10() - elseif(VESPA_OS_DISTRO_COMBINED STREQUAL "rhel 7.7" OR - VESPA_OS_DISTRO_COMBINED STREQUAL "rhel 7.8") + elseif(VESPA_OS_DISTRO STREQUAL "rhel" AND + VESPA_OS_DISTRO_VERSION VERSION_GREATER_EQUAL "7" AND + VESPA_OS_DISTRO_VERSION VERSION_LESS "8") setup_vespa_default_build_settings_rhel_7() - elseif(VESPA_OS_DISTRO_COMBINED STREQUAL "rhel 8.1") + elseif(VESPA_OS_DISTRO STREQUAL "rhel" AND + VESPA_OS_DISTRO_VERSION VERSION_GREATER_EQUAL "8" AND + VESPA_OS_DISTRO_VERSION VERSION_LESS "9") setup_vespa_default_build_settings_rhel_8() elseif(VESPA_OS_DISTRO_COMBINED STREQUAL "centos 7") setup_vespa_default_build_settings_centos_7() 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 deb14c40e7a..149c03710aa 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -39,7 +39,8 @@ import static com.yahoo.vespa.flags.FetchVector.Dimension.ZONE_ID; public class Flags { private static volatile TreeMap<FlagId, FlagDefinition> flags = new TreeMap<>(); - public static final UnboundIntFlag DROP_CACHES = defineIntFlag("drop-caches", 3, + public static final UnboundIntFlag DROP_CACHES = defineIntFlag( + "drop-caches", 3, "The int value to write into /proc/sys/vm/drop_caches for each tick. " + "1 is page cache, 2 is dentries inodes, 3 is both page cache and dentries inodes, etc.", "Takes effect on next tick.", @@ -196,12 +197,6 @@ public class Flags { "Takes effect on restart of Docker container", NODE_TYPE, APPLICATION_ID, HOSTNAME); - public static final UnboundStringFlag TLS_FOR_ZOOKEEPER_QUORUM_COMMUNICATION = defineStringFlag( - "tls-for-zookeeper-quorum-communication", "TLS_WITH_PORT_UNIFICATION", - "How to setup TLS for ZooKeeper quorum communication. Valid values are OFF, PORT_UNIFICATION, TLS_WITH_PORT_UNIFICATION, TLS_ONLY", - "Takes effect on restart of config server", - NODE_TYPE, HOSTNAME); - public static final UnboundStringFlag TLS_FOR_ZOOKEEPER_CLIENT_SERVER_COMMUNICATION = defineStringFlag( "tls-for-zookeeper-client-server-communication", "OFF", "How to setup TLS for ZooKeeper client/server communication. Valid values are OFF, PORT_UNIFICATION, TLS_WITH_PORT_UNIFICATION, TLS_ONLY", diff --git a/model-integration/src/main/java/ai/vespa/rankingexpression/importer/operations/Reshape.java b/model-integration/src/main/java/ai/vespa/rankingexpression/importer/operations/Reshape.java index d8e806ae7e4..78eb5a0e7bb 100644 --- a/model-integration/src/main/java/ai/vespa/rankingexpression/importer/operations/Reshape.java +++ b/model-integration/src/main/java/ai/vespa/rankingexpression/importer/operations/Reshape.java @@ -166,7 +166,7 @@ public class Reshape extends IntermediateOperation { ExpressionNode previousSize = new ConstantNode(new DoubleValue(previousInnerSize)); ExpressionNode mod = new ArithmeticNode(unrolled, ArithmeticOperator.MODULO, previousSize); ExpressionNode div = new ArithmeticNode(new EmbracedNode(mod), ArithmeticOperator.DIVIDE, size); - inputDimensionExpression = new EmbracedNode(new FunctionNode(Function.floor, div)); + inputDimensionExpression = new EmbracedNode(div); } dimensionValues.add(new com.yahoo.tensor.functions.Slice.DimensionValue<>(Optional.of(inputDimensionName), wrapScalar(inputDimensionExpression))); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java index d42efd7ba29..45ed8db3491 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java @@ -398,7 +398,7 @@ public final class Node { public enum State { - /** This host has been requested (from OpenStack) but is not yet ready for use */ + /** This node has been requested, but is not yet ready for use */ provisioned, /** This node is free and ready for use */ diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RetiringUpgrader.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RetiringUpgrader.java index 2601060146b..8b11c10e3dd 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RetiringUpgrader.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RetiringUpgrader.java @@ -9,8 +9,10 @@ import com.yahoo.vespa.hosted.provision.NodeRepository; import java.time.Duration; import java.time.Instant; +import java.util.List; import java.util.Optional; import java.util.logging.Logger; +import java.util.stream.Collectors; /** * An upgrader that retires and deprovisions nodes on stale OS versions. Retirement of each node is spread out in time, @@ -55,21 +57,25 @@ public class RetiringUpgrader implements Upgrader { // No action needed in this implementation. } - /** Retire and deprovision given node */ - private void retire(Node node, Version target, Instant now) { - try (var lock = nodeRepository.lock(node)) { - Optional<Node> currentNode = nodeRepository.getNode(node.hostname()); + /** Retire and deprovision given host and its children */ + private void retire(Node host, Version target, Instant now) { + if (!host.type().isDockerHost()) throw new IllegalArgumentException("Cannot retire non-host " + host); + try (var lock = nodeRepository.lock(host)) { + Optional<Node> currentNode = nodeRepository.getNode(host.hostname()); if (currentNode.isEmpty()) return; - node = currentNode.get(); - NodeType nodeType = node.type(); - LOG.info("Retiring and deprovisioning " + node + ": On stale OS version " + - node.status().osVersion().current().map(Version::toFullString).orElse("<unset>") + + host = currentNode.get(); + NodeType nodeType = host.type(); + List<Node> nodesToRetire = nodeRepository.list().childrenOf(host).stream() + .map(child -> child.with(child.status().withWantToRetire(true))) + .collect(Collectors.toList()); + LOG.info("Retiring and deprovisioning " + host + ": On stale OS version " + + host.status().osVersion().current().map(Version::toFullString).orElse("<unset>") + ", want " + target); - nodeRepository.write(node.with(node.status() - .withWantToRetire(true) - .withWantToDeprovision(true) - .withOsVersion(node.status().osVersion().withWanted(Optional.of(target)))), - lock); + nodesToRetire.add(host.with(host.status() + .withWantToRetire(true) + .withWantToDeprovision(true) + .withOsVersion(host.status().osVersion().withWanted(Optional.of(target))))); + nodeRepository.write(nodesToRetire, lock); nodeRepository.osVersions().writeChange((change) -> change.withRetirementAt(now, nodeType)); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java index fdbf58cda3d..5c990f0a3f3 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java @@ -52,8 +52,7 @@ public class CapacityPolicies { target = target.with(NodeResources.DiskSpeed.any).with(NodeResources.StorageType.any); // Dev does not cap the cpu of containers since usage is spotty: Allocate just a small amount exclusively - // Do not cap in AWS as hosts are allocated on demand and 1-to-1, so the node can use the entire host - if (zone.environment() == Environment.dev && !zone.region().value().contains("aws-")) + if (zone.environment() == Environment.dev && zone.getCloud().allowHostSharing()) target = target.withVcpu(0.1); return target; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java index ca9cbc418bb..0544b675a6f 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java @@ -113,7 +113,7 @@ class NodeAllocation { if (requestedNodes.considerRetiring()) { boolean wantToRetireNode = false; - // if ( ! nodeResourceLimits.isWithinRealLimits(offered, cluster)) wantToRetireNode = true; + if ( ! nodeResourceLimits.isWithinRealLimits(offered, cluster)) wantToRetireNode = true; if (violatesParentHostPolicy(this.nodes, offered)) wantToRetireNode = true; if ( ! hasCompatibleFlavor(node)) wantToRetireNode = true; if (offered.status().wantToRetire()) wantToRetireNode = true; @@ -127,12 +127,10 @@ class NodeAllocation { } } else if (! saturated() && hasCompatibleFlavor(node)) { - /* if ( ! nodeResourceLimits.isWithinRealLimits(offered, cluster)) { ++rejectedDueToInsufficientRealResources; continue; } - */ if ( violatesParentHostPolicy(this.nodes, offered)) { ++rejectedDueToClashingParentHost; continue; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java index 4f66794fa84..913357b16ca 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java @@ -163,11 +163,11 @@ public class NodeRepositoryProvisioner implements Provisioner { AllocatableClusterResources currentResources = nodes.isEmpty() ? new AllocatableClusterResources(requested.minResources(), clusterSpec.type()) // new deployment: Use min : new AllocatableClusterResources(nodes, nodeRepository); - return ensureWithin(Limits.of(requested), currentResources); + return within(Limits.of(requested), currentResources); } /** Make the minimal adjustments needed to the current resources to stay within the limits */ - private ClusterResources ensureWithin(Limits limits, AllocatableClusterResources current) { + private ClusterResources within(Limits limits, AllocatableClusterResources current) { if (limits.isEmpty()) return current.toAdvertisedClusterResources(); if (limits.min().equals(limits.max())) return limits.min(); @@ -202,7 +202,7 @@ public class NodeRepositoryProvisioner implements Provisioner { Optional.of(nodeAllocation.membership()), node.status().vespaVersion(), nodeAllocation.networkPorts(), - requestedResources == NodeResources.unspecified ? Optional.empty() : Optional.of(requestedResources), + requestedResources.isUnspecified() ? Optional.empty() : Optional.of(requestedResources), node.status().dockerImage())); if (nodeAllocation.networkPorts().isPresent()) { log.log(Level.FINE, () -> "Prepared node " + node.hostname() + " has port allocations"); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeResourceLimits.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeResourceLimits.java index 94d25d7cabc..db54fd316f6 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeResourceLimits.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeResourceLimits.java @@ -43,8 +43,11 @@ public class NodeResourceLimits { return true; } - public NodeResources enlargeToLegal(NodeResources advertisedResources, ClusterSpec.Type clusterType) { - return advertisedResources.withMemoryGb(Math.max(minAdvertisedMemoryGb(clusterType), advertisedResources.memoryGb())); + public NodeResources enlargeToLegal(NodeResources requested, ClusterSpec.Type clusterType) { + if (requested.isUnspecified()) return requested; + + return requested.withMemoryGb(Math.max(minAdvertisedMemoryGb(clusterType), requested.memoryGb())) + .withDiskGb(Math.max(minAdvertisedDiskGb(requested), requested.diskGb())); } private double minAdvertisedMemoryGb(ClusterSpec.Type clusterType) { @@ -58,6 +61,7 @@ public class NodeResourceLimits { } private double minAdvertisedDiskGb(NodeResources requested) { + if (requested.storageType() == NodeResources.StorageType.local && nodeRepository.zone().getCloud().dynamicProvisioning()) { if (nodeRepository.zone().system() == SystemName.Public) @@ -65,11 +69,11 @@ public class NodeResourceLimits { else return 55 + minRealDiskGb(); } - return minRealDiskGb(); + return 4 + minRealDiskGb(); } private double minRealDiskGb() { - return 10; + return 6; } private void illegal(String type, String resource, ClusterSpec cluster, double requested, double minAllowed) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java index 8b01868e75e..5b7d67a86a5 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java @@ -161,7 +161,7 @@ public class AutoscalingTest { tester.addMeasurements(Resource.memory, 0.05f, 120, application1); tester.addMeasurements(Resource.disk, 0.05f, 120, application1); tester.assertResources("Scaling down to limit since resource usage is low", - 4, 1, 1.8, 7.4, 8.5, + 4, 1, 1.8, 7.4, 10.0, tester.autoscale(application1, cluster1.id(), min, max)); } @@ -179,7 +179,7 @@ public class AutoscalingTest { tester.deploy(application1, cluster1, 5, 5, new NodeResources(3.0, 10, 10, 1)); tester.addMeasurements(Resource.cpu, 0.3f, 1f, 240, application1); tester.assertResources("Scaling up since resource usage is too high", - 6, 6, 3.6, 8.0, 8.0, + 6, 6, 3.6, 8.0, 10.0, tester.autoscale(application1, cluster1.id(), min, max)); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java index 7d508306846..75d83f00ebb 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.provision.os; import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import com.yahoo.test.ManualClock; import com.yahoo.vespa.hosted.provision.Node; @@ -150,7 +151,12 @@ public class OsVersionsTest { var versions = new OsVersions(tester.nodeRepository(), new RetiringUpgrader(tester.nodeRepository())); var clock = (ManualClock) tester.nodeRepository().clock(); int hostCount = 10; - provisionInfraApplication(hostCount); + // Provision hosts and children + List<Node> hosts = provisionInfraApplication(hostCount); + NodeResources resources = new NodeResources(2, 4, 8, 1); + for (var host : hosts) { + tester.makeReadyVirtualDockerNodes(2, resources, host.hostname()); + } Supplier<NodeList> hostNodes = () -> tester.nodeRepository().list() .nodeType(NodeType.host) .not().state(Node.State.deprovisioned); @@ -167,7 +173,9 @@ public class OsVersionsTest { // Nothing happens on next resume as first host has not spent its budget versions.resumeUpgradeOf(NodeType.host, true); - assertEquals(1, hostNodes.get().deprovisioning().size()); + NodeList nodesDeprovisioning = hostNodes.get().deprovisioning(); + assertEquals(1, nodesDeprovisioning.size()); + assertEquals(2, retiringChildrenOf(nodesDeprovisioning.asList().get(0)).size()); // Budget has been spent and another host is retired clock.advance(nodeBudget); @@ -181,8 +189,9 @@ public class OsVersionsTest { for (int i = 0; i < hostCount - 2; i++) { clock.advance(nodeBudget); versions.resumeUpgradeOf(NodeType.host, true); - NodeList nodesDeprovisioning = hostNodes.get().deprovisioning(); + nodesDeprovisioning = hostNodes.get().deprovisioning(); assertEquals(1, nodesDeprovisioning.size()); + assertEquals(2, retiringChildrenOf(nodesDeprovisioning.asList().get(0)).size()); completeUpgradeOf(nodesDeprovisioning.asList()); } @@ -226,6 +235,10 @@ public class OsVersionsTest { assertEquals(hostCount, hostNodes.get().onOsVersion(version1).size()); } + private NodeList retiringChildrenOf(Node parent) { + return tester.nodeRepository().list().childrenOf(parent).filter(child -> child.status().wantToRetire()); + } + private List<Node> provisionInfraApplication(int nodeCount) { return provisionInfraApplication(nodeCount, NodeType.host); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java index 5eb3c394cab..f8fe0d22e84 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java @@ -408,6 +408,24 @@ public class ProvisioningTest { } @Test + public void test_node_limits_only() { + Flavor hostFlavor = new Flavor(new NodeResources(20, 40, 100, 4)); + ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east"))) + .flavors(List.of(hostFlavor)) + .build(); + tester.makeReadyHosts(30, hostFlavor.resources()).deployZoneApp(); + + ApplicationId app1 = tester.makeApplicationId("app1"); + ClusterSpec cluster1 = ClusterSpec.request(ClusterSpec.Type.content, new ClusterSpec.Id("cluster1")).vespaVersion("7").build(); + + tester.activate(app1, cluster1, Capacity.from(new ClusterResources(2, 1, NodeResources.unspecified), + new ClusterResources(4, 1, NodeResources.unspecified))); + tester.assertNodes("Initial allocation at min with default resources", + 2, 1, 1.5, 8, 50, 0.3, + app1, cluster1); + } + + @Test public void test_changing_limits() { Flavor hostFlavor = new Flavor(new NodeResources(20, 40, 100, 4)); ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east"))) diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java index f7fa3b29b67..227ebf8e692 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java @@ -236,10 +236,24 @@ public class ProvisioningTester { /** Assert on the current *non retired* nodes */ public void assertNodes(String explanation, int nodes, int groups, double vcpu, double memory, double disk, ApplicationId app, ClusterSpec cluster) { - assertNodes(explanation, nodes, groups, vcpu, memory, disk, DiskSpeed.getDefault(), StorageType.getDefault(), app, cluster); + assertNodes(explanation, nodes, groups, vcpu, memory, disk, 0.1, app, cluster); } - public void assertNodes(String explanation, int nodes, int groups, double vcpu, double memory, double disk, + /** Assert on the current *non retired* nodes */ + public void assertNodes(String explanation, int nodes, int groups, double vcpu, double memory, double disk, double bandwidth, + ApplicationId app, ClusterSpec cluster) { + assertNodes(explanation, nodes, groups, vcpu, memory, disk, bandwidth, DiskSpeed.getDefault(), StorageType.getDefault(), app, cluster); + } + + public void assertNodes(String explanation, int nodes, int groups, + double vcpu, double memory, double disk, + DiskSpeed diskSpeed, StorageType storageType, + ApplicationId app, ClusterSpec cluster) { + assertNodes(explanation, nodes, groups, vcpu, memory, disk, 0.1, diskSpeed, storageType, app, cluster); + } + + public void assertNodes(String explanation, int nodes, int groups, + double vcpu, double memory, double disk, double bandwidth, DiskSpeed diskSpeed, StorageType storageType, ApplicationId app, ClusterSpec cluster) { List<Node> nodeList = nodeRepository.list().owner(app).cluster(cluster.id()).not().retired().asList(); @@ -250,7 +264,7 @@ public class ProvisioningTester { groups, nodeList.stream().map(n -> n.allocation().get().membership().cluster().group().get()).distinct().count()); for (Node node : nodeList) { - var expected = new NodeResources(vcpu, memory, disk, 0.1, diskSpeed, storageType); + var expected = new NodeResources(vcpu, memory, disk, bandwidth, diskSpeed, storageType); assertTrue(explanation + ": Resources: Expected " + expected + " but was " + node.flavor().resources(), expected.compatibleWith(node.flavor().resources())); } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostInfosServiceImpl.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostInfosServiceImpl.java index cbb6902066e..d5cf0b39fda 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostInfosServiceImpl.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostInfosServiceImpl.java @@ -4,7 +4,6 @@ package com.yahoo.vespa.orchestrator.status; import com.yahoo.config.provision.ApplicationId; import com.yahoo.jdisc.Timer; -import java.util.logging.Level; import com.yahoo.path.Path; import com.yahoo.vespa.applicationmodel.ApplicationInstanceReference; import com.yahoo.vespa.applicationmodel.HostName; @@ -14,12 +13,12 @@ import com.yahoo.vespa.orchestrator.status.json.WireHostInfo; import org.apache.zookeeper.KeeperException.NoNodeException; import java.time.Instant; +import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.logging.Level; import java.util.logging.Logger; -import java.util.stream.Collectors; /** * Handles all ZooKeeper data structures related to each active application, including HostInfo. @@ -42,18 +41,32 @@ public class HostInfosServiceImpl implements HostInfosService { public HostInfos getHostInfos(ApplicationInstanceReference reference) { ApplicationId application = OrchestratorUtil.toApplicationId(reference); String hostsRootPath = hostsPath(application); - if (uncheck(() -> curator.framework().checkExists().forPath(hostsRootPath)) == null) { + + List<String> hostnames; + try { + hostnames = curator.framework().getChildren().forPath(hostsRootPath); + } catch (NoNodeException e) { return new HostInfos(); - } else { - List<String> hostnames = uncheck(() -> curator.framework().getChildren().forPath(hostsRootPath)); - Map<HostName, HostInfo> hostInfos = hostnames.stream().collect(Collectors.toMap( - hostname -> new HostName(hostname), - hostname -> { - byte[] bytes = uncheck(() -> curator.framework().getData().forPath(hostsRootPath + "/" + hostname)); - return WireHostInfo.deserialize(bytes); - })); - return new HostInfos(hostInfos); + } catch (Exception e) { + throw new RuntimeException(e); } + + var hostInfos = new HashMap<HostName, HostInfo>(); + for (var hostname : hostnames) { + byte[] bytes; + try { + bytes = curator.framework().getData().forPath(hostsRootPath + "/" + hostname); + } catch (NoNodeException e) { + // OK, node has been removed since getChildren() + continue; + } catch (Exception e) { + throw new RuntimeException(e); + } + + hostInfos.put(new HostName(hostname), WireHostInfo.deserialize(bytes)); + } + + return new HostInfos(hostInfos); } @Override diff --git a/parent/pom.xml b/parent/pom.xml index 082a086c418..9225909cfb5 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -780,7 +780,7 @@ <protobuf.version>3.7.0</protobuf.version> <surefire.version>2.22.0</surefire.version> <tensorflow.version>1.12.0</tensorflow.version> - <zookeeper.client.version>3.5.6</zookeeper.client.version> + <zookeeper.client.version>3.5.8</zookeeper.client.version> <zookeeper.server.version>3.5.8</zookeeper.server.version> <doclint>all</doclint> diff --git a/searchcommon/src/vespa/searchcommon/attribute/attributecontent.h b/searchcommon/src/vespa/searchcommon/attribute/attributecontent.h index 72ce1754d71..67b269139d3 100644 --- a/searchcommon/src/vespa/searchcommon/attribute/attributecontent.h +++ b/searchcommon/src/vespa/searchcommon/attribute/attributecontent.h @@ -33,7 +33,7 @@ public: * Creates a new object with an initial capacity of 16 without dynamic allocation. **/ AttributeContent() : - _dynamicBuf(NULL), + _dynamicBuf(nullptr), _size(0), _capacity(16) { @@ -42,7 +42,7 @@ public: * Destructs the object. **/ ~AttributeContent() { - if (_dynamicBuf != NULL) { + if (_dynamicBuf != nullptr) { delete [] _dynamicBuf; } } @@ -53,7 +53,7 @@ public: * @return iterator **/ const T * begin() const { - if (_dynamicBuf != NULL) { + if (_dynamicBuf != nullptr) { return _dynamicBuf; } return _staticBuf; @@ -102,7 +102,7 @@ public: * @return read/write pointer. **/ T * data() { - if (_dynamicBuf != NULL) { + if (_dynamicBuf != nullptr) { return _dynamicBuf; } return _staticBuf; @@ -126,7 +126,7 @@ public: **/ void allocate(uint32_t n) { if (n > _capacity) { - if (_dynamicBuf != NULL) { + if (_dynamicBuf != nullptr) { delete [] _dynamicBuf; } _dynamicBuf = new T[n]; @@ -141,8 +141,7 @@ public: * @param attribute the attribute vector * @param docId the docId **/ - void fill(const search::attribute::IAttributeVector & attribute, - search::attribute::IAttributeVector::DocId docId) + void fill(const IAttributeVector & attribute, IAttributeVector::DocId docId) { uint32_t count = attribute.get(docId, data(), capacity()); while (count > capacity()) { diff --git a/searchcore/CMakeLists.txt b/searchcore/CMakeLists.txt index 412652821d1..fe0504b25af 100644 --- a/searchcore/CMakeLists.txt +++ b/searchcore/CMakeLists.txt @@ -59,6 +59,7 @@ vespa_define_module( src/tests/proton/attribute/attribute_manager src/tests/proton/attribute/attribute_populator src/tests/proton/attribute/attribute_usage_filter + src/tests/proton/attribute/attribute_usage_sampler_functor src/tests/proton/attribute/attributes_state_explorer src/tests/proton/attribute/document_field_extractor src/tests/proton/attribute/document_field_populator diff --git a/searchcore/src/tests/proton/attribute/attribute_usage_sampler_functor/CMakeLists.txt b/searchcore/src/tests/proton/attribute/attribute_usage_sampler_functor/CMakeLists.txt new file mode 100644 index 00000000000..2cb6bc65df3 --- /dev/null +++ b/searchcore/src/tests/proton/attribute/attribute_usage_sampler_functor/CMakeLists.txt @@ -0,0 +1,10 @@ +# Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +vespa_add_executable(searchcore_attribute_usage_sampler_functor_test_app TEST + SOURCES + attribute_usage_sampler_functor_test.cpp + DEPENDS + searchcore_attribute + searchcore_pcommon + gtest +) +vespa_add_test(NAME searchcore_attribute_usage_sampler_functor_test_app COMMAND searchcore_attribute_usage_sampler_functor_test_app) diff --git a/searchcore/src/tests/proton/attribute/attribute_usage_sampler_functor/attribute_usage_sampler_functor_test.cpp b/searchcore/src/tests/proton/attribute/attribute_usage_sampler_functor/attribute_usage_sampler_functor_test.cpp new file mode 100644 index 00000000000..cb092aaa910 --- /dev/null +++ b/searchcore/src/tests/proton/attribute/attribute_usage_sampler_functor/attribute_usage_sampler_functor_test.cpp @@ -0,0 +1,132 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include <vespa/config-attributes.h> +#include <vespa/searchlib/attribute/attributefactory.h> +#include <vespa/searchlib/attribute/attributevector.h> +#include <vespa/searchlib/attribute/integerbase.h> +#include <vespa/searchcore/proton/attribute/attribute_config_inspector.h> +#include <vespa/searchcore/proton/attribute/attribute_usage_filter.h> +#include <vespa/searchcore/proton/attribute/attribute_usage_sampler_context.h> +#include <vespa/searchcore/proton/attribute/attribute_usage_sampler_functor.h> +#include <vespa/searchcore/proton/common/transient_memory_usage_provider.h> +#include <vespa/vespalib/gtest/gtest.h> +#include <vespa/vespalib/stllike/string.h> + +using vespa::config::search::AttributesConfig; +using vespa::config::search::AttributesConfigBuilder; +using search::AttributeVector; +using search::attribute::Config; + +namespace proton { + +namespace { + +AttributesConfig::Attribute build_single_config(const vespalib::string& name, bool fast_search) +{ + AttributesConfigBuilder::Attribute builder; + builder.name = name; + builder.datatype = AttributesConfig::Attribute::Datatype::INT32; + builder.collectiontype = AttributesConfig::Attribute::Collectiontype::WEIGHTEDSET; + builder.fastsearch = fast_search; + return builder; +} + +AttributesConfig build_config(bool fast_search) +{ + AttributesConfigBuilder builder; + builder.attribute.emplace_back(build_single_config("a1", fast_search)); + builder.attribute.emplace_back(build_single_config("a2", fast_search)); + return builder; +} + +std::shared_ptr<AttributeVector> build_attribute_vector(const vespalib::string& name, const AttributeConfigInspector& attribute_config_inspector, uint32_t docs) +{ + auto attribute_vector = search::AttributeFactory::createAttribute(name, *attribute_config_inspector.get_config(name)); + attribute_vector->addReservedDoc(); + for (uint32_t wanted_doc_id = 1; wanted_doc_id <= docs; ++wanted_doc_id) { + uint32_t doc_id = 0; + attribute_vector->addDoc(doc_id); + assert(doc_id == wanted_doc_id); + attribute_vector->clearDoc(doc_id); + auto &integer_attribute_vector = dynamic_cast<search::IntegerAttribute &>(*attribute_vector); + integer_attribute_vector.append(doc_id, 10, 1); + integer_attribute_vector.append(doc_id, 11, 1); + } + attribute_vector->commit(true); + return attribute_vector; +} + +} + +class AttributeUsageSamplerFunctorTest : public ::testing::Test { +protected: + AttributeUsageFilter _filter; + std::shared_ptr<TransientMemoryUsageProvider> _transient_memory_usage_provider; +public: + AttributeUsageSamplerFunctorTest(); + ~AttributeUsageSamplerFunctorTest(); +protected: + void sample_usage(bool sample_a1, bool sample_a2, bool old_fast_search, bool new_fast_search = true); + size_t get_transient_memory_usage() const { return _transient_memory_usage_provider->get_transient_memory_usage(); } +}; + +AttributeUsageSamplerFunctorTest::AttributeUsageSamplerFunctorTest() + : _filter(), + _transient_memory_usage_provider(std::make_shared<TransientMemoryUsageProvider>()) +{ +} + +AttributeUsageSamplerFunctorTest::~AttributeUsageSamplerFunctorTest() = default; + +void +AttributeUsageSamplerFunctorTest::sample_usage(bool sample_a1, bool sample_a2, bool old_fast_search, bool new_fast_search) +{ + auto old_config = build_config(old_fast_search); + auto old_inspector = std::make_shared<AttributeConfigInspector>(old_config); + auto av1 = build_attribute_vector("a1", *old_inspector, 1); + auto av2 = build_attribute_vector("a2", *old_inspector, 3); + EXPECT_EQ(av1->getEnumeratedSave(), old_fast_search); + auto new_config = build_config(new_fast_search); + auto new_inspector = std::make_shared<AttributeConfigInspector>(new_config); + auto context = std::make_shared<AttributeUsageSamplerContext>(_filter, new_inspector, _transient_memory_usage_provider); + if (sample_a1) { + AttributeUsageSamplerFunctor functor1(context, "ready"); + functor1(*av1); + } + if (sample_a2) { + AttributeUsageSamplerFunctor functor2(context, "ready"); + functor2(*av2); + } +} + +TEST_F(AttributeUsageSamplerFunctorTest, plain_attribute_vector_requires_no_transient_memory_for_load) +{ + sample_usage(true, true, false, false); + EXPECT_EQ(0u, get_transient_memory_usage()); +} + +TEST_F(AttributeUsageSamplerFunctorTest, fast_search_attribute_vector_requires_transient_memory_for_load) +{ + sample_usage(true, false, true, true); + EXPECT_EQ(24u, get_transient_memory_usage()); +} + +TEST_F(AttributeUsageSamplerFunctorTest, fast_search_attribute_vector_requires_more_transient_memory_for_load_from_unenumerated) +{ + sample_usage(true, false, false, true); + EXPECT_EQ(40u, get_transient_memory_usage()); +} + +TEST_F(AttributeUsageSamplerFunctorTest, transient_memory_aggregation_function_for_attribute_usage_sampler_context_is_max) +{ + sample_usage(true, false, true, true); + EXPECT_EQ(24u, get_transient_memory_usage()); + sample_usage(false, true, true, true); + EXPECT_EQ(72u, get_transient_memory_usage()); + sample_usage(true, true, true, true); + EXPECT_EQ(72u, get_transient_memory_usage()); +} + +} + +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp b/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp index 1f0f3566b1d..6eaa1bd373a 100644 --- a/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp +++ b/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp @@ -3,11 +3,14 @@ #include <vespa/document/repo/documenttyperepo.h> #include <vespa/document/test/make_bucket_space.h> #include <vespa/fastos/thread.h> +#include <vespa/config-attributes.h> +#include <vespa/searchcore/proton/attribute/attribute_config_inspector.h> #include <vespa/searchcore/proton/attribute/attribute_usage_filter.h> #include <vespa/searchcore/proton/attribute/i_attribute_manager.h> #include <vespa/searchcore/proton/bucketdb/bucket_create_notifier.h> #include <vespa/searchcore/proton/common/doctypename.h> #include <vespa/searchcore/proton/common/feedtoken.h> +#include <vespa/searchcore/proton/common/transient_memory_usage_provider.h> #include <vespa/searchcore/proton/documentmetastore/operation_listener.h> #include <vespa/searchcore/proton/feedoperation/moveoperation.h> #include <vespa/searchcore/proton/feedoperation/pruneremoveddocumentsoperation.h> @@ -60,6 +63,7 @@ using storage::spi::Timestamp; using vespalib::Slime; using vespalib::makeClosure; using vespalib::makeTask; +using vespa::config::search::AttributesConfigBuilder; using BlockedReason = IBlockableMaintenanceJob::BlockedReason; @@ -885,6 +889,8 @@ MaintenanceControllerFixture::injectMaintenanceJobs() _jobTrackers, *this, _readyAttributeManager, _notReadyAttributeManager, + std::make_unique<const AttributeConfigInspector>(AttributesConfigBuilder()), + std::make_shared<TransientMemoryUsageProvider>(), _attributeUsageFilter); } } diff --git a/searchcore/src/vespa/searchcore/proton/attribute/CMakeLists.txt b/searchcore/src/vespa/searchcore/proton/attribute/CMakeLists.txt index 550d0a1c4c4..c20f279503f 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/CMakeLists.txt +++ b/searchcore/src/vespa/searchcore/proton/attribute/CMakeLists.txt @@ -5,6 +5,7 @@ vespa_add_library(searchcore_attribute STATIC attribute_aspect_delayer.cpp attribute_collection_spec_factory.cpp attribute_collection_spec.cpp + attribute_config_inspector.cpp attribute_directory.cpp attribute_factory.cpp attribute_initializer.cpp @@ -13,6 +14,7 @@ vespa_add_library(searchcore_attribute STATIC attribute_manager_initializer.cpp attribute_populator.cpp attribute_spec.cpp + attribute_transient_memory_calculator.cpp attribute_type_matcher.cpp attribute_usage_filter.cpp attribute_usage_sampler_context.cpp diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_config_inspector.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_config_inspector.cpp new file mode 100644 index 00000000000..d950b5e8ed3 --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_config_inspector.cpp @@ -0,0 +1,30 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "attribute_config_inspector.h" +#include <vespa/searchlib/attribute/configconverter.h> +#include <vespa/config-attributes.h> +#include <vespa/vespalib/stllike/hash_map.hpp> + +using search::attribute::ConfigConverter; + +namespace proton { + +AttributeConfigInspector::AttributeConfigInspector(const AttributesConfig& config) + : _hash() +{ + for (auto& attr : config.attribute) { + auto res = _hash.insert(std::make_pair(attr.name, ConfigConverter::convert(attr))); + assert(res.second); + } +} + +AttributeConfigInspector::~AttributeConfigInspector() = default; + +const search::attribute::Config* +AttributeConfigInspector::get_config(const vespalib::string& name) const +{ + auto itr = _hash.find(name); + return (itr != _hash.end()) ? &itr->second : nullptr; +} + +} diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_config_inspector.h b/searchcore/src/vespa/searchcore/proton/attribute/attribute_config_inspector.h new file mode 100644 index 00000000000..8952b34c6ff --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_config_inspector.h @@ -0,0 +1,26 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include <vespa/vespalib/stllike/string.h> +#include <vespa/vespalib/stllike/hash_map.h> +#include <vespa/searchcommon/attribute/config.h> + +namespace vespa::config::search::internal { class InternalAttributesType; } + +namespace proton { + +/* + * Class used to find attribute config given attribute name based on config + * from config server. + */ +class AttributeConfigInspector { + vespalib::hash_map<vespalib::string, search::attribute::Config> _hash; +public: + using AttributesConfig = const vespa::config::search::internal::InternalAttributesType; + AttributeConfigInspector(const AttributesConfig& config); + ~AttributeConfigInspector(); + const search::attribute::Config* get_config(const vespalib::string& name) const; +}; + +} diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_initializer.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_initializer.cpp index 88e4d7a2191..368e4ce12f2 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_initializer.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_initializer.cpp @@ -4,6 +4,7 @@ #include "attributedisklayout.h" #include "attribute_directory.h" #include "i_attribute_factory.h" +#include "attribute_transient_memory_calculator.h" #include <vespa/searchcore/proton/common/eventlogger.h> #include <vespa/vespalib/data/fileheader.h> #include <vespa/vespalib/stllike/asciistream.h> @@ -12,8 +13,6 @@ #include <vespa/searchlib/util/fileutil.h> #include <vespa/searchlib/attribute/attribute_header.h> #include <vespa/searchlib/attribute/attributevector.h> -#include <vespa/searchlib/attribute/loadedenumvalue.h> -#include <vespa/searchlib/attribute/loadedvalue.h> #include <vespa/fastos/file.h> #include <vespa/log/log.h> @@ -263,29 +262,8 @@ size_t AttributeInitializer::get_transient_memory_usage() const { if (_header_ok) { - auto &header = *_header; - if (_spec.getConfig().fastSearch()) { - if (header.getEnumerated()) { - return sizeof(search::attribute::LoadedEnumAttribute) * header.get_total_value_count(); - } else { - switch (_spec.getConfig().basicType().type()) { - case BasicType::Type::INT8: - return sizeof(search::attribute::LoadedValue<int8_t>) * header.get_total_value_count(); - case BasicType::Type::INT16: - return sizeof(search::attribute::LoadedValue<int16_t>) * header.get_total_value_count(); - case BasicType::Type::INT32: - return sizeof(search::attribute::LoadedValue<int32_t>) * header.get_total_value_count(); - case BasicType::Type::INT64: - return sizeof(search::attribute::LoadedValue<int64_t>) * header.get_total_value_count(); - case BasicType::Type::FLOAT: - return sizeof(search::attribute::LoadedValue<float>) * header.get_total_value_count(); - case BasicType::Type::DOUBLE: - return sizeof(search::attribute::LoadedValue<double>) * header.get_total_value_count(); - default: - return 0u; - } - } - } + AttributeTransientMemoryCalculator get_transient_memory_usage; + return get_transient_memory_usage(*_header, _spec.getConfig()); } return 0u; } diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_transient_memory_calculator.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_transient_memory_calculator.cpp new file mode 100644 index 00000000000..37706328149 --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_transient_memory_calculator.cpp @@ -0,0 +1,63 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "attribute_transient_memory_calculator.h" +#include <vespa/searchlib/attribute/attribute_header.h> +#include <vespa/searchcommon/attribute/config.h> +#include <vespa/searchlib/attribute/loadedenumvalue.h> +#include <vespa/searchlib/attribute/loadedvalue.h> +#include <vespa/searchlib/attribute/attributevector.h> + +using search::attribute::BasicType; + +namespace proton { + +namespace { + +size_t +get_transient_memory_usage(bool old_enumerated, + const search::attribute::Config& new_config, + uint64_t total_value_count) +{ + if (new_config.fastSearch()) { + if (old_enumerated) { + return sizeof(search::attribute::LoadedEnumAttribute) * total_value_count; + } else { + switch (new_config.basicType().type()) { + case BasicType::Type::INT8: + return sizeof(search::attribute::LoadedValue<int8_t>) * total_value_count; + case BasicType::Type::INT16: + return sizeof(search::attribute::LoadedValue<int16_t>) * total_value_count; + case BasicType::Type::INT32: + return sizeof(search::attribute::LoadedValue<int32_t>) * total_value_count; + case BasicType::Type::INT64: + return sizeof(search::attribute::LoadedValue<int64_t>) * total_value_count; + case BasicType::Type::FLOAT: + return sizeof(search::attribute::LoadedValue<float>) * total_value_count; + case BasicType::Type::DOUBLE: + return sizeof(search::attribute::LoadedValue<double>) * total_value_count; + default: + ; + } + } + } + return 0u; +} + +} +size_t +AttributeTransientMemoryCalculator::operator()(const search::AttributeVector& attribute_vector, + const search::attribute::Config& new_config) const +{ + uint64_t total_value_count = attribute_vector.getStatus().getNumValues(); + bool old_enumerated = attribute_vector.getEnumeratedSave(); + return get_transient_memory_usage(old_enumerated, new_config, total_value_count); +} + +size_t +AttributeTransientMemoryCalculator::operator()(const search::attribute::AttributeHeader& old_header, + const search::attribute::Config& new_config) const +{ + return get_transient_memory_usage(old_header.getEnumerated(), new_config, old_header.get_total_value_count()); +}; + +} diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_transient_memory_calculator.h b/searchcore/src/vespa/searchcore/proton/attribute/attribute_transient_memory_calculator.h new file mode 100644 index 00000000000..34ab8f02768 --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_transient_memory_calculator.h @@ -0,0 +1,34 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include <cstddef> +#include <cstdint> + +namespace search { class AttributeVector; } + +namespace search::attribute { + +class AttributeHeader; +class Config; + +} + +namespace proton { + +/** + * Class to calculate transient memory during load of attribute vector + * in the future based on current attribute vector and new config. + */ +class AttributeTransientMemoryCalculator +{ +public: + AttributeTransientMemoryCalculator() = default; + ~AttributeTransientMemoryCalculator() = default; + size_t operator()(const search::AttributeVector& attribute_vector, + const search::attribute::Config& new_config) const; + size_t operator()(const search::attribute::AttributeHeader& old_header, + const search::attribute::Config& new_config) const; +}; + +} diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_sampler_context.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_sampler_context.cpp index 39235fb8d00..43c9e52315b 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_sampler_context.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_sampler_context.cpp @@ -2,28 +2,35 @@ #include "attribute_usage_sampler_context.h" #include "attribute_usage_filter.h" +#include <vespa/searchcore/proton/common/transient_memory_usage_provider.h> namespace proton { -AttributeUsageSamplerContext::AttributeUsageSamplerContext(AttributeUsageFilter &filter) +AttributeUsageSamplerContext::AttributeUsageSamplerContext(AttributeUsageFilter &filter, std::shared_ptr<const AttributeConfigInspector> attribute_config_inspector, std::shared_ptr<TransientMemoryUsageProvider> transient_memory_usage_provider) : _usage(), + _transient_memory_usage(0u), _lock(), - _filter(filter) + _filter(filter), + _attribute_config_inspector(std::move(attribute_config_inspector)), + _transient_memory_usage_provider(std::move(transient_memory_usage_provider)) { } AttributeUsageSamplerContext::~AttributeUsageSamplerContext() { _filter.setAttributeStats(_usage); + _transient_memory_usage_provider->set_transient_memory_usage(_transient_memory_usage); } void AttributeUsageSamplerContext::merge(const search::AddressSpaceUsage &usage, + size_t transient_memory_usage, const vespalib::string &attributeName, const vespalib::string &subDbName) { Guard guard(_lock); _usage.merge(usage, attributeName, subDbName); + _transient_memory_usage = std::max(_transient_memory_usage, transient_memory_usage); } } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_sampler_context.h b/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_sampler_context.h index 21e4ff6a90f..6f33ac483c1 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_sampler_context.h +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_sampler_context.h @@ -8,10 +8,13 @@ namespace proton { class AttributeUsageFilter; +class AttributeConfigInspector; +class TransientMemoryUsageProvider; /* - * Context for sampling attribute usage stats. When instance is - * destroyed, the aggregated stats is passed on to attribute usage filter. + * Context for sampling attribute usage stats and transient memory usage. + * When instance is destroyed, the aggregated stats is passed on to + * attribute usage filter and the transient memory usage provider. */ class AttributeUsageSamplerContext { @@ -19,16 +22,22 @@ class AttributeUsageSamplerContext using Guard = std::lock_guard<Mutex>; AttributeUsageStats _usage; + size_t _transient_memory_usage; Mutex _lock; AttributeUsageFilter &_filter; + std::shared_ptr<const AttributeConfigInspector> _attribute_config_inspector; + std::shared_ptr<TransientMemoryUsageProvider> _transient_memory_usage_provider; public: - AttributeUsageSamplerContext(AttributeUsageFilter &filter); + AttributeUsageSamplerContext(AttributeUsageFilter &filter, std::shared_ptr<const AttributeConfigInspector> attribute_config_inspector, std::shared_ptr<TransientMemoryUsageProvider> transient_memory_usage_provider); ~AttributeUsageSamplerContext(); void merge(const search::AddressSpaceUsage &usage, + size_t transient_memory_usage, const vespalib::string &attributeName, const vespalib::string &subDbName); + const AttributeConfigInspector& get_attribute_config_inspector() const { return *_attribute_config_inspector; } const AttributeUsageStats & getUsage() const { return _usage; } + size_t get_transient_memory_usage() const { return _transient_memory_usage; } }; } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_sampler_functor.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_sampler_functor.cpp index a1a8409e93b..685433cd5c7 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_sampler_functor.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_usage_sampler_functor.cpp @@ -2,8 +2,12 @@ #include "attribute_usage_sampler_functor.h" #include "attribute_usage_sampler_context.h" +#include "attribute_config_inspector.h" +#include "attribute_transient_memory_calculator.h" #include <vespa/searchlib/attribute/attributevector.h> +using search::attribute::BasicType; + namespace proton { AttributeUsageSamplerFunctor::AttributeUsageSamplerFunctor( @@ -23,7 +27,14 @@ AttributeUsageSamplerFunctor::operator()(const search::attribute::IAttributeVect const auto & attributeVector = dynamic_cast<const search::AttributeVector &>(iAttributeVector); search::AddressSpaceUsage usage = attributeVector.getAddressSpaceUsage(); vespalib::string attributeName = attributeVector.getName(); - _samplerContext->merge(usage, attributeName, _subDbName); + auto& old_config = attributeVector.getConfig(); + auto* current_config = _samplerContext->get_attribute_config_inspector().get_config(attributeName); + if (current_config == nullptr) { + current_config = &old_config; + } + AttributeTransientMemoryCalculator get_transient_memory_usage; + size_t transient_memory_usage = get_transient_memory_usage(attributeVector, *current_config); + _samplerContext->merge(usage, transient_memory_usage, attributeName, _subDbName); } } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/common/CMakeLists.txt b/searchcore/src/vespa/searchcore/proton/common/CMakeLists.txt index f3303f36199..7a9426ae716 100644 --- a/searchcore/src/vespa/searchcore/proton/common/CMakeLists.txt +++ b/searchcore/src/vespa/searchcore/proton/common/CMakeLists.txt @@ -20,6 +20,7 @@ vespa_add_library(searchcore_pcommon STATIC selectpruner.cpp state_reporter_utils.cpp statusreport.cpp + transient_memory_usage_provider.cpp DEPENDS searchcore_proton_metrics searchcore_fconfig diff --git a/searchcore/src/vespa/searchcore/proton/common/i_transient_memory_usage_provider.h b/searchcore/src/vespa/searchcore/proton/common/i_transient_memory_usage_provider.h new file mode 100644 index 00000000000..cd87150195c --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/common/i_transient_memory_usage_provider.h @@ -0,0 +1,20 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include <cstddef> + +namespace proton { + +/* + * Interface class providing transient memory usage, e.g. extra memory needed + * for loading or saving an attribute vector. It provides an aggregated view + * over several components (e.g. all attribute vectors for a document type). + */ +class ITransientMemoryUsageProvider { +public: + virtual ~ITransientMemoryUsageProvider() = default; + virtual size_t get_transient_memory_usage() const = 0; +}; + +} diff --git a/searchcore/src/vespa/searchcore/proton/common/transient_memory_usage_provider.cpp b/searchcore/src/vespa/searchcore/proton/common/transient_memory_usage_provider.cpp new file mode 100644 index 00000000000..0752af29a0a --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/common/transient_memory_usage_provider.cpp @@ -0,0 +1,27 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "transient_memory_usage_provider.h" + +namespace proton { + +TransientMemoryUsageProvider::TransientMemoryUsageProvider() + : ITransientMemoryUsageProvider(), + _transient_memory_usage(0u) +{ +} + +TransientMemoryUsageProvider::~TransientMemoryUsageProvider() = default; + +size_t +TransientMemoryUsageProvider::get_transient_memory_usage() const +{ + return _transient_memory_usage.load(std::memory_order_relaxed); +} + +void +TransientMemoryUsageProvider::set_transient_memory_usage(size_t transient_memory_usage) +{ + _transient_memory_usage.store(transient_memory_usage, std::memory_order_relaxed); +} + +} diff --git a/searchcore/src/vespa/searchcore/proton/common/transient_memory_usage_provider.h b/searchcore/src/vespa/searchcore/proton/common/transient_memory_usage_provider.h new file mode 100644 index 00000000000..8cda4278ad8 --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/common/transient_memory_usage_provider.h @@ -0,0 +1,25 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include "i_transient_memory_usage_provider.h" + +#include <atomic> + +namespace proton { + +/* + * Class providing transient memory usage, e.g. extra memory needed + * for loading or saving an attribute vector. It provides an aggregated view + * over several components (e.g. all attribute vectors for a document type). + */ +class TransientMemoryUsageProvider : public ITransientMemoryUsageProvider { + std::atomic<size_t> _transient_memory_usage; +public: + TransientMemoryUsageProvider(); + virtual ~TransientMemoryUsageProvider(); + size_t get_transient_memory_usage() const override; + void set_transient_memory_usage(size_t transient_memory_usage); +}; + +} diff --git a/searchcore/src/vespa/searchcore/proton/metrics/resource_usage_metrics.cpp b/searchcore/src/vespa/searchcore/proton/metrics/resource_usage_metrics.cpp index 43e4e4d3f75..c890592b411 100644 --- a/searchcore/src/vespa/searchcore/proton/metrics/resource_usage_metrics.cpp +++ b/searchcore/src/vespa/searchcore/proton/metrics/resource_usage_metrics.cpp @@ -10,6 +10,7 @@ ResourceUsageMetrics::ResourceUsageMetrics(metrics::MetricSet *parent) diskUtilization("disk_utilization", {}, "The relative amount of disk used compared to the disk resource limit", this), memory("memory", {}, "The relative amount of memory used by this process (value in the range [0, 1])", this), memoryUtilization("memory_utilization", {}, "The relative amount of memory used compared to the memory resource limit", this), + transient_memory("transient_memory", {}, "The relative amount of transient memory needed for load (value in the range [0, 1])", this), memoryMappings("memory_mappings", {}, "The number of mapped memory areas", this), openFileDescriptors("open_file_descriptors", {}, "The number of open files", this), feedingBlocked("feeding_blocked", {}, "Whether feeding is blocked due to resource limits being reached (value is either 0 or 1)", this) diff --git a/searchcore/src/vespa/searchcore/proton/metrics/resource_usage_metrics.h b/searchcore/src/vespa/searchcore/proton/metrics/resource_usage_metrics.h index 497a0fe2ba0..8f7036ee832 100644 --- a/searchcore/src/vespa/searchcore/proton/metrics/resource_usage_metrics.h +++ b/searchcore/src/vespa/searchcore/proton/metrics/resource_usage_metrics.h @@ -15,6 +15,7 @@ struct ResourceUsageMetrics : metrics::MetricSet metrics::DoubleValueMetric diskUtilization; metrics::DoubleValueMetric memory; metrics::DoubleValueMetric memoryUtilization; + metrics::DoubleValueMetric transient_memory; metrics::LongValueMetric memoryMappings; metrics::LongValueMetric openFileDescriptors; metrics::LongValueMetric feedingBlocked; diff --git a/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_filter.cpp b/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_filter.cpp index 3191d82bf11..4855577e712 100644 --- a/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_filter.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_filter.cpp @@ -156,6 +156,7 @@ DiskMemUsageFilter::DiskMemUsageFilter(const HwInfo &hwInfo) _hwInfo(hwInfo), _memoryStats(), _diskUsedSizeBytes(), + _transient_memory_usage(0u), _config(), _state(), _acceptWrite(true), @@ -182,6 +183,13 @@ DiskMemUsageFilter::setDiskUsedSize(uint64_t diskUsedSizeBytes) } void +DiskMemUsageFilter::set_transient_memory_usage(size_t transient_memory_usage) +{ + Guard guard(_lock); + _transient_memory_usage = transient_memory_usage; +} + +void DiskMemUsageFilter::setConfig(Config config_in) { Guard guard(_lock); @@ -203,6 +211,20 @@ DiskMemUsageFilter::getDiskUsedSize() const return _diskUsedSizeBytes; } +size_t +DiskMemUsageFilter::get_transient_memory_usage() const +{ + Guard guard(_lock); + return _transient_memory_usage; +} + +double +DiskMemUsageFilter::get_relative_transient_memory_usage() const +{ + Guard guard(_lock); + return static_cast<double>(_transient_memory_usage) / _hwInfo.memory().sizeBytes(); +} + DiskMemUsageFilter::Config DiskMemUsageFilter::getConfig() const { @@ -230,7 +252,6 @@ DiskMemUsageFilter::getAcceptState() const return _state; } - void DiskMemUsageFilter::addDiskMemUsageListener(IDiskMemUsageListener *listener) { diff --git a/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_filter.h b/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_filter.h index afb8c3d2c7a..6d6fad257ac 100644 --- a/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_filter.h +++ b/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_filter.h @@ -43,6 +43,7 @@ private: HwInfo _hwInfo; vespalib::ProcessMemoryStats _memoryStats; uint64_t _diskUsedSizeBytes; + size_t _transient_memory_usage; Config _config; State _state; std::atomic<bool> _acceptWrite; @@ -59,9 +60,12 @@ public: ~DiskMemUsageFilter() override; void setMemoryStats(vespalib::ProcessMemoryStats memoryStats_in); void setDiskUsedSize(uint64_t diskUsedSizeBytes); + void set_transient_memory_usage(size_t transient_memory_usage); void setConfig(Config config); vespalib::ProcessMemoryStats getMemoryStats() const; uint64_t getDiskUsedSize() const; + size_t get_transient_memory_usage() const; + double get_relative_transient_memory_usage() const; Config getConfig() const; const HwInfo &getHwInfo() const { return _hwInfo; } DiskMemUsageState usageState() const; diff --git a/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_sampler.cpp b/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_sampler.cpp index a3fb1244c45..f7f505cd754 100644 --- a/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_sampler.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_sampler.cpp @@ -3,6 +3,7 @@ #include "disk_mem_usage_sampler.h" #include <vespa/vespalib/util/scheduledexecutor.h> #include <vespa/vespalib/util/lambdatask.h> +#include <vespa/searchcore/proton/common/i_transient_memory_usage_provider.h> #include <filesystem> using vespalib::makeLambdaTask; @@ -14,7 +15,9 @@ DiskMemUsageSampler::DiskMemUsageSampler(const std::string &path_in, const Confi _path(path_in), _sampleInterval(60s), _lastSampleTime(vespalib::steady_clock::now()), - _periodicTimer() + _periodicTimer(), + _lock(), + _transient_memory_usage_providers() { setConfig(config); } @@ -49,6 +52,7 @@ DiskMemUsageSampler::sampleUsage() { sampleMemoryUsage(); sampleDiskUsage(); + sample_transient_memory_usage(); } namespace { @@ -118,4 +122,37 @@ DiskMemUsageSampler::sampleMemoryUsage() _filter.setMemoryStats(vespalib::ProcessMemoryStats::create()); } +void +DiskMemUsageSampler::sample_transient_memory_usage() +{ + size_t max_transient_memory_usage = 0; + { + std::lock_guard<std::mutex> guard(_lock); + for (auto provider : _transient_memory_usage_providers) { + auto transient_memory_usage = provider->get_transient_memory_usage(); + max_transient_memory_usage = std::max(max_transient_memory_usage, transient_memory_usage); + } + } + _filter.set_transient_memory_usage(max_transient_memory_usage); +} + +void +DiskMemUsageSampler::add_transient_memory_usage_provider(std::shared_ptr<const ITransientMemoryUsageProvider> provider) +{ + std::lock_guard<std::mutex> guard(_lock); + _transient_memory_usage_providers.push_back(provider); +} + +void +DiskMemUsageSampler::remove_transient_memory_usage_provider(std::shared_ptr<const ITransientMemoryUsageProvider> provider) +{ + std::lock_guard<std::mutex> guard(_lock); + for (auto itr = _transient_memory_usage_providers.begin(); itr != _transient_memory_usage_providers.end(); ++itr) { + if (*itr == provider) { + _transient_memory_usage_providers.erase(itr); + break; + } + } +} + } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_sampler.h b/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_sampler.h index bbfc95ff82c..38e80a6cec1 100644 --- a/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_sampler.h +++ b/searchcore/src/vespa/searchcore/proton/server/disk_mem_usage_sampler.h @@ -9,6 +9,8 @@ namespace vespalib { class ScheduledExecutor; } namespace proton { +class ITransientMemoryUsageProvider; + /* * Class to sample disk and memory usage used for filtering write operations. */ @@ -18,10 +20,13 @@ class DiskMemUsageSampler { vespalib::duration _sampleInterval; vespalib::steady_time _lastSampleTime; std::unique_ptr<vespalib::ScheduledExecutor> _periodicTimer; + std::mutex _lock; + std::vector<std::shared_ptr<const ITransientMemoryUsageProvider>> _transient_memory_usage_providers; void sampleUsage(); void sampleDiskUsage(); void sampleMemoryUsage(); + void sample_transient_memory_usage(); public: struct Config { DiskMemUsageFilter::Config filterConfig; @@ -55,6 +60,8 @@ public: const DiskMemUsageFilter &writeFilter() const { return _filter; } IDiskMemUsageNotifier ¬ifier() { return _filter; } + void add_transient_memory_usage_provider(std::shared_ptr<const ITransientMemoryUsageProvider> provider); + void remove_transient_memory_usage_provider(std::shared_ptr<const ITransientMemoryUsageProvider> provider); }; diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp index 55f95ce0518..7567aa9c322 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp @@ -13,10 +13,12 @@ #include "maintenance_jobs_injector.h" #include "reconfig_params.h" #include <vespa/document/repo/documenttyperepo.h> +#include <vespa/searchcore/proton/attribute/attribute_config_inspector.h> #include <vespa/searchcore/proton/attribute/attribute_writer.h> #include <vespa/searchcore/proton/attribute/imported_attributes_repo.h> #include <vespa/searchcore/proton/common/eventlogger.h> #include <vespa/searchcore/proton/common/statusreport.h> +#include <vespa/searchcore/proton/common/transient_memory_usage_provider.h> #include <vespa/searchcore/proton/docsummary/isummarymanager.h> #include <vespa/searchcore/proton/feedoperation/noopoperation.h> #include <vespa/searchcore/proton/index/index_writer.h> @@ -163,6 +165,7 @@ DocumentDB::DocumentDB(const vespalib::string &baseDir, _state(), _dmUsageForwarder(_writeService.master()), _writeFilter(), + _transient_memory_usage_provider(std::make_shared<TransientMemoryUsageProvider>()), _feedHandler(_writeService, tlsSpec, docTypeName, _state, *this, _writeFilter, *this, tlsDirectWriter), _subDBs(*this, *this, _feedHandler, _docTypeName, _writeService, warmupExecutor, fileHeaderContext, metricsWireService, getMetrics(), queryLimiter, clock, _configMutex, _baseDir, @@ -929,7 +932,7 @@ DocumentDB::hasDocument(const document::DocumentId &id) } void -DocumentDB::injectMaintenanceJobs(const DocumentDBMaintenanceConfig &config) +DocumentDB::injectMaintenanceJobs(const DocumentDBMaintenanceConfig &config, std::unique_ptr<const AttributeConfigInspector> attribute_config_inspector) { // Called by executor thread _maintenanceController.killJobs(); @@ -954,6 +957,8 @@ DocumentDB::injectMaintenanceJobs(const DocumentDBMaintenanceConfig &config) _visibility, // ICommitable _subDBs.getReadySubDB()->getAttributeManager(), _subDBs.getNotReadySubDB()->getAttributeManager(), + std::move(attribute_config_inspector), + _transient_memory_usage_provider, _writeFilter); } @@ -963,18 +968,21 @@ DocumentDB::performStartMaintenance() // Called by executor thread // Only start once, after replay done - DocumentDBMaintenanceConfig::SP maintenanceConfig; + std::shared_ptr<DocumentDBConfig> activeConfig; { lock_guard guard(_configMutex); if (_state.getClosed()) return; - assert(_activeConfigSnapshot); - maintenanceConfig = _activeConfigSnapshot->getMaintenanceConfigSP(); + activeConfig = _activeConfigSnapshot; } + assert(activeConfig); if (_maintenanceController.getStopping()) { return; } - injectMaintenanceJobs(*maintenanceConfig); + auto maintenanceConfig = activeConfig->getMaintenanceConfigSP(); + const auto &attributes_config = activeConfig->getAttributesConfig(); + auto attribute_config_inspector = std::make_unique<AttributeConfigInspector>(attributes_config); + injectMaintenanceJobs(*maintenanceConfig, std::move(attribute_config_inspector)); _maintenanceController.start(maintenanceConfig); } @@ -992,10 +1000,12 @@ DocumentDB::forwardMaintenanceConfig() assert(activeConfig); DocumentDBMaintenanceConfig::SP maintenanceConfig(activeConfig->getMaintenanceConfigSP()); + const auto &attributes_config = activeConfig->getAttributesConfig(); + auto attribute_config_inspector = std::make_unique<AttributeConfigInspector>(attributes_config); if (!_state.getClosed()) { if (_maintenanceController.getStarted() && !_maintenanceController.getStopping()) { - injectMaintenanceJobs(*maintenanceConfig); + injectMaintenanceJobs(*maintenanceConfig, std::move(attribute_config_inspector)); } _maintenanceController.newConfig(maintenanceConfig); } @@ -1086,4 +1096,10 @@ DocumentDB::getDistributionKey() const return _owner.getDistributionKey(); } +std::shared_ptr<const ITransientMemoryUsageProvider> +DocumentDB::transient_memory_usage_provider() +{ + return _transient_memory_usage_provider; +} + } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb.h b/searchcore/src/vespa/searchcore/proton/server/documentdb.h index 917d753683a..33dfa85b628 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentdb.h +++ b/searchcore/src/vespa/searchcore/proton/server/documentdb.h @@ -44,10 +44,13 @@ namespace search { namespace vespa::config::search::core::internal { class InternalProtonType; } namespace proton { +class AttributeConfigInspector; class IDocumentDBOwner; +class ITransientMemoryUsageProvider; struct MetricsWireService; class StatusReport; class ExecutorThreadingServiceStats; +class TransientMemoryUsageProvider; namespace matching { class SessionManager; } @@ -132,6 +135,7 @@ private: DDBState _state; DiskMemUsageForwarder _dmUsageForwarder; AttributeUsageFilter _writeFilter; + std::shared_ptr<TransientMemoryUsageProvider> _transient_memory_usage_provider; FeedHandler _feedHandler; DocumentSubDBCollection _subDBs; @@ -412,7 +416,7 @@ public: /** * Implements IFeedHandlerOwner **/ - void injectMaintenanceJobs(const DocumentDBMaintenanceConfig &config); + void injectMaintenanceJobs(const DocumentDBMaintenanceConfig &config, std::unique_ptr<const AttributeConfigInspector> attribute_config_inspector); void performStartMaintenance(); void stopMaintenance(); void forwardMaintenanceConfig(); @@ -435,6 +439,7 @@ public: void enterOnlineState(); void waitForOnlineState(); IDiskMemUsageListener *diskMemUsageListener() { return &_dmUsageForwarder; } + std::shared_ptr<const ITransientMemoryUsageProvider> transient_memory_usage_provider(); }; } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.cpp b/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.cpp index 483497eb008..d69f0365fc0 100644 --- a/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.cpp @@ -9,6 +9,7 @@ #include "prune_session_cache_job.h" #include "pruneremoveddocumentsjob.h" #include "sample_attribute_usage_job.h" +#include <vespa/searchcore/proton/attribute/attribute_config_inspector.h> using vespalib::system_clock; @@ -99,6 +100,8 @@ MaintenanceJobsInjector::injectJobs(MaintenanceController &controller, ICommitable &commit, IAttributeManagerSP readyAttributeManager, IAttributeManagerSP notReadyAttributeManager, + std::unique_ptr<const AttributeConfigInspector> attribute_config_inspector, + std::shared_ptr<TransientMemoryUsageProvider> transient_memory_usage_provider, AttributeUsageFilter &attributeUsageFilter) { controller.registerJobInMasterThread(std::make_unique<HeartBeatJob>(hbHandler, config.getHeartBeatConfig())); controller.registerJobInDefaultPool(std::make_unique<PruneSessionCacheJob>(scPruner, config.getSessionCachePruneInterval())); @@ -123,7 +126,9 @@ MaintenanceJobsInjector::injectJobs(MaintenanceController &controller, notReadyAttributeManager, attributeUsageFilter, docTypeName, - config.getAttributeUsageSampleInterval())); + config.getAttributeUsageSampleInterval(), + std::move(attribute_config_inspector), + transient_memory_usage_provider)); } } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.h b/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.h index 94203e144dc..f0859335919 100644 --- a/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.h +++ b/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.h @@ -12,6 +12,7 @@ namespace proton { +class AttributeConfigInspector; class IPruneRemovedDocumentsHandler; struct IDocumentMoveHandler; class IBucketModifiedHandler; @@ -21,6 +22,7 @@ struct IBucketStateCalculator; struct IAttributeManager; class AttributeUsageFilter; class IDiskMemUsageNotifier; +class TransientMemoryUsageProvider; namespace bucketdb { class IBucketCreateNotifier; } /** @@ -53,6 +55,8 @@ struct MaintenanceJobsInjector ICommitable & commit, IAttributeManagerSP readyAttributeManager, IAttributeManagerSP notReadyAttributeManager, + std::unique_ptr<const AttributeConfigInspector> attribute_config_inspector, + std::shared_ptr<TransientMemoryUsageProvider> transient_memory_usage_provider, AttributeUsageFilter &attributeUsageFilter); }; diff --git a/searchcore/src/vespa/searchcore/proton/server/proton.cpp b/searchcore/src/vespa/searchcore/proton/server/proton.cpp index c886b371064..c7000119861 100644 --- a/searchcore/src/vespa/searchcore/proton/server/proton.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/proton.cpp @@ -606,6 +606,7 @@ Proton::addDocumentDB(const document::DocumentType &docType, auto flushHandler = std::make_shared<FlushHandlerProxy>(ret); _flushEngine->putFlushHandler(docTypeName, flushHandler); _diskMemUsageSampler->notifier().addDiskMemUsageListener(ret->diskMemUsageListener()); + _diskMemUsageSampler->add_transient_memory_usage_provider(ret->transient_memory_usage_provider()); return ret; } @@ -643,6 +644,7 @@ Proton::removeDocumentDB(const DocTypeName &docTypeName) _metricsEngine->removeMetricsHook(old->getMetricsUpdateHook()); _metricsEngine->removeDocumentDBMetrics(old->getMetrics()); _diskMemUsageSampler->notifier().removeDiskMemUsageListener(old->diskMemUsageListener()); + _diskMemUsageSampler->remove_transient_memory_usage_provider(old->transient_memory_usage_provider()); // Caller should have removed & drained relevant timer tasks old->close(); } @@ -710,6 +712,7 @@ Proton::updateMetrics(const vespalib::MonitorGuard &) metrics.resourceUsage.diskUtilization.set(usageState.diskState().utilization()); metrics.resourceUsage.memory.set(usageState.memoryState().usage()); metrics.resourceUsage.memoryUtilization.set(usageState.memoryState().utilization()); + metrics.resourceUsage.transient_memory.set(usageFilter.get_relative_transient_memory_usage()); metrics.resourceUsage.memoryMappings.set(usageFilter.getMemoryStats().getMappingsCount()); metrics.resourceUsage.openFileDescriptors.set(FastOS_File::count_open_files()); metrics.resourceUsage.feedingBlocked.set((usageFilter.acceptWriteOperation() ? 0.0 : 1.0)); diff --git a/searchcore/src/vespa/searchcore/proton/server/resource_usage_explorer.cpp b/searchcore/src/vespa/searchcore/proton/server/resource_usage_explorer.cpp index 6f1fd8b90e0..83c268fc755 100644 --- a/searchcore/src/vespa/searchcore/proton/server/resource_usage_explorer.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/resource_usage_explorer.cpp @@ -47,6 +47,8 @@ ResourceUsageExplorer::get_state(const vespalib::slime::Inserter &inserter, bool memory.setDouble("utilization", usageState.memoryState().utilization()); memory.setLong("physicalMemory", _usageFilter.getHwInfo().memory().sizeBytes()); convertMemoryStatsToSlime(_usageFilter.getMemoryStats(), memory.setObject("stats")); + size_t transient_memory = _usageFilter.get_transient_memory_usage(); + memory.setLong("transient", transient_memory); } else { object.setDouble("disk", usageState.diskState().usage()); object.setDouble("memory", usageState.memoryState().usage()); diff --git a/searchcore/src/vespa/searchcore/proton/server/sample_attribute_usage_job.cpp b/searchcore/src/vespa/searchcore/proton/server/sample_attribute_usage_job.cpp index 1f5f29c7708..a6497966891 100644 --- a/searchcore/src/vespa/searchcore/proton/server/sample_attribute_usage_job.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/sample_attribute_usage_job.cpp @@ -2,6 +2,7 @@ #include "sample_attribute_usage_job.h" #include <vespa/searchcore/proton/attribute/i_attribute_manager.h> +#include <vespa/searchcore/proton/attribute/attribute_config_inspector.h> #include <vespa/searchcore/proton/attribute/attribute_usage_filter.h> #include <vespa/searchcore/proton/attribute/attribute_usage_sampler_context.h> #include <vespa/searchcore/proton/attribute/attribute_usage_sampler_functor.h> @@ -13,11 +14,15 @@ SampleAttributeUsageJob(IAttributeManagerSP readyAttributeManager, IAttributeManagerSP notReadyAttributeManager, AttributeUsageFilter &attributeUsageFilter, const vespalib::string &docTypeName, - vespalib::duration interval) + vespalib::duration interval, + std::unique_ptr<const AttributeConfigInspector> attribute_config_inspector, + std::shared_ptr<TransientMemoryUsageProvider> transient_memory_usage_provider) : IMaintenanceJob("sample_attribute_usage." + docTypeName, vespalib::duration::zero(), interval), _readyAttributeManager(readyAttributeManager), _notReadyAttributeManager(notReadyAttributeManager), - _attributeUsageFilter(attributeUsageFilter) + _attributeUsageFilter(attributeUsageFilter), + _attribute_config_inspector(std::move(attribute_config_inspector)), + _transient_memory_usage_provider(std::move(transient_memory_usage_provider)) { } @@ -26,7 +31,7 @@ SampleAttributeUsageJob::~SampleAttributeUsageJob() = default; bool SampleAttributeUsageJob::run() { - auto context = std::make_shared<AttributeUsageSamplerContext> (_attributeUsageFilter); + auto context = std::make_shared<AttributeUsageSamplerContext> (_attributeUsageFilter, _attribute_config_inspector, _transient_memory_usage_provider); _readyAttributeManager->asyncForEachAttribute(std::make_shared<AttributeUsageSamplerFunctor>(context, "ready")); _notReadyAttributeManager->asyncForEachAttribute(std::make_shared<AttributeUsageSamplerFunctor>(context, "notready")); return true; diff --git a/searchcore/src/vespa/searchcore/proton/server/sample_attribute_usage_job.h b/searchcore/src/vespa/searchcore/proton/server/sample_attribute_usage_job.h index 72a0bf1a665..6efbfb67c4c 100644 --- a/searchcore/src/vespa/searchcore/proton/server/sample_attribute_usage_job.h +++ b/searchcore/src/vespa/searchcore/proton/server/sample_attribute_usage_job.h @@ -7,7 +7,9 @@ namespace proton { struct IAttributeManager; +class AttributeConfigInspector; class AttributeUsageFilter; +class TransientMemoryUsageProvider; /** * Class used to sample attribute resource usage and pass aggregated @@ -21,12 +23,16 @@ class SampleAttributeUsageJob : public IMaintenanceJob IAttributeManagerSP _readyAttributeManager; IAttributeManagerSP _notReadyAttributeManager; AttributeUsageFilter &_attributeUsageFilter; + std::shared_ptr<const AttributeConfigInspector> _attribute_config_inspector; + std::shared_ptr<TransientMemoryUsageProvider> _transient_memory_usage_provider; public: SampleAttributeUsageJob(IAttributeManagerSP readyAttributeManager, IAttributeManagerSP notReadyAttributeManager, AttributeUsageFilter &attributeUsageFilter, const vespalib::string &docTypeName, - vespalib::duration interval); + vespalib::duration interval, + std::unique_ptr<const AttributeConfigInspector> attribute_config_inspector, + std::shared_ptr<TransientMemoryUsageProvider> transient_memory_usage_provider); ~SampleAttributeUsageJob() override; bool run() override; diff --git a/searchlib/src/tests/fef/fef_test.cpp b/searchlib/src/tests/fef/fef_test.cpp index 4d1163de6ee..50f8a770618 100644 --- a/searchlib/src/tests/fef/fef_test.cpp +++ b/searchlib/src/tests/fef/fef_test.cpp @@ -67,12 +67,11 @@ TEST("test TermFieldMatchDataAppend") EXPECT_EQUAL(1u, tmd.capacity()); tmd.appendPosition(pos); EXPECT_EQUAL(2u, tmd.size()); - EXPECT_EQUAL(2u, tmd.capacity()); + EXPECT_EQUAL(42u, tmd.capacity()); uint32_t resizeCount(0); const TermFieldMatchDataPosition * prev = tmd.begin(); for (size_t i(2); i < std::numeric_limits<uint16_t>::max(); i++) { EXPECT_EQUAL(i, tmd.size()); - EXPECT_EQUAL(std::min(size_t(std::numeric_limits<uint16_t>::max()), vespalib::roundUp2inN(i)), tmd.capacity()); tmd.appendPosition(pos); const TermFieldMatchDataPosition * cur = tmd.begin(); if (cur != prev) { @@ -80,13 +79,15 @@ TEST("test TermFieldMatchDataAppend") resizeCount++; } } - EXPECT_EQUAL(15u, resizeCount); - EXPECT_EQUAL(std::numeric_limits<uint16_t>::max(), tmd.size()); - EXPECT_EQUAL(std::numeric_limits<uint16_t>::max(), tmd.capacity()); - tmd.appendPosition(pos); - EXPECT_EQUAL(prev, tmd.begin()); + EXPECT_EQUAL(11u, resizeCount); EXPECT_EQUAL(std::numeric_limits<uint16_t>::max(), tmd.size()); EXPECT_EQUAL(std::numeric_limits<uint16_t>::max(), tmd.capacity()); + for (size_t i(0); i < 10; i++) { + tmd.appendPosition(pos); + EXPECT_EQUAL(prev, tmd.begin()); + EXPECT_EQUAL(std::numeric_limits<uint16_t>::max(), tmd.size()); + EXPECT_EQUAL(std::numeric_limits<uint16_t>::max(), tmd.capacity()); + } } TEST("verify size of essential fef classes") { diff --git a/searchlib/src/tests/fef/termfieldmodel/termfieldmodel_test.cpp b/searchlib/src/tests/fef/termfieldmodel/termfieldmodel_test.cpp index a75b91cb78a..3de34047ecd 100644 --- a/searchlib/src/tests/fef/termfieldmodel/termfieldmodel_test.cpp +++ b/searchlib/src/tests/fef/termfieldmodel/termfieldmodel_test.cpp @@ -215,11 +215,11 @@ TEST("append positions") { tfmd.appendPosition(pos); tfmd.appendPosition(pos2); EXPECT_EQUAL(2u, tfmd.size()); - EXPECT_EQUAL(2u, tfmd.capacity()); + EXPECT_EQUAL(42u, tfmd.capacity()); TermFieldMatchDataPosition pos3(0x31020304, 0x30203040, 0x31223344, 0x32345678); tfmd.appendPosition(pos3); EXPECT_EQUAL(3u, tfmd.size()); - EXPECT_EQUAL(4u, tfmd.capacity()); + EXPECT_EQUAL(42u, tfmd.capacity()); EXPECT_EQUAL(0x01020304u, tfmd.begin()->getElementId()); EXPECT_EQUAL(0x10203040u, tfmd.begin()->getPosition()); EXPECT_EQUAL(0x11223344, tfmd.begin()->getElementWeight()); diff --git a/searchlib/src/tests/fef/termmatchdatamerger/CMakeLists.txt b/searchlib/src/tests/fef/termmatchdatamerger/CMakeLists.txt index 4eb00cacf38..d01b3f84436 100644 --- a/searchlib/src/tests/fef/termmatchdatamerger/CMakeLists.txt +++ b/searchlib/src/tests/fef/termmatchdatamerger/CMakeLists.txt @@ -1,8 +1,10 @@ # Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +find_package(GTest REQUIRED) vespa_add_executable(searchlib_termmatchdatamerger_test_app TEST SOURCES termmatchdatamerger_test.cpp DEPENDS searchlib + GTest::GTest ) vespa_add_test(NAME searchlib_termmatchdatamerger_test_app COMMAND searchlib_termmatchdatamerger_test_app) diff --git a/searchlib/src/tests/fef/termmatchdatamerger/termmatchdatamerger_test.cpp b/searchlib/src/tests/fef/termmatchdatamerger/termmatchdatamerger_test.cpp index a8670f43a6b..eb04c34b595 100644 --- a/searchlib/src/tests/fef/termmatchdatamerger/termmatchdatamerger_test.cpp +++ b/searchlib/src/tests/fef/termmatchdatamerger/termmatchdatamerger_test.cpp @@ -1,11 +1,8 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/log/log.h> -LOG_SETUP("termmatchdatamerger_test"); -#include <vespa/vespalib/testkit/testapp.h> - #include <vespa/searchlib/fef/termfieldmatchdata.h> #include <vespa/searchlib/fef/termfieldmatchdataarray.h> #include <vespa/searchlib/fef/termmatchdatamerger.h> +#include <vespa/vespalib/gtest/gtest.h> using namespace search::fef; @@ -21,19 +18,7 @@ TermFieldMatchDataPosition make_pos(uint32_t pos) } // namespace <unnamed> -class Test : public vespalib::TestApp -{ -public: - void testMergeEmptyInput(); - void testMergeSimple(); - void testMergeMultifield(); - void testMergeDuplicates(); - void testMergeFieldLength(); - int Main() override; -}; - -void -Test::testMergeEmptyInput() +TEST(TermMatchDataMergerTest, merge_empty_input) { TermFieldMatchData out; TermFieldMatchDataArray output; @@ -48,12 +33,11 @@ Test::testMergeEmptyInput() uint32_t docid = 5; in.reset(docid); merger.merge(docid); - EXPECT_EQUAL(docid, out.getDocId()); + EXPECT_EQ(docid, out.getDocId()); EXPECT_TRUE(out.begin() == out.end()); } -void -Test::testMergeSimple() +TEST(TermMatchDataMergerTest, merge_simple) { TermFieldMatchData a; TermFieldMatchData b; @@ -86,26 +70,26 @@ Test::testMergeSimple() merger.merge(docid); - EXPECT_EQUAL(docid, out.getDocId()); - EXPECT_EQUAL(8u, out.end() - out.begin()); - - EXPECT_EQUAL( 5u, out.begin()[0].getPosition()); - EXPECT_EQUAL( 7u, out.begin()[1].getPosition()); - EXPECT_EQUAL(10u, out.begin()[2].getPosition()); - EXPECT_EQUAL(15u, out.begin()[3].getPosition()); - EXPECT_EQUAL(20u, out.begin()[4].getPosition()); - EXPECT_EQUAL(22u, out.begin()[5].getPosition()); - EXPECT_EQUAL(27u, out.begin()[6].getPosition()); - EXPECT_EQUAL(28u, out.begin()[7].getPosition()); - - EXPECT_EQUAL(0.25, out.begin()[0].getMatchExactness()); - EXPECT_EQUAL( 0.5, out.begin()[1].getMatchExactness()); - EXPECT_EQUAL( 1.5, out.begin()[2].getMatchExactness()); - EXPECT_EQUAL( 1.0, out.begin()[3].getMatchExactness()); - EXPECT_EQUAL( 4.0, out.begin()[4].getMatchExactness()); - EXPECT_EQUAL(0.75, out.begin()[5].getMatchExactness()); - EXPECT_EQUAL( 3.0, out.begin()[6].getMatchExactness()); - EXPECT_EQUAL( 7.5, out.begin()[7].getMatchExactness()); + EXPECT_EQ(docid, out.getDocId()); + EXPECT_EQ(8u, out.end() - out.begin()); + + EXPECT_EQ( 5u, out.begin()[0].getPosition()); + EXPECT_EQ( 7u, out.begin()[1].getPosition()); + EXPECT_EQ(10u, out.begin()[2].getPosition()); + EXPECT_EQ(15u, out.begin()[3].getPosition()); + EXPECT_EQ(20u, out.begin()[4].getPosition()); + EXPECT_EQ(22u, out.begin()[5].getPosition()); + EXPECT_EQ(27u, out.begin()[6].getPosition()); + EXPECT_EQ(28u, out.begin()[7].getPosition()); + + EXPECT_EQ(0.25, out.begin()[0].getMatchExactness()); + EXPECT_EQ( 0.5, out.begin()[1].getMatchExactness()); + EXPECT_EQ( 1.5, out.begin()[2].getMatchExactness()); + EXPECT_EQ( 1.0, out.begin()[3].getMatchExactness()); + EXPECT_EQ( 4.0, out.begin()[4].getMatchExactness()); + EXPECT_EQ(0.75, out.begin()[5].getMatchExactness()); + EXPECT_EQ( 3.0, out.begin()[6].getMatchExactness()); + EXPECT_EQ( 7.5, out.begin()[7].getMatchExactness()); // one stale input @@ -117,24 +101,22 @@ Test::testMergeSimple() merger.merge(docid); - EXPECT_EQUAL(docid, out.getDocId()); - EXPECT_EQUAL(3u, out.end() - out.begin()); + EXPECT_EQ(docid, out.getDocId()); + EXPECT_EQ(3u, out.end() - out.begin()); - EXPECT_EQUAL( 5u, out.begin()[0].getPosition()); - EXPECT_EQUAL(10u, out.begin()[1].getPosition()); - EXPECT_EQUAL(15u, out.begin()[2].getPosition()); + EXPECT_EQ( 5u, out.begin()[0].getPosition()); + EXPECT_EQ(10u, out.begin()[1].getPosition()); + EXPECT_EQ(15u, out.begin()[2].getPosition()); // both inputs are stale docid = 15; merger.merge(docid); - EXPECT_NOT_EQUAL(docid, out.getDocId()); + EXPECT_NE(docid, out.getDocId()); } - -void -Test::testMergeMultifield() +TEST(TermMatchDataMergerTest, merge_multiple_fields) { TermFieldMatchData a; TermFieldMatchData b; @@ -174,30 +156,29 @@ Test::testMergeMultifield() merger.merge(docid); - EXPECT_EQUAL(docid, out1.getDocId()); - EXPECT_EQUAL(docid, out2.getDocId()); - EXPECT_NOT_EQUAL(docid, out3.getDocId()); + EXPECT_EQ(docid, out1.getDocId()); + EXPECT_EQ(docid, out2.getDocId()); + EXPECT_NE(docid, out3.getDocId()); - EXPECT_EQUAL(2u, out1.end() - out1.begin()); - EXPECT_EQUAL(3u, out2.end() - out2.begin()); + EXPECT_EQ(2u, out1.end() - out1.begin()); + EXPECT_EQ(3u, out2.end() - out2.begin()); - EXPECT_EQUAL( 5u, out1.begin()[0].getPosition()); - EXPECT_EQUAL(15u, out1.begin()[1].getPosition()); + EXPECT_EQ( 5u, out1.begin()[0].getPosition()); + EXPECT_EQ(15u, out1.begin()[1].getPosition()); - EXPECT_EQUAL( 5u, out2.begin()[0].getPosition()); - EXPECT_EQUAL( 7u, out2.begin()[1].getPosition()); - EXPECT_EQUAL(20u, out2.begin()[2].getPosition()); + EXPECT_EQ( 5u, out2.begin()[0].getPosition()); + EXPECT_EQ( 7u, out2.begin()[1].getPosition()); + EXPECT_EQ(20u, out2.begin()[2].getPosition()); - EXPECT_EQUAL(1.0, out1.begin()[0].getMatchExactness()); - EXPECT_EQUAL(1.0, out1.begin()[1].getMatchExactness()); + EXPECT_EQ(1.0, out1.begin()[0].getMatchExactness()); + EXPECT_EQ(1.0, out1.begin()[1].getMatchExactness()); - EXPECT_EQUAL(1.5, out2.begin()[0].getMatchExactness()); - EXPECT_EQUAL(0.5, out2.begin()[1].getMatchExactness()); - EXPECT_EQUAL(1.5, out2.begin()[2].getMatchExactness()); + EXPECT_EQ(1.5, out2.begin()[0].getMatchExactness()); + EXPECT_EQ(0.5, out2.begin()[1].getMatchExactness()); + EXPECT_EQ(1.5, out2.begin()[2].getMatchExactness()); } -void -Test::testMergeDuplicates() +TEST(TermMatchDataMergerTest, merge_duplicates) { TermFieldMatchData a; TermFieldMatchData b; @@ -225,23 +206,22 @@ Test::testMergeDuplicates() merger.merge(docid); - EXPECT_EQUAL(docid, out.getDocId()); - EXPECT_EQUAL(5u, out.end() - out.begin()); - - EXPECT_EQUAL( 3u, out.begin()[0].getPosition()); - EXPECT_EQUAL(1.5, out.begin()[0].getMatchExactness()); - EXPECT_EQUAL( 5u, out.begin()[1].getPosition()); - EXPECT_EQUAL(0.5, out.begin()[1].getMatchExactness()); - EXPECT_EQUAL(10u, out.begin()[2].getPosition()); - EXPECT_EQUAL(1.5, out.begin()[2].getMatchExactness()); - EXPECT_EQUAL(15u, out.begin()[3].getPosition()); - EXPECT_EQUAL(1.5, out.begin()[3].getMatchExactness()); - EXPECT_EQUAL(17u, out.begin()[4].getPosition()); - EXPECT_EQUAL(1.5, out.begin()[4].getMatchExactness()); + EXPECT_EQ(docid, out.getDocId()); + EXPECT_EQ(5u, out.end() - out.begin()); + + EXPECT_EQ( 3u, out.begin()[0].getPosition()); + EXPECT_EQ(1.5, out.begin()[0].getMatchExactness()); + EXPECT_EQ( 5u, out.begin()[1].getPosition()); + EXPECT_EQ(0.5, out.begin()[1].getMatchExactness()); + EXPECT_EQ(10u, out.begin()[2].getPosition()); + EXPECT_EQ(1.5, out.begin()[2].getMatchExactness()); + EXPECT_EQ(15u, out.begin()[3].getPosition()); + EXPECT_EQ(1.5, out.begin()[3].getMatchExactness()); + EXPECT_EQ(17u, out.begin()[4].getPosition()); + EXPECT_EQ(1.5, out.begin()[4].getMatchExactness()); } -void -Test::testMergeFieldLength() +TEST(TermMatchDataMergerTest, merge_max_element_length) { TermFieldMatchData a; TermFieldMatchData b; @@ -261,20 +241,93 @@ Test::testMergeFieldLength() b.appendPosition(make_pos(2)); merger.merge(docid); - EXPECT_EQUAL(docid, out.getDocId()); - EXPECT_EQUAL(1000u, out.getIterator().getFieldLength()); + EXPECT_EQ(docid, out.getDocId()); + EXPECT_EQ(1000u, out.getIterator().getFieldLength()); +} + +class TermMatchDataMergerTest2 : public ::testing::Test +{ +protected: + TermFieldMatchData a; + TermFieldMatchData b; + MDMIs input; + TermFieldMatchData out; + TermFieldMatchDataArray output; + TermMatchDataMerger merger; + + TermMatchDataMergerTest2() + : a(), + b(), + input({{&a, 0.5},{&b, 1.5}}), + out(), + output(), + merger(input, output.add(&out)) + { + } +}; + +TEST_F(TermMatchDataMergerTest2, merge_no_normal_features) +{ + out.setNeedNormalFeatures(false); + + uint32_t docid = 5; + + a.reset(docid); + a.appendPosition(make_pos(5)); + + b.reset(docid); + b.appendPosition(make_pos(3)); + + merger.merge(docid); + EXPECT_EQ(docid, out.getDocId()); + EXPECT_EQ(0u, out.size()); +} + +TEST_F(TermMatchDataMergerTest2, merge_interleaved_features) +{ + out.setNeedNormalFeatures(false); + out.setNeedInterleavedFeatures(true); + + uint32_t docid = 5; + + a.reset(docid); + a.setNumOccs(1); + a.setFieldLength(30); + + b.reset(docid); + b.setNumOccs(1); + b.setFieldLength(35); + + merger.merge(docid); + EXPECT_EQ(docid, out.getDocId()); + EXPECT_EQ(2u, out.getNumOccs()); + EXPECT_EQ(35u, out.getFieldLength()); } -int -Test::Main() +TEST_F(TermMatchDataMergerTest2, merge_interleaved_features_with_detected_duplicate) { - TEST_INIT("termmatchdatamerger_test"); - testMergeEmptyInput(); - testMergeSimple(); - testMergeMultifield(); - testMergeDuplicates(); - testMergeFieldLength(); - TEST_DONE(); + out.setNeedNormalFeatures(true); + out.setNeedInterleavedFeatures(true); + + uint32_t docid = 5; + + a.reset(docid); + a.setNumOccs(1); + a.setFieldLength(30); + a.appendPosition(make_pos(5)); + + b.reset(docid); + b.setNumOccs(1); + b.setFieldLength(30); + b.appendPosition(make_pos(5)); + + merger.merge(docid); + EXPECT_EQ(docid, out.getDocId()); + EXPECT_EQ(1u, out.end() - out.begin()); + EXPECT_EQ( 5u, out.begin()[0].getPosition()); + EXPECT_EQ(1.5, out.begin()[0].getMatchExactness()); + EXPECT_EQ(1u, out.getNumOccs()); + EXPECT_EQ(30u, out.getFieldLength()); } -TEST_APPHOOK(Test); +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchlib/src/tests/queryeval/blueprint/intermediate_blueprints_test.cpp b/searchlib/src/tests/queryeval/blueprint/intermediate_blueprints_test.cpp index bec1691df23..eb6e49747a1 100644 --- a/searchlib/src/tests/queryeval/blueprint/intermediate_blueprints_test.cpp +++ b/searchlib/src/tests/queryeval/blueprint/intermediate_blueprints_test.cpp @@ -1291,7 +1291,7 @@ TEST("require that children does not optimize when parents refuse them to") { } } -TEST("require_that_unpack_optimization_is_overruled_by_equiv") { +TEST("require_that_unpack_optimization_is_not_overruled_by_equiv") { FieldSpecBaseList fields; fields.add(FieldSpecBase(1, 1)); fields.add(FieldSpecBase(2, 2)); @@ -1322,7 +1322,7 @@ TEST("require_that_unpack_optimization_is_overruled_by_equiv") { EXPECT_EQUAL("search::queryeval::EquivImpl<true>", search->getClassName()); { const MultiSearch & e = dynamic_cast<const MultiSearch &>(*search); - EXPECT_EQUAL("search::queryeval::OrLikeSearch<true, search::queryeval::(anonymous namespace)::FullUnpack>", + EXPECT_EQUAL("search::queryeval::OrLikeSearch<true, search::queryeval::(anonymous namespace)::SelectiveUnpack>", e.getChildren()[0]->getClassName()); } @@ -1332,7 +1332,7 @@ TEST("require_that_unpack_optimization_is_overruled_by_equiv") { EXPECT_EQUAL("search::queryeval::EquivImpl<true>", search->getClassName()); { const MultiSearch & e = dynamic_cast<const MultiSearch &>(*search); - EXPECT_EQUAL("search::queryeval::OrLikeSearch<true, search::queryeval::(anonymous namespace)::FullUnpack>", + EXPECT_EQUAL("search::queryeval::OrLikeSearch<true, search::queryeval::NoUnpack>", e.getChildren()[0]->getClassName()); } } diff --git a/searchlib/src/tests/queryeval/equiv/CMakeLists.txt b/searchlib/src/tests/queryeval/equiv/CMakeLists.txt index 368f12e34a5..246b2ce0426 100644 --- a/searchlib/src/tests/queryeval/equiv/CMakeLists.txt +++ b/searchlib/src/tests/queryeval/equiv/CMakeLists.txt @@ -1,8 +1,10 @@ # Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +find_package(GTest REQUIRED) vespa_add_executable(searchlib_equiv_test_app TEST SOURCES equiv_test.cpp DEPENDS searchlib + GTest::GTest ) vespa_add_test(NAME searchlib_equiv_test_app COMMAND searchlib_equiv_test_app) diff --git a/searchlib/src/tests/queryeval/equiv/equiv_test.cpp b/searchlib/src/tests/queryeval/equiv/equiv_test.cpp index ecd1c8cd218..412130ecaab 100644 --- a/searchlib/src/tests/queryeval/equiv/equiv_test.cpp +++ b/searchlib/src/tests/queryeval/equiv/equiv_test.cpp @@ -1,11 +1,11 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include <vespa/log/log.h> LOG_SETUP("equiv_test"); -#include <vespa/vespalib/testkit/testapp.h> #include <vespa/searchlib/queryeval/leaf_blueprints.h> #include <vespa/searchlib/queryeval/intermediate_blueprints.h> #include <vespa/searchlib/queryeval/equiv_blueprint.h> #include <vespa/searchlib/fef/matchdatalayout.h> +#include <vespa/vespalib/gtest/gtest.h> using namespace search::queryeval; using search::fef::MatchData; @@ -14,22 +14,28 @@ using search::fef::TermFieldHandle; using search::fef::TermFieldMatchData; using search::fef::FieldPositionsIterator; -class Test : public vespalib::TestApp { -public: - void testEquiv(); - int Main() override; +class EquivTest : public ::testing::Test { +protected: + EquivTest(); + ~EquivTest(); + + void test_equiv(bool strict, bool unpack_normal_features, bool unpack_interleaved_features); }; +EquivTest::EquivTest() = default; + +EquivTest::~EquivTest() = default; + void -Test::testEquiv() +EquivTest::test_equiv(bool strict, bool unpack_normal_features, bool unpack_interleaved_features) { FakeResult a; FakeResult b; FakeResult c; - a.doc(5).pos(1); - b.doc(5).pos(2); - c.doc(5).pos(3).doc(10).pos(4); + a.doc(5).pos(1).len(30).field_length(30).num_occs(1); + b.doc(5).pos(2).len(30).field_length(30).num_occs(1); + c.doc(5).pos(3).len(30).field_length(30).num_occs(1).doc(10).pos(4).len(35).field_length(35).num_occs(1); MatchDataLayout subLayout; TermFieldHandle fbh11 = subLayout.allocTermField(1); @@ -39,91 +45,151 @@ Test::testEquiv() FieldSpecBaseList fields; fields.add(FieldSpecBase(1, 1)); fields.add(FieldSpecBase(2, 2)); - EquivBlueprint *eq_b = new EquivBlueprint(fields, subLayout); - - eq_b->addTerm(Blueprint::UP(new FakeBlueprint(FieldSpec("foo", 1, fbh11), a)), 1.0); - eq_b->addTerm(Blueprint::UP(new FakeBlueprint(FieldSpec("bar", 2, fbh21), b)), 1.0); - eq_b->addTerm(Blueprint::UP(new FakeBlueprint(FieldSpec("bar", 2, fbh22), c)), 1.0); - - Blueprint::UP bp(eq_b); - for (int i = 0; i <= 1; ++i) { - bool strict = (i == 0); - TEST_STATE(strict ? "strict" : "non-strict"); - MatchData::UP md = MatchData::makeTestInstance(100, 10); - bp->fetchPostings(ExecuteInfo::create(strict)); - SearchIterator::UP search = bp->createSearch(*md, strict); - search->initFullRange(); - - EXPECT_TRUE(!search->seek(3)); - if (!strict) { - EXPECT_EQUAL(SearchIterator::beginId(), search->getDocId()); - EXPECT_TRUE(search->seek(5u)); - } - EXPECT_EQUAL(5u, search->getDocId()); - { // test doc 5 results - search->unpack(5u); - { - TermFieldMatchData &data = *md->resolveTermField(1); - EXPECT_EQUAL(1u, data.getFieldId()); - EXPECT_EQUAL(5u, data.getDocId()); - FieldPositionsIterator itr = data.getIterator(); - EXPECT_EQUAL(1u, itr.size()); + auto bp = std::make_unique<EquivBlueprint>(fields, subLayout); + + bp->addTerm(std::make_unique<FakeBlueprint>(FieldSpec("foo", 1, fbh11), a), 1.0); + bp->addTerm(std::make_unique<FakeBlueprint>(FieldSpec("bar", 2, fbh21), b), 1.0); + bp->addTerm(std::make_unique<FakeBlueprint>(FieldSpec("bar", 2, fbh22), c), 1.0); + + MatchData::UP md = MatchData::makeTestInstance(100, 10); + for (uint32_t field_id = 1; field_id <= 2; ++field_id) { + TermFieldMatchData &data = *md->resolveTermField(field_id); + data.setNeedNormalFeatures(unpack_normal_features); + data.setNeedInterleavedFeatures(unpack_interleaved_features); + } + bp->fetchPostings(ExecuteInfo::create(strict)); + SearchIterator::UP search = bp->createSearch(*md, strict); + search->initFullRange(); + + EXPECT_TRUE(!search->seek(3)); + if (!strict) { + EXPECT_EQ(SearchIterator::beginId(), search->getDocId()); + EXPECT_TRUE(search->seek(5u)); + } + EXPECT_EQ(5u, search->getDocId()); + { // test doc 5 results + search->unpack(5u); + { + TermFieldMatchData &data = *md->resolveTermField(1); + EXPECT_EQ(1u, data.getFieldId()); + EXPECT_EQ(5u, data.getDocId()); + FieldPositionsIterator itr = data.getIterator(); + if (unpack_normal_features) { + EXPECT_EQ(1u, itr.size()); ASSERT_TRUE(itr.valid()); - EXPECT_EQUAL(1u, itr.getPosition()); + EXPECT_EQ(1u, itr.getPosition()); itr.next(); - EXPECT_TRUE(!itr.valid()); } - { - TermFieldMatchData &data = *md->resolveTermField(2); - EXPECT_EQUAL(2u, data.getFieldId()); - EXPECT_EQUAL(5u, data.getDocId()); - FieldPositionsIterator itr = data.getIterator(); - EXPECT_EQUAL(2u, itr.size()); + EXPECT_TRUE(!itr.valid()); + if (unpack_interleaved_features) { + EXPECT_EQ(1u, data.getNumOccs()); + EXPECT_EQ(30u, data.getFieldLength()); + } else { + EXPECT_EQ(0u, data.getNumOccs()); + EXPECT_EQ(0u, data.getFieldLength()); + } + } + { + TermFieldMatchData &data = *md->resolveTermField(2); + EXPECT_EQ(2u, data.getFieldId()); + EXPECT_EQ(5u, data.getDocId()); + FieldPositionsIterator itr = data.getIterator(); + if (unpack_normal_features) { + EXPECT_EQ(2u, itr.size()); ASSERT_TRUE(itr.valid()); - EXPECT_EQUAL(2u, itr.getPosition()); + EXPECT_EQ(2u, itr.getPosition()); itr.next(); ASSERT_TRUE(itr.valid()); - EXPECT_EQUAL(3u, itr.getPosition()); + EXPECT_EQ(3u, itr.getPosition()); itr.next(); - EXPECT_TRUE(!itr.valid()); + } + EXPECT_TRUE(!itr.valid()); + if (unpack_interleaved_features) { + EXPECT_EQ(2u, data.getNumOccs()); + EXPECT_EQ(30u, data.getFieldLength()); + } else { + EXPECT_EQ(0u, data.getNumOccs()); + EXPECT_EQ(0u, data.getFieldLength()); } } - EXPECT_TRUE(!search->seek(7)); - if (!strict) { - EXPECT_EQUAL(5u, search->getDocId()); - EXPECT_TRUE(search->seek(10u)); - } - EXPECT_EQUAL(10u, search->getDocId()); - { // test doc 10 results - search->unpack(10u); - EXPECT_EQUAL(5u, md->resolveTermField(1)->getDocId()); // no match - { - TermFieldMatchData &data = *md->resolveTermField(2); - EXPECT_EQUAL(2u, data.getFieldId()); - EXPECT_EQUAL(10u, data.getDocId()); - FieldPositionsIterator itr = data.getIterator(); - EXPECT_EQUAL(1u, itr.size()); + } + EXPECT_TRUE(!search->seek(7)); + if (!strict) { + EXPECT_EQ(5u, search->getDocId()); + EXPECT_TRUE(search->seek(10u)); + } + EXPECT_EQ(10u, search->getDocId()); + { // test doc 10 results + search->unpack(10u); + EXPECT_EQ(5u, md->resolveTermField(1)->getDocId()); // no match + { + TermFieldMatchData &data = *md->resolveTermField(2); + EXPECT_EQ(2u, data.getFieldId()); + EXPECT_EQ(10u, data.getDocId()); + FieldPositionsIterator itr = data.getIterator(); + if (unpack_normal_features) { + EXPECT_EQ(1u, itr.size()); ASSERT_TRUE(itr.valid()); - EXPECT_EQUAL(4u, itr.getPosition()); + EXPECT_EQ(4u, itr.getPosition()); itr.next(); - EXPECT_TRUE(!itr.valid()); + } + EXPECT_TRUE(!itr.valid()); + if (unpack_interleaved_features) { + EXPECT_EQ(1u, data.getNumOccs()); + EXPECT_EQ(35u, data.getFieldLength()); + } else { + EXPECT_EQ(0u, data.getNumOccs()); + EXPECT_EQ(0u, data.getFieldLength()); } } - EXPECT_TRUE(!search->seek(13)); - if (strict) { - EXPECT_TRUE(search->isAtEnd()); - } else { - EXPECT_EQUAL(10u, search->getDocId()); - } } + EXPECT_TRUE(!search->seek(13)); + if (strict) { + EXPECT_TRUE(search->isAtEnd()); + } else { + EXPECT_EQ(10u, search->getDocId()); + } +} + + +TEST_F(EquivTest, nonstrict) +{ + test_equiv(false, true, false); +} + +TEST_F(EquivTest, strict) +{ + test_equiv(true, true, false); +} + +TEST_F(EquivTest, nonstrict_no_normal_no_interleaved) +{ + test_equiv(false, false, false); +} + +TEST_F(EquivTest, strict_no_normal_no_interleaved) +{ + test_equiv(true, false, false); +} + +TEST_F(EquivTest, nonstrict_no_normal_interleaved) +{ + test_equiv(false, false, true); +} + +TEST_F(EquivTest, strict_no_normal_interleaved) +{ + test_equiv(true, false, true); +} + +TEST_F(EquivTest, nonstrict_normal_interleaved) +{ + test_equiv(false, true, true); } -int -Test::Main() +TEST_F(EquivTest, strict_normal_interleaved) { - TEST_INIT("equiv_test"); - testEquiv(); - TEST_DONE(); + test_equiv(true, true, true); } -TEST_APPHOOK(Test); +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchlib/src/tests/queryeval/multibitvectoriterator/multibitvectoriterator_test.cpp b/searchlib/src/tests/queryeval/multibitvectoriterator/multibitvectoriterator_test.cpp index 12c38d3fe02..296d06d0b7b 100644 --- a/searchlib/src/tests/queryeval/multibitvectoriterator/multibitvectoriterator_test.cpp +++ b/searchlib/src/tests/queryeval/multibitvectoriterator/multibitvectoriterator_test.cpp @@ -34,6 +34,7 @@ public: void testAndWith(bool invert); void testEndGuard(bool invert); void testIteratorConformance(); + void testUnpackOfOr(); template<typename T> void testThatOptimizePreservesUnpack(); template <typename T> @@ -44,6 +45,7 @@ public: void testSearch(bool strict, bool invert); int Main() override; private: + void verifyUnpackOfOr(const UnpackInfo & unpackInfo); void verifySelectiveUnpack(SearchIterator & s, const TermFieldMatchData * tfmd); void searchAndCompare(SearchIterator::UP s, uint32_t docIdLimit); void setup(); @@ -58,6 +60,8 @@ private: for (int i = 0; i < 3; ++i) { if (_bvs_inverted[i]->testBit(1)) { _bvs[i]->clearBit(1); + } else { + _bvs[i]->setBit(1); } } } @@ -253,6 +257,63 @@ Test::testThatOptimizePreservesUnpack() fixup_bitvectors(); } +void verifyOrUnpack(SearchIterator & s, TermFieldMatchData tfmd[3]) { + s.initFullRange(); + s.seek(1); + for (size_t i = 0; i < 3; i++) { + EXPECT_EQUAL(0u, tfmd[i].getDocId()); + } + s.unpack(1); + EXPECT_EQUAL(0u, tfmd[0].getDocId()); + EXPECT_EQUAL(1u, tfmd[1].getDocId()); + EXPECT_EQUAL(0u, tfmd[2].getDocId()); +} + +void +Test::testUnpackOfOr() { + _bvs[0]->clearBit(1); + _bvs[1]->setBit(1); + _bvs[2]->clearBit(1); + UnpackInfo all; + all.forceAll(); + verifyUnpackOfOr(all); + + UnpackInfo unpackInfo; + unpackInfo.add(1); + unpackInfo.add(2); + verifyUnpackOfOr(unpackInfo); + + fixup_bitvectors(); +} + +void +Test::verifyUnpackOfOr(const UnpackInfo &unpackInfo) +{ + TermFieldMatchData tfmdA[3]; + MultiSearch::Children children; + children.push_back(createIter(0, false, tfmdA[0], false).release()); + children.push_back(createIter(1, false, tfmdA[1], false).release()); + children.push_back(createIter(2, false, tfmdA[2], false).release()); + SearchIterator::UP s(OrSearch::create(children, false, unpackInfo)); + verifyOrUnpack(*s, tfmdA); + + for (auto & tfmd : tfmdA) { + tfmd.resetOnlyDocId(0); + } + + const MultiSearch * ms = dynamic_cast<const MultiSearch *>(s.get()); + EXPECT_TRUE(ms != nullptr); + EXPECT_EQUAL(3u, ms->getChildren().size()); + + s = MultiBitVectorIteratorBase::optimize(std::move(s)); + s->initFullRange(); + ms = dynamic_cast<const MultiBitVectorIteratorBase *>(s.get()); + EXPECT_TRUE(ms != nullptr); + EXPECT_EQUAL(3u, ms->getChildren().size()); + verifyOrUnpack(*s, tfmdA); + +} + void Test::verifySelectiveUnpack(SearchIterator & s, const TermFieldMatchData * tfmd) { @@ -584,6 +645,8 @@ Test::Main() testThatOptimizePreservesUnpack<OrSearch>(); testThatOptimizePreservesUnpack<AndSearch>(); TEST_FLUSH(); + testUnpackOfOr(); + TEST_FLUSH(); testEndGuard(false); testEndGuard(true); TEST_FLUSH(); diff --git a/searchlib/src/vespa/searchlib/common/bitvectoriterator.cpp b/searchlib/src/vespa/searchlib/common/bitvectoriterator.cpp index 9c17dfa6f74..18cece7b8b0 100644 --- a/searchlib/src/vespa/searchlib/common/bitvectoriterator.cpp +++ b/searchlib/src/vespa/searchlib/common/bitvectoriterator.cpp @@ -2,7 +2,6 @@ #include "bitvectoriterator.h" #include <vespa/searchlib/queryeval/emptysearch.h> -#include <vespa/searchlib/fef/termfieldmatchdata.h> #include <vespa/searchlib/fef/termfieldmatchdataarray.h> #include <vespa/vespalib/objects/visit.h> @@ -39,12 +38,6 @@ BitVectorIterator::visitMembers(vespalib::ObjectVisitor &visitor) const visit(visitor, "termfieldmatchdata.docid", _tfmd.getDocId()); } -void -BitVectorIterator::doUnpack(uint32_t docId) -{ - _tfmd.resetOnlyDocId(docId); -} - template<bool inverse> class BitVectorIteratorT : public BitVectorIterator { public: diff --git a/searchlib/src/vespa/searchlib/common/bitvectoriterator.h b/searchlib/src/vespa/searchlib/common/bitvectoriterator.h index 6200837d449..8b1112de97e 100644 --- a/searchlib/src/vespa/searchlib/common/bitvectoriterator.h +++ b/searchlib/src/vespa/searchlib/common/bitvectoriterator.h @@ -4,11 +4,11 @@ #include "bitvector.h" #include <vespa/searchlib/queryeval/searchiterator.h> +#include <vespa/searchlib/fef/termfieldmatchdata.h> namespace search { namespace fef { class TermFieldMatchDataArray; } -namespace fef { class TermFieldMatchData; } class BitVectorIterator : public queryeval::SearchIterator { @@ -20,7 +20,9 @@ protected: const BitVector & _bv; private: void visitMembers(vespalib::ObjectVisitor &visitor) const override; - void doUnpack(uint32_t docId) override; + void doUnpack(uint32_t docId) override final { + _tfmd.resetOnlyDocId(docId); + } bool isBitVector() const override { return true; } fef::TermFieldMatchData &_tfmd; public: diff --git a/searchlib/src/vespa/searchlib/features/attributematchfeature.cpp b/searchlib/src/vespa/searchlib/features/attributematchfeature.cpp index bae7b2a1157..bf52e5e0b5e 100644 --- a/searchlib/src/vespa/searchlib/features/attributematchfeature.cpp +++ b/searchlib/src/vespa/searchlib/features/attributematchfeature.cpp @@ -60,7 +60,6 @@ AttributeMatchExecutor<T>::Computer::Computer(const IQueryEnvironment & env, Att _valueCount(0), _md(nullptr) { - _buffer.allocate(_params.attribute->getMaxValueCount()); QueryTermHelper queryTerms(env); for (const QueryTerm & qt : queryTerms.terms()) { _totalTermWeight += qt.termData()->getWeight().percent(); diff --git a/searchlib/src/vespa/searchlib/fef/termfieldmatchdata.cpp b/searchlib/src/vespa/searchlib/fef/termfieldmatchdata.cpp index 781135bf246..7e3e0f5e4bc 100644 --- a/searchlib/src/vespa/searchlib/fef/termfieldmatchdata.cpp +++ b/searchlib/src/vespa/searchlib/fef/termfieldmatchdata.cpp @@ -104,7 +104,8 @@ TermFieldMatchData::swap(TermFieldMatchData &rhs) namespace { -constexpr size_t MAX_ELEMS = std::numeric_limits<uint16_t>::max(); +constexpr size_t MAX_ELEMS = std::numeric_limits<uint16_t>::max(); +constexpr size_t INITIAL_ELEMS = 1024/sizeof(TermFieldMatchDataPosition); } @@ -128,7 +129,7 @@ TermFieldMatchData::allocateVector() { assert(_sz < 2); assert(!allocated()); - size_t newSize = 2; + size_t newSize = INITIAL_ELEMS; TermFieldMatchDataPosition * n = new TermFieldMatchDataPosition[newSize]; if (_sz > 0) { n[0] = *getFixed(); diff --git a/searchlib/src/vespa/searchlib/fef/termmatchdatamerger.cpp b/searchlib/src/vespa/searchlib/fef/termmatchdatamerger.cpp index 97cb829e30c..973e11fc0d2 100644 --- a/searchlib/src/vespa/searchlib/fef/termmatchdatamerger.cpp +++ b/searchlib/src/vespa/searchlib/fef/termmatchdatamerger.cpp @@ -41,31 +41,52 @@ TermMatchDataMerger::merge(uint32_t docid, { _scratch.clear(); bool wasMatch = false; + bool needs_normal_features = out.needs_normal_features(); + bool needs_interleaved_features = out.needs_interleaved_features(); + uint32_t num_occs = 0u; + uint16_t field_length = 0u; for (size_t i = 0; i < in.size(); ++i) { const TermFieldMatchData *md = in[i].matchData; if (md->getDocId() == docid) { - for (const TermFieldMatchDataPosition &iter : *md) { - double exactness = in[i].exactness * iter.getMatchExactness(); - _scratch.push_back(iter); - _scratch.back().setMatchExactness(exactness); + if (needs_normal_features) { + for (const TermFieldMatchDataPosition &iter : *md) { + double exactness = in[i].exactness * iter.getMatchExactness(); + _scratch.push_back(iter); + _scratch.back().setMatchExactness(exactness); + } + } + if (needs_interleaved_features) { + num_occs += md->getNumOccs(); + field_length = std::max(field_length, md->getFieldLength()); } wasMatch = true; } } if (wasMatch) { out.reset(docid); - if (_scratch.size() > 0) { - std::sort(_scratch.begin(), _scratch.end(), - TermFieldMatchDataPosition::compareWithExactness); - TermFieldMatchDataPosition prev = _scratch[0]; - for (size_t i = 1; i < _scratch.size(); ++i) { - const TermFieldMatchDataPosition &curr = _scratch[i]; - if (prev.key() < curr.key()) { - out.appendPosition(prev); - prev = curr; + if (needs_normal_features) { + num_occs = 0; + if (_scratch.size() > 0) { + std::sort(_scratch.begin(), _scratch.end(), + TermFieldMatchDataPosition::compareWithExactness); + TermFieldMatchDataPosition prev = _scratch[0]; + for (size_t i = 1; i < _scratch.size(); ++i) { + const TermFieldMatchDataPosition &curr = _scratch[i]; + if (prev.key() < curr.key()) { + out.appendPosition(prev); + prev = curr; + ++num_occs; + } } + out.appendPosition(prev); + ++num_occs; } - out.appendPosition(prev); + } + if (needs_interleaved_features) { + constexpr uint32_t max_num_occs = std::numeric_limits<uint16_t>::max(); + uint16_t capped_num_occs = std::min(num_occs, max_num_occs); + out.setNumOccs(std::min(capped_num_occs, field_length)); + out.setFieldLength(field_length); } } } diff --git a/searchlib/src/vespa/searchlib/queryeval/equiv_blueprint.cpp b/searchlib/src/vespa/searchlib/queryeval/equiv_blueprint.cpp index 08a05b25772..cf378c95487 100644 --- a/searchlib/src/vespa/searchlib/queryeval/equiv_blueprint.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/equiv_blueprint.cpp @@ -4,9 +4,42 @@ #include "equivsearch.h" #include "field_spec.hpp" #include <vespa/vespalib/objects/visit.hpp> +#include <vespa/vespalib/stllike/hash_map.hpp> namespace search::queryeval { +namespace { + +class UnpackNeed +{ + bool _needs_normal_features; + bool _needs_interleaved_features; +public: + UnpackNeed() + : _needs_normal_features(false), + _needs_interleaved_features(false) + { + } + + void observe(const fef::TermFieldMatchData &output) + { + if (output.needs_normal_features()) { + _needs_normal_features = true; + } + if (output.needs_interleaved_features()) { + _needs_interleaved_features = true; + } + } + + void notify(fef::TermFieldMatchData &input) const + { + input.setNeedNormalFeatures(_needs_normal_features); + input.setNeedInterleavedFeatures(_needs_interleaved_features); + } +}; + +}; + EquivBlueprint::EquivBlueprint(const FieldSpecBaseList &fields, fef::MatchDataLayout subtree_mdl) : ComplexLeafBlueprint(fields), @@ -26,10 +59,16 @@ EquivBlueprint::createLeafSearch(const fef::TermFieldMatchDataArray &outputs, bo fef::MatchData::UP md = _layout.createMatchData(); MultiSearch::Children children(_terms.size()); fef::TermMatchDataMerger::Inputs childMatch; + vespalib::hash_map<uint16_t, UnpackNeed> unpack_needs(outputs.size()); + for (size_t i = 0; i < outputs.size(); ++i) { + unpack_needs[outputs[i]->getFieldId()].observe(*outputs[i]); + } for (size_t i = 0; i < _terms.size(); ++i) { const State &childState = _terms[i]->getState(); for (size_t j = 0; j < childState.numFields(); ++j) { - childMatch.emplace_back(childState.field(j).resolve(*md), _exactness[i]); + auto *child_term_field_match_data = childState.field(j).resolve(*md); + unpack_needs[child_term_field_match_data->getFieldId()].notify(*child_term_field_match_data); + childMatch.emplace_back(child_term_field_match_data, _exactness[i]); } children[i] = _terms[i]->createSearch(*md, strict).release(); } diff --git a/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.cpp b/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.cpp index bba331284f9..e9e34b4f5ce 100644 --- a/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.cpp @@ -177,7 +177,9 @@ MultiBitVectorIteratorBase::doUnpack(uint32_t docid) MultiSearch::doUnpack(docid); } else { auto &children = getChildren(); - _unpackInfo.each([&children,docid](size_t i){children[i]->doUnpack(docid);}, children.size()); + _unpackInfo.each([&children,docid](size_t i) { + static_cast<BitVectorIterator *>(children[i])->unpack(docid); + }, children.size()); } } diff --git a/searchlib/src/vespa/searchlib/queryeval/orsearch.cpp b/searchlib/src/vespa/searchlib/queryeval/orsearch.cpp index 977080f8266..b5e0ff9c423 100644 --- a/searchlib/src/vespa/searchlib/queryeval/orsearch.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/orsearch.cpp @@ -5,8 +5,7 @@ #include "termwise_helper.h" #include <vespa/searchlib/common/bitvector.h> -namespace search { -namespace queryeval { +namespace search::queryeval { namespace { @@ -88,7 +87,6 @@ OrSearch::create(const MultiSearch::Children &children, bool strict) { SearchIterator * OrSearch::create(const MultiSearch::Children &children, bool strict, const UnpackInfo & unpackInfo) { - (void) unpackInfo; if (strict) { if (unpackInfo.unpackAll()) { return new OrLikeSearch<true, FullUnpack>(children, FullUnpack()); @@ -108,5 +106,4 @@ OrSearch::create(const MultiSearch::Children &children, bool strict, const Unpac } } -} // namespace queryeval -} // namespace search +} diff --git a/zookeeper-server/zookeeper-server-3.5/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperServerImpl.java b/zookeeper-server/zookeeper-server-3.5/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperServerImpl.java index 1f4be8852a3..1f66c919a41 100644 --- a/zookeeper-server/zookeeper-server-3.5/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperServerImpl.java +++ b/zookeeper-server/zookeeper-server-3.5/src/main/java/com/yahoo/vespa/zookeeper/VespaZooKeeperServerImpl.java @@ -108,6 +108,7 @@ public class VespaZooKeeperServerImpl extends AbstractComponent implements Runna sb.append("admin.enableServer=false").append("\n"); // Need NettyServerCnxnFactory to be able to use TLS for communication sb.append("serverCnxnFactory=org.apache.zookeeper.server.NettyServerCnxnFactory").append("\n"); + sb.append("quorumListenOnAllIPs=true").append("\n"); ensureThisServerIsRepresented(config.myid(), config.server()); config.server().forEach(server -> addServerToCfg(sb, server)); SSLContext sslContext = new SslContextBuilder().build(); diff --git a/zookeeper-server/zookeeper-server-3.5/src/test/java/com/yahoo/vespa/zookeeper/VespaZooKeeperServerImplTest.java b/zookeeper-server/zookeeper-server-3.5/src/test/java/com/yahoo/vespa/zookeeper/VespaZooKeeperServerImplTest.java index 863d2dba708..72351244cce 100644 --- a/zookeeper-server/zookeeper-server-3.5/src/test/java/com/yahoo/vespa/zookeeper/VespaZooKeeperServerImplTest.java +++ b/zookeeper-server/zookeeper-server-3.5/src/test/java/com/yahoo/vespa/zookeeper/VespaZooKeeperServerImplTest.java @@ -181,7 +181,8 @@ public class VespaZooKeeperServerImplTest { "autopurge.snapRetainCount=15\n" + "4lw.commands.whitelist=conf,cons,crst,dirs,dump,envi,mntr,ruok,srst,srvr,stat,wchs\n" + "admin.enableServer=false\n" + - "serverCnxnFactory=org.apache.zookeeper.server.NettyServerCnxnFactory\n"; + "serverCnxnFactory=org.apache.zookeeper.server.NettyServerCnxnFactory\n" + + "quorumListenOnAllIPs=true\n"; } private String quorumKeyStoreAndTrustStoreConfig(File jksKeyStoreFilePath, File caCertificatesFilePath) { |