From ce018d5590d44f5efdd83884be7b681bff653be2 Mon Sep 17 00:00:00 2001 From: Aaron Chung Date: Mon, 4 May 2026 22:48:51 -0700 Subject: [PATCH] Batch-load VM details in CapacityManagerImpl.updateCapacityForHost to eliminate N+1 queries updateCapacityForHost called listDetailsKeyPairs per VM (50-100+ round-trips per host). Now batch-loads all VM details in a single WHERE vm_id IN (...) query before the loops. - Add listDetailsKeyPairs(List, List) to ResourceDetailsDao/DaoBase - Extract VM_DETAIL_KEYS_FOR_CAPACITY constant, add batchGetVmDetailsForCapacityCalculation - Replace per-VM getVmDetailsForCapacityCalculation with map lookup in both loops - Add test for mixed static/dynamic offerings verifying batch path and capacity math --- .../resourcedetail/ResourceDetailsDao.java | 2 + .../ResourceDetailsDaoBase.java | 26 +++++ .../cloud/capacity/CapacityManagerImpl.java | 42 +++++-- .../capacity/CapacityManagerImplTest.java | 107 ++++++++++++++++++ 4 files changed, 166 insertions(+), 11 deletions(-) diff --git a/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/ResourceDetailsDao.java b/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/ResourceDetailsDao.java index 1102de16e4ea..853c2611d205 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/ResourceDetailsDao.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/ResourceDetailsDao.java @@ -98,6 +98,8 @@ public interface ResourceDetailsDao extends GenericDao Map listDetailsKeyPairs(long resourceId, List keys); + Map> listDetailsKeyPairs(List resourceIds, List keys); + Map listDetailsKeyPairs(long resourceId, boolean forDisplay); Map listDetailsVisibility(long resourceId); diff --git a/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/ResourceDetailsDaoBase.java b/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/ResourceDetailsDaoBase.java index eafaed182abd..f289e00fdb03 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/ResourceDetailsDaoBase.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/ResourceDetailsDaoBase.java @@ -39,6 +39,7 @@ import javax.inject.Inject; public abstract class ResourceDetailsDaoBase extends GenericDaoBase implements ResourceDetailsDao { + private static final int IN_CLAUSE_BATCH_SIZE = 1000; @Inject private ConfigurationDao configDao; @@ -122,6 +123,31 @@ public Map listDetailsKeyPairs(long resourceId, List key return results.stream().collect(Collectors.toMap(R::getName, R::getValue)); } + @Override + public Map> listDetailsKeyPairs(List resourceIds, List keys) { + if (CollectionUtils.isEmpty(resourceIds)) { + return new HashMap<>(); + } + SearchBuilder sb = createSearchBuilder(); + sb.and("resourceId", sb.entity().getResourceId(), SearchCriteria.Op.IN); + sb.and("name", sb.entity().getName(), SearchCriteria.Op.IN); + sb.done(); + + Map> result = new HashMap<>(resourceIds.size()); + for (int i = 0; i < resourceIds.size(); i += IN_CLAUSE_BATCH_SIZE) { + List batch = resourceIds.subList(i, Math.min(i + IN_CLAUSE_BATCH_SIZE, resourceIds.size())); + SearchCriteria sc = sb.create(); + sc.setParameters("resourceId", batch.toArray()); + sc.setParameters("name", keys.toArray()); + List results = search(sc, null); + for (R r : results) { + result.computeIfAbsent(r.getResourceId(), k -> new HashMap<>()) + .put(r.getName(), r.getValue()); + } + } + return result; + } + public Map listDetailsVisibility(long resourceId) { SearchCriteria sc = AllFieldsSearch.create(); sc.setParameters("resourceId", resourceId); diff --git a/server/src/main/java/com/cloud/capacity/CapacityManagerImpl.java b/server/src/main/java/com/cloud/capacity/CapacityManagerImpl.java index 2de9abc827ec..c7f810c28d2a 100644 --- a/server/src/main/java/com/cloud/capacity/CapacityManagerImpl.java +++ b/server/src/main/java/com/cloud/capacity/CapacityManagerImpl.java @@ -19,6 +19,8 @@ import static com.cloud.utils.NumbersUtil.toHumanReadableSize; import java.net.URI; +import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -668,13 +670,19 @@ protected ServiceOfferingVO getServiceOffering(long id) { return serviceOfferingVO; } + private static final List VM_DETAIL_KEYS_FOR_CAPACITY = List.of( + VmDetailConstants.CPU_OVER_COMMIT_RATIO, + VmDetailConstants.MEMORY_OVER_COMMIT_RATIO, + UsageEventVO.DynamicParameters.memory.name(), + UsageEventVO.DynamicParameters.cpuNumber.name(), + UsageEventVO.DynamicParameters.cpuSpeed.name()); + protected Map getVmDetailsForCapacityCalculation(long vmId) { - return _vmInstanceDetailsDao.listDetailsKeyPairs(vmId, - List.of(VmDetailConstants.CPU_OVER_COMMIT_RATIO, - VmDetailConstants.MEMORY_OVER_COMMIT_RATIO, - UsageEventVO.DynamicParameters.memory.name(), - UsageEventVO.DynamicParameters.cpuNumber.name(), - UsageEventVO.DynamicParameters.cpuSpeed.name())); + return _vmInstanceDetailsDao.listDetailsKeyPairs(vmId, VM_DETAIL_KEYS_FOR_CAPACITY); + } + + protected Map> batchGetVmDetailsForCapacityCalculation(List vmIds) { + return _vmInstanceDetailsDao.listDetailsKeyPairs(vmIds, VM_DETAIL_KEYS_FOR_CAPACITY); } @DB @@ -698,6 +706,21 @@ public void updateCapacityForHost(final Host host) { logger.debug("Found {} VMs are Migrating from {}", vosMigrating.size(), host); vms.addAll(vosMigrating); + + List vmsByLastHostId = _vmDao.listByLastHostId(host.getId()); + logger.debug("Found {} VM, not running on {}", vmsByLastHostId.size(), host); + + List allVmIds = new ArrayList<>(vms.size() + vmsByLastHostId.size()); + for (VMInstanceVO vm : vms) { + allVmIds.add(vm.getId()); + } + for (VMInstanceVO vm : vmsByLastHostId) { + allVmIds.add(vm.getId()); + } + Map> allVmDetails = allVmIds.isEmpty() + ? Collections.emptyMap() + : batchGetVmDetailsForCapacityCalculation(allVmIds); + Pair clusterValues = clusterValuesCache.get(host.getClusterId()); Float clusterCpuOvercommitRatio = Float.parseFloat(clusterValues.first()); @@ -705,7 +728,7 @@ public void updateCapacityForHost(final Host host) { for (VMInstanceVO vm : vms) { Float cpuOvercommitRatio = 1.0f; Float ramOvercommitRatio = 1.0f; - Map vmDetails = getVmDetailsForCapacityCalculation(vm.getId()); + Map vmDetails = allVmDetails.getOrDefault(vm.getId(), Collections.emptyMap()); String vmDetailCpu = vmDetails.get(VmDetailConstants.CPU_OVER_COMMIT_RATIO); String vmDetailRam = vmDetails.get(VmDetailConstants.MEMORY_OVER_COMMIT_RATIO); // if vmDetailCpu or vmDetailRam is not null it means it is running in a overcommitted cluster. @@ -736,16 +759,13 @@ public void updateCapacityForHost(final Host host) { } } - List vmsByLastHostId = _vmDao.listByLastHostId(host.getId()); - logger.debug("Found {} VM, not running on {}", vmsByLastHostId.size(), host); - for (VMInstanceVO vm : vmsByLastHostId) { Float cpuOvercommitRatio = 1.0f; Float ramOvercommitRatio = 1.0f; long lastModificationTime = Optional.ofNullable(vm.getUpdateTime()).orElse(vm.getCreated()).getTime(); long secondsSinceLastUpdate = (DateUtil.currentGMTTime().getTime() - lastModificationTime) / 1000; if (secondsSinceLastUpdate < _vmCapacityReleaseInterval) { - Map vmDetails = getVmDetailsForCapacityCalculation(vm.getId()); + Map vmDetails = allVmDetails.getOrDefault(vm.getId(), Collections.emptyMap()); String vmDetailCpu = vmDetails.get(VmDetailConstants.CPU_OVER_COMMIT_RATIO); String vmDetailRam = vmDetails.get(VmDetailConstants.MEMORY_OVER_COMMIT_RATIO); if (vmDetailCpu != null) { diff --git a/server/src/test/java/com/cloud/capacity/CapacityManagerImplTest.java b/server/src/test/java/com/cloud/capacity/CapacityManagerImplTest.java index a69830e2e0bb..11f5d9302439 100644 --- a/server/src/test/java/com/cloud/capacity/CapacityManagerImplTest.java +++ b/server/src/test/java/com/cloud/capacity/CapacityManagerImplTest.java @@ -20,19 +20,23 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.any; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import org.apache.cloudstack.utils.bytescale.ByteScaleUtils; +import org.apache.cloudstack.utils.cache.LazyCache; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -41,13 +45,20 @@ import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; +import com.cloud.capacity.dao.CapacityDao; import com.cloud.dc.ClusterDetailsDao; import com.cloud.host.Host; +import com.cloud.host.HostVO; +import com.cloud.host.dao.HostDao; import com.cloud.offering.ServiceOffering; import com.cloud.service.ServiceOfferingVO; import com.cloud.service.dao.ServiceOfferingDao; import com.cloud.utils.Pair; +import com.cloud.event.UsageEventVO; +import com.cloud.resource.ResourceState; +import com.cloud.vm.VMInstanceVO; import com.cloud.vm.VmDetailConstants; +import com.cloud.vm.dao.VMInstanceDao; @RunWith(MockitoJUnitRunner.class) public class CapacityManagerImplTest { @@ -55,6 +66,12 @@ public class CapacityManagerImplTest { ClusterDetailsDao clusterDetailsDao; @Mock ServiceOfferingDao serviceOfferingDao; + @Mock + CapacityDao capacityDao; + @Mock + HostDao hostDao; + @Mock + VMInstanceDao vmInstanceDao; @Spy @InjectMocks @@ -64,6 +81,7 @@ public class CapacityManagerImplTest { private Host host; private ServiceOffering offering; private static final long CLUSTER_ID = 1L; + private static final long HOST_ID = 100L; private static final int OFFERING_CPU = 4; private static final int OFFERING_CPU_SPEED = 2000; private static final int OFFERING_MEMORY = 4096; @@ -179,4 +197,93 @@ public void testCheckIfHostHasCpuCapabilityButNoCapacity() { eq(ByteScaleUtils.mebibytesToBytes(OFFERING_MEMORY)), eq(false), eq(cpuOvercommit), eq(memoryOvercommit), eq(false)); } + + @Test + public void testUpdateCapacityForHostMixedStaticAndDynamic() throws Exception { + HostVO testHost = mock(HostVO.class); + when(testHost.getId()).thenReturn(HOST_ID); + when(testHost.getClusterId()).thenReturn(CLUSTER_ID); + when(testHost.getResourceState()).thenReturn(ResourceState.Enabled); + when(testHost.getCpus()).thenReturn(16); + when(testHost.getSpeed()).thenReturn(2000L); + when(testHost.getTotalMemory()).thenReturn(32L * 1024 * 1024 * 1024); + + Pair clusterOvercommit = new Pair<>("2.0", "1.5"); + doReturn(clusterOvercommit).when(capacityManager).getClusterValues(CLUSTER_ID); + java.lang.reflect.Field cacheField = CapacityManagerImpl.class.getDeclaredField("clusterValuesCache"); + cacheField.setAccessible(true); + cacheField.set(capacityManager, new LazyCache<>(128, 60, capacityManager::getClusterValues)); + + long staticVmId = 1L; + long dynamicVmId = 2L; + long staticOfferingId = 10L; + long dynamicOfferingId = 20L; + + VMInstanceVO staticVm = mock(VMInstanceVO.class); + when(staticVm.getId()).thenReturn(staticVmId); + when(staticVm.getServiceOfferingId()).thenReturn(staticOfferingId); + + VMInstanceVO dynamicVm = mock(VMInstanceVO.class); + when(dynamicVm.getId()).thenReturn(dynamicVmId); + when(dynamicVm.getServiceOfferingId()).thenReturn(dynamicOfferingId); + + when(vmInstanceDao.listIdServiceOfferingForUpVmsByHostId(HOST_ID)) + .thenReturn(new ArrayList<>(List.of(staticVm, dynamicVm))); + when(vmInstanceDao.listIdServiceOfferingForVmsMigratingFromHost(HOST_ID)) + .thenReturn(Collections.emptyList()); + when(vmInstanceDao.listByLastHostId(HOST_ID)) + .thenReturn(Collections.emptyList()); + + ServiceOfferingVO staticOffering = mock(ServiceOfferingVO.class); + when(staticOffering.isDynamic()).thenReturn(false); + when(staticOffering.getCpu()).thenReturn(2); + when(staticOffering.getSpeed()).thenReturn(2000); + when(staticOffering.getRamSize()).thenReturn(2048); + + ServiceOfferingVO dynamicOffering = mock(ServiceOfferingVO.class); + when(dynamicOffering.isDynamic()).thenReturn(true); + + doReturn(staticOffering).when(capacityManager).getServiceOffering(staticOfferingId); + doReturn(dynamicOffering).when(capacityManager).getServiceOffering(dynamicOfferingId); + + Map> batchDetails = new HashMap<>(); + batchDetails.put(staticVmId, Map.of( + VmDetailConstants.CPU_OVER_COMMIT_RATIO, "2.0", + VmDetailConstants.MEMORY_OVER_COMMIT_RATIO, "1.5")); + batchDetails.put(dynamicVmId, Map.of( + VmDetailConstants.CPU_OVER_COMMIT_RATIO, "2.0", + VmDetailConstants.MEMORY_OVER_COMMIT_RATIO, "1.5", + UsageEventVO.DynamicParameters.cpuNumber.name(), "4", + UsageEventVO.DynamicParameters.cpuSpeed.name(), "2500", + UsageEventVO.DynamicParameters.memory.name(), "4096")); + doReturn(batchDetails).when(capacityManager).batchGetVmDetailsForCapacityCalculation(anyList()); + + CapacityVO cpuCapVO = new CapacityVO(HOST_ID, 1L, 1L, CLUSTER_ID, 0, 16 * 2000L, Capacity.CAPACITY_TYPE_CPU); + cpuCapVO.setReservedCapacity(0); + CapacityVO memCapVO = new CapacityVO(HOST_ID, 1L, 1L, CLUSTER_ID, 0, 32L * 1024 * 1024 * 1024, Capacity.CAPACITY_TYPE_MEMORY); + memCapVO.setReservedCapacity(0); + CapacityVO cpuCoreCapVO = new CapacityVO(HOST_ID, 1L, 1L, CLUSTER_ID, 0, 16L, CapacityVO.CAPACITY_TYPE_CPU_CORE); + cpuCoreCapVO.setReservedCapacity(0); + + when(capacityDao.listByHostIdTypes(eq(HOST_ID), anyList())) + .thenReturn(List.of(cpuCapVO, memCapVO, cpuCoreCapVO)); + when(capacityDao.update(anyLong(), any(CapacityVO.class))).thenReturn(true); + + capacityManager.updateCapacityForHost(testHost); + + // static VM: cpu = (2 * 2000 / 2.0) * 2.0 = 4000, mem = (2048 * 1024 * 1024 / 1.5) * 1.5 = 2048MB + // dynamic VM: cpu = (4 * 2500 / 2.0) * 2.0 = 10000, mem = (4096 * 1024 * 1024 / 1.5) * 1.5 = 4096MB + long expectedUsedCpu = 4000 + 10000; + long expectedUsedMem = (2048L + 4096L) * 1024 * 1024; + long expectedUsedCpuCore = 2 + 4; + + assertEquals(expectedUsedCpu, cpuCapVO.getUsedCapacity()); + assertEquals(expectedUsedMem, memCapVO.getUsedCapacity()); + assertEquals(expectedUsedCpuCore, cpuCoreCapVO.getUsedCapacity()); + assertEquals(0, cpuCapVO.getReservedCapacity()); + assertEquals(0, memCapVO.getReservedCapacity()); + + verify(capacityManager).batchGetVmDetailsForCapacityCalculation(anyList()); + verify(capacityManager, never()).getVmDetailsForCapacityCalculation(anyLong()); + } }