Merge pull request #25422 from vespa-engine/hmusum/stop-using-shared-hosts-min-count

Stop using shared hosts mincount from flag when provisioning

2 files changed, 0 insertions, 79 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java
index e3e192eba5f..5769f978089 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java
@@ -14,11 +14,9 @@ import com.yahoo.jdisc.Metric;
 import com.yahoo.lang.MutableInteger;
 import com.yahoo.transaction.Mutex;
 import com.yahoo.vespa.flags.FlagSource;
-import com.yahoo.vespa.flags.JacksonFlag;
 import com.yahoo.vespa.flags.ListFlag;
 import com.yahoo.vespa.flags.PermanentFlags;
 import com.yahoo.vespa.flags.custom.ClusterCapacity;
-import com.yahoo.vespa.flags.custom.SharedHost;
 import com.yahoo.vespa.hosted.provision.LockedNodeList;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.NodeList;
@@ -33,7 +31,6 @@ import com.yahoo.vespa.hosted.provision.provisioning.NodeCandidate;
 import com.yahoo.vespa.hosted.provision.provisioning.NodePrioritizer;
 import com.yahoo.vespa.hosted.provision.provisioning.NodeSpec;
 import com.yahoo.vespa.hosted.provision.provisioning.ProvisionedHost;
-
 import java.time.Duration;
 import java.time.Instant;
 import java.util.ArrayList;
@@ -43,7 +40,6 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
-import java.util.function.Function;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
@@ -58,7 +54,6 @@ public class HostCapacityMaintainer extends NodeRepositoryMaintainer {
 
     private final HostProvisioner hostProvisioner;
     private final ListFlag<ClusterCapacity> preprovisionCapacityFlag;
-    private final JacksonFlag<SharedHost> sharedHostFlag;
 
     HostCapacityMaintainer(NodeRepository nodeRepository,
                            Duration interval,
@@ -68,7 +63,6 @@ public class HostCapacityMaintainer extends NodeRepositoryMaintainer {
         super(nodeRepository, interval, metric);
         this.hostProvisioner = hostProvisioner;
         this.preprovisionCapacityFlag = PermanentFlags.PREPROVISION_CAPACITY.bindTo(flagSource);
-        this.sharedHostFlag = PermanentFlags.SHARED_HOST.bindTo(flagSource);
     }
 
     @Override
@@ -132,28 +126,9 @@ public class HostCapacityMaintainer extends NodeRepositoryMaintainer {
      */
     private List<Node> provision(NodeList nodeList) {
         var nodes = new ArrayList<>(provisionUntilNoDeficit(nodeList));
-        var sharedHosts = new HashMap<>(findSharedHosts(nodeList));
-        int minCount = sharedHostFlag.value().getMinCount();
-        int deficit = minCount - sharedHosts.size();
-        if (deficit > 0) {
-            provisionHosts(deficit, NodeResources.unspecified())
-                    .forEach(host -> {
-                        sharedHosts.put(host.hostname(), host);
-                        nodes.add(host);
-                    });
-        }
-
         return candidatesForRemoval(nodes).stream()
                 .sorted(Comparator.comparing(node -> node.history().events().stream()
                                                          .map(History.Event::at).min(Comparator.naturalOrder()).orElse(Instant.MIN)))
-                .filter(node -> {
-                    if (!sharedHosts.containsKey(node.hostname()) || sharedHosts.size() > minCount) {
-                        sharedHosts.remove(node.hostname());
-                        return true;
-                    } else {
-                        return false;
-                    }
-                })
                 .toList();
     }
 
@@ -186,14 +161,6 @@ public class HostCapacityMaintainer extends NodeRepositoryMaintainer {
                                                      node.state() == Node.State.failed);
     }
 
-    private Map<String, Node> findSharedHosts(NodeList nodeList) {
-        return nodeList.stream()
-                .filter(node -> nodeRepository().nodes().canAllocateTenantNodeTo(node, true))
-                .filter(node -> node.reservedTo().isEmpty())
-                .filter(node -> node.exclusiveToApplicationId().isEmpty())
-                .collect(Collectors.toMap(Node::hostname, Function.identity()));
-    }
-
     /**
      * @return the nodes in {@code nodeList} plus all hosts provisioned, plus all preprovision capacity
      *         nodes that were allocated.
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainerTest.java
index 28530ddd39a..27a944a471e 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainerTest.java
@@ -22,7 +22,6 @@ import com.yahoo.net.HostName;
 import com.yahoo.vespa.flags.InMemoryFlagSource;
 import com.yahoo.vespa.flags.PermanentFlags;
 import com.yahoo.vespa.flags.custom.ClusterCapacity;
-import com.yahoo.vespa.flags.custom.SharedHost;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.NodeList;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
@@ -76,7 +75,6 @@ public class HostCapacityMaintainerTest {
 
         tester.maintain();
         assertTrue(tester.nodeRepository.nodes().node("host2").isEmpty());
-        assertTrue(tester.nodeRepository.nodes().node("host3").isEmpty());
     }
 
     @Test
@@ -208,50 +206,6 @@ public class HostCapacityMaintainerTest {
     }
 
     @Test
-    public void verify_min_count_of_shared_hosts() {
-        // What's going on here?  We are trying to verify the impact of varying the minimum number of
-        // shared hosts (SharedHost.minCount()).
-        //
-        // addInitialNodes() adds 4 tenant hosts:
-        //   host1    shared   !removable   # not removable because it got child nodes w/allocation
-        //   host2   !shared    removable   # not counted as a shared host because it is failed
-        //   host3    shared    removable
-        //   host4    shared   !removable   # not removable because it got child nodes w/allocation
-        //
-        // Hosts 1, 3, and 4 count as "shared hosts" with respect to the minCount lower boundary.
-        // Hosts 3 and 4 are removable, that is they will be deprovisioned as excess hosts unless
-        // prevented by minCount.
-
-        // minCount=0: All (2) removable hosts are deprovisioned
-        assertWithMinCount(0, 0, 2);
-        // minCount=1: The same thing happens, because there are 2 shared hosts left
-        assertWithMinCount(1, 0, 2);
-        assertWithMinCount(2, 0, 2);
-        // minCount=3: since we require 3 shared hosts, host3 is not deprovisioned.
-        assertWithMinCount(3, 0, 1);
-        // 4 shared hosts require we provision 1 shared host
-        assertWithMinCount(4, 1, 1);
-        // 5 shared hosts require we provision 2 shared hosts
-        assertWithMinCount(5, 2, 1);
-        assertWithMinCount(6, 3, 1);
-    }
-
-    private void assertWithMinCount(int minCount, int provisionCount, int deprovisionCount) {
-        var tester = new DynamicProvisioningTester().addInitialNodes();
-        tester.hostProvisioner.overrideHostFlavor("host4");
-
-        tester.flagSource.withJacksonFlag(PermanentFlags.SHARED_HOST.id(), new SharedHost(null, minCount), SharedHost.class);
-        tester.maintain();
-        assertEquals(provisionCount, tester.hostProvisioner.provisionedHosts().size());
-        assertEquals(deprovisionCount, tester.hostProvisioner.deprovisionedHosts());
-
-        // Verify next maintain is a no-op
-        tester.maintain();
-        assertEquals(provisionCount, tester.hostProvisioner.provisionedHosts().size());
-        assertEquals(deprovisionCount, tester.hostProvisioner.deprovisionedHosts());
-    }
-
-    @Test
     public void does_not_remove_if_host_provisioner_failed() {
         var tester = new DynamicProvisioningTester();
         Node host2 = tester.addNode("host2", Optional.empty(), NodeType.host, Node.State.failed, DynamicProvisioningTester.tenantApp);