From 245ce970c8827d81eb7b9a30455fddecc817660b Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 17 Jan 2024 01:39:33 +0530 Subject: [PATCH 01/12] Use free/total instead of free metric to calculate imbalance --- .../cluster/ClusterDrsAlgorithm.java | 44 +++++++++---------- .../apache/cloudstack/cluster/Balanced.java | 23 +++++----- .../cloudstack/cluster/BalancedTest.java | 23 +++++----- .../apache/cloudstack/cluster/Condensed.java | 18 +++++--- .../cloudstack/cluster/CondensedTest.java | 23 +++++----- .../cluster/ClusterDrsServiceImpl.java | 26 ++++++----- .../cluster/ClusterDrsServiceImplTest.java | 2 - 7 files changed, 82 insertions(+), 77 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java b/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java index 889c49298ecc..5b50fc909360 100644 --- a/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java +++ b/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java @@ -42,16 +42,16 @@ public interface ClusterDrsAlgorithm extends Adapter { * @param clusterId * the ID of the cluster to check * @param cpuList - * a list of CPU allocated values for each host in the cluster + * a list of pair of free CPU & total CPU for each host in the cluster * @param memoryList - * a list of memory allocated values for each host in the cluster + * a list of pair of free memory & total memory values for each host in the cluster * * @return true if a DRS operation is needed, false otherwise * * @throws ConfigurationException * if there is an error in the configuration */ - boolean needsDrs(long clusterId, List cpuList, List memoryList) throws ConfigurationException; + boolean needsDrs(long clusterId, List> cpuList, List> memoryList) throws ConfigurationException; /** @@ -66,17 +66,17 @@ public interface ClusterDrsAlgorithm extends Adapter { * @param destHost * the destination host for the virtual machine * @param hostCpuFreeMap - * a map of host IDs to the amount of CPU free on each host + * a map of host IDs to the pair of amount of free CPU and total CPU on each host * @param hostMemoryFreeMap - * a map of host IDs to the amount of memory free on each host + * a map of host IDs to the pair of amount of free memory free and total memory on each host * @param requiresStorageMotion * whether storage motion is required for the virtual machine * * @return a ternary containing improvement, cost, benefit */ Ternary getMetrics(long clusterId, VirtualMachine vm, ServiceOffering serviceOffering, - Host destHost, Map hostCpuFreeMap, - Map hostMemoryFreeMap, Boolean requiresStorageMotion); + Host destHost, Map> hostCpuFreeMap, + Map> hostMemoryFreeMap, Boolean requiresStorageMotion); /** * Calculates the imbalance of the cluster after a virtual machine migration. @@ -95,25 +95,25 @@ Ternary getMetrics(long clusterId, VirtualMachine vm, Se * @return a pair containing the CPU and memory imbalance of the cluster after the migration */ default Pair getImbalancePostMigration(ServiceOffering serviceOffering, VirtualMachine vm, - Host destHost, Map hostCpuFreeMap, - Map hostMemoryFreeMap) { - List postCpuList = new ArrayList<>(); - List postMemoryList = new ArrayList<>(); + Host destHost, Map> hostCpuFreeMap, + Map> hostMemoryFreeMap) { + List postCpuList = new ArrayList<>(); + List postMemoryList = new ArrayList<>(); final int vmCpu = serviceOffering.getCpu() * serviceOffering.getSpeed(); final long vmRam = serviceOffering.getRamSize() * 1024L * 1024L; for (Long hostId : hostCpuFreeMap.keySet()) { - long cpu = hostCpuFreeMap.get(hostId); - long memory = hostMemoryFreeMap.get(hostId); + long cpu = hostCpuFreeMap.get(hostId).first(); + long memory = hostMemoryFreeMap.get(hostId).first(); if (hostId == destHost.getId()) { - postCpuList.add(cpu - vmCpu); - postMemoryList.add(memory - vmRam); + postCpuList.add((double)(cpu - vmCpu)/hostCpuFreeMap.get(hostId).second()); + postMemoryList.add((double)(memory - vmRam)/hostMemoryFreeMap.get(hostId).second()); } else if (hostId.equals(vm.getHostId())) { - postCpuList.add(cpu + vmCpu); - postMemoryList.add(memory + vmRam); + postCpuList.add((double)(cpu + vmCpu)/hostCpuFreeMap.get(hostId).second()); + postMemoryList.add((double)(memory + vmRam)/hostMemoryFreeMap.get(hostId).second()); } else { - postCpuList.add(cpu); - postMemoryList.add(memory); + postCpuList.add((double)cpu/hostCpuFreeMap.get(hostId).second()); + postMemoryList.add((double)memory/hostMemoryFreeMap.get(hostId).second()); } } return new Pair<>(getClusterImbalance(postCpuList), getClusterImbalance(postMemoryList)); @@ -129,7 +129,7 @@ default Pair getImbalancePostMigration(ServiceOffering serviceOf * Cluster Imbalance, Ic = σc / mavg , where σc is the standard deviation and * mavg is the mean metric value for the cluster. */ - default Double getClusterImbalance(List metricList) { + default Double getClusterImbalance(List metricList) { Double clusterMeanMetric = getClusterMeanMetric(metricList); Double clusterStandardDeviation = getClusterStandardDeviation(metricList, clusterMeanMetric); return clusterStandardDeviation / clusterMeanMetric; @@ -142,7 +142,7 @@ default Double getClusterImbalance(List metricList) { * Cluster Mean Metric, mavg = (∑mi) / N, where mi is a measurable metric for a * resource ‘i’ in a cluster with total N number of resources. */ - default Double getClusterMeanMetric(List metricList) { + default Double getClusterMeanMetric(List metricList) { return new Mean().evaluate(metricList.stream().mapToDouble(i -> i).toArray()); } @@ -157,7 +157,7 @@ default Double getClusterMeanMetric(List metricList) { * mean metric value and mi is a measurable metric for some resource ‘i’ in the * cluster with total N number of resources. */ - default Double getClusterStandardDeviation(List metricList, Double mean) { + default Double getClusterStandardDeviation(List metricList, Double mean) { if (mean != null) { return new StandardDeviation(false).evaluate(metricList.stream().mapToDouble(i -> i).toArray(), mean); } else { diff --git a/plugins/drs/cluster/balanced/src/main/java/org/apache/cloudstack/cluster/Balanced.java b/plugins/drs/cluster/balanced/src/main/java/org/apache/cloudstack/cluster/Balanced.java index dc15a8205607..a3e5e22a9a12 100644 --- a/plugins/drs/cluster/balanced/src/main/java/org/apache/cloudstack/cluster/Balanced.java +++ b/plugins/drs/cluster/balanced/src/main/java/org/apache/cloudstack/cluster/Balanced.java @@ -27,9 +27,9 @@ import com.cloud.vm.VirtualMachine; import javax.naming.ConfigurationException; -import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsImbalanceThreshold; import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsMetric; @@ -42,15 +42,17 @@ public String getName() { } @Override - public boolean needsDrs(long clusterId, List cpuList, List memoryList) throws ConfigurationException { - Double cpuImbalance = getClusterImbalance(cpuList); - Double memoryImbalance = getClusterImbalance(memoryList); + public boolean needsDrs(long clusterId, List> cpuList, List> memoryList) throws ConfigurationException { double threshold = getThreshold(clusterId); String metric = ClusterDrsMetric.valueIn(clusterId); switch (metric) { case "cpu": + List cpuRatioList = cpuList.stream().map(pair -> (double) pair.first()/ pair.second()).collect(Collectors.toList()); + Double cpuImbalance = getClusterImbalance(cpuRatioList); return cpuImbalance > threshold; case "memory": + List memoryRatioList = memoryList.stream().map(pair -> (double) pair.first()/ pair.second()).collect(Collectors.toList()); + Double memoryImbalance = getClusterImbalance(memoryRatioList); return memoryImbalance > threshold; default: throw new ConfigurationException( @@ -65,13 +67,14 @@ private double getThreshold(long clusterId) throws ConfigurationException { @Override public Ternary getMetrics(long clusterId, VirtualMachine vm, ServiceOffering serviceOffering, Host destHost, - Map hostCpuUsedMap, Map hostMemoryUsedMap, + Map> hostCpuFreeMap, Map> hostMemoryFreeMap, Boolean requiresStorageMotion) { - Double preCpuImbalance = getClusterImbalance(new ArrayList<>(hostCpuUsedMap.values())); - Double preMemoryImbalance = getClusterImbalance(new ArrayList<>(hostMemoryUsedMap.values())); - - Pair imbalancePair = getImbalancePostMigration(serviceOffering, vm, destHost, hostCpuUsedMap, - hostMemoryUsedMap); + List cpuList = hostCpuFreeMap.values().stream().map(pair -> (double) pair.first() / pair.second()).collect(Collectors.toList()); + Double preCpuImbalance = getClusterImbalance(cpuList); + List memoryList = hostMemoryFreeMap.values().stream().map(pair -> (double) pair.first() / pair.second()).collect(Collectors.toList()); + Double preMemoryImbalance = getClusterImbalance(memoryList); + Pair imbalancePair = getImbalancePostMigration(serviceOffering, vm, destHost, hostCpuFreeMap, + hostMemoryFreeMap); Double postCpuImbalance = imbalancePair.first(); Double postMemoryImbalance = imbalancePair.second(); diff --git a/plugins/drs/cluster/balanced/src/test/java/org/apache/cloudstack/cluster/BalancedTest.java b/plugins/drs/cluster/balanced/src/test/java/org/apache/cloudstack/cluster/BalancedTest.java index 0da807e65c37..4b39536715b1 100644 --- a/plugins/drs/cluster/balanced/src/test/java/org/apache/cloudstack/cluster/BalancedTest.java +++ b/plugins/drs/cluster/balanced/src/test/java/org/apache/cloudstack/cluster/BalancedTest.java @@ -22,6 +22,7 @@ import com.cloud.host.Host; import com.cloud.service.ServiceOfferingVO; import com.cloud.service.dao.ServiceOfferingDao; +import com.cloud.utils.Pair; import com.cloud.utils.Ternary; import com.cloud.vm.VirtualMachine; import org.apache.cloudstack.framework.config.ConfigKey; @@ -37,6 +38,7 @@ import javax.naming.ConfigurationException; import java.lang.reflect.Field; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -66,9 +68,7 @@ public class BalancedTest { Map> hostVmMap; - List cpuList, memoryList; - - Map hostCpuFreeMap, hostMemoryFreeMap; + Map> hostCpuFreeMap, hostMemoryFreeMap; @Mock @@ -102,16 +102,13 @@ public void setUp() throws NoSuchFieldException, IllegalAccessException { overrideDefaultConfigValue(ClusterDrsImbalanceThreshold, "_defaultValue", "0.5"); - cpuList = Arrays.asList(1L, 2L); - memoryList = Arrays.asList(512L, 2048L); - hostCpuFreeMap = new HashMap<>(); - hostCpuFreeMap.put(1L, 2000L); - hostCpuFreeMap.put(2L, 1000L); + hostCpuFreeMap.put(1L, new Pair<>(2000L, 10000L)); + hostCpuFreeMap.put(2L, new Pair<>(1000L, 10000L)); hostMemoryFreeMap = new HashMap<>(); - hostMemoryFreeMap.put(1L, 2048L * 1024L * 1024L); - hostMemoryFreeMap.put(2L, 512L * 1024L * 1024L); + hostMemoryFreeMap.put(1L, new Pair<>(2048L * 1024L * 1024L, 8192L * 1024L * 1024L)); + hostMemoryFreeMap.put(2L, new Pair<>(512L * 1024L * 1024L, 8192L * 1024L * 1024L)); } private void overrideDefaultConfigValue(final ConfigKey configKey, final String name, @@ -144,7 +141,7 @@ public void tearDown() throws Exception { @Test public void needsDrsWithCpu() throws ConfigurationException, NoSuchFieldException, IllegalAccessException { overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "cpu"); - assertFalse(balanced.needsDrs(clusterId, cpuList, memoryList)); + assertFalse(balanced.needsDrs(clusterId, new ArrayList<>(hostCpuFreeMap.values()), new ArrayList<>(hostMemoryFreeMap.values()))); } /* @@ -154,14 +151,14 @@ public void needsDrsWithCpu() throws ConfigurationException, NoSuchFieldExceptio @Test public void needsDrsWithMemory() throws ConfigurationException, NoSuchFieldException, IllegalAccessException { overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "memory"); - assertTrue(balanced.needsDrs(clusterId, cpuList, memoryList)); + assertTrue(balanced.needsDrs(clusterId, new ArrayList<>(hostCpuFreeMap.values()), new ArrayList<>(hostMemoryFreeMap.values()))); } /* 3. cluster with "unknown" metric */ @Test public void needsDrsWithUnknown() throws ConfigurationException, NoSuchFieldException, IllegalAccessException { overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "unknown"); - assertThrows(ConfigurationException.class, () -> balanced.needsDrs(clusterId, cpuList, memoryList)); + assertThrows(ConfigurationException.class, () -> balanced.needsDrs(clusterId, new ArrayList<>(hostCpuFreeMap.values()), new ArrayList<>(hostMemoryFreeMap.values()))); } /** diff --git a/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java b/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java index aefd11905efa..31ca1fad5569 100644 --- a/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java +++ b/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java @@ -27,9 +27,9 @@ import com.cloud.vm.VirtualMachine; import javax.naming.ConfigurationException; -import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsImbalanceThreshold; import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsMetric; @@ -42,15 +42,17 @@ public String getName() { } @Override - public boolean needsDrs(long clusterId, List cpuList, List memoryList) throws ConfigurationException { - Double cpuImbalance = getClusterImbalance(cpuList); - Double memoryImbalance = getClusterImbalance(memoryList); + public boolean needsDrs(long clusterId, List> cpuList, List> memoryList) throws ConfigurationException { double threshold = getThreshold(clusterId); String metric = ClusterDrsMetric.valueIn(clusterId); switch (metric) { case "cpu": + List cpuRatioList = cpuList.stream().map(pair -> (double) pair.first() / pair.second()).collect(Collectors.toList()); + Double cpuImbalance = getClusterImbalance(cpuRatioList); return cpuImbalance < threshold; case "memory": + List memoryRatioList = memoryList.stream().map(pair -> (double) pair.first() / pair.second()).collect(Collectors.toList()); + Double memoryImbalance = getClusterImbalance(memoryRatioList); return memoryImbalance < threshold; default: throw new ConfigurationException( @@ -65,10 +67,12 @@ private double getThreshold(long clusterId) throws ConfigurationException { @Override public Ternary getMetrics(long clusterId, VirtualMachine vm, ServiceOffering serviceOffering, Host destHost, - Map hostCpuUsedMap, Map hostMemoryUsedMap, + Map> hostCpuUsedMap, Map> hostMemoryUsedMap, Boolean requiresStorageMotion) { - Double preCpuImbalance = getClusterImbalance(new ArrayList<>(hostCpuUsedMap.values())); - Double preMemoryImbalance = getClusterImbalance(new ArrayList<>(hostMemoryUsedMap.values())); + List cpuList = hostCpuUsedMap.values().stream().map(pair -> (double) pair.first() / pair.second()).collect(Collectors.toList()); + Double preCpuImbalance = getClusterImbalance(cpuList); + List memoryList = hostMemoryUsedMap.values().stream().map(pair -> (double) pair.first() / pair.second()).collect(Collectors.toList()); + Double preMemoryImbalance = getClusterImbalance(memoryList); Pair imbalancePair = getImbalancePostMigration(serviceOffering, vm, destHost, hostCpuUsedMap, hostMemoryUsedMap); diff --git a/plugins/drs/cluster/condensed/src/test/java/org/apache/cloudstack/cluster/CondensedTest.java b/plugins/drs/cluster/condensed/src/test/java/org/apache/cloudstack/cluster/CondensedTest.java index 0ba2b66379aa..28e88d43984b 100644 --- a/plugins/drs/cluster/condensed/src/test/java/org/apache/cloudstack/cluster/CondensedTest.java +++ b/plugins/drs/cluster/condensed/src/test/java/org/apache/cloudstack/cluster/CondensedTest.java @@ -21,6 +21,7 @@ import com.cloud.host.Host; import com.cloud.service.ServiceOfferingVO; +import com.cloud.utils.Pair; import com.cloud.utils.Ternary; import com.cloud.vm.VirtualMachine; import org.apache.cloudstack.framework.config.ConfigKey; @@ -35,6 +36,7 @@ import javax.naming.ConfigurationException; import java.lang.reflect.Field; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -64,9 +66,7 @@ public class CondensedTest { Map> hostVmMap; - List cpuList, memoryList; - - Map hostCpuFreeMap, hostMemoryFreeMap; + Map> hostCpuFreeMap, hostMemoryFreeMap; private AutoCloseable closeable; @@ -95,16 +95,13 @@ public void setUp() throws NoSuchFieldException, IllegalAccessException { overrideDefaultConfigValue(ClusterDrsImbalanceThreshold, "_defaultValue", "0.5"); - cpuList = Arrays.asList(1L, 2L); - memoryList = Arrays.asList(512L, 2048L); - hostCpuFreeMap = new HashMap<>(); - hostCpuFreeMap.put(1L, 2000L); - hostCpuFreeMap.put(2L, 1000L); + hostCpuFreeMap.put(1L, new Pair<>(2000L, 10000L)); + hostCpuFreeMap.put(2L, new Pair<>(1000L, 10000L)); hostMemoryFreeMap = new HashMap<>(); - hostMemoryFreeMap.put(1L, 2048L * 1024L * 1024L); - hostMemoryFreeMap.put(2L, 512L * 1024L * 1024L); + hostMemoryFreeMap.put(1L, new Pair<>(2048L * 1024L * 1024L, 8192L * 1024L * 1024L)); + hostMemoryFreeMap.put(2L, new Pair<>(512L * 1024L * 1024L, 8192L * 1024L * 1024L)); } private void overrideDefaultConfigValue(final ConfigKey configKey, @@ -138,7 +135,7 @@ public void tearDown() throws Exception { @Test public void needsDrsWithCpu() throws ConfigurationException, NoSuchFieldException, IllegalAccessException { overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "cpu"); - assertTrue(condensed.needsDrs(clusterId, cpuList, memoryList)); + assertTrue(condensed.needsDrs(clusterId, new ArrayList<>(hostCpuFreeMap.values()), new ArrayList<>(hostMemoryFreeMap.values()))); } /* @@ -148,14 +145,14 @@ public void needsDrsWithCpu() throws ConfigurationException, NoSuchFieldExceptio @Test public void needsDrsWithMemory() throws ConfigurationException, NoSuchFieldException, IllegalAccessException { overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "memory"); - assertFalse(condensed.needsDrs(clusterId, cpuList, memoryList)); + assertFalse(condensed.needsDrs(clusterId, new ArrayList<>(hostCpuFreeMap.values()), new ArrayList<>(hostMemoryFreeMap.values()))); } /* 3. cluster with "unknown" metric */ @Test public void needsDrsWithUnknown() throws ConfigurationException, NoSuchFieldException, IllegalAccessException { overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "unknown"); - assertThrows(ConfigurationException.class, () -> condensed.needsDrs(clusterId, cpuList, memoryList)); + assertThrows(ConfigurationException.class, () -> condensed.needsDrs(clusterId, new ArrayList<>(hostCpuFreeMap.values()), new ArrayList<>(hostMemoryFreeMap.values()))); } /** diff --git a/server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java b/server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java index f949233e8e85..8d566dbd7e5a 100644 --- a/server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java +++ b/server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java @@ -353,10 +353,16 @@ List> getDrsPlan(Cluster cluster, List hostJoinList = hostJoinDao.searchByIds( hostList.stream().map(HostVO::getId).toArray(Long[]::new)); - Map hostCpuMap = hostJoinList.stream().collect(Collectors.toMap(HostJoinVO::getId, - hostJoin -> hostJoin.getCpus() * hostJoin.getSpeed() - hostJoin.getCpuReservedCapacity() - hostJoin.getCpuUsedCapacity())); - Map hostMemoryMap = hostJoinList.stream().collect(Collectors.toMap(HostJoinVO::getId, - hostJoin -> hostJoin.getTotalMemory() - hostJoin.getMemUsedCapacity() - hostJoin.getMemReservedCapacity())); + Map> hostCpuMap = hostJoinList.stream().collect(Collectors.toMap(HostJoinVO::getId, + hostJoin -> { + long totalCpu = hostJoin.getCpus() * hostJoin.getSpeed() - hostJoin.getCpuReservedCapacity(); + return new Pair<>(totalCpu - hostJoin.getCpuUsedCapacity(), totalCpu); + })); + Map> hostMemoryMap = hostJoinList.stream().collect(Collectors.toMap(HostJoinVO::getId, + hostJoin -> { + long totalMemory = hostJoin.getTotalMemory() - hostJoin.getMemUsedCapacity(); + return new Pair<>(totalMemory - hostJoin.getMemUsedCapacity(), totalMemory); + })); Map vmIdServiceOfferingMap = new HashMap<>(); @@ -387,10 +393,10 @@ List> getDrsPlan(Cluster cluster, long vmCpu = (long) serviceOffering.getCpu() * serviceOffering.getSpeed(); long vmMemory = serviceOffering.getRamSize() * 1024L * 1024L; - hostCpuMap.put(vm.getHostId(), hostCpuMap.get(vm.getHostId()) + vmCpu); - hostCpuMap.put(destHost.getId(), hostCpuMap.get(destHost.getId()) - vmCpu); - hostMemoryMap.put(vm.getHostId(), hostMemoryMap.get(vm.getHostId()) + vmMemory); - hostMemoryMap.put(destHost.getId(), hostMemoryMap.get(destHost.getId()) - vmMemory); + hostCpuMap.get(vm.getHostId()).set(hostCpuMap.get(vm.getHostId()).first() + vmCpu, hostCpuMap.get(vm.getHostId()).second()); + hostCpuMap.get(destHost.getId()).set(hostCpuMap.get(destHost.getId()).first() - vmCpu, hostCpuMap.get(destHost.getId()).second()); + hostMemoryMap.get(vm.getHostId()).set(hostMemoryMap.get(vm.getHostId()).first() + vmMemory, hostMemoryMap.get(vm.getHostId()).second()); + hostMemoryMap.get(destHost.getId()).set(hostMemoryMap.get(destHost.getId()).first() - vmMemory, hostMemoryMap.get(destHost.getId()).second()); vm.setHostId(destHost.getId()); iteration++; } @@ -443,8 +449,8 @@ Map> getHostVmMap(List hostList, List getBestMigration(Cluster cluster, ClusterDrsAlgorithm algorithm, List vmList, Map vmIdServiceOfferingMap, - Map hostCpuCapacityMap, - Map hostMemoryCapacityMap) { + Map> hostCpuCapacityMap, + Map> hostMemoryCapacityMap) { double improvement = 0; Pair bestMigration = new Pair<>(null, null); diff --git a/server/src/test/java/org/apache/cloudstack/cluster/ClusterDrsServiceImplTest.java b/server/src/test/java/org/apache/cloudstack/cluster/ClusterDrsServiceImplTest.java index e82b39a47ecc..480bc67dd918 100644 --- a/server/src/test/java/org/apache/cloudstack/cluster/ClusterDrsServiceImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/cluster/ClusterDrsServiceImplTest.java @@ -180,14 +180,12 @@ public void testGetDrsPlan() throws ConfigurationException { Mockito.when(hostJoin1.getCpuUsedCapacity()).thenReturn(1000L); Mockito.when(hostJoin1.getCpuReservedCapacity()).thenReturn(0L); Mockito.when(hostJoin1.getMemUsedCapacity()).thenReturn(1024L); - Mockito.when(hostJoin1.getMemReservedCapacity()).thenReturn(512L); HostJoinVO hostJoin2 = Mockito.mock(HostJoinVO.class); Mockito.when(hostJoin2.getId()).thenReturn(2L); Mockito.when(hostJoin2.getCpuUsedCapacity()).thenReturn(1000L); Mockito.when(hostJoin2.getCpuReservedCapacity()).thenReturn(0L); Mockito.when(hostJoin2.getMemUsedCapacity()).thenReturn(1024L); - Mockito.when(hostJoin2.getMemReservedCapacity()).thenReturn(512L); List vmList = new ArrayList<>(); vmList.add(vm1); From 1b818878669d85b92d3b478ad5bdd99637cc8fac Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 17 Jan 2024 01:42:10 +0530 Subject: [PATCH 02/12] Filter out hosts for condensed while checking imbalance --- .../apache/cloudstack/cluster/Condensed.java | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java b/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java index 31ca1fad5569..522099d9df32 100644 --- a/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java +++ b/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java @@ -27,6 +27,7 @@ import com.cloud.vm.VirtualMachine; import javax.naming.ConfigurationException; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.stream.Collectors; @@ -47,11 +48,27 @@ public boolean needsDrs(long clusterId, List> cpuList, List cpuRatioList = cpuList.stream().map(pair -> (double) pair.first() / pair.second()).collect(Collectors.toList()); + List cpuRatioList = new ArrayList<>(); + for (Pair pair : cpuList) { + Double ratio = (double) pair.first() / pair.second(); + // To ensure that we calculate imbalance in cases where a few hosts are already condensed + // but other hosts aren't. This is to prevent the case where we don't migrate VMs because + // imbalance > 1 but the cluster is not condensed. + if (pair.first() < 0.05 * pair.second()) continue; + cpuRatioList.add(ratio); + } Double cpuImbalance = getClusterImbalance(cpuRatioList); return cpuImbalance < threshold; case "memory": - List memoryRatioList = memoryList.stream().map(pair -> (double) pair.first() / pair.second()).collect(Collectors.toList()); + List memoryRatioList = new ArrayList<>(); + for (Pair pair : memoryList) { + Double ratio = (double) pair.first() / pair.second(); + // To ensure that we calculate imbalance in cases where a few hosts are already condensed + // but other hosts aren't. This is to prevent the case where we don't migrate VMs because + // imbalance > 1 but the cluster is not condensed. + if (pair.first() < 0.05 * pair.second()) continue; + memoryRatioList.add(ratio); + } Double memoryImbalance = getClusterImbalance(memoryRatioList); return memoryImbalance < threshold; default: From 9382c14f94708ec99cd63039ed41cde54bcd06ef Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 17 Jan 2024 21:33:45 +0530 Subject: [PATCH 03/12] Make DRS more configurable --- .../cluster/ClusterDrsAlgorithm.java | 144 +++++++++++++----- .../cloudstack/cluster/ClusterDrsService.java | 13 ++ .../apache/cloudstack/cluster/Balanced.java | 60 ++------ .../cloudstack/cluster/BalancedTest.java | 42 ++--- .../apache/cloudstack/cluster/Condensed.java | 86 +++-------- .../cloudstack/cluster/CondensedTest.java | 35 ++--- .../cluster/ClusterDrsServiceImpl.java | 27 ++-- .../cluster/ClusterDrsServiceImplTest.java | 4 +- 8 files changed, 194 insertions(+), 217 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java b/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java index 5b50fc909360..ae3916b97dbd 100644 --- a/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java +++ b/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java @@ -21,7 +21,6 @@ import com.cloud.host.Host; import com.cloud.offering.ServiceOffering; -import com.cloud.utils.Pair; import com.cloud.utils.Ternary; import com.cloud.utils.component.Adapter; import com.cloud.vm.VirtualMachine; @@ -33,6 +32,10 @@ import java.util.List; import java.util.Map; +import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsMetricType; +import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsMetricUseRatio; +import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsMetric; + public interface ClusterDrsAlgorithm extends Adapter { /** @@ -42,16 +45,16 @@ public interface ClusterDrsAlgorithm extends Adapter { * @param clusterId * the ID of the cluster to check * @param cpuList - * a list of pair of free CPU & total CPU for each host in the cluster + * a list of Ternary of used, reserved & total CPU for each host in the cluster * @param memoryList - * a list of pair of free memory & total memory values for each host in the cluster + * a list of Ternary of used, reserved & total memory values for each host in the cluster * * @return true if a DRS operation is needed, false otherwise * * @throws ConfigurationException * if there is an error in the configuration */ - boolean needsDrs(long clusterId, List> cpuList, List> memoryList) throws ConfigurationException; + boolean needsDrs(long clusterId, List> cpuList, List> memoryList) throws ConfigurationException; /** @@ -65,18 +68,18 @@ public interface ClusterDrsAlgorithm extends Adapter { * the service offering for the virtual machine * @param destHost * the destination host for the virtual machine - * @param hostCpuFreeMap - * a map of host IDs to the pair of amount of free CPU and total CPU on each host - * @param hostMemoryFreeMap - * a map of host IDs to the pair of amount of free memory free and total memory on each host + * @param hostCpuMap + * a map of host IDs to the Ternary of used, reserved and total CPU on each host + * @param hostMemoryMap + * a map of host IDs to the Ternary of used, reserved and total memory on each host * @param requiresStorageMotion * whether storage motion is required for the virtual machine * * @return a ternary containing improvement, cost, benefit */ Ternary getMetrics(long clusterId, VirtualMachine vm, ServiceOffering serviceOffering, - Host destHost, Map> hostCpuFreeMap, - Map> hostMemoryFreeMap, Boolean requiresStorageMotion); + Host destHost, Map> hostCpuMap, + Map> hostMemoryMap, Boolean requiresStorageMotion) throws ConfigurationException; /** * Calculates the imbalance of the cluster after a virtual machine migration. @@ -87,36 +90,65 @@ Ternary getMetrics(long clusterId, VirtualMachine vm, Se * the virtual machine being migrated * @param destHost * the destination host for the virtual machine - * @param hostCpuFreeMap - * a map of host IDs to the amount of CPU free on each host - * @param hostMemoryFreeMap - * a map of host IDs to the amount of memory free on each host + * @param hostCpuMap + * a map of host IDs to the Ternary of used, reserved and total CPU on each host + * @param hostMemoryMap + * a map of host IDs to the Ternary of used, reserved and total memory on each host * * @return a pair containing the CPU and memory imbalance of the cluster after the migration */ - default Pair getImbalancePostMigration(ServiceOffering serviceOffering, VirtualMachine vm, - Host destHost, Map> hostCpuFreeMap, - Map> hostMemoryFreeMap) { - List postCpuList = new ArrayList<>(); - List postMemoryList = new ArrayList<>(); - final int vmCpu = serviceOffering.getCpu() * serviceOffering.getSpeed(); - final long vmRam = serviceOffering.getRamSize() * 1024L * 1024L; - - for (Long hostId : hostCpuFreeMap.keySet()) { - long cpu = hostCpuFreeMap.get(hostId).first(); - long memory = hostMemoryFreeMap.get(hostId).first(); + default Double getImbalancePostMigration(ServiceOffering serviceOffering, VirtualMachine vm, + Host destHost, Map> hostCpuMap, + Map> hostMemoryMap) throws ConfigurationException { + String metric = ClusterDrsMetric.valueIn(destHost.getClusterId()); + long vmMetric; + Map> hostMetricsMap; + switch (metric) { + case "cpu": + hostMetricsMap = hostCpuMap; + vmMetric = (long) serviceOffering.getCpu() * serviceOffering.getSpeed(); + break; + case "memory": + hostMetricsMap = hostMemoryMap; + vmMetric = serviceOffering.getRamSize() * 1024L * 1024L; + break; + default: + throw new ConfigurationException( + String.format("Invalid metric: %s for cluster: %d", metric, destHost.getClusterId())); + } + + boolean useRatio = ClusterDrsMetricUseRatio.valueIn(destHost.getClusterId()); + List list = new ArrayList<>(); + for (Long hostId : hostMetricsMap.keySet()) { + Ternary ternary = hostMetricsMap.get(hostId); + long used = ternary.first(); + long actualTotal = ternary.third() - ternary.second(); + long free = actualTotal - ternary.first(); + if (hostId == destHost.getId()) { - postCpuList.add((double)(cpu - vmCpu)/hostCpuFreeMap.get(hostId).second()); - postMemoryList.add((double)(memory - vmRam)/hostMemoryFreeMap.get(hostId).second()); + used += vmMetric; + free -= vmMetric; } else if (hostId.equals(vm.getHostId())) { - postCpuList.add((double)(cpu + vmCpu)/hostCpuFreeMap.get(hostId).second()); - postMemoryList.add((double)(memory + vmRam)/hostMemoryFreeMap.get(hostId).second()); - } else { - postCpuList.add((double)cpu/hostCpuFreeMap.get(hostId).second()); - postMemoryList.add((double)memory/hostMemoryFreeMap.get(hostId).second()); + used -= vmMetric; + free += vmMetric; + } + + switch (ClusterDrsMetricType.valueIn(destHost.getClusterId())) { + case "free": + if (useRatio) { + list.add((double) free / actualTotal); + } else { + list.add((double) free); + } + case "used": + if (useRatio) { + list.add((double) used / actualTotal); + } else { + list.add((double) used); + } } } - return new Pair<>(getClusterImbalance(postCpuList), getClusterImbalance(postMemoryList)); + return getImbalance(list); } /** @@ -129,7 +161,24 @@ default Pair getImbalancePostMigration(ServiceOffering serviceOf * Cluster Imbalance, Ic = σc / mavg , where σc is the standard deviation and * mavg is the mean metric value for the cluster. */ - default Double getClusterImbalance(List metricList) { + default Double getClusterImbalance(Long clusterId, List> cpuList, List> memoryList, Float skipThreshold) throws ConfigurationException { + String metric = ClusterDrsMetric.valueIn(clusterId); + List list; + switch (metric) { + case "cpu": + list = getMetricList(clusterId, cpuList, skipThreshold); + break; + case "memory": + list = getMetricList(clusterId, memoryList, skipThreshold); + break; + default: + throw new ConfigurationException( + String.format("Invalid metric: %s for cluster: %d", metric, clusterId)); + } + return getImbalance(list); + } + + private Double getImbalance(List metricList) { Double clusterMeanMetric = getClusterMeanMetric(metricList); Double clusterStandardDeviation = getClusterStandardDeviation(metricList, clusterMeanMetric); return clusterStandardDeviation / clusterMeanMetric; @@ -164,4 +213,31 @@ default Double getClusterStandardDeviation(List metricList, Double mean) return new StandardDeviation(false).evaluate(metricList.stream().mapToDouble(i -> i).toArray()); } } + + default List getMetricList(Long clusterId, List> hostMetricsList, Float skipThreshold) { + boolean useRatio = ClusterDrsMetricUseRatio.valueIn(clusterId); + List list = new ArrayList<>(); + for (Ternary ternary : hostMetricsList) { + long used = ternary.first(); + long actualTotal = ternary.third() - ternary.second(); + long free = actualTotal - ternary.first(); + switch (ClusterDrsMetricType.valueIn(clusterId)) { + case "free": + if (skipThreshold != null && free < skipThreshold * actualTotal) continue; + if (useRatio) { + list.add((double) free / actualTotal); + } else { + list.add((double) free); + } + case "used": + if (skipThreshold != null && used > skipThreshold * actualTotal) continue; + if (useRatio) { + list.add((double) used / actualTotal); + } else { + list.add((double) used); + } + } + } + return list; + } } diff --git a/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsService.java b/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsService.java index 91be8c535a46..1553ef8ddd0b 100644 --- a/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsService.java +++ b/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsService.java @@ -66,6 +66,19 @@ public interface ClusterDrsService extends Manager, Configurable, Scheduler { true, ConfigKey.Scope.Cluster, null, "DRS metric", null, null, null, ConfigKey.Kind.Select, "memory,cpu"); + ConfigKey ClusterDrsMetricType = new ConfigKey<>(String.class, "drs.metric.type", ConfigKey.CATEGORY_ADVANCED, + "used", + "The metric type used to measure imbalance in a cluster. This can completely change the imbalance value. Possible values are free, used.", + true, ConfigKey.Scope.Cluster, null, "DRS metric", null, null, null, ConfigKey.Kind.Select, + "free,used"); + + ConfigKey ClusterDrsMetricUseRatio = new ConfigKey<>(Boolean.class, "drs.metric.use.ratio", ConfigKey.CATEGORY_ADVANCED, + "true", + "Whether to use ratio of selected metric & total. Useful when the cluster has hosts with different capacities", + true, ConfigKey.Scope.Cluster, null, "DRS metric", null, null, null, ConfigKey.Kind.Select, + "true,false"); + + /** * Generate a DRS plan for a cluster and save it as per the parameters * diff --git a/plugins/drs/cluster/balanced/src/main/java/org/apache/cloudstack/cluster/Balanced.java b/plugins/drs/cluster/balanced/src/main/java/org/apache/cloudstack/cluster/Balanced.java index a3e5e22a9a12..fa8924d3bab5 100644 --- a/plugins/drs/cluster/balanced/src/main/java/org/apache/cloudstack/cluster/Balanced.java +++ b/plugins/drs/cluster/balanced/src/main/java/org/apache/cloudstack/cluster/Balanced.java @@ -21,18 +21,16 @@ import com.cloud.host.Host; import com.cloud.offering.ServiceOffering; -import com.cloud.utils.Pair; import com.cloud.utils.Ternary; import com.cloud.utils.component.AdapterBase; import com.cloud.vm.VirtualMachine; import javax.naming.ConfigurationException; +import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsImbalanceThreshold; -import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsMetric; public class Balanced extends AdapterBase implements ClusterDrsAlgorithm { @@ -42,61 +40,31 @@ public String getName() { } @Override - public boolean needsDrs(long clusterId, List> cpuList, List> memoryList) throws ConfigurationException { + public boolean needsDrs(long clusterId, List> cpuList, + List> memoryList) throws ConfigurationException { double threshold = getThreshold(clusterId); - String metric = ClusterDrsMetric.valueIn(clusterId); - switch (metric) { - case "cpu": - List cpuRatioList = cpuList.stream().map(pair -> (double) pair.first()/ pair.second()).collect(Collectors.toList()); - Double cpuImbalance = getClusterImbalance(cpuRatioList); - return cpuImbalance > threshold; - case "memory": - List memoryRatioList = memoryList.stream().map(pair -> (double) pair.first()/ pair.second()).collect(Collectors.toList()); - Double memoryImbalance = getClusterImbalance(memoryRatioList); - return memoryImbalance > threshold; - default: - throw new ConfigurationException( - String.format("Invalid metric: %s for cluster: %d", metric, clusterId)); - } + Double imbalance = getClusterImbalance(clusterId, cpuList, memoryList, null); + return imbalance > threshold; } - private double getThreshold(long clusterId) throws ConfigurationException { + private double getThreshold(long clusterId) { return 1.0 - ClusterDrsImbalanceThreshold.valueIn(clusterId); } @Override public Ternary getMetrics(long clusterId, VirtualMachine vm, - ServiceOffering serviceOffering, Host destHost, - Map> hostCpuFreeMap, Map> hostMemoryFreeMap, - Boolean requiresStorageMotion) { - List cpuList = hostCpuFreeMap.values().stream().map(pair -> (double) pair.first() / pair.second()).collect(Collectors.toList()); - Double preCpuImbalance = getClusterImbalance(cpuList); - List memoryList = hostMemoryFreeMap.values().stream().map(pair -> (double) pair.first() / pair.second()).collect(Collectors.toList()); - Double preMemoryImbalance = getClusterImbalance(memoryList); - Pair imbalancePair = getImbalancePostMigration(serviceOffering, vm, destHost, hostCpuFreeMap, - hostMemoryFreeMap); - Double postCpuImbalance = imbalancePair.first(); - Double postMemoryImbalance = imbalancePair.second(); + ServiceOffering serviceOffering, Host destHost, + Map> hostCpuMap, Map> hostMemoryMap, + Boolean requiresStorageMotion) throws ConfigurationException { + Double preImbalance = getClusterImbalance(clusterId, new ArrayList<>(hostCpuMap.values()), new ArrayList<>(hostMemoryMap.values()), null); + Double postImbalance = getImbalancePostMigration(serviceOffering, vm, destHost, hostCpuMap, hostMemoryMap); // This needs more research to determine the cost and benefit of a migration // TODO: Cost should be a factor of the VM size and the host capacity // TODO: Benefit should be a factor of the VM size and the host capacity and the number of VMs on the host - double cost = 0.0; - double benefit = 1.0; - - String metric = ClusterDrsMetric.valueIn(clusterId); - final double improvement; - switch (metric) { - case "cpu": - improvement = preCpuImbalance - postCpuImbalance; - break; - case "memory": - improvement = preMemoryImbalance - postMemoryImbalance; - break; - default: - improvement = preCpuImbalance + preMemoryImbalance - postCpuImbalance - postMemoryImbalance; - } - + final double improvement = preImbalance - postImbalance; + final double cost = 0.0; + final double benefit = 1.0; return new Ternary<>(improvement, cost, benefit); } } diff --git a/plugins/drs/cluster/balanced/src/test/java/org/apache/cloudstack/cluster/BalancedTest.java b/plugins/drs/cluster/balanced/src/test/java/org/apache/cloudstack/cluster/BalancedTest.java index 4b39536715b1..a1562b52e384 100644 --- a/plugins/drs/cluster/balanced/src/test/java/org/apache/cloudstack/cluster/BalancedTest.java +++ b/plugins/drs/cluster/balanced/src/test/java/org/apache/cloudstack/cluster/BalancedTest.java @@ -21,8 +21,6 @@ import com.cloud.host.Host; import com.cloud.service.ServiceOfferingVO; -import com.cloud.service.dao.ServiceOfferingDao; -import com.cloud.utils.Pair; import com.cloud.utils.Ternary; import com.cloud.vm.VirtualMachine; import org.apache.cloudstack.framework.config.ConfigKey; @@ -31,7 +29,6 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; -import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; import org.mockito.junit.MockitoJUnitRunner; @@ -68,12 +65,7 @@ public class BalancedTest { Map> hostVmMap; - Map> hostCpuFreeMap, hostMemoryFreeMap; - - - @Mock - private ServiceOfferingDao serviceOfferingDao; - + Map> hostCpuFreeMap, hostMemoryFreeMap; private AutoCloseable closeable; @@ -98,21 +90,21 @@ public void setUp() throws NoSuchFieldException, IllegalAccessException { Mockito.when(serviceOffering.getCpu()).thenReturn(1); Mockito.when(serviceOffering.getSpeed()).thenReturn(1000); - Mockito.when(serviceOffering.getRamSize()).thenReturn(512); + Mockito.when(serviceOffering.getRamSize()).thenReturn(1024); overrideDefaultConfigValue(ClusterDrsImbalanceThreshold, "_defaultValue", "0.5"); hostCpuFreeMap = new HashMap<>(); - hostCpuFreeMap.put(1L, new Pair<>(2000L, 10000L)); - hostCpuFreeMap.put(2L, new Pair<>(1000L, 10000L)); + hostCpuFreeMap.put(1L, new Ternary<>(1000L, 0L, 10000L)); + hostCpuFreeMap.put(2L, new Ternary<>(2000L, 0L, 10000L)); hostMemoryFreeMap = new HashMap<>(); - hostMemoryFreeMap.put(1L, new Pair<>(2048L * 1024L * 1024L, 8192L * 1024L * 1024L)); - hostMemoryFreeMap.put(2L, new Pair<>(512L * 1024L * 1024L, 8192L * 1024L * 1024L)); + hostMemoryFreeMap.put(1L, new Ternary<>(512L * 1024L * 1024L, 0L, 8192L * 1024L * 1024L)); + hostMemoryFreeMap.put(2L, new Ternary<>(2048L * 1024L * 1024L, 0L, 8192L * 1024L * 1024L)); } private void overrideDefaultConfigValue(final ConfigKey configKey, final String name, - final Object o) throws IllegalAccessException, NoSuchFieldException { + final Object o) throws IllegalAccessException, NoSuchFieldException { Field f = ConfigKey.class.getDeclaredField(name); f.setAccessible(true); f.set(configKey, o); @@ -156,7 +148,7 @@ public void needsDrsWithMemory() throws ConfigurationException, NoSuchFieldExcep /* 3. cluster with "unknown" metric */ @Test - public void needsDrsWithUnknown() throws ConfigurationException, NoSuchFieldException, IllegalAccessException { + public void needsDrsWithUnknown() throws NoSuchFieldException, IllegalAccessException { overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "unknown"); assertThrows(ConfigurationException.class, () -> balanced.needsDrs(clusterId, new ArrayList<>(hostCpuFreeMap.values()), new ArrayList<>(hostMemoryFreeMap.values()))); } @@ -185,7 +177,7 @@ public void needsDrsWithUnknown() throws ConfigurationException, NoSuchFieldExce improvement = 0.3333 - 0.3333 = 0.0 */ @Test - public void getMetricsWithCpu() throws NoSuchFieldException, IllegalAccessException { + public void getMetricsWithCpu() throws NoSuchFieldException, IllegalAccessException, ConfigurationException { overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "cpu"); Ternary result = balanced.getMetrics(clusterId, vm3, serviceOffering, destHost, hostCpuFreeMap, hostMemoryFreeMap, false); @@ -199,7 +191,7 @@ public void getMetricsWithCpu() throws NoSuchFieldException, IllegalAccessExcept improvement = 0.6 - 0.2 = 0.4 */ @Test - public void getMetricsWithMemory() throws NoSuchFieldException, IllegalAccessException { + public void getMetricsWithMemory() throws NoSuchFieldException, IllegalAccessException, ConfigurationException { overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "memory"); Ternary result = balanced.getMetrics(clusterId, vm3, serviceOffering, destHost, hostCpuFreeMap, hostMemoryFreeMap, false); @@ -207,18 +199,4 @@ public void getMetricsWithMemory() throws NoSuchFieldException, IllegalAccessExc assertEquals(0, result.second(), 0.0); assertEquals(1, result.third(), 0.0); } - - /* - 3. cluster with default metric - improvement = 0.3333 + 0.6 - 0.3333 - 0.2 = 0.4 - */ - @Test - public void getMetricsWithDefault() throws NoSuchFieldException, IllegalAccessException { - overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "both"); - Ternary result = balanced.getMetrics(clusterId, vm3, serviceOffering, destHost, - hostCpuFreeMap, hostMemoryFreeMap, false); - assertEquals(0.4, result.first(), 0.01); - assertEquals(0, result.second(), 0.0); - assertEquals(1, result.third(), 0.0); - } } diff --git a/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java b/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java index 522099d9df32..3810aca42b53 100644 --- a/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java +++ b/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java @@ -21,99 +21,61 @@ import com.cloud.host.Host; import com.cloud.offering.ServiceOffering; -import com.cloud.utils.Pair; import com.cloud.utils.Ternary; import com.cloud.utils.component.AdapterBase; import com.cloud.vm.VirtualMachine; +import org.apache.cloudstack.framework.config.ConfigKey; import javax.naming.ConfigurationException; import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsImbalanceThreshold; -import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsMetric; +import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsMetricType; public class Condensed extends AdapterBase implements ClusterDrsAlgorithm { + ConfigKey ClusterDrsImbalanceSkipThreshold = new ConfigKey<>(Float.class, + "drs.imbalance.condensed.skip.threshold", ConfigKey.CATEGORY_ADVANCED, "0.95", + "Threshold to ignore the metric for a host while calculating the imbalance to decide " + + "whether DRS is required for a cluster.This is to avoid cases when the calculated imbalance" + + " gets skewed due to a single host having a very high/low metric value resulting in imbalance" + + " being higher than 1. If " + ClusterDrsMetricType.key() + " is 'free', set a lower value and if it is 'used' " + + "set a higher value. The value should be between 0.0 and 1.0", + true, ConfigKey.Scope.Cluster, null, "DRS imbalance", null, null, null); + @Override public String getName() { return "condensed"; } @Override - public boolean needsDrs(long clusterId, List> cpuList, List> memoryList) throws ConfigurationException { + public boolean needsDrs(long clusterId, List> cpuList, + List> memoryList) throws ConfigurationException { double threshold = getThreshold(clusterId); - String metric = ClusterDrsMetric.valueIn(clusterId); - switch (metric) { - case "cpu": - List cpuRatioList = new ArrayList<>(); - for (Pair pair : cpuList) { - Double ratio = (double) pair.first() / pair.second(); - // To ensure that we calculate imbalance in cases where a few hosts are already condensed - // but other hosts aren't. This is to prevent the case where we don't migrate VMs because - // imbalance > 1 but the cluster is not condensed. - if (pair.first() < 0.05 * pair.second()) continue; - cpuRatioList.add(ratio); - } - Double cpuImbalance = getClusterImbalance(cpuRatioList); - return cpuImbalance < threshold; - case "memory": - List memoryRatioList = new ArrayList<>(); - for (Pair pair : memoryList) { - Double ratio = (double) pair.first() / pair.second(); - // To ensure that we calculate imbalance in cases where a few hosts are already condensed - // but other hosts aren't. This is to prevent the case where we don't migrate VMs because - // imbalance > 1 but the cluster is not condensed. - if (pair.first() < 0.05 * pair.second()) continue; - memoryRatioList.add(ratio); - } - Double memoryImbalance = getClusterImbalance(memoryRatioList); - return memoryImbalance < threshold; - default: - throw new ConfigurationException( - String.format("Invalid metric: %s for cluster: %d", metric, clusterId)); - } + Double imbalance = getClusterImbalance(clusterId, cpuList, memoryList, ClusterDrsImbalanceSkipThreshold.valueIn(clusterId)); + return imbalance < threshold; } - private double getThreshold(long clusterId) throws ConfigurationException { + private double getThreshold(long clusterId) { return ClusterDrsImbalanceThreshold.valueIn(clusterId); } @Override public Ternary getMetrics(long clusterId, VirtualMachine vm, - ServiceOffering serviceOffering, Host destHost, - Map> hostCpuUsedMap, Map> hostMemoryUsedMap, - Boolean requiresStorageMotion) { - List cpuList = hostCpuUsedMap.values().stream().map(pair -> (double) pair.first() / pair.second()).collect(Collectors.toList()); - Double preCpuImbalance = getClusterImbalance(cpuList); - List memoryList = hostMemoryUsedMap.values().stream().map(pair -> (double) pair.first() / pair.second()).collect(Collectors.toList()); - Double preMemoryImbalance = getClusterImbalance(memoryList); - - Pair imbalancePair = getImbalancePostMigration(serviceOffering, vm, destHost, hostCpuUsedMap, - hostMemoryUsedMap); - Double postCpuImbalance = imbalancePair.first(); - Double postMemoryImbalance = imbalancePair.second(); + ServiceOffering serviceOffering, Host destHost, + Map> hostCpuMap, Map> hostMemoryMap, + Boolean requiresStorageMotion) throws ConfigurationException { + Double preImbalance = getClusterImbalance(clusterId, new ArrayList<>(hostCpuMap.values()), new ArrayList<>(hostMemoryMap.values()), null); + Double postImbalance = getImbalancePostMigration(serviceOffering, vm, destHost, hostCpuMap, hostMemoryMap); // This needs more research to determine the cost and benefit of a migration // TODO: Cost should be a factor of the VM size and the host capacity // TODO: Benefit should be a factor of the VM size and the host capacity and the number of VMs on the host - double cost = 0; - double benefit = 1; - - String metric = ClusterDrsMetric.valueIn(clusterId); - double improvement; - switch (metric) { - case "cpu": - improvement = postCpuImbalance - preCpuImbalance; - break; - case "memory": - improvement = postMemoryImbalance - preMemoryImbalance; - break; - default: - improvement = postCpuImbalance + postMemoryImbalance - preCpuImbalance - preMemoryImbalance; - } + final double improvement = postImbalance - preImbalance; + final double cost = 0; + final double benefit = 1; return new Ternary<>(improvement, cost, benefit); } } diff --git a/plugins/drs/cluster/condensed/src/test/java/org/apache/cloudstack/cluster/CondensedTest.java b/plugins/drs/cluster/condensed/src/test/java/org/apache/cloudstack/cluster/CondensedTest.java index 28e88d43984b..d50727745347 100644 --- a/plugins/drs/cluster/condensed/src/test/java/org/apache/cloudstack/cluster/CondensedTest.java +++ b/plugins/drs/cluster/condensed/src/test/java/org/apache/cloudstack/cluster/CondensedTest.java @@ -21,7 +21,6 @@ import com.cloud.host.Host; import com.cloud.service.ServiceOfferingVO; -import com.cloud.utils.Pair; import com.cloud.utils.Ternary; import com.cloud.vm.VirtualMachine; import org.apache.cloudstack.framework.config.ConfigKey; @@ -66,7 +65,7 @@ public class CondensedTest { Map> hostVmMap; - Map> hostCpuFreeMap, hostMemoryFreeMap; + Map> hostCpuFreeMap, hostMemoryFreeMap; private AutoCloseable closeable; @@ -96,17 +95,17 @@ public void setUp() throws NoSuchFieldException, IllegalAccessException { overrideDefaultConfigValue(ClusterDrsImbalanceThreshold, "_defaultValue", "0.5"); hostCpuFreeMap = new HashMap<>(); - hostCpuFreeMap.put(1L, new Pair<>(2000L, 10000L)); - hostCpuFreeMap.put(2L, new Pair<>(1000L, 10000L)); + hostCpuFreeMap.put(1L, new Ternary<>(1000L, 0L, 10000L)); + hostCpuFreeMap.put(2L, new Ternary<>(2000L, 0L, 10000L)); hostMemoryFreeMap = new HashMap<>(); - hostMemoryFreeMap.put(1L, new Pair<>(2048L * 1024L * 1024L, 8192L * 1024L * 1024L)); - hostMemoryFreeMap.put(2L, new Pair<>(512L * 1024L * 1024L, 8192L * 1024L * 1024L)); + hostMemoryFreeMap.put(1L, new Ternary<>(512L * 1024L * 1024L, 0L, 8192L * 1024L * 1024L)); + hostMemoryFreeMap.put(2L, new Ternary<>(2048L * 1024L * 1024L, 0L, 8192L * 1024L * 1024L)); } private void overrideDefaultConfigValue(final ConfigKey configKey, - final String name, - final Object o) throws IllegalAccessException, NoSuchFieldException { + final String name, + final Object o) throws IllegalAccessException, NoSuchFieldException { Field f = ConfigKey.class.getDeclaredField(name); f.setAccessible(true); f.set(configKey, o); @@ -150,7 +149,7 @@ public void needsDrsWithMemory() throws ConfigurationException, NoSuchFieldExcep /* 3. cluster with "unknown" metric */ @Test - public void needsDrsWithUnknown() throws ConfigurationException, NoSuchFieldException, IllegalAccessException { + public void needsDrsWithUnknown() throws NoSuchFieldException, IllegalAccessException { overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "unknown"); assertThrows(ConfigurationException.class, () -> condensed.needsDrs(clusterId, new ArrayList<>(hostCpuFreeMap.values()), new ArrayList<>(hostMemoryFreeMap.values()))); } @@ -179,7 +178,7 @@ public void needsDrsWithUnknown() throws ConfigurationException, NoSuchFieldExce improvement = 0.3333 - 0.3333 = 0.0 */ @Test - public void getMetricsWithCpu() throws NoSuchFieldException, IllegalAccessException { + public void getMetricsWithCpu() throws NoSuchFieldException, IllegalAccessException, ConfigurationException { overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "cpu"); Ternary result = condensed.getMetrics(clusterId, vm3, serviceOffering, destHost, hostCpuFreeMap, hostMemoryFreeMap, false); @@ -193,7 +192,7 @@ public void getMetricsWithCpu() throws NoSuchFieldException, IllegalAccessExcept improvement = 0.2 - 0.6 = -0.4 */ @Test - public void getMetricsWithMemory() throws NoSuchFieldException, IllegalAccessException { + public void getMetricsWithMemory() throws NoSuchFieldException, IllegalAccessException, ConfigurationException { overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "memory"); Ternary result = condensed.getMetrics(clusterId, vm3, serviceOffering, destHost, hostCpuFreeMap, hostMemoryFreeMap, false); @@ -201,18 +200,4 @@ public void getMetricsWithMemory() throws NoSuchFieldException, IllegalAccessExc assertEquals(0, result.second(), 0.0); assertEquals(1, result.third(), 0.0); } - - /* - 3. cluster with default metric - improvement = 0.3333 + 0.2 - 0.3333 - 0.6 = -0.4 - */ - @Test - public void getMetricsWithDefault() throws NoSuchFieldException, IllegalAccessException { - overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "both"); - Ternary result = condensed.getMetrics(clusterId, vm3, serviceOffering, destHost, - hostCpuFreeMap, hostMemoryFreeMap, false); - assertEquals(-0.4, result.first(), 0.0001); - assertEquals(0, result.second(), 0.0); - assertEquals(1, result.third(), 0.0); - } } diff --git a/server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java b/server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java index 8d566dbd7e5a..a0573f69ac52 100644 --- a/server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java +++ b/server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java @@ -353,16 +353,10 @@ List> getDrsPlan(Cluster cluster, List hostJoinList = hostJoinDao.searchByIds( hostList.stream().map(HostVO::getId).toArray(Long[]::new)); - Map> hostCpuMap = hostJoinList.stream().collect(Collectors.toMap(HostJoinVO::getId, - hostJoin -> { - long totalCpu = hostJoin.getCpus() * hostJoin.getSpeed() - hostJoin.getCpuReservedCapacity(); - return new Pair<>(totalCpu - hostJoin.getCpuUsedCapacity(), totalCpu); - })); - Map> hostMemoryMap = hostJoinList.stream().collect(Collectors.toMap(HostJoinVO::getId, - hostJoin -> { - long totalMemory = hostJoin.getTotalMemory() - hostJoin.getMemUsedCapacity(); - return new Pair<>(totalMemory - hostJoin.getMemUsedCapacity(), totalMemory); - })); + Map> hostCpuMap = hostJoinList.stream().collect(Collectors.toMap(HostJoinVO::getId, + hostJoin -> new Ternary<>(hostJoin.getCpuUsedCapacity(), hostJoin.getCpuReservedCapacity(), hostJoin.getCpus() * hostJoin.getSpeed()))); + Map> hostMemoryMap = hostJoinList.stream().collect(Collectors.toMap(HostJoinVO::getId, + hostJoin -> new Ternary<>(hostJoin.getMemUsedCapacity(), hostJoin.getMemReservedCapacity(), hostJoin.getTotalMemory()))); Map vmIdServiceOfferingMap = new HashMap<>(); @@ -393,10 +387,11 @@ List> getDrsPlan(Cluster cluster, long vmCpu = (long) serviceOffering.getCpu() * serviceOffering.getSpeed(); long vmMemory = serviceOffering.getRamSize() * 1024L * 1024L; - hostCpuMap.get(vm.getHostId()).set(hostCpuMap.get(vm.getHostId()).first() + vmCpu, hostCpuMap.get(vm.getHostId()).second()); - hostCpuMap.get(destHost.getId()).set(hostCpuMap.get(destHost.getId()).first() - vmCpu, hostCpuMap.get(destHost.getId()).second()); - hostMemoryMap.get(vm.getHostId()).set(hostMemoryMap.get(vm.getHostId()).first() + vmMemory, hostMemoryMap.get(vm.getHostId()).second()); - hostMemoryMap.get(destHost.getId()).set(hostMemoryMap.get(destHost.getId()).first() - vmMemory, hostMemoryMap.get(destHost.getId()).second()); + // Updating the map as per the migration + hostCpuMap.get(vm.getHostId()).first(hostCpuMap.get(vm.getHostId()).first() - vmCpu); + hostCpuMap.get(destHost.getId()).first(hostCpuMap.get(destHost.getId()).first() + vmCpu); + hostMemoryMap.get(vm.getHostId()).first(hostMemoryMap.get(vm.getHostId()).first() - vmMemory); + hostMemoryMap.get(destHost.getId()).first(hostMemoryMap.get(destHost.getId()).first() + vmMemory); vm.setHostId(destHost.getId()); iteration++; } @@ -449,8 +444,8 @@ Map> getHostVmMap(List hostList, List getBestMigration(Cluster cluster, ClusterDrsAlgorithm algorithm, List vmList, Map vmIdServiceOfferingMap, - Map> hostCpuCapacityMap, - Map> hostMemoryCapacityMap) { + Map> hostCpuCapacityMap, + Map> hostMemoryCapacityMap) throws ConfigurationException { double improvement = 0; Pair bestMigration = new Pair<>(null, null); diff --git a/server/src/test/java/org/apache/cloudstack/cluster/ClusterDrsServiceImplTest.java b/server/src/test/java/org/apache/cloudstack/cluster/ClusterDrsServiceImplTest.java index 480bc67dd918..8aed790af0a8 100644 --- a/server/src/test/java/org/apache/cloudstack/cluster/ClusterDrsServiceImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/cluster/ClusterDrsServiceImplTest.java @@ -297,7 +297,7 @@ public void testGenerateDrsPlan() throws ConfigurationException { Mockito.when(clusterDrsService.getResponseObjectForMigrations(Mockito.anyList())).thenReturn( List.of(migrationResponse)); - try(MockedStatic ignored = Mockito.mockStatic(ActionEventUtils.class)) { + try (MockedStatic ignored = Mockito.mockStatic(ActionEventUtils.class)) { Mockito.when(ActionEventUtils.onActionEvent(Mockito.anyLong(), Mockito.anyLong(), Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), @@ -348,7 +348,7 @@ public void testUpdateOldPlanMigrations() { } @Test - public void testGetBestMigration() { + public void testGetBestMigration() throws ConfigurationException { ClusterVO cluster = Mockito.mock(ClusterVO.class); Mockito.when(cluster.getId()).thenReturn(1L); From 66574233870443f6bf205442958e4b46812e67e2 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Thu, 18 Jan 2024 18:10:53 +0530 Subject: [PATCH 04/12] code refactor --- .../cluster/ClusterDrsAlgorithm.java | 172 +++++++++--------- 1 file changed, 90 insertions(+), 82 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java b/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java index ae3916b97dbd..332eb7fe692b 100644 --- a/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java +++ b/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java @@ -21,6 +21,7 @@ import com.cloud.host.Host; import com.cloud.offering.ServiceOffering; +import com.cloud.utils.Pair; import com.cloud.utils.Ternary; import com.cloud.utils.component.Adapter; import com.cloud.vm.VirtualMachine; @@ -32,9 +33,9 @@ import java.util.List; import java.util.Map; +import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsMetric; import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsMetricType; import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsMetricUseRatio; -import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsMetric; public interface ClusterDrsAlgorithm extends Adapter { @@ -54,7 +55,8 @@ public interface ClusterDrsAlgorithm extends Adapter { * @throws ConfigurationException * if there is an error in the configuration */ - boolean needsDrs(long clusterId, List> cpuList, List> memoryList) throws ConfigurationException; + boolean needsDrs(long clusterId, List> cpuList, + List> memoryList) throws ConfigurationException; /** @@ -78,8 +80,9 @@ public interface ClusterDrsAlgorithm extends Adapter { * @return a ternary containing improvement, cost, benefit */ Ternary getMetrics(long clusterId, VirtualMachine vm, ServiceOffering serviceOffering, - Host destHost, Map> hostCpuMap, - Map> hostMemoryMap, Boolean requiresStorageMotion) throws ConfigurationException; + Host destHost, Map> hostCpuMap, + Map> hostMemoryMap, + Boolean requiresStorageMotion) throws ConfigurationException; /** * Calculates the imbalance of the cluster after a virtual machine migration. @@ -98,84 +101,52 @@ Ternary getMetrics(long clusterId, VirtualMachine vm, Se * @return a pair containing the CPU and memory imbalance of the cluster after the migration */ default Double getImbalancePostMigration(ServiceOffering serviceOffering, VirtualMachine vm, - Host destHost, Map> hostCpuMap, - Map> hostMemoryMap) throws ConfigurationException { - String metric = ClusterDrsMetric.valueIn(destHost.getClusterId()); - long vmMetric; - Map> hostMetricsMap; - switch (metric) { - case "cpu": - hostMetricsMap = hostCpuMap; - vmMetric = (long) serviceOffering.getCpu() * serviceOffering.getSpeed(); - break; - case "memory": - hostMetricsMap = hostMemoryMap; - vmMetric = serviceOffering.getRamSize() * 1024L * 1024L; - break; - default: - throw new ConfigurationException( - String.format("Invalid metric: %s for cluster: %d", metric, destHost.getClusterId())); - } + Host destHost, Map> hostCpuMap, + Map> hostMemoryMap) throws ConfigurationException { + Pair>> pair = getHostMetricsMapAndType(destHost.getClusterId(), serviceOffering, hostCpuMap, hostMemoryMap); + long vmMetric = pair.first(); + Map> hostMetricsMap = pair.second(); - boolean useRatio = ClusterDrsMetricUseRatio.valueIn(destHost.getClusterId()); List list = new ArrayList<>(); for (Long hostId : hostMetricsMap.keySet()) { - Ternary ternary = hostMetricsMap.get(hostId); - long used = ternary.first(); - long actualTotal = ternary.third() - ternary.second(); - long free = actualTotal - ternary.first(); - - if (hostId == destHost.getId()) { - used += vmMetric; - free -= vmMetric; - } else if (hostId.equals(vm.getHostId())) { - used -= vmMetric; - free += vmMetric; - } - - switch (ClusterDrsMetricType.valueIn(destHost.getClusterId())) { - case "free": - if (useRatio) { - list.add((double) free / actualTotal); - } else { - list.add((double) free); - } - case "used": - if (useRatio) { - list.add((double) used / actualTotal); - } else { - list.add((double) used); - } - } + list.add(getMetricValuePostMigration(destHost.getClusterId(), hostMetricsMap.get(hostId), vmMetric, hostId, destHost.getId(), vm.getHostId())); } return getImbalance(list); } - /** - * The cluster imbalance is defined as the percentage deviation from the mean - * for a configured metric of the cluster. The standard deviation is used as a - * mathematical tool to normalize the metric data for all the resource and the - * percentage deviation provides an easy tool to compare a cluster’s current - * state against the defined imbalance threshold. Because this is essentially a - * percentage, the value is a number between 0.0 and 1.0. - * Cluster Imbalance, Ic = σc / mavg , where σc is the standard deviation and - * mavg is the mean metric value for the cluster. - */ - default Double getClusterImbalance(Long clusterId, List> cpuList, List> memoryList, Float skipThreshold) throws ConfigurationException { + private Pair>> getHostMetricsMapAndType(Long clusterId, + ServiceOffering serviceOffering, Map> hostCpuMap, + Map> hostMemoryMap) throws ConfigurationException { String metric = ClusterDrsMetric.valueIn(clusterId); - List list; + Pair>> pair; switch (metric) { case "cpu": - list = getMetricList(clusterId, cpuList, skipThreshold); + pair = new Pair<>((long) serviceOffering.getCpu() * serviceOffering.getSpeed(), hostCpuMap); break; case "memory": - list = getMetricList(clusterId, memoryList, skipThreshold); + pair = new Pair<>(serviceOffering.getRamSize() * 1024L * 1024L, hostMemoryMap); break; default: throw new ConfigurationException( String.format("Invalid metric: %s for cluster: %d", metric, clusterId)); } - return getImbalance(list); + return pair; + } + + private Double getMetricValuePostMigration(Long clusterId, Ternary metrics, long vmMetric, + long hostId, long destHostId, long vmHostId) { + long used = metrics.first(); + long actualTotal = metrics.third() - metrics.second(); + long free = actualTotal - metrics.first(); + + if (hostId == destHostId) { + used += vmMetric; + free -= vmMetric; + } else if (hostId == vmHostId) { + used -= vmMetric; + free += vmMetric; + } + return getMetricValue(clusterId, used, free, actualTotal, null); } private Double getImbalance(List metricList) { @@ -184,6 +155,27 @@ private Double getImbalance(List metricList) { return clusterStandardDeviation / clusterMeanMetric; } + private Double getMetricValue(Long clusterId, double used, double free, double total, Float skipThreshold) { + boolean useRatio = ClusterDrsMetricUseRatio.valueIn(clusterId); + switch (ClusterDrsMetricType.valueIn(clusterId)) { + case "free": + if (skipThreshold != null && free < skipThreshold * total) return null; + if (useRatio) { + return free / total; + } else { + return free; + } + case "used": + if (skipThreshold != null && used > skipThreshold * total) return null; + if (useRatio) { + return used / total; + } else { + return used; + } + } + return null; + } + /** * Mean is the average of a collection or set of metrics. In context of a DRS * cluster, the cluster metrics defined as the average metrics value for some @@ -214,28 +206,44 @@ default Double getClusterStandardDeviation(List metricList, Double mean) } } - default List getMetricList(Long clusterId, List> hostMetricsList, Float skipThreshold) { - boolean useRatio = ClusterDrsMetricUseRatio.valueIn(clusterId); + /** + * The cluster imbalance is defined as the percentage deviation from the mean + * for a configured metric of the cluster. The standard deviation is used as a + * mathematical tool to normalize the metric data for all the resource and the + * percentage deviation provides an easy tool to compare a cluster’s current + * state against the defined imbalance threshold. Because this is essentially a + * percentage, the value is a number between 0.0 and 1.0. + * Cluster Imbalance, Ic = σc / mavg , where σc is the standard deviation and + * mavg is the mean metric value for the cluster. + */ + default Double getClusterImbalance(Long clusterId, List> cpuList, + List> memoryList, Float skipThreshold) throws ConfigurationException { + String metric = ClusterDrsMetric.valueIn(clusterId); + List list; + switch (metric) { + case "cpu": + list = getMetricList(clusterId, cpuList, skipThreshold); + break; + case "memory": + list = getMetricList(clusterId, memoryList, skipThreshold); + break; + default: + throw new ConfigurationException( + String.format("Invalid metric: %s for cluster: %d", metric, clusterId)); + } + return getImbalance(list); + } + + default List getMetricList(Long clusterId, List> hostMetricsList, + Float skipThreshold) { List list = new ArrayList<>(); for (Ternary ternary : hostMetricsList) { long used = ternary.first(); long actualTotal = ternary.third() - ternary.second(); long free = actualTotal - ternary.first(); - switch (ClusterDrsMetricType.valueIn(clusterId)) { - case "free": - if (skipThreshold != null && free < skipThreshold * actualTotal) continue; - if (useRatio) { - list.add((double) free / actualTotal); - } else { - list.add((double) free); - } - case "used": - if (skipThreshold != null && used > skipThreshold * actualTotal) continue; - if (useRatio) { - list.add((double) used / actualTotal); - } else { - list.add((double) used); - } + Double metricValue = getMetricValue(clusterId, used, free, actualTotal, skipThreshold); + if (metricValue != null) { + list.add(metricValue); } } return list; From dd281427dbad3c84f7c5c9837dd31a5ee4fa0c5d Mon Sep 17 00:00:00 2001 From: Vishesh Date: Fri, 19 Jan 2024 12:10:35 +0530 Subject: [PATCH 05/12] Add unit tests --- .../cluster/ClusterDrsAlgorithm.java | 30 +++++-- .../cluster/ClusterDrsAlgorithmTest.java | 89 +++++++++++++++++++ 2 files changed, 110 insertions(+), 9 deletions(-) create mode 100644 api/src/test/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithmTest.java diff --git a/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java b/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java index 332eb7fe692b..63303c7089d2 100644 --- a/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java +++ b/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java @@ -117,7 +117,7 @@ default Double getImbalancePostMigration(ServiceOffering serviceOffering, Virtua private Pair>> getHostMetricsMapAndType(Long clusterId, ServiceOffering serviceOffering, Map> hostCpuMap, Map> hostMemoryMap) throws ConfigurationException { - String metric = ClusterDrsMetric.valueIn(clusterId); + String metric = getClusterDrsMetric(clusterId); Pair>> pair; switch (metric) { case "cpu": @@ -155,22 +155,26 @@ private Double getImbalance(List metricList) { return clusterStandardDeviation / clusterMeanMetric; } - private Double getMetricValue(Long clusterId, double used, double free, double total, Float skipThreshold) { - boolean useRatio = ClusterDrsMetricUseRatio.valueIn(clusterId); - switch (ClusterDrsMetricType.valueIn(clusterId)) { + default String getClusterDrsMetric(long clusterId) { + return ClusterDrsMetric.valueIn(clusterId); + } + + default Double getMetricValue(long clusterId, long used, long free, long total, Float skipThreshold) { + boolean useRatio = getDrsMetricUseRatio(clusterId); + switch (getDrsMetricType(clusterId)) { case "free": if (skipThreshold != null && free < skipThreshold * total) return null; if (useRatio) { - return free / total; + return (double) free / total; } else { - return free; + return (double) free; } case "used": if (skipThreshold != null && used > skipThreshold * total) return null; if (useRatio) { - return used / total; + return (double) used / total; } else { - return used; + return (double) used; } } return null; @@ -206,6 +210,14 @@ default Double getClusterStandardDeviation(List metricList, Double mean) } } + default boolean getDrsMetricUseRatio(long clusterId) { + return ClusterDrsMetricUseRatio.valueIn(clusterId); + } + + default String getDrsMetricType(long clusterId) { + return ClusterDrsMetricType.valueIn(clusterId); + } + /** * The cluster imbalance is defined as the percentage deviation from the mean * for a configured metric of the cluster. The standard deviation is used as a @@ -218,7 +230,7 @@ default Double getClusterStandardDeviation(List metricList, Double mean) */ default Double getClusterImbalance(Long clusterId, List> cpuList, List> memoryList, Float skipThreshold) throws ConfigurationException { - String metric = ClusterDrsMetric.valueIn(clusterId); + String metric = getClusterDrsMetric(clusterId); List list; switch (metric) { case "cpu": diff --git a/api/src/test/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithmTest.java b/api/src/test/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithmTest.java new file mode 100644 index 000000000000..13fa7dcc88e5 --- /dev/null +++ b/api/src/test/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithmTest.java @@ -0,0 +1,89 @@ +/* + * 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 org.apache.cloudstack.cluster; + +import com.cloud.utils.Ternary; +import junit.framework.TestCase; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnitRunner; + +import java.util.List; + +import static org.mockito.Mockito.doReturn; + +@RunWith(MockitoJUnitRunner.class) +public class ClusterDrsAlgorithmTest extends TestCase { + + @Spy + ClusterDrsAlgorithm drsAlgorithm; + + @Test + public void testGetMetricValue() { + List> testData = List.of( + new Ternary<>(true, "free", 0.4), + new Ternary<>(false, "free", 40.0), + new Ternary<>(true, "used", 0.3), + new Ternary<>(false, "used", 30.0) + ); + + long used = 30; + long free = 40; + long total = 100; + + for (Ternary data : testData) { + boolean useRatio = data.first(); + String metricType = data.second(); + double expectedValue = data.third(); + + doReturn(useRatio).when(drsAlgorithm).getDrsMetricUseRatio(1L); + doReturn(metricType).when(drsAlgorithm).getDrsMetricType(1L); + + assertEquals(expectedValue, drsAlgorithm.getMetricValue(1, used, free, total, null)); + } + } + + @Test + public void testGetMetricValueWithSkipThreshold() { + List> testData = List.of( + new Ternary<>(true, "free", 0.15), + new Ternary<>(false, "free", 15.0), + new Ternary<>(true, "used", null), + new Ternary<>(false, "used", null) + ); + + long used = 80; + long free = 15; + long total = 100; + + for (Ternary data : testData) { + boolean useRatio = data.first(); + String metricType = data.second(); + Double expectedValue = data.third(); + float skipThreshold = metricType.equals("free") ? 0.1f : 0.7f; + + doReturn(useRatio).when(drsAlgorithm).getDrsMetricUseRatio(1L); + doReturn(metricType).when(drsAlgorithm).getDrsMetricType(1L); + + assertEquals(expectedValue, drsAlgorithm.getMetricValue(1L, used, free, total, skipThreshold)); + } + } +} From c10865901b7e89d822223fec06387467dd9f3a4f Mon Sep 17 00:00:00 2001 From: Vishesh Date: Mon, 22 Jan 2024 12:29:28 +0530 Subject: [PATCH 06/12] fixup --- .../cloudstack/cluster/ClusterDrsService.java | 4 ++-- .../org/apache/cloudstack/cluster/Condensed.java | 16 ++++++++++++++-- .../cluster/ClusterDrsServiceImpl.java | 4 ++-- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsService.java b/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsService.java index 1553ef8ddd0b..5650145f56b6 100644 --- a/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsService.java +++ b/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsService.java @@ -69,13 +69,13 @@ public interface ClusterDrsService extends Manager, Configurable, Scheduler { ConfigKey ClusterDrsMetricType = new ConfigKey<>(String.class, "drs.metric.type", ConfigKey.CATEGORY_ADVANCED, "used", "The metric type used to measure imbalance in a cluster. This can completely change the imbalance value. Possible values are free, used.", - true, ConfigKey.Scope.Cluster, null, "DRS metric", null, null, null, ConfigKey.Kind.Select, + true, ConfigKey.Scope.Cluster, null, "DRS metric type", null, null, null, ConfigKey.Kind.Select, "free,used"); ConfigKey ClusterDrsMetricUseRatio = new ConfigKey<>(Boolean.class, "drs.metric.use.ratio", ConfigKey.CATEGORY_ADVANCED, "true", "Whether to use ratio of selected metric & total. Useful when the cluster has hosts with different capacities", - true, ConfigKey.Scope.Cluster, null, "DRS metric", null, null, null, ConfigKey.Kind.Select, + true, ConfigKey.Scope.Cluster, null, "DRS metric use ratio", null, null, null, ConfigKey.Kind.Select, "true,false"); diff --git a/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java b/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java index 3810aca42b53..6689d090b725 100644 --- a/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java +++ b/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java @@ -25,6 +25,7 @@ import com.cloud.utils.component.AdapterBase; import com.cloud.vm.VirtualMachine; import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.Configurable; import javax.naming.ConfigurationException; import java.util.ArrayList; @@ -34,7 +35,7 @@ import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsImbalanceThreshold; import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsMetricType; -public class Condensed extends AdapterBase implements ClusterDrsAlgorithm { +public class Condensed extends AdapterBase implements ClusterDrsAlgorithm, Configurable { ConfigKey ClusterDrsImbalanceSkipThreshold = new ConfigKey<>(Float.class, "drs.imbalance.condensed.skip.threshold", ConfigKey.CATEGORY_ADVANCED, "0.95", @@ -43,13 +44,24 @@ public class Condensed extends AdapterBase implements ClusterDrsAlgorithm { " gets skewed due to a single host having a very high/low metric value resulting in imbalance" + " being higher than 1. If " + ClusterDrsMetricType.key() + " is 'free', set a lower value and if it is 'used' " + "set a higher value. The value should be between 0.0 and 1.0", - true, ConfigKey.Scope.Cluster, null, "DRS imbalance", null, null, null); + true, ConfigKey.Scope.Cluster, null, "DRS imbalance skip threshold for Condensed algorithm", + null, null, null); @Override public String getName() { return "condensed"; } + @Override + public String getConfigComponentName() { + return Condensed.class.getSimpleName(); + } + + @Override + public ConfigKey[] getConfigKeys() { + return new ConfigKey[]{ClusterDrsImbalanceSkipThreshold}; + } + @Override public boolean needsDrs(long clusterId, List> cpuList, List> memoryList) throws ConfigurationException { diff --git a/server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java b/server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java index a0573f69ac52..316129db0516 100644 --- a/server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java +++ b/server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java @@ -628,8 +628,8 @@ public String getConfigComponentName() { @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[]{ClusterDrsPlanExpireInterval, ClusterDrsEnabled, ClusterDrsInterval, - ClusterDrsMaxMigrations, ClusterDrsAlgorithm, ClusterDrsImbalanceThreshold, ClusterDrsMetric}; + return new ConfigKey[]{ClusterDrsPlanExpireInterval, ClusterDrsEnabled, ClusterDrsInterval, ClusterDrsMaxMigrations, + ClusterDrsAlgorithm, ClusterDrsImbalanceThreshold, ClusterDrsMetric, ClusterDrsMetricType, ClusterDrsMetricUseRatio}; } @Override From 7877b43a38d8ede9a1d697cedba34951a4e30740 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 24 Jan 2024 11:04:48 +0530 Subject: [PATCH 07/12] Fix validation for drs.imbalance.condensed.skip.threshold --- .../cloudstack/cluster/ClusterDrsService.java | 10 +++++++ .../apache/cloudstack/cluster/Condensed.java | 27 +++---------------- .../ConfigurationManagerImpl.java | 1 + .../cluster/ClusterDrsServiceImpl.java | 3 ++- 4 files changed, 16 insertions(+), 25 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsService.java b/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsService.java index 5650145f56b6..ba6a6464fc20 100644 --- a/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsService.java +++ b/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsService.java @@ -78,6 +78,16 @@ public interface ClusterDrsService extends Manager, Configurable, Scheduler { true, ConfigKey.Scope.Cluster, null, "DRS metric use ratio", null, null, null, ConfigKey.Kind.Select, "true,false"); + ConfigKey ClusterDrsImbalanceSkipThreshold = new ConfigKey<>(Float.class, + "drs.imbalance.condensed.skip.threshold", ConfigKey.CATEGORY_ADVANCED, "0.95", + "Threshold to ignore the metric for a host while calculating the imbalance to decide " + + "whether DRS is required for a cluster.This is to avoid cases when the calculated imbalance" + + " gets skewed due to a single host having a very high/low metric value resulting in imbalance" + + " being higher than 1. If " + ClusterDrsMetricType.key() + " is 'free', set a lower value and if it is 'used' " + + "set a higher value. The value should be between 0.0 and 1.0", + true, ConfigKey.Scope.Cluster, null, "DRS imbalance skip threshold for Condensed algorithm", + null, null, null); + /** * Generate a DRS plan for a cluster and save it as per the parameters diff --git a/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java b/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java index 6689d090b725..4aaa1983d36d 100644 --- a/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java +++ b/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java @@ -24,44 +24,23 @@ import com.cloud.utils.Ternary; import com.cloud.utils.component.AdapterBase; import com.cloud.vm.VirtualMachine; -import org.apache.cloudstack.framework.config.ConfigKey; -import org.apache.cloudstack.framework.config.Configurable; + import javax.naming.ConfigurationException; import java.util.ArrayList; import java.util.List; import java.util.Map; +import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsImbalanceSkipThreshold; import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsImbalanceThreshold; -import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsMetricType; - -public class Condensed extends AdapterBase implements ClusterDrsAlgorithm, Configurable { - ConfigKey ClusterDrsImbalanceSkipThreshold = new ConfigKey<>(Float.class, - "drs.imbalance.condensed.skip.threshold", ConfigKey.CATEGORY_ADVANCED, "0.95", - "Threshold to ignore the metric for a host while calculating the imbalance to decide " + - "whether DRS is required for a cluster.This is to avoid cases when the calculated imbalance" + - " gets skewed due to a single host having a very high/low metric value resulting in imbalance" + - " being higher than 1. If " + ClusterDrsMetricType.key() + " is 'free', set a lower value and if it is 'used' " + - "set a higher value. The value should be between 0.0 and 1.0", - true, ConfigKey.Scope.Cluster, null, "DRS imbalance skip threshold for Condensed algorithm", - null, null, null); +public class Condensed extends AdapterBase implements ClusterDrsAlgorithm { @Override public String getName() { return "condensed"; } - @Override - public String getConfigComponentName() { - return Condensed.class.getSimpleName(); - } - - @Override - public ConfigKey[] getConfigKeys() { - return new ConfigKey[]{ClusterDrsImbalanceSkipThreshold}; - } - @Override public boolean needsDrs(long clusterId, List> cpuList, List> memoryList) throws ConfigurationException { diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 94d992f86369..d84673efd6b1 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -577,6 +577,7 @@ private void weightBasedParametersForValidation() { weightBasedParametersForValidation.add(Config.VmUserDispersionWeight.key()); weightBasedParametersForValidation.add(CapacityManager.SecondaryStorageCapacityThreshold.key()); weightBasedParametersForValidation.add(ClusterDrsService.ClusterDrsImbalanceThreshold.key()); + weightBasedParametersForValidation.add(ClusterDrsService.ClusterDrsImbalanceSkipThreshold.key()); } diff --git a/server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java b/server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java index 316129db0516..ad72c60b87c4 100644 --- a/server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java +++ b/server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java @@ -629,7 +629,8 @@ public String getConfigComponentName() { @Override public ConfigKey[] getConfigKeys() { return new ConfigKey[]{ClusterDrsPlanExpireInterval, ClusterDrsEnabled, ClusterDrsInterval, ClusterDrsMaxMigrations, - ClusterDrsAlgorithm, ClusterDrsImbalanceThreshold, ClusterDrsMetric, ClusterDrsMetricType, ClusterDrsMetricUseRatio}; + ClusterDrsAlgorithm, ClusterDrsImbalanceThreshold, ClusterDrsMetric, ClusterDrsMetricType, ClusterDrsMetricUseRatio, + ClusterDrsImbalanceSkipThreshold}; } @Override From c9eb8cea6f69905cac585b24c223a2bac06c28d6 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 7 Feb 2024 01:48:51 +0530 Subject: [PATCH 08/12] Add logging and other minor changes for drs --- .../cluster/ClusterDrsAlgorithm.java | 18 +++++----- .../cluster/ClusterDrsAlgorithmTest.java | 30 ++++++++++------ .../apache/cloudstack/cluster/Balanced.java | 31 ++++++++++++---- .../apache/cloudstack/cluster/Condensed.java | 35 ++++++++++++++----- .../metrics/MetricsServiceImpl.java | 23 ++++++++++-- .../response/ClusterMetricsResponse.java | 8 +++++ ui/public/locales/en.json | 1 + ui/src/components/view/ListView.vue | 3 ++ ui/src/config/section/infra/clusters.js | 4 +-- ui/src/views/infra/ClusterDRSTab.vue | 18 +++++++--- 10 files changed, 126 insertions(+), 45 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java b/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java index 63303c7089d2..15f7fcd81741 100644 --- a/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java +++ b/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java @@ -149,17 +149,17 @@ private Double getMetricValuePostMigration(Long clusterId, Ternary metricList) { + private static Double getImbalance(List metricList) { Double clusterMeanMetric = getClusterMeanMetric(metricList); Double clusterStandardDeviation = getClusterStandardDeviation(metricList, clusterMeanMetric); return clusterStandardDeviation / clusterMeanMetric; } - default String getClusterDrsMetric(long clusterId) { + static String getClusterDrsMetric(long clusterId) { return ClusterDrsMetric.valueIn(clusterId); } - default Double getMetricValue(long clusterId, long used, long free, long total, Float skipThreshold) { + static Double getMetricValue(long clusterId, long used, long free, long total, Float skipThreshold) { boolean useRatio = getDrsMetricUseRatio(clusterId); switch (getDrsMetricType(clusterId)) { case "free": @@ -187,7 +187,7 @@ default Double getMetricValue(long clusterId, long used, long free, long total, * Cluster Mean Metric, mavg = (∑mi) / N, where mi is a measurable metric for a * resource ‘i’ in a cluster with total N number of resources. */ - default Double getClusterMeanMetric(List metricList) { + static Double getClusterMeanMetric(List metricList) { return new Mean().evaluate(metricList.stream().mapToDouble(i -> i).toArray()); } @@ -202,7 +202,7 @@ default Double getClusterMeanMetric(List metricList) { * mean metric value and mi is a measurable metric for some resource ‘i’ in the * cluster with total N number of resources. */ - default Double getClusterStandardDeviation(List metricList, Double mean) { + static Double getClusterStandardDeviation(List metricList, Double mean) { if (mean != null) { return new StandardDeviation(false).evaluate(metricList.stream().mapToDouble(i -> i).toArray(), mean); } else { @@ -210,11 +210,11 @@ default Double getClusterStandardDeviation(List metricList, Double mean) } } - default boolean getDrsMetricUseRatio(long clusterId) { + static boolean getDrsMetricUseRatio(long clusterId) { return ClusterDrsMetricUseRatio.valueIn(clusterId); } - default String getDrsMetricType(long clusterId) { + static String getDrsMetricType(long clusterId) { return ClusterDrsMetricType.valueIn(clusterId); } @@ -228,7 +228,7 @@ default String getDrsMetricType(long clusterId) { * Cluster Imbalance, Ic = σc / mavg , where σc is the standard deviation and * mavg is the mean metric value for the cluster. */ - default Double getClusterImbalance(Long clusterId, List> cpuList, + static Double getClusterImbalance(Long clusterId, List> cpuList, List> memoryList, Float skipThreshold) throws ConfigurationException { String metric = getClusterDrsMetric(clusterId); List list; @@ -246,7 +246,7 @@ default Double getClusterImbalance(Long clusterId, List getMetricList(Long clusterId, List> hostMetricsList, + static List getMetricList(Long clusterId, List> hostMetricsList, Float skipThreshold) { List list = new ArrayList<>(); for (Ternary ternary : hostMetricsList) { diff --git a/api/src/test/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithmTest.java b/api/src/test/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithmTest.java index 13fa7dcc88e5..883798109443 100644 --- a/api/src/test/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithmTest.java +++ b/api/src/test/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithmTest.java @@ -23,19 +23,21 @@ import junit.framework.TestCase; import org.junit.Test; import org.junit.runner.RunWith; -import org.mockito.Spy; +import org.mockito.MockedStatic; +import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; import java.util.List; -import static org.mockito.Mockito.doReturn; +import static org.apache.cloudstack.cluster.ClusterDrsAlgorithm.getMetricValue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyFloat; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) public class ClusterDrsAlgorithmTest extends TestCase { - @Spy - ClusterDrsAlgorithm drsAlgorithm; - @Test public void testGetMetricValue() { List> testData = List.of( @@ -54,10 +56,13 @@ public void testGetMetricValue() { String metricType = data.second(); double expectedValue = data.third(); - doReturn(useRatio).when(drsAlgorithm).getDrsMetricUseRatio(1L); - doReturn(metricType).when(drsAlgorithm).getDrsMetricType(1L); + try (MockedStatic ignored = Mockito.mockStatic(ClusterDrsAlgorithm.class)) { + when(ClusterDrsAlgorithm.getDrsMetricUseRatio(1L)).thenReturn(useRatio); + when(ClusterDrsAlgorithm.getDrsMetricType(1L)).thenReturn(metricType); + when(ClusterDrsAlgorithm.getMetricValue(anyLong(), anyLong(), anyLong(), anyLong(), any())).thenCallRealMethod(); - assertEquals(expectedValue, drsAlgorithm.getMetricValue(1, used, free, total, null)); + assertEquals(expectedValue, getMetricValue(1, used, free, total, null)); + } } } @@ -80,10 +85,13 @@ public void testGetMetricValueWithSkipThreshold() { Double expectedValue = data.third(); float skipThreshold = metricType.equals("free") ? 0.1f : 0.7f; - doReturn(useRatio).when(drsAlgorithm).getDrsMetricUseRatio(1L); - doReturn(metricType).when(drsAlgorithm).getDrsMetricType(1L); + try (MockedStatic ignored = Mockito.mockStatic(ClusterDrsAlgorithm.class)) { + when(ClusterDrsAlgorithm.getDrsMetricUseRatio(1L)).thenReturn(useRatio); + when(ClusterDrsAlgorithm.getDrsMetricType(1L)).thenReturn(metricType); + when(ClusterDrsAlgorithm.getMetricValue(anyLong(), anyLong(), anyLong(), anyLong(), anyFloat())).thenCallRealMethod(); - assertEquals(expectedValue, drsAlgorithm.getMetricValue(1L, used, free, total, skipThreshold)); + assertEquals(expectedValue, ClusterDrsAlgorithm.getMetricValue(1L, used, free, total, skipThreshold)); + } } } } diff --git a/plugins/drs/cluster/balanced/src/main/java/org/apache/cloudstack/cluster/Balanced.java b/plugins/drs/cluster/balanced/src/main/java/org/apache/cloudstack/cluster/Balanced.java index fa8924d3bab5..289394a9685a 100644 --- a/plugins/drs/cluster/balanced/src/main/java/org/apache/cloudstack/cluster/Balanced.java +++ b/plugins/drs/cluster/balanced/src/main/java/org/apache/cloudstack/cluster/Balanced.java @@ -24,6 +24,7 @@ import com.cloud.utils.Ternary; import com.cloud.utils.component.AdapterBase; import com.cloud.vm.VirtualMachine; +import org.apache.log4j.Logger; import javax.naming.ConfigurationException; import java.util.ArrayList; @@ -34,31 +35,47 @@ public class Balanced extends AdapterBase implements ClusterDrsAlgorithm { - @Override - public String getName() { - return "balanced"; - } + private static final Logger logger = Logger.getLogger(Balanced.class); @Override public boolean needsDrs(long clusterId, List> cpuList, List> memoryList) throws ConfigurationException { double threshold = getThreshold(clusterId); - Double imbalance = getClusterImbalance(clusterId, cpuList, memoryList, null); - return imbalance > threshold; + Double imbalance = ClusterDrsAlgorithm.getClusterImbalance(clusterId, cpuList, memoryList, null); + String drsMetric = ClusterDrsAlgorithm.getClusterDrsMetric(clusterId); + String metricType = ClusterDrsAlgorithm.getDrsMetricType(clusterId); + Boolean useRatio = ClusterDrsAlgorithm.getDrsMetricUseRatio(clusterId); + if (imbalance > threshold) { + logger.debug(String.format("Cluster %d needs DRS. Imbalance: %s Threshold: %s Algorithm: %s DRS metric: %s Metric Type: %s Use ratio: %s", + clusterId, imbalance, threshold, getName(), drsMetric, metricType, useRatio)); + return true; + } else { + logger.debug(String.format("Cluster %d does not need DRS. Imbalance: %s Threshold: %s Algorithm: %s DRS metric: %s Metric Type: %s Use ratio: %s", + clusterId, imbalance, threshold, getName(), drsMetric, metricType, useRatio)); + return false; + } } private double getThreshold(long clusterId) { return 1.0 - ClusterDrsImbalanceThreshold.valueIn(clusterId); } + @Override + public String getName() { + return "balanced"; + } + @Override public Ternary getMetrics(long clusterId, VirtualMachine vm, ServiceOffering serviceOffering, Host destHost, Map> hostCpuMap, Map> hostMemoryMap, Boolean requiresStorageMotion) throws ConfigurationException { - Double preImbalance = getClusterImbalance(clusterId, new ArrayList<>(hostCpuMap.values()), new ArrayList<>(hostMemoryMap.values()), null); + Double preImbalance = ClusterDrsAlgorithm.getClusterImbalance(clusterId, new ArrayList<>(hostCpuMap.values()), new ArrayList<>(hostMemoryMap.values()), null); Double postImbalance = getImbalancePostMigration(serviceOffering, vm, destHost, hostCpuMap, hostMemoryMap); + logger.debug(String.format("Cluster %d pre-imbalance: %s post-imbalance: %s Algorithm: %s VM: %s dest Host: %s", + clusterId, preImbalance, postImbalance, getName(), vm.getUuid(), destHost.getUuid())); + // This needs more research to determine the cost and benefit of a migration // TODO: Cost should be a factor of the VM size and the host capacity // TODO: Benefit should be a factor of the VM size and the host capacity and the number of VMs on the host diff --git a/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java b/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java index 4aaa1983d36d..8fe82dab5fbd 100644 --- a/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java +++ b/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java @@ -24,7 +24,7 @@ import com.cloud.utils.Ternary; import com.cloud.utils.component.AdapterBase; import com.cloud.vm.VirtualMachine; - +import org.apache.log4j.Logger; import javax.naming.ConfigurationException; import java.util.ArrayList; @@ -36,31 +36,50 @@ public class Condensed extends AdapterBase implements ClusterDrsAlgorithm { - @Override - public String getName() { - return "condensed"; - } + private static final Logger logger = Logger.getLogger(Condensed.class); @Override public boolean needsDrs(long clusterId, List> cpuList, List> memoryList) throws ConfigurationException { double threshold = getThreshold(clusterId); - Double imbalance = getClusterImbalance(clusterId, cpuList, memoryList, ClusterDrsImbalanceSkipThreshold.valueIn(clusterId)); - return imbalance < threshold; + Float skipThreshold = ClusterDrsImbalanceSkipThreshold.valueIn(clusterId); + Double imbalance = ClusterDrsAlgorithm.getClusterImbalance(clusterId, cpuList, memoryList, skipThreshold); + String drsMetric = ClusterDrsAlgorithm.getClusterDrsMetric(clusterId); + String metricType = ClusterDrsAlgorithm.getDrsMetricType(clusterId); + Boolean useRatio = ClusterDrsAlgorithm.getDrsMetricUseRatio(clusterId); + if (imbalance < threshold) { + + logger.debug(String.format("Cluster %d needs DRS. Imbalance: %s Threshold: %s Algorithm: %s DRS metric: %s Metric Type: %s Use ratio: %s SkipThreshold: %s", + clusterId, imbalance, threshold, getName(), drsMetric, metricType, useRatio, skipThreshold)); + return true; + } else { + logger.debug(String.format("Cluster %d does not need DRS. Imbalance: %s Threshold: %s Algorithm: %s DRS metric: %s Metric Type: %s Use ratio: %s SkipThreshold: %s", + clusterId, imbalance, threshold, getName(), drsMetric, metricType, useRatio, skipThreshold)); + return false; + } } private double getThreshold(long clusterId) { return ClusterDrsImbalanceThreshold.valueIn(clusterId); } + @Override + public String getName() { + return "condensed"; + } + @Override public Ternary getMetrics(long clusterId, VirtualMachine vm, ServiceOffering serviceOffering, Host destHost, Map> hostCpuMap, Map> hostMemoryMap, Boolean requiresStorageMotion) throws ConfigurationException { - Double preImbalance = getClusterImbalance(clusterId, new ArrayList<>(hostCpuMap.values()), new ArrayList<>(hostMemoryMap.values()), null); + Double preImbalance = ClusterDrsAlgorithm.getClusterImbalance(clusterId, new ArrayList<>(hostCpuMap.values()), + new ArrayList<>(hostMemoryMap.values()), null); Double postImbalance = getImbalancePostMigration(serviceOffering, vm, destHost, hostCpuMap, hostMemoryMap); + logger.debug(String.format("Cluster %d pre-imbalance: %s post-imbalance: %s Algorithm: %s VM: %s destHost: %s", + clusterId, preImbalance, postImbalance, getName(), vm.getUuid(), destHost.getUuid())); + // This needs more research to determine the cost and benefit of a migration // TODO: Cost should be a factor of the VM size and the host capacity // TODO: Benefit should be a factor of the VM size and the host capacity and the number of VMs on the host diff --git a/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java b/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java index 51c020fc2375..e185a6ba2301 100644 --- a/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java +++ b/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java @@ -29,7 +29,9 @@ import java.util.Properties; import javax.inject.Inject; +import javax.naming.ConfigurationException; +import com.cloud.utils.Ternary; import org.apache.cloudstack.api.ApiErrorCode; import org.apache.cloudstack.api.ListClustersMetricsCmd; import org.apache.cloudstack.api.ListDbMetricsCmd; @@ -55,6 +57,7 @@ import org.apache.cloudstack.api.response.UserVmResponse; import org.apache.cloudstack.api.response.VolumeResponse; import org.apache.cloudstack.api.response.ZoneResponse; +import org.apache.cloudstack.cluster.ClusterDrsAlgorithm; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.management.ManagementServerHost.State; import org.apache.cloudstack.response.ClusterMetricsResponse; @@ -762,10 +765,13 @@ public List listClusterMetrics(Pair> cpuList = new ArrayList<>(); + List> memoryList = new ArrayList<>(); + for (final Host host: hostDao.findByClusterId(clusterId)) { if (host == null || host.getType() != Host.Type.Routing) { continue; @@ -774,7 +780,18 @@ public List listClusterMetrics(Pair(hostJoin.getCpuUsedCapacity(), hostJoin.getCpuReservedCapacity(), hostJoin.getCpus() * hostJoin.getSpeed())); + memoryList.add(new Ternary<>(hostJoin.getMemUsedCapacity(), hostJoin.getMemReservedCapacity(), hostJoin.getTotalMemory())); + } + + try { + Double imbalance = ClusterDrsAlgorithm.getClusterImbalance(clusterId, cpuList, memoryList, null); + metricsResponse.setDrsImbalance(imbalance.isNaN() ? null : imbalance); + } catch (ConfigurationException e) { + LOGGER.warn("Failed to get cluster imbalance for cluster " + clusterId, e); } metricsResponse.setState(clusterResponse.getAllocationState(), clusterResponse.getManagedState()); diff --git a/plugins/metrics/src/main/java/org/apache/cloudstack/response/ClusterMetricsResponse.java b/plugins/metrics/src/main/java/org/apache/cloudstack/response/ClusterMetricsResponse.java index 18ea57d711c9..ced29a0ba07b 100644 --- a/plugins/metrics/src/main/java/org/apache/cloudstack/response/ClusterMetricsResponse.java +++ b/plugins/metrics/src/main/java/org/apache/cloudstack/response/ClusterMetricsResponse.java @@ -94,6 +94,10 @@ public class ClusterMetricsResponse extends ClusterResponse implements HostMetri @Param(description = "memory allocated disable threshold exceeded") private Boolean memoryAllocatedDisableThresholdExceeded; + @SerializedName("drsimbalance") + @Param(description = "DRS imbalance for the cluster") + private Double drsImbalance; + public void setState(final String allocationState, final String managedState) { this.state = allocationState; if (managedState.equals("Unmanaged")) { @@ -208,4 +212,8 @@ public void setMemoryAllocatedDisableThreshold(final Long memAllocated, final Lo this.memoryAllocatedDisableThresholdExceeded = (1.0 * memAllocated / memTotal) > threshold; } } + + public void setDrsImbalance(Double drsImbalance) { + this.drsImbalance = drsImbalance; + } } diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index 71da3c6d0aae..ffa63952da11 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -782,6 +782,7 @@ "label.dpd": "Dead peer detection", "label.driver": "Driver", "label.drs": "DRS", +"label.drsimbalance": "DRS imbalance", "label.drs.plan": "DRS Plan", "label.drs.generate.plan": "Generate DRS plan", "label.drs.no.plan.generated": "No DRS plan has been generated as the cluster is not imbalanced according to the threshold set", diff --git a/ui/src/components/view/ListView.vue b/ui/src/components/view/ListView.vue index 2beec672a3c2..99fb3d6a4a62 100644 --- a/ui/src/components/view/ListView.vue +++ b/ui/src/components/view/ListView.vue @@ -337,6 +337,9 @@ + diff --git a/ui/src/config/section/infra/clusters.js b/ui/src/config/section/infra/clusters.js index 8fc4ebd54a9c..ab8bea6b79d4 100644 --- a/ui/src/config/section/infra/clusters.js +++ b/ui/src/config/section/infra/clusters.js @@ -26,7 +26,7 @@ export default { permission: ['listClustersMetrics'], columns: () => { const fields = ['name', 'state', 'allocationstate', 'clustertype', 'hypervisortype', 'hosts'] - const metricsFields = ['cpuused', 'cpumaxdeviation', 'cpuallocated', 'cputotal', 'memoryused', 'memorymaxdeviation', 'memoryallocated', 'memorytotal'] + const metricsFields = ['cpuused', 'cpumaxdeviation', 'cpuallocated', 'cputotal', 'memoryused', 'memorymaxdeviation', 'memoryallocated', 'memorytotal', 'drsimbalance'] if (store.getters.metrics) { fields.push(...metricsFields) } @@ -34,7 +34,7 @@ export default { fields.push('zonename') return fields }, - details: ['name', 'id', 'allocationstate', 'clustertype', 'managedstate', 'hypervisortype', 'podname', 'zonename'], + details: ['name', 'id', 'allocationstate', 'clustertype', 'managedstate', 'hypervisortype', 'podname', 'zonename', 'drsimbalance'], related: [{ name: 'host', title: 'label.hosts', diff --git a/ui/src/views/infra/ClusterDRSTab.vue b/ui/src/views/infra/ClusterDRSTab.vue index 4ff255bf718c..d53199ced87a 100644 --- a/ui/src/views/infra/ClusterDRSTab.vue +++ b/ui/src/views/infra/ClusterDRSTab.vue @@ -57,7 +57,8 @@ :columns="migrationColumns" :dataSource="record.migrations" :rowKey="(record, index) => index" - :pagination="{hideOnSinglePage: true, showSizeChanger: true}"> + :pagination="{hideOnSinglePage: true, showSizeChanger: true}" + @resizeColumn="resizeColumn">