Merge pull request #21407 from vespa-engine/bratseth/reuse-indexesv7.550.61

Reuse node indexes

2 files changed, 48 insertions, 4 deletions

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;
 
@@ -145,6 +151,39 @@ public class ProvisioningTest {
     }
 
     @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<Integer> 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<Integer> 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<Integer> 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);
         }