From 96147d82d9aedac1040ad857f05d1a4dbd239f5f Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Fri, 25 Feb 2022 16:27:21 +0100 Subject: Reuse node indexes --- .../main/java/com/yahoo/config/provision/Zone.java | 3 ++ .../java/com/yahoo/vespa/flags/FlagSource.java | 4 ++- .../src/main/java/com/yahoo/vespa/flags/Flags.java | 7 ++++ .../yahoo/vespa/hosted/provision/node/Nodes.java | 5 ++- .../provisioning/NodeRepositoryProvisioner.java | 10 ++++-- .../hosted/provision/provisioning/Preparer.java | 10 +++--- .../provision/provisioning/ProvisioningTest.java | 39 ++++++++++++++++++++++ .../provision/provisioning/ProvisioningTester.java | 13 +++++--- 8 files changed, 77 insertions(+), 14 deletions(-) diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/Zone.java b/config-provisioning/src/main/java/com/yahoo/config/provision/Zone.java index d550c5287cc..8fcb11232c6 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/Zone.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/Zone.java @@ -68,6 +68,9 @@ public class Zone { return region; } + /** Returns the string "region.environment" */ + public String systemLocalValue() { return environment + "." + region; } + /** Do not use */ public static Zone defaultZone() { return new Zone(Cloud.defaultCloud(), SystemName.defaultSystem(), Environment.defaultEnvironment(), RegionName.defaultName()); diff --git a/flags/src/main/java/com/yahoo/vespa/flags/FlagSource.java b/flags/src/main/java/com/yahoo/vespa/flags/FlagSource.java index 34e69e735fa..1572286207d 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/FlagSource.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/FlagSource.java @@ -9,6 +9,8 @@ import java.util.Optional; * @author hakonhall */ public interface FlagSource { - /** Get raw flag for the given vector (specifying hostname, application id, etc). */ + + /** Returns the raw flag for the given vector (specifying hostname, application id, etc). */ Optional fetch(FlagId id, FetchVector vector); + } 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 271c75fe446..0ef2e51fe1f 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -383,6 +383,13 @@ public class Flags { "Takes effect at redeployment", ZONE_ID, APPLICATION_ID); + public static final UnboundBooleanFlag REUSE_NODE_INDEXES = defineFeatureFlag( + "reuse-node-indexes", false, + List.of("bratseth"), "2022-02-25", "2022-04-25", + "Whether we should reuse earlier indexes when allocating new nodes", + "Takes effect immediately", + ZONE_ID); + /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, List owners, String createdAt, String expiresAt, String description, diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java index e5e5ce27cf1..cb82585fe01 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java @@ -280,8 +280,8 @@ public class Nodes { } public List deallocateRecursively(String hostname, Agent agent, String reason) { - Node nodeToDirty = node(hostname).orElseThrow(() -> - new IllegalArgumentException("Could not deallocate " + hostname + ": Node not found")); + Node nodeToDirty = node(hostname).orElseThrow(() -> new IllegalArgumentException("Could not deallocate " + + hostname + ": Node not found")); List nodesToDirty = (nodeToDirty.type().isHost() ? @@ -289,7 +289,6 @@ public class Nodes { Stream.of(nodeToDirty)) .filter(node -> node.state() != Node.State.dirty) .collect(Collectors.toList()); - List hostnamesNotAllowedToDirty = nodesToDirty.stream() .filter(node -> node.state() != Node.State.provisioned) .filter(node -> node.state() != Node.State.failed) 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 7d15a2b30b1..d890c144fd6 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 @@ -17,7 +17,9 @@ import com.yahoo.config.provision.ProvisionLogger; import com.yahoo.config.provision.Provisioner; import com.yahoo.config.provision.Zone; import com.yahoo.transaction.Mutex; +import com.yahoo.vespa.flags.FetchVector; import com.yahoo.vespa.flags.FlagSource; +import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; @@ -54,6 +56,7 @@ public class NodeRepositoryProvisioner implements Provisioner { private final AllocationOptimizer allocationOptimizer; private final CapacityPolicies capacityPolicies; private final Zone zone; + private FlagSource flagSource; private final Preparer preparer; private final Activator activator; private final Optional loadBalancerProvisioner; @@ -62,11 +65,13 @@ public class NodeRepositoryProvisioner implements Provisioner { @Inject public NodeRepositoryProvisioner(NodeRepository nodeRepository, Zone zone, - ProvisionServiceProvider provisionServiceProvider, FlagSource flagSource) { + ProvisionServiceProvider provisionServiceProvider, + FlagSource flagSource) { this.nodeRepository = nodeRepository; this.allocationOptimizer = new AllocationOptimizer(nodeRepository); this.capacityPolicies = new CapacityPolicies(nodeRepository); this.zone = zone; + this.flagSource = flagSource; this.loadBalancerProvisioner = provisionServiceProvider.getLoadBalancerService(nodeRepository) .map(lbService -> new LoadBalancerProvisioner(nodeRepository, lbService)); this.nodeResourceLimits = new NodeResourceLimits(nodeRepository); @@ -113,7 +118,8 @@ public class NodeRepositoryProvisioner implements Provisioner { : requested.minResources().nodeResources(); nodeSpec = NodeSpec.from(requested.type()); } - return asSortedHosts(preparer.prepare(application, cluster, nodeSpec, groups), resources); + var reuseIndexes = Flags.REUSE_NODE_INDEXES.bindTo(flagSource).with(FetchVector.Dimension.ZONE_ID, zone.systemLocalValue()).value(); + return asSortedHosts(preparer.prepare(application, cluster, nodeSpec, groups, reuseIndexes), resources); } @Override diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java index 838d6751d09..a6f16d3d65e 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java @@ -34,9 +34,10 @@ class Preparer { } /** Prepare all required resources for the given application and cluster */ - public List prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes, int wantedGroups) { + public List prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes, int wantedGroups, + boolean reuseIndexes) { try { - var nodes = prepareNodes(application, cluster, requestedNodes, wantedGroups); + var nodes = prepareNodes(application, cluster, requestedNodes, wantedGroups, reuseIndexes); prepareLoadBalancer(application, cluster, requestedNodes); return nodes; } @@ -55,13 +56,14 @@ class Preparer { // Note: This operation may make persisted changes to the set of reserved and inactive nodes, // but it may not change the set of active nodes, as the active nodes must stay in sync with the // active config model which is changed on activate - private List prepareNodes(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes, int wantedGroups) { + private List prepareNodes(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes, + int wantedGroups, boolean reuseIndexes) { NodesAndHosts allNodesAndHosts = groupPreparer.createNodesAndHostUnlocked(); NodeList appNodes = allNodesAndHosts.nodes().owner(application); List surplusNodes = findNodesInRemovableGroups(appNodes, cluster, wantedGroups); List usedIndices = appNodes.cluster(cluster.id()).mapToList(node -> node.allocation().get().membership().index()); - NodeIndices indices = new NodeIndices(usedIndices, ! cluster.type().isContent()); + NodeIndices indices = new NodeIndices(usedIndices, reuseIndexes || ! cluster.type().isContent()); List acceptedNodes = new ArrayList<>(); for (int groupIndex = 0; groupIndex < wantedGroups; groupIndex++) { 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 d0eb55469b6..ba9da20b615 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 @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.provision.provisioning; import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ApplicationTransaction; import com.yahoo.config.provision.Capacity; import com.yahoo.config.provision.ClusterMembership; import com.yahoo.config.provision.ClusterResources; @@ -16,11 +17,14 @@ import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.NodeAllocationException; import com.yahoo.config.provision.ParentHostUnavailableException; +import com.yahoo.config.provision.ProvisionLock; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.Zone; +import com.yahoo.transaction.NestedTransaction; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; +import com.yahoo.vespa.hosted.provision.NodeMutex; import com.yahoo.vespa.hosted.provision.maintenance.ReservationExpirer; import com.yahoo.vespa.hosted.provision.maintenance.TestMetric; import com.yahoo.vespa.hosted.provision.node.Agent; @@ -32,6 +36,7 @@ import com.yahoo.vespa.service.duper.InfraApplication; import org.junit.Test; import java.time.Duration; +import java.time.Instant; import java.util.Collection; import java.util.HashSet; import java.util.Iterator; @@ -47,6 +52,7 @@ import java.util.stream.Collectors; import static java.time.temporal.ChronoUnit.MILLIS; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -144,6 +150,39 @@ public class ProvisioningTest { tester.assertRestartCount(application1, allFilter, hostFilter, clusterTypeFilter, clusterIdFilter); } + @Test + public void application_deployment_reuses_node_indexes() { + ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east"))).build(); + + ApplicationId app1 = ProvisioningTester.applicationId("app1"); + + tester.makeReadyHosts(21, defaultResources).activateTenantHosts(); + + // deploy + SystemState state1 = prepare(app1, 2, 2, 3, 3, defaultResources, tester); + tester.activate(app1, state1.allHosts); + Set state1Indexes = state1.allHosts.stream().map(hostSpec -> hostSpec.membership().get().index()).collect(Collectors.toSet()); + + // deallocate 2 nodes with index 0 + NodeMutex node1 = tester.nodeRepository().nodes().lockAndGet(tester.removeOne(state1.container0).hostname()).get(); + NodeMutex node2 = tester.nodeRepository().nodes().lockAndGet(tester.removeOne(state1.content0).hostname()).get(); + tester.nodeRepository().nodes().write(node1.node().withoutAllocation(), node1); + tester.nodeRepository().nodes().write(node2.node().withoutAllocation(), node1); + + // redeploy to get new nodes + SystemState state2 = prepare(app1, 2, 2, 3, 3, defaultResources, tester); + Set state2Indexes = state2.allHosts.stream().map(hostSpec -> hostSpec.membership().get().index()).collect(Collectors.toSet()); + assertEquals("Indexes are reused", state1Indexes, state2Indexes); + + // if nodes are e.g failed indexes are not reused as they are still allocated + tester.nodeRepository().nodes().fail(tester.removeOne(state2.container0).hostname(), Agent.system, "test"); + tester.nodeRepository().nodes().fail(tester.removeOne(state2.content0).hostname(), Agent.system, "test"); + SystemState state3 = prepare(app1, 2, 2, 3, 3, defaultResources, tester); + Set state3Indexes = state3.allHosts.stream().map(hostSpec -> hostSpec.membership().get().index()).collect(Collectors.toSet()); + assertNotEquals("Indexes are not reused", state2Indexes, state3Indexes); + + } + @Test public void nodeVersionIsReturnedIfSet() { ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.dev, RegionName.from("us-east"))).build(); 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 b781f397f70..7b988f83c01 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 @@ -30,6 +30,7 @@ import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.mock.MockCurator; import com.yahoo.vespa.curator.transaction.CuratorTransaction; import com.yahoo.vespa.flags.FlagSource; +import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; @@ -66,9 +67,6 @@ import java.util.stream.Collectors; import static com.yahoo.config.provision.NodeResources.StorageType.local; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.mock; /** * A test utility for provisioning tests. @@ -151,6 +149,7 @@ public class ProvisioningTester { public CapacityPolicies capacityPolicies() { return capacityPolicies; } public NodeList getNodes(ApplicationId id, Node.State ... inState) { return nodeRepository.nodes().list(inState).owner(id); } public InMemoryFlagSource flagSource() { return (InMemoryFlagSource) nodeRepository.flagSource(); } + public Node node(String hostname) { return nodeRepository.nodes().node(hostname).get(); } public int decideSize(Capacity capacity, ApplicationId application) { return capacityPolicies.applyOn(capacity, application).minResources().nodes(); @@ -688,6 +687,12 @@ public class ProvisioningTester { return this; } + private FlagSource defaultFlagSource() { + var flagSource = new InMemoryFlagSource(); + flagSource.withBooleanFlag(Flags.REUSE_NODE_INDEXES.id(), true); + return flagSource; + } + public ProvisioningTester build() { return new ProvisioningTester(Optional.ofNullable(curator).orElseGet(MockCurator::new), new NodeFlavors(Optional.ofNullable(flavorsConfig).orElseGet(ProvisioningTester::createConfig)), @@ -698,7 +703,7 @@ public class ProvisioningTester { Optional.ofNullable(orchestrator).orElseGet(OrchestratorMock::new), hostProvisioner, new LoadBalancerServiceMock(), - Optional.ofNullable(flagSource).orElseGet(InMemoryFlagSource::new), + Optional.ofNullable(flagSource).orElse(defaultFlagSource()), spareCount); } -- cgit v1.2.3 From d00919409df940b84bdb612bc3e27e11e8c0de4f Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Sun, 27 Feb 2022 16:36:29 +0100 Subject: Fix comment --- config-provisioning/src/main/java/com/yahoo/config/provision/Zone.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/Zone.java b/config-provisioning/src/main/java/com/yahoo/config/provision/Zone.java index 8fcb11232c6..03c35f789b1 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/Zone.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/Zone.java @@ -68,7 +68,7 @@ public class Zone { return region; } - /** Returns the string "region.environment" */ + /** Returns the string "environment.region" */ public String systemLocalValue() { return environment + "." + region; } /** Do not use */ -- cgit v1.2.3 From d34904697419c851e4457120b823cc3b5c84b094 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Sun, 27 Feb 2022 16:38:02 +0100 Subject: Use flag source ref from node repo --- .../hosted/provision/provisioning/NodeRepositoryProvisioner.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 d890c144fd6..4dc199dd7b2 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 @@ -56,7 +56,6 @@ public class NodeRepositoryProvisioner implements Provisioner { private final AllocationOptimizer allocationOptimizer; private final CapacityPolicies capacityPolicies; private final Zone zone; - private FlagSource flagSource; private final Preparer preparer; private final Activator activator; private final Optional loadBalancerProvisioner; @@ -71,7 +70,6 @@ public class NodeRepositoryProvisioner implements Provisioner { this.allocationOptimizer = new AllocationOptimizer(nodeRepository); this.capacityPolicies = new CapacityPolicies(nodeRepository); this.zone = zone; - this.flagSource = flagSource; this.loadBalancerProvisioner = provisionServiceProvider.getLoadBalancerService(nodeRepository) .map(lbService -> new LoadBalancerProvisioner(nodeRepository, lbService)); this.nodeResourceLimits = new NodeResourceLimits(nodeRepository); @@ -118,7 +116,9 @@ public class NodeRepositoryProvisioner implements Provisioner { : requested.minResources().nodeResources(); nodeSpec = NodeSpec.from(requested.type()); } - var reuseIndexes = Flags.REUSE_NODE_INDEXES.bindTo(flagSource).with(FetchVector.Dimension.ZONE_ID, zone.systemLocalValue()).value(); + var reuseIndexes = Flags.REUSE_NODE_INDEXES.bindTo(nodeRepository.flagSource()) + .with(FetchVector.Dimension.ZONE_ID, zone.systemLocalValue()) + .value(); return asSortedHosts(preparer.prepare(application, cluster, nodeSpec, groups, reuseIndexes), resources); } -- cgit v1.2.3