diff --git a/engine/orchestration/src/main/java/com/cloud/cluster/agentlb/ClusterBasedAgentLoadBalancerPlanner.java b/engine/orchestration/src/main/java/com/cloud/cluster/agentlb/ClusterBasedAgentLoadBalancerPlanner.java index 5b05b4df0423..57627484cc1a 100644 --- a/engine/orchestration/src/main/java/com/cloud/cluster/agentlb/ClusterBasedAgentLoadBalancerPlanner.java +++ b/engine/orchestration/src/main/java/com/cloud/cluster/agentlb/ClusterBasedAgentLoadBalancerPlanner.java @@ -88,37 +88,54 @@ public List getHostsToRebalance(ManagementServerHostVO ms, int avLoad) { hostToClusterMap = sortByClusterSize(hostToClusterMap); int hostsToGive = allHosts.size() - avLoad; - int hostsLeftToGive = hostsToGive; - int hostsLeft = directHosts.size(); - List hostsToReturn = new ArrayList(); logger.debug("Management server {} can give away {} as it currently owns {} and the " + "average agent load in the system is {}; finalyzing list of hosts to give away...", ms, hostsToGive, allHosts.size(), avLoad); + List hostsToReturn = selectHostsToGiveAway(hostToClusterMap, hostsToGive, directHosts.size()); + + logger.debug("Management server {} is ready to give away {} hosts", ms, hostsToReturn.size()); + return hostsToReturn; + } + + /** + * Picks hosts to hand over from a management server that is over its average agent load. + * Whole clusters are preferred (so a hypervisor cluster stays on a single management server + * whenever possible), but when no combination of whole clusters can satisfy {@code hostsToGive} + * (e.g. a single cluster holds all the hosts), a partial subset of the last cluster considered + * is taken instead of giving away nothing, so rebalancing still makes progress. + */ + protected List selectHostsToGiveAway(Map> hostToClusterMap, int hostsToGive, int totalDirectHosts) { + int hostsLeftToGive = hostsToGive; + int hostsLeft = totalDirectHosts; + List hostsToReturn = new ArrayList(); + for (Long cluster : hostToClusterMap.keySet()) { List hostsInCluster = hostToClusterMap.get(cluster); hostsLeft = hostsLeft - hostsInCluster.size(); - if (hostsToReturn.size() < hostsToGive) { - logger.debug("Trying cluster id=" + cluster); - - if (hostsInCluster.size() > hostsLeftToGive) { - logger.debug("Skipping cluster id=" + cluster + " as it has more hosts than we need: " + hostsInCluster.size() + " vs " + hostsLeftToGive); - if (hostsLeft >= hostsLeftToGive) { - continue; - } else { - break; - } - } else { - logger.debug("Taking all " + hostsInCluster.size() + " hosts: " + hostsInCluster + " from cluster id=" + cluster); - hostsToReturn.addAll(hostsInCluster); - hostsLeftToGive = hostsLeftToGive - hostsInCluster.size(); - } + if (hostsToReturn.size() >= hostsToGive) { + break; + } + + logger.debug("Trying cluster id=" + cluster); + + if (hostsInCluster.size() <= hostsLeftToGive) { + logger.debug("Taking all " + hostsInCluster.size() + " hosts: " + hostsInCluster + " from cluster id=" + cluster); + hostsToReturn.addAll(hostsInCluster); + hostsLeftToGive = hostsLeftToGive - hostsInCluster.size(); + } else if (hostsLeft >= hostsLeftToGive) { + logger.debug("Skipping cluster id=" + cluster + " as it has more hosts than we need: " + hostsInCluster.size() + " vs " + hostsLeftToGive + + ", and remaining clusters can still satisfy the quota"); + continue; } else { + logger.debug("No combination of whole clusters can satisfy the quota; taking a partial subset of " + hostsLeftToGive + + " hosts from cluster id=" + cluster + " instead of giving away nothing"); + hostsToReturn.addAll(hostsInCluster.subList(0, hostsLeftToGive)); + hostsLeftToGive = 0; break; } } - logger.debug("Management server {} is ready to give away {} hosts", ms, hostsToReturn.size()); return hostsToReturn; } diff --git a/engine/orchestration/src/test/java/com/cloud/cluster/agentlb/ClusterBasedAgentLoadBalancerPlannerTest.java b/engine/orchestration/src/test/java/com/cloud/cluster/agentlb/ClusterBasedAgentLoadBalancerPlannerTest.java new file mode 100644 index 000000000000..523c79d0aae3 --- /dev/null +++ b/engine/orchestration/src/test/java/com/cloud/cluster/agentlb/ClusterBasedAgentLoadBalancerPlannerTest.java @@ -0,0 +1,98 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.cluster.agentlb; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; + +import org.junit.Test; + +import com.cloud.host.HostVO; + +public class ClusterBasedAgentLoadBalancerPlannerTest { + + private final ClusterBasedAgentLoadBalancerPlanner planner = new ClusterBasedAgentLoadBalancerPlanner(); + + private HostVO host(long clusterId) { + HostVO host = new HostVO("guid-" + clusterId + "-" + System.nanoTime()); + host.setClusterId(clusterId); + return host; + } + + private LinkedHashMap> clusterOf(int... clusterSizes) { + LinkedHashMap> map = new LinkedHashMap<>(); + long clusterId = 1; + for (int size : clusterSizes) { + List hosts = new ArrayList<>(); + for (int i = 0; i < size; i++) { + hosts.add(host(clusterId)); + } + map.put(clusterId, hosts); + clusterId++; + } + return map; + } + + @Test + public void singleOversizedClusterGivesAwayPartialHosts() { + LinkedHashMap> hostToClusterMap = clusterOf(2); + int totalDirectHosts = 2; + int avLoad = 1; + int hostsToGive = totalDirectHosts - avLoad; + + List hostsToReturn = planner.selectHostsToGiveAway(hostToClusterMap, hostsToGive, totalDirectHosts); + + assertEquals("a lone oversized cluster must still give away hosts to satisfy the quota", 1, hostsToReturn.size()); + } + + @Test + public void wholeClusterIsPreferredWhenItExactlyFitsTheQuota() { + LinkedHashMap> hostToClusterMap = clusterOf(2, 3); + int totalDirectHosts = 5; + int hostsToGive = 2; + + List hostsToReturn = planner.selectHostsToGiveAway(hostToClusterMap, hostsToGive, totalDirectHosts); + + assertEquals(2, hostsToReturn.size()); + assertTrue("should take the whole 2-host cluster rather than split the larger one", hostToClusterMap.get(1L).containsAll(hostsToReturn)); + } + + @Test + public void smallerClusterIsPreferredOverSplittingWhenBothCanSatisfyQuota() { + LinkedHashMap> hostToClusterMap = clusterOf(5, 2); + int totalDirectHosts = 7; + int hostsToGive = 2; + + List hostsToReturn = planner.selectHostsToGiveAway(hostToClusterMap, hostsToGive, totalDirectHosts); + + assertEquals(2, hostsToReturn.size()); + assertTrue("should skip the oversized cluster and take the smaller cluster whole", hostToClusterMap.get(2L).containsAll(hostsToReturn)); + } + + @Test + public void noHostsGivenAwayWhenAlreadyUnderThreshold() { + LinkedHashMap> hostToClusterMap = clusterOf(1); + + List hostsToReturn = planner.selectHostsToGiveAway(hostToClusterMap, 0, 1); + + assertTrue(hostsToReturn.isEmpty()); + } +}