diff options
18 files changed, 133 insertions, 113 deletions
diff --git a/client/go/script-utils/configserver/fix_dirs_and_files.go b/client/go/script-utils/configserver/fix_dirs_and_files.go index 0c0eb9af2f9..4fcafd80202 100644 --- a/client/go/script-utils/configserver/fix_dirs_and_files.go +++ b/client/go/script-utils/configserver/fix_dirs_and_files.go @@ -19,11 +19,9 @@ func makeFixSpec() util.FixSpec { } func fixDirsAndFiles(fixSpec util.FixSpec) { - fixSpec.FixDir("conf/zookeeper") // TODO: Remove when files are only written to var/zookeeper/conf fixSpec.FixDir("var/zookeeper") fixSpec.FixDir("var/zookeeper/conf") fixSpec.FixDir("var/zookeeper/version-2") - fixSpec.FixFile("conf/zookeeper/zookeeper.cfg") // TODO: Remove when files are only written to var/zookeeper/conf fixSpec.FixFile("var/zookeeper/conf/zookeeper.cfg") fixSpec.FixFile("var/zookeeper/myid") } diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/Capacity.java b/config-provisioning/src/main/java/com/yahoo/config/provision/Capacity.java index dd8d181e1df..e63750e0e11 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/Capacity.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/Capacity.java @@ -26,7 +26,7 @@ public final class Capacity { if (max.smallerThan(min)) throw new IllegalArgumentException("The max capacity must be larger than the min capacity, but got min " + min + " and max " + max); - if (!min.equals(max) && Stream.of(min, max).anyMatch(cr -> !cr.nodeResources().gpuResources().isDefault())) + if (!min.equals(max) && Stream.of(min, max).anyMatch(cr -> !cr.nodeResources().gpuResources().isZero())) throw new IllegalArgumentException("Capacity range does not allow GPU, got min " + min + " and max " + max); this.min = min; this.max = max; diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java index 92513636eaf..2f2310c3703 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java @@ -123,7 +123,7 @@ public class NodeResources { public record GpuResources(int count, double memoryGb) { - private static final GpuResources none = new GpuResources(0, 0); + private static final GpuResources zero = new GpuResources(0, 0); public GpuResources { validate(memoryGb, "memory"); @@ -138,9 +138,14 @@ public class NodeResources { this.memoryGb < other.memoryGb; } + public boolean isZero() { return this.equals(zero); } + + public static GpuResources zero() { return zero; } + public boolean isDefault() { return this.equals(getDefault()); } - public static GpuResources getDefault() { return none; } + /** Returns zero gpu resources. */ + public static GpuResources getDefault() { return zero; } public GpuResources plus(GpuResources other) { return new GpuResources(this.count + other.count, this.memoryGb + other.memoryGb); @@ -274,7 +279,7 @@ public class NodeResources { /** Returns this with all numbers set to 0 */ public NodeResources justNonNumbers() { if (isUnspecified()) return unspecified(); - return withVcpu(0).withMemoryGb(0).withDiskGb(0).withBandwidthGbps(0).with(GpuResources.getDefault()); + return withVcpu(0).withMemoryGb(0).withDiskGb(0).withBandwidthGbps(0).with(GpuResources.zero()); } public NodeResources subtract(NodeResources other) { @@ -499,6 +504,10 @@ public class NodeResources { return value; } + public boolean isZero() { + return this.equals(zero); + } + public static NodeResources zero() { return zero; } } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Group.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Group.java index fbea58054da..3d3efce2ab2 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Group.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Group.java @@ -2,8 +2,6 @@ package com.yahoo.search.dispatch.searchcluster; import java.util.List; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicLong; import java.util.logging.Logger; /** @@ -21,12 +19,15 @@ public class Group { private final int id; private final List<Node> nodes; - private final AtomicBoolean hasSufficientCoverage = new AtomicBoolean(true); - private final AtomicBoolean hasFullCoverage = new AtomicBoolean(true); - private final AtomicLong activeDocuments = new AtomicLong(0); - private final AtomicLong targetActiveDocuments = new AtomicLong(0); - private final AtomicBoolean isBlockingWrites = new AtomicBoolean(false); - private final AtomicBoolean isBalanced = new AtomicBoolean(true); + + // Using volatile to ensure visibility for reader. + // All udates are done in a single writer thread + private volatile boolean hasSufficientCoverage = true; + private volatile boolean hasFullCoverage = true; + private volatile long activeDocuments = 0; + private volatile long targetActiveDocuments = 0; + private volatile boolean isBlockingWrites = false; + private volatile boolean isBalanced = true; public Group(int id, List<Node> nodes) { this.id = id; @@ -53,11 +54,11 @@ public class Group { * (compared to other groups) that is should receive traffic */ public boolean hasSufficientCoverage() { - return hasSufficientCoverage.get(); + return hasSufficientCoverage; } void setHasSufficientCoverage(boolean sufficientCoverage) { - hasSufficientCoverage.lazySet(sufficientCoverage); + hasSufficientCoverage = sufficientCoverage; } public int workingNodes() { @@ -66,39 +67,38 @@ public class Group { public void aggregateNodeValues() { long activeDocs = nodes.stream().filter(node -> node.isWorking() == Boolean.TRUE).mapToLong(Node::getActiveDocuments).sum(); - activeDocuments.set(activeDocs); - long targetActiveDocs = nodes.stream().filter(node -> node.isWorking() == Boolean.TRUE).mapToLong(Node::getTargetActiveDocuments).sum(); - targetActiveDocuments.set(targetActiveDocs); - isBlockingWrites.set(nodes.stream().anyMatch(Node::isBlockingWrites)); + activeDocuments = activeDocs; + targetActiveDocuments = nodes.stream().filter(node -> node.isWorking() == Boolean.TRUE).mapToLong(Node::getTargetActiveDocuments).sum(); + isBlockingWrites = nodes.stream().anyMatch(Node::isBlockingWrites); int numWorkingNodes = workingNodes(); if (numWorkingNodes > 0) { long average = activeDocs / numWorkingNodes; long skew = nodes.stream().filter(node -> node.isWorking() == Boolean.TRUE).mapToLong(node -> Math.abs(node.getActiveDocuments() - average)).sum(); boolean balanced = skew <= activeDocs * maxContentSkew; - if (balanced != isBalanced.get()) { + if (balanced != isBalanced) { if (!isSparse()) log.info("Content in " + this + ", with " + numWorkingNodes + "/" + nodes.size() + " working nodes, is " + (balanced ? "" : "not ") + "well balanced. Current deviation: " + skew * 100 / activeDocs + "%. Active documents: " + activeDocs + ", skew: " + skew + ", average: " + average + (balanced ? "" : ". Top-k summary fetch optimization is deactivated.")); - isBalanced.set(balanced); + isBalanced = balanced; } } else { - isBalanced.set(true); + isBalanced = true; } } /** Returns the active documents on this group. If unknown, 0 is returned. */ - long activeDocuments() { return activeDocuments.get(); } + long activeDocuments() { return activeDocuments; } /** Returns the target active documents on this group. If unknown, 0 is returned. */ - long targetActiveDocuments() { return targetActiveDocuments.get(); } + long targetActiveDocuments() { return targetActiveDocuments; } /** Returns whether any node in this group is currently blocking write operations */ - public boolean isBlockingWrites() { return isBlockingWrites.get(); } + public boolean isBlockingWrites() { return isBlockingWrites; } /** Returns whether the nodes in the group have about the same number of documents */ - public boolean isBalanced() { return isBalanced.get(); } + public boolean isBalanced() { return isBalanced; } /** Returns whether this group has too few documents per node to expect it to be balanced */ public boolean isSparse() { @@ -107,7 +107,8 @@ public class Group { } public boolean fullCoverageStatusChanged(boolean hasFullCoverageNow) { - boolean previousState = hasFullCoverage.getAndSet(hasFullCoverageNow); + boolean previousState = hasFullCoverage; + hasFullCoverage = hasFullCoverageNow; return previousState != hasFullCoverageNow; } diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Node.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Node.java index 73a3c3742cc..38d51585ae4 100644 --- a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Node.java +++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/Node.java @@ -2,7 +2,6 @@ package com.yahoo.search.dispatch.searchcluster; import java.util.Objects; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; /** @@ -14,17 +13,17 @@ import java.util.concurrent.atomic.AtomicLong; public class Node { private final int key; - private int pathIndex; private final String hostname; private final int group; + private int pathIndex; - private final AtomicBoolean statusIsKnown = new AtomicBoolean(false); - private final AtomicBoolean working = new AtomicBoolean(true); - private final AtomicLong activeDocuments = new AtomicLong(0); - private final AtomicLong targetActiveDocuments = new AtomicLong(0); private final AtomicLong pingSequence = new AtomicLong(0); private final AtomicLong lastPong = new AtomicLong(0); - private final AtomicBoolean isBlockingWrites = new AtomicBoolean(false); + private volatile long activeDocuments = 0; + private volatile long targetActiveDocuments = 0; + private volatile boolean statusIsKnown = false; + private volatile boolean working = true; + private volatile boolean isBlockingWrites = false; public Node(int key, String hostname, int group) { this.key = key; @@ -63,30 +62,30 @@ public class Node { public int group() { return group; } public void setWorking(boolean working) { - this.statusIsKnown.lazySet(true); - this.working.lazySet(working); + this.statusIsKnown = true; + this.working = working; if ( ! working ) { - activeDocuments.set(0); - targetActiveDocuments.set(0); + activeDocuments = 0; + targetActiveDocuments = 0; } } /** Returns whether this node is currently responding to requests, or null if status is not known */ public Boolean isWorking() { - return statusIsKnown.get() ? working.get() : null; + return statusIsKnown ? working : null; } /** Updates the active documents on this node */ - public void setActiveDocuments(long documents) { this.activeDocuments.set(documents); } - public void setTargetActiveDocuments(long documents) { this.targetActiveDocuments.set(documents); } + public void setActiveDocuments(long documents) { this.activeDocuments = documents; } + public void setTargetActiveDocuments(long documents) { this.targetActiveDocuments = documents; } /** Returns the active documents on this node. If unknown, 0 is returned. */ - long getActiveDocuments() { return activeDocuments.get(); } - long getTargetActiveDocuments() { return targetActiveDocuments.get(); } + long getActiveDocuments() { return activeDocuments; } + long getTargetActiveDocuments() { return targetActiveDocuments; } - public void setBlockingWrites(boolean isBlockingWrites) { this.isBlockingWrites.set(isBlockingWrites); } + public void setBlockingWrites(boolean isBlockingWrites) { this.isBlockingWrites = isBlockingWrites; } - boolean isBlockingWrites() { return isBlockingWrites.get(); } + boolean isBlockingWrites() { return isBlockingWrites; } @Override public int hashCode() { return Objects.hash(hostname, key, pathIndex, group); } @@ -106,7 +105,7 @@ public class Node { @Override public String toString() { return "search node key = " + key + " hostname = "+ hostname + " path = " + pathIndex + " in group " + group + - " statusIsKnown = " + statusIsKnown.get() + " working = " + working.get() + + " statusIsKnown = " + statusIsKnown + " working = " + working + " activeDocs = " + getActiveDocuments() + " targetActiveDocs = " + getTargetActiveDocuments(); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java index 4e2eb03f01d..d721528f13b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java @@ -383,10 +383,12 @@ public class RoutingPolicies { nameServiceForwarderIn(allocation.deployment.zoneId()).createTxt(challenge.name(), List.of(challenge.data()), Priority.high); Instant doom = controller.clock().instant().plusSeconds(30); while (controller.clock().instant().isBefore(doom)) { - if (controller.curator().readNameServiceQueue().requests().stream() - .noneMatch(request -> request.name().equals(Optional.of(challenge.name())))) { - challenge.trigger().run(); - return; + try (Mutex lock = controller.curator().lockNameServiceQueue()) { + if (controller.curator().readNameServiceQueue().requests().stream() + .noneMatch(request -> request.name().equals(Optional.of(challenge.name())))) { + challenge.trigger().run(); + return; + } } Thread.sleep(100); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java index 867e03258f9..fb85b876adf 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java @@ -39,6 +39,7 @@ import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock; import com.yahoo.vespa.hosted.rotation.config.RotationsConfig; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; import java.time.Duration; import java.time.Instant; @@ -511,6 +512,7 @@ public class RoutingPoliciesTest { } @Test + @Timeout(30) void private_dns_for_vpc_endpoint() { // Challenge answered for endpoint RoutingPoliciesTester tester = new RoutingPoliciesTester(); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/Container.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/Container.java index e0e11ad0a3a..fb789874acf 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/Container.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/Container.java @@ -19,17 +19,15 @@ public class Container extends PartialContainer { private final ContainerResources resources; private final int conmonPid; private final List<Network> networks; - private final List<String> createCommand; public Container(ContainerId id, ContainerName name, Instant createdAt, State state, String imageId, DockerImage image, Map<String, String> labels, int pid, int conmonPid, String hostname, - ContainerResources resources, List<Network> networks, boolean managed, List<String> createCommand) { + ContainerResources resources, List<Network> networks, boolean managed) { super(id, name, createdAt, state, imageId, image, labels, pid, managed); this.hostname = Objects.requireNonNull(hostname); this.resources = Objects.requireNonNull(resources); this.conmonPid = conmonPid; this.networks = List.copyOf(Objects.requireNonNull(networks)); - this.createCommand = List.copyOf(Objects.requireNonNull(createCommand)); } /** The hostname of this, if any */ @@ -52,23 +50,18 @@ public class Container extends PartialContainer { return networks; } - /** The command used to create this */ - public List<String> createCommand() { - return createCommand; - } - @Override public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; if (!super.equals(o)) return false; - Container container = (Container) o; - return conmonPid == container.conmonPid && hostname.equals(container.hostname) && resources.equals(container.resources) && networks.equals(container.networks) && createCommand.equals(container.createCommand); + Container that = (Container) o; + return conmonPid == that.conmonPid && hostname.equals(that.hostname) && resources.equals(that.resources) && networks.equals(that.networks); } @Override public int hashCode() { - return Objects.hash(super.hashCode(), hostname, resources, conmonPid, networks, createCommand); + return Objects.hash(super.hashCode(), hostname, resources, conmonPid, networks); } /** The network of a container */ diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerEngine.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerEngine.java index 323a2223bd1..2aa1d12c491 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerEngine.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerEngine.java @@ -35,9 +35,6 @@ public interface ContainerEngine { /** Remove given container. The container will be stopped if necessary */ void removeContainer(TaskContext context, PartialContainer container); - /** Returns whether the given container should be re-created to apply new configuration */ - boolean shouldRecreate(NodeAgentContext context, Container container); - /** Get container for given context */ Optional<Container> getContainer(NodeAgentContext context); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperations.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperations.java index f144552343d..9060261b806 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperations.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperations.java @@ -57,10 +57,6 @@ public class ContainerOperations { containerEngine.updateContainer(context, containerId, containerResources); } - public boolean shouldRecreate(NodeAgentContext context, Container container) { - return containerEngine.shouldRecreate(context, container); - } - public Optional<Container> getContainer(NodeAgentContext context) { return containerEngine.getContainer(context); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java index dc7e3587407..20ea29381f3 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java @@ -332,34 +332,34 @@ public class NodeAgentImpl implements NodeAgent { } private List<String> shouldRemoveContainer(NodeAgentContext context, Container existingContainer) { - NodeState nodeState = context.node().state(); + final NodeState nodeState = context.node().state(); List<String> reasons = new ArrayList<>(); - if (nodeState == NodeState.dirty || nodeState == NodeState.provisioned) { + if (nodeState == NodeState.dirty || nodeState == NodeState.provisioned) reasons.add("Node in state " + nodeState + ", container should no longer be running"); - } + if (context.node().wantedDockerImage().isPresent() && !context.node().wantedDockerImage().get().equals(existingContainer.image())) { - reasons.add("The node is supposed to run a new image: " + reasons.add("The node is supposed to run a new Docker image: " + existingContainer.image().asString() + " -> " + context.node().wantedDockerImage().get().asString()); } - if (!existingContainer.state().isRunning()) { + + if (!existingContainer.state().isRunning()) reasons.add("Container no longer running"); - } + if (currentRebootGeneration < context.node().wantedRebootGeneration()) { reasons.add(String.format("Container reboot wanted. Current: %d, Wanted: %d", currentRebootGeneration, context.node().wantedRebootGeneration())); } + ContainerResources wantedContainerResources = getContainerResources(context); if (!wantedContainerResources.equalsMemory(existingContainer.resources())) { reasons.add("Container should be running with different memory allocation, wanted: " + wantedContainerResources.toStringMemory() + ", actual: " + existingContainer.resources().toStringMemory()); } - if (containerOperations.shouldRecreate(context, existingContainer)) { - reasons.add("Container should be re-created to apply new configuration"); - } - if (containerState == STARTING) { + + if (containerState == STARTING) reasons.add("Container failed to start"); - } + return reasons; } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerEngineMock.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerEngineMock.java index d8888cb7267..2d3a4976fe5 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerEngineMock.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerEngineMock.java @@ -28,7 +28,6 @@ public class ContainerEngineMock implements ContainerEngine { private final Map<ContainerName, Container> containers = new ConcurrentHashMap<>(); private final Map<String, ImageDownload> images = new ConcurrentHashMap<>(); - private boolean asyncImageDownload = false; public ContainerEngineMock asyncImageDownload(boolean enabled) { @@ -113,11 +112,6 @@ public class ContainerEngineMock implements ContainerEngine { } @Override - public boolean shouldRecreate(NodeAgentContext context, Container container) { - return false; - } - - @Override public void updateContainer(NodeAgentContext context, ContainerId containerId, ContainerResources containerResources) { Container container = requireContainer(context.containerName()); containers.put(container.name(), new Container(containerId, container.name(), container.createdAt(), container.state(), @@ -125,8 +119,7 @@ public class ContainerEngineMock implements ContainerEngine { container.labels(), container.pid(), container.conmonPid(), container.hostname(), containerResources, container.networks(), - container.managed(), - container.createCommand())); + container.managed())); } @Override @@ -207,8 +200,7 @@ public class ContainerEngineMock implements ContainerEngine { context.hostname().value(), containerResources, List.of(), - true, - List.of()); + true); } private static class ImageDownload { diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperationsTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperationsTest.java index 8fb65e4bd47..9a5ca8c805e 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperationsTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperationsTest.java @@ -59,7 +59,7 @@ public class ContainerOperationsTest { private Container createContainer(String name, boolean managed) { return new Container(new ContainerId("id-of-" + name), new ContainerName(name), Instant.EPOCH, PartialContainer.State.running, "image-id", DockerImage.EMPTY, Map.of(), 42, 43, name, - ContainerResources.UNLIMITED, List.of(), managed, List.of()); + ContainerResources.UNLIMITED, List.of(), managed); } } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImagePrunerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImagePrunerTest.java index 0d83a397d33..2ef6780dff6 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImagePrunerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/image/ContainerImagePrunerTest.java @@ -123,7 +123,7 @@ public class ContainerImagePrunerTest { return new Container(new ContainerId("id-of-" + name), new ContainerName(name), Instant.EPOCH, Container.State.running, imageId, DockerImage.EMPTY, Map.of(), 42, 43, name + ".example.com", ContainerResources.UNLIMITED, - List.of(), true, List.of()); + List.of(), true); } private static class Tester { diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java index 63a7242c96c..fb132c9b717 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java @@ -358,15 +358,6 @@ public class NodeAgentImplTest { verify(orchestrator, times(1)).resume(eq(hostName)); verify(nodeRepository, times(1)).updateNodeAttributes(eq(hostName), eq(new NodeAttributes() .withRebootGeneration(wantedRebootGeneration))); - - // Re-create if new container config needs to be applied - when(containerOperations.shouldRecreate(eq(context), any())).thenReturn(true); - nodeAgent.doConverge(context); - verify(containerOperations, times(2)).removeContainer(eq(context), any()); - verify(containerOperations, times(2)).createContainer(eq(context), any()); - verify(orchestrator, times(2)).resume(eq(hostName)); - verify(nodeRepository, times(2)).updateNodeAttributes(eq(hostName), eq(new NodeAttributes() - .withRebootGeneration(wantedRebootGeneration))); } @Test @@ -824,8 +815,7 @@ public class NodeAgentImplTest { hostName, containerResources, List.of(), - true, - List.of())) : + true)) : Optional.empty(); }).when(containerOperations).getContainer(any()); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java index 55b2bfc8329..4ad52a51df9 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java @@ -36,7 +36,7 @@ public abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidat private static final NodeResources zeroResources = new NodeResources(0, 0, 0, 0, NodeResources.DiskSpeed.any, NodeResources.StorageType.any, - NodeResources.Architecture.getDefault(), NodeResources.GpuResources.getDefault()); + NodeResources.Architecture.getDefault(), NodeResources.GpuResources.zero()); /** The free capacity on the parent of this node, before adding this node to it */ protected final NodeResources freeParentCapacity; 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 1022981f445..0ed4f4ee9b0 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 @@ -259,7 +259,7 @@ public class NodeRepositoryProvisioner implements Provisioner { } private static NodeResources requireCompatibleResources(NodeResources nodeResources, ClusterSpec cluster) { - if (cluster.type() != ClusterSpec.Type.container && !nodeResources.gpuResources().isDefault()) { + if (cluster.type() != ClusterSpec.Type.container && !nodeResources.gpuResources().isZero()) { throw new IllegalArgumentException(cluster.id() + " of type " + cluster.type() + " does not support GPU resources"); } return nodeResources; diff --git a/searchlib/src/tests/tensor/hnsw_index/hnsw_index_test.cpp b/searchlib/src/tests/tensor/hnsw_index/hnsw_index_test.cpp index bea371c78a8..d8e189717c3 100644 --- a/searchlib/src/tests/tensor/hnsw_index/hnsw_index_test.cpp +++ b/searchlib/src/tests/tensor/hnsw_index/hnsw_index_test.cpp @@ -62,14 +62,13 @@ public: return *this; } vespalib::eval::TypedCells get_vector(uint32_t docid, uint32_t subspace) const override { - (void) subspace; - ArrayRef ref(_vectors[docid]); - return vespalib::eval::TypedCells(ref); + return get_vectors(docid).cells(subspace); } VectorBundle get_vectors(uint32_t docid) const override { ArrayRef ref(_vectors[docid]); - assert(_subspace_type.size() == ref.size()); - return VectorBundle(ref.data(), 1, _subspace_type); + assert((ref.size() % _subspace_type.size()) == 0); + uint32_t subspaces = ref.size() / _subspace_type.size(); + return VectorBundle(ref.data(), subspaces, _subspace_type); } void clear() { _vectors.clear(); } @@ -160,6 +159,20 @@ public: ASSERT_EQ(exp_levels.size(), act_node.size()); EXPECT_EQ(exp_levels, act_node.levels()); } + void expect_top_3_by_docid(const vespalib::string& label, std::vector<float> qv, std::vector<uint32_t> exp) { + SCOPED_TRACE(label); + uint32_t k = 3; + uint32_t explore_k = 100; + vespalib::ArrayRef qv_ref(qv); + vespalib::eval::TypedCells qv_cells(qv_ref); + auto got_by_docid = index->find_top_k(k, qv_cells, explore_k, 10000.0); + std::vector<uint32_t> act; + act.reserve(got_by_docid.size()); + for (auto& hit : got_by_docid) { + act.emplace_back(hit.docid); + } + EXPECT_EQ(exp, act); + } void expect_top_3(uint32_t docid, std::vector<uint32_t> exp_hits) { uint32_t k = 3; auto qv = vectors.get_vector(docid, 0); @@ -740,7 +753,37 @@ TYPED_TEST(HnswIndexTest, hnsw_graph_can_be_saved_and_loaded) this->init(false); this->load_index(data); this->check_savetest_index("after load"); - } +} + +using HnswMultiIndexTest = HnswIndexTest<HnswIndex<HnswIndexType::MULTI>>; + +TEST_F(HnswMultiIndexTest, duplicate_docid_is_removed) +{ + this->init(false); + this->vectors + .set(1, {0, 0, 0, 2}) + .set(2, {1, 0}) + .set(3, {1, 2}) + .set(4, {2, 0, 2, 2}); + /* + * Graph showing documents at column x row y, origo in lower left corner. + * + * 1 3 4 + * . . . + * 1 2 4 + */ + for (uint32_t docid = 1; docid < 5; ++docid) { + this->add_document(docid); + } + this->expect_top_3_by_docid("{0, 0}", {0, 0}, {1, 2, 4}); + this->expect_top_3_by_docid("{0, 1}", {0, 1}, {1, 2, 3}); + this->expect_top_3_by_docid("{0, 2}", {0, 2}, {1, 3, 4}); + this->expect_top_3_by_docid("{1, 0}", {1, 0}, {1, 2, 4}); + this->expect_top_3_by_docid("{1, 2}", {1, 2}, {1, 3, 4}); + this->expect_top_3_by_docid("{2, 0}", {2, 0}, {1, 2, 4}); + this->expect_top_3_by_docid("{2, 1}", {2, 1}, {2, 3, 4}); + this->expect_top_3_by_docid("{2, 2}", {2, 2}, {1, 3, 4}); +}; TEST(LevelGeneratorTest, gives_various_levels) { @@ -789,7 +832,6 @@ TEST(LevelGeneratorTest, gives_various_levels) EXPECT_TRUE(hist.size() < 14); } - template <typename IndexType> class TwoPhaseTest : public HnswIndexTest<IndexType> { public: @@ -853,5 +895,4 @@ TYPED_TEST(TwoPhaseTest, two_phase_add) this->expect_levels(nodeids[0], {{2}, {4}}); } - GTEST_MAIN_RUN_ALL_TESTS() |