This is an automated email from the ASF dual-hosted git repository.
nvazquez pushed a commit to branch 4.22
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.22 by this push:
new 768298a2111 Batch VM and host lookups in StorageManagerImpl to
eliminate N+1 queries during cleanup and pool connect (#13353)
768298a2111 is described below
commit 768298a21116c27a38f98db797896a322cf832e7
Author: Vishesh <[email protected]>
AuthorDate: Mon Oct 19 04:00:06 2026 +0530
Batch VM and host lookups in StorageManagerImpl to eliminate N+1 queries
during cleanup and pool connect (#13353)
* Batch VM and host lookups in StorageManagerImpl to eliminate N+1 queries
during cleanup and pool connect
cleanupStorage: replaced per-volume
_vmInstanceDao.findById(vol.getInstanceId())
with a single batch SELECT ... WHERE id IN (...) before the loop, using a
HashMap for O(1) lookups. Added null guard for volumes with null instanceId.
connectHostsToPool: replaced per-thread _hostDao.findById(hostId) with a
batch load before thread pool submission. Each thread now reads from a
pre-built HashMap instead of hitting the DB. Added null guard with warning
log for hosts removed between list assembly and batch read.
Added VMInstanceDao.listByIds(List<Long>) with empty-list guard and
IN-clause
SearchBuilder, matching the established pattern in HostDao.
* Fix test failures
* Address comments
---------
Co-authored-by: Aaron Chung <[email protected]>
---
.../main/java/com/cloud/vm/dao/VMInstanceDao.java | 2 ++
.../java/com/cloud/vm/dao/VMInstanceDaoImpl.java | 16 ++++++++++++++++
...CloudStackPrimaryDataStoreLifeCycleImplTest.java | 2 ++
.../ScaleIOPrimaryDataStoreLifeCycleTest.java | 3 +++
.../java/com/cloud/storage/StorageManagerImpl.java | 21 +++++++++++++++++++--
5 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDao.java
b/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDao.java
index 4fd3e729e0d..fbc4ea669e5 100755
--- a/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDao.java
+++ b/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDao.java
@@ -197,4 +197,6 @@ public interface VMInstanceDao extends
GenericDao<VMInstanceVO, Long>, StateDao<
List<VMInstanceVO> listDeleteProtectedVmsByAccountId(long accountId);
List<VMInstanceVO> listDeleteProtectedVmsByDomainIds(Set<Long> domainIds);
+
+ List<VMInstanceVO> listByIds(List<Long> ids);
}
diff --git
a/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java
b/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java
index d8c9b9253c8..b22d7e0274a 100755
--- a/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java
+++ b/engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java
@@ -21,6 +21,7 @@ import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.List;
@@ -106,6 +107,7 @@ public class VMInstanceDaoImpl extends
GenericDaoBase<VMInstanceVO, Long> implem
protected SearchBuilder<VMInstanceVO> LastHostAndStatesSearch;
protected SearchBuilder<VMInstanceVO> VmsNotInClusterUsingPool;
protected SearchBuilder<VMInstanceVO> IdsPowerStateSelectSearch;
+ protected SearchBuilder<VMInstanceVO> IdsSearch;
GenericSearchBuilder<VMInstanceVO, Integer> CountByOfferingId;
GenericSearchBuilder<VMInstanceVO, Integer> CountUserVmNotInDomain;
SearchBuilder<VMInstanceVO> DeleteProtectedVmSearchByAccount;
@@ -386,6 +388,10 @@ public class VMInstanceDaoImpl extends
GenericDaoBase<VMInstanceVO, Long> implem
DeleteProtectedVmSearchByDomainIds.and(ApiConstants.DELETE_PROTECTION,
DeleteProtectedVmSearchByDomainIds.entity().isDeleteProtection(), Op.EQ);
DeleteProtectedVmSearchByDomainIds.and(ApiConstants.REMOVED,
DeleteProtectedVmSearchByDomainIds.entity().getRemoved(), Op.NULL);
DeleteProtectedVmSearchByDomainIds.done();
+
+ IdsSearch = createSearchBuilder();
+ IdsSearch.and("ids", IdsSearch.entity().getId(), Op.IN);
+ IdsSearch.done();
}
@Override
@@ -1336,4 +1342,14 @@ public class VMInstanceDaoImpl extends
GenericDaoBase<VMInstanceVO, Long> implem
Filter filter = new Filter(VMInstanceVO.class, null, false, 0L, 10L);
return listBy(sc, filter);
}
+
+ @Override
+ public List<VMInstanceVO> listByIds(List<Long> ids) {
+ if (CollectionUtils.isEmpty(ids)) {
+ return Collections.emptyList();
+ }
+ SearchCriteria<VMInstanceVO> sc = IdsSearch.create();
+ sc.setParameters("ids", ids.toArray());
+ return listBy(sc);
+ }
}
diff --git
a/plugins/storage/volume/default/src/test/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImplTest.java
b/plugins/storage/volume/default/src/test/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImplTest.java
index 3b533d588d3..5d765267178 100644
---
a/plugins/storage/volume/default/src/test/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImplTest.java
+++
b/plugins/storage/volume/default/src/test/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImplTest.java
@@ -26,6 +26,7 @@ import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import java.util.Arrays;
+import java.util.List;
import org.apache.cloudstack.engine.subsystem.api.storage.ClusterScope;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
@@ -153,6 +154,7 @@ public class CloudStackPrimaryDataStoreLifeCycleImplTest
extends TestCase {
.thenReturn(Arrays.asList(host1, host2));
when(hostDao.findById(anyLong())).thenReturn(mock(HostVO.class));
+ when(hostDao.listByIds(List.of(1L, 2L))).thenReturn(List.of(host1,
host2));
when(agentMgr.easySend(anyLong(),
Mockito.any(ModifyStoragePoolCommand.class))).thenReturn(answer);
when(answer.getResult()).thenReturn(true);
diff --git
a/plugins/storage/volume/scaleio/src/test/java/org/apache/cloudstack/storage/datastore/lifecycle/ScaleIOPrimaryDataStoreLifeCycleTest.java
b/plugins/storage/volume/scaleio/src/test/java/org/apache/cloudstack/storage/datastore/lifecycle/ScaleIOPrimaryDataStoreLifeCycleTest.java
index 324f3c08cb8..ac7075ee462 100644
---
a/plugins/storage/volume/scaleio/src/test/java/org/apache/cloudstack/storage/datastore/lifecycle/ScaleIOPrimaryDataStoreLifeCycleTest.java
+++
b/plugins/storage/volume/scaleio/src/test/java/org/apache/cloudstack/storage/datastore/lifecycle/ScaleIOPrimaryDataStoreLifeCycleTest.java
@@ -151,6 +151,7 @@ public class ScaleIOPrimaryDataStoreLifeCycleTest {
when(resourceManager.getEligibleUpAndEnabledHostsInZoneForStorageConnection(dataStore,
scope.getScopeId(), Hypervisor.HypervisorType.KVM))
.thenReturn(Arrays.asList(host1, host2));
+ when(hostDao.listByIds(List.of(1L, 2L))).thenReturn(List.of(host1,
host2));
when(dataStoreMgr.getDataStore(anyLong(),
eq(DataStoreRole.Primary))).thenReturn(store);
when(store.isShared()).thenReturn(true);
@@ -164,6 +165,8 @@ public class ScaleIOPrimaryDataStoreLifeCycleTest {
boolean result =
scaleIOPrimaryDataStoreLifeCycleTest.attachZone(dataStore, scope,
Hypervisor.HypervisorType.KVM);
assertThat(result).isTrue();
+ Mockito.verify(hostListener).hostConnect(host1, store);
+ Mockito.verify(hostListener).hostConnect(host2, store);
}
@Test(expected = CloudRuntimeException.class)
diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java
b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java
index 0b2aca08e08..587a5269cba 100644
--- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java
+++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java
@@ -1846,6 +1846,10 @@ public class StorageManagerImpl extends ManagerBase
implements StorageManager, C
if (CollectionUtils.isEmpty(hostIds)) {
return;
}
+ Map<Long, HostVO> hostMap = new HashMap<>();
+ for (HostVO h : _hostDao.listByIds(hostIds)) {
+ hostMap.put(h.getId(), h);
+ }
CopyOnWriteArrayList<Long> poolHostIds = new CopyOnWriteArrayList<>();
ExecutorService executorService =
Executors.newFixedThreadPool(Math.max(1, Math.min(hostIds.size(),
StoragePoolHostConnectWorkers.value())));
@@ -1856,10 +1860,14 @@ public class StorageManagerImpl extends ManagerBase
implements StorageManager, C
if (exceptionOccurred.get()) {
return null;
}
+ HostVO host = hostMap.get(hostId);
+ if (host == null) {
+ logger.warn("Host [{}] not found, skipping pool connection
for {}", hostId, primaryStore);
+ return null;
+ }
Transaction.execute(new
TransactionCallbackWithExceptionNoReturn<Exception>() {
@Override
public void doInTransactionWithoutResult(TransactionStatus
status) throws Exception {
- HostVO host = _hostDao.findById(hostId);
try {
connectHostToSharedPool(host,
primaryStore.getId());
poolHostIds.add(hostId);
@@ -2126,9 +2134,18 @@ public class StorageManagerImpl extends ManagerBase
implements StorageManager, C
cleanupSecondaryStorage(recurring);
List<VolumeVO> vols =
volumeDao.listVolumesToBeDestroyed(new Date(System.currentTimeMillis() -
((long)StorageCleanupDelay.value() << 10)));
+ List<Long> rootVolumeVmIds = vols.stream()
+ .filter(v -> Type.ROOT.equals(v.getVolumeType())
&& v.getInstanceId() != null)
+ .map(VolumeVO::getInstanceId)
+ .distinct()
+ .collect(Collectors.toList());
+ Map<Long, VMInstanceVO> vmMap = new HashMap<>();
+ for (VMInstanceVO vm :
_vmInstanceDao.listByIds(rootVolumeVmIds)) {
+ vmMap.put(vm.getId(), vm);
+ }
for (VolumeVO vol : vols) {
if (Type.ROOT.equals(vol.getVolumeType())) {
- VMInstanceVO vmInstanceVO =
_vmInstanceDao.findById(vol.getInstanceId());
+ VMInstanceVO vmInstanceVO = vol.getInstanceId() !=
null ? vmMap.get(vol.getInstanceId()) : null;
if (vmInstanceVO != null &&
vmInstanceVO.getState() == State.Destroyed) {
logger.debug("ROOT volume [{}] will not be
expunged because the VM is [{}], therefore this volume will be expunged with
the VM"
+ " cleanup job.", vol,
vmInstanceVO.getState());
