Merge pull request #15638 from vespa-engine/hakonhall/remove-compact-preprovision-capacity-flag

Remove compact-preprovision-capacity flag

2 files changed, 2 insertions, 100 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java
index 392bcc5aa08..119f0a69551 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainer.java
@@ -12,7 +12,6 @@ import com.yahoo.config.provision.OutOfCapacityException;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.lang.MutableInteger;
 import com.yahoo.transaction.Mutex;
-import com.yahoo.vespa.flags.BooleanFlag;
 import com.yahoo.vespa.flags.FlagSource;
 import com.yahoo.vespa.flags.Flags;
 import com.yahoo.vespa.flags.JacksonFlag;
@@ -42,7 +41,6 @@ import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.HashMap;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -63,7 +61,6 @@ public class DynamicProvisioningMaintainer extends NodeRepositoryMaintainer {
 
     private final HostProvisioner hostProvisioner;
     private final ListFlag<ClusterCapacity> preprovisionCapacityFlag;
-    private final BooleanFlag compactPreprovisionCapacityFlag;
     private final JacksonFlag<SharedHost> sharedHostFlag;
 
     DynamicProvisioningMaintainer(NodeRepository nodeRepository,
@@ -74,7 +71,6 @@ public class DynamicProvisioningMaintainer extends NodeRepositoryMaintainer {
         super(nodeRepository, interval, metric);
         this.hostProvisioner = hostProvisioner;
         this.preprovisionCapacityFlag = Flags.PREPROVISION_CAPACITY.bindTo(flagSource);
-        this.compactPreprovisionCapacityFlag = Flags.COMPACT_PREPROVISION_CAPACITY.bindTo(flagSource);
         this.sharedHostFlag = Flags.SHARED_HOST.bindTo(flagSource);
     }
 
@@ -146,70 +142,12 @@ public class DynamicProvisioningMaintainer extends NodeRepositoryMaintainer {
     /**
      * Provision hosts to ensure there is room to allocate spare nodes.
      *
-     * @param nodes list of all nodes
+     * @param nodeList list of all nodes
      * @return excess hosts that can safely be deprovisioned: An excess host 1. contains no nodes allocated
      *         to an application, and assuming the spare nodes have been allocated, and 2. is not parked
      *         without wantToDeprovision (which means an operator is looking at the node).
      */
-    private List<Node> provision(NodeList nodes) {
-        boolean compactPreprovisionCapacity = compactPreprovisionCapacityFlag.value();
-        if (compactPreprovisionCapacity) {
-            return findExcessHosts(nodes);
-        } else {
-            return legacyProvision(nodes);
-        }
-    }
-
-    private List<Node> legacyProvision(NodeList nodes) {
-        Map<String, Node> hostsByHostname = new HashMap<>(nodes.hosts().asList().stream()
-                .filter(host -> host.state() != Node.State.parked || host.status().wantToDeprovision())
-                .collect(Collectors.toMap(Node::hostname, Function.identity())));
-
-        nodes.asList().stream()
-                .filter(node -> node.allocation().isPresent())
-                .flatMap(node -> node.parentHostname().stream())
-                .distinct()
-                .forEach(hostsByHostname::remove);
-
-        List<Node> excessHosts = new ArrayList<>(hostsByHostname.values());
-
-        var capacity = new ArrayList<>(targetCapacity());
-        for (Iterator<NodeResources> it = capacity.iterator(); it.hasNext() && !excessHosts.isEmpty(); ) {
-            NodeResources resources = it.next();
-            excessHosts.stream()
-                       .filter(nodeRepository()::canAllocateTenantNodeTo)
-                       .filter(host -> nodeRepository().resourcesCalculator()
-                                                       .advertisedResourcesOf(host.flavor())
-                                                       .satisfies(resources))
-                       .min(Comparator.comparingInt(n -> n.flavor().cost()))
-                       .ifPresent(host -> {
-                           excessHosts.remove(host);
-                           it.remove();
-                       });
-        }
-
-        // Pre-provisioning is best effort, do one host at a time
-        capacity.forEach(resources -> {
-            try {
-                Version osVersion = nodeRepository().osVersions().targetFor(NodeType.host).orElse(Version.emptyVersion);
-                List<Node> hosts = hostProvisioner.provisionHosts(nodeRepository().database().getProvisionIndexes(1),
-                                                                  resources, ApplicationId.defaultId(), osVersion,
-                                                                  HostSharing.shared)
-                                                  .stream()
-                                                  .map(ProvisionedHost::generateHost)
-                                                  .collect(Collectors.toList());
-                nodeRepository().addNodes(hosts, Agent.DynamicProvisioningMaintainer);
-            } catch (OutOfCapacityException | IllegalArgumentException | IllegalStateException e) {
-                log.log(Level.WARNING, "Failed to pre-provision " + resources + ": " + e.getMessage());
-            } catch (RuntimeException e) {
-                log.log(Level.WARNING, "Failed to pre-provision " + resources + ", will retry in " + interval(), e);
-            }
-        });
-
-        return excessHosts;
-    }
-
-    private List<Node> findExcessHosts(NodeList nodeList) {
+    private List<Node> provision(NodeList nodeList) {
         final List<Node> nodes = new ArrayList<>(provisionUntilNoDeficit(nodeList));
 
 
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java
index c747d95d41f..cc23f29dde6 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java
@@ -103,19 +103,6 @@ public class DynamicProvisioningMaintainerTest {
     }
 
     @Test
-    public void does_not_deprovision_when_preprovisioning_enabled__legacy() {
-        var tester = new DynamicProvisioningTester().addInitialNodes();
-        tester.flagSource.withListFlag(Flags.PREPROVISION_CAPACITY.id(), List.of(new ClusterCapacity(1, 1, 3, 2, 1.0)), ClusterCapacity.class);
-        tester.flagSource.withBooleanFlag(Flags.COMPACT_PREPROVISION_CAPACITY.id(), false);
-        Optional<Node> failedHost = tester.nodeRepository.getNode("host2");
-        assertTrue(failedHost.isPresent());
-
-        tester.maintainer.maintain();
-        assertTrue("Failed host is deprovisioned", tester.nodeRepository.getNode(failedHost.get().hostname()).isEmpty());
-        assertEquals(1, tester.hostProvisioner.deprovisionedHosts);
-    }
-
-    @Test
     public void does_not_deprovision_when_preprovisioning_enabled() {
         var tester = new DynamicProvisioningTester().addInitialNodes();
         tester.flagSource.withListFlag(Flags.PREPROVISION_CAPACITY.id(), List.of(new ClusterCapacity(1, 1, 3, 2, 1.0)), ClusterCapacity.class);
@@ -128,29 +115,6 @@ public class DynamicProvisioningMaintainerTest {
     }
 
     @Test
-    public void provision_deficit_and_deprovision_excess__legacy() {
-        var tester = new DynamicProvisioningTester().addInitialNodes();
-        tester.flagSource.withListFlag(Flags.PREPROVISION_CAPACITY.id(),
-                List.of(new ClusterCapacity(2, 48, 128, 1000, 10.0),
-                        new ClusterCapacity(1, 16, 24, 100, 1.0)),
-                ClusterCapacity.class);
-        tester.flagSource.withBooleanFlag(Flags.COMPACT_PREPROVISION_CAPACITY.id(), false);
-        assertTrue(tester.nodeRepository.getNode("host2").isPresent());
-        assertEquals(0, tester.hostProvisioner.provisionedHosts.size());
-
-        // failed host2 is removed
-        Optional<Node> failedHost = tester.nodeRepository.getNode("host2");
-        assertTrue(failedHost.isPresent());
-        tester.maintainer.maintain();
-        assertTrue("Failed host is deprovisioned", tester.nodeRepository.getNode(failedHost.get().hostname()).isEmpty());
-        assertTrue("Host with matching resources is kept", tester.nodeRepository.getNode("host3").isPresent());
-
-        // Two more hosts are provisioned with expected resources
-        NodeResources resources = new NodeResources(48, 128, 1000, 10);
-        assertEquals(2, tester.provisionedHostsMatching(resources));
-    }
-
-    @Test
     public void provision_deficit_and_deprovision_excess() {
         var tester = new DynamicProvisioningTester().addInitialNodes();
         tester.flagSource.withListFlag(Flags.PREPROVISION_CAPACITY.id(),