Skip to content

Commit 9550ebe

Browse files
sliceofapplepievishesh92
authored andcommitted
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.
1 parent 21b2025 commit 9550ebe

3 files changed

Lines changed: 34 additions & 2 deletions

File tree

engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDao.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,4 +197,6 @@ List<VMInstanceVO> searchRemovedByRemoveDate(final Date startDate, final Date en
197197
List<VMInstanceVO> listDeleteProtectedVmsByAccountId(long accountId);
198198

199199
List<VMInstanceVO> listDeleteProtectedVmsByDomainIds(Set<Long> domainIds);
200+
201+
List<VMInstanceVO> listByIds(List<Long> ids);
200202
}

engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1336,4 +1336,17 @@ public List<VMInstanceVO> listDeleteProtectedVmsByDomainIds(Set<Long> domainIds)
13361336
Filter filter = new Filter(VMInstanceVO.class, null, false, 0L, 10L);
13371337
return listBy(sc, filter);
13381338
}
1339+
1340+
@Override
1341+
public List<VMInstanceVO> listByIds(List<Long> ids) {
1342+
if (ids == null || ids.isEmpty()) {
1343+
return new ArrayList<>();
1344+
}
1345+
SearchBuilder<VMInstanceVO> sb = createSearchBuilder();
1346+
sb.and("id", sb.entity().getId(), Op.IN);
1347+
sb.done();
1348+
SearchCriteria<VMInstanceVO> sc = sb.create();
1349+
sc.setParameters("id", ids.toArray());
1350+
return listBy(sc);
1351+
}
13391352
}

server/src/main/java/com/cloud/storage/StorageManagerImpl.java

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1846,6 +1846,10 @@ public void connectHostsToPool(DataStore primaryStore, List<Long> hostIds, Scope
18461846
if (CollectionUtils.isEmpty(hostIds)) {
18471847
return;
18481848
}
1849+
Map<Long, HostVO> hostMap = new HashMap<>();
1850+
for (HostVO h : _hostDao.listByIds(hostIds)) {
1851+
hostMap.put(h.getId(), h);
1852+
}
18491853
CopyOnWriteArrayList<Long> poolHostIds = new CopyOnWriteArrayList<>();
18501854
ExecutorService executorService = Executors.newFixedThreadPool(Math.max(1, Math.min(hostIds.size(),
18511855
StoragePoolHostConnectWorkers.value())));
@@ -1856,10 +1860,14 @@ public void connectHostsToPool(DataStore primaryStore, List<Long> hostIds, Scope
18561860
if (exceptionOccurred.get()) {
18571861
return null;
18581862
}
1863+
HostVO host = hostMap.get(hostId);
1864+
if (host == null) {
1865+
logger.warn("Host [{}] not found, skipping pool connection for {}", hostId, primaryStore);
1866+
return null;
1867+
}
18591868
Transaction.execute(new TransactionCallbackWithExceptionNoReturn<Exception>() {
18601869
@Override
18611870
public void doInTransactionWithoutResult(TransactionStatus status) throws Exception {
1862-
HostVO host = _hostDao.findById(hostId);
18631871
try {
18641872
connectHostToSharedPool(host, primaryStore.getId());
18651873
poolHostIds.add(hostId);
@@ -2126,9 +2134,18 @@ public void cleanupStorage(boolean recurring) {
21262134
cleanupSecondaryStorage(recurring);
21272135

21282136
List<VolumeVO> vols = volumeDao.listVolumesToBeDestroyed(new Date(System.currentTimeMillis() - ((long)StorageCleanupDelay.value() << 10)));
2137+
List<Long> rootVolumeVmIds = vols.stream()
2138+
.filter(v -> Type.ROOT.equals(v.getVolumeType()) && v.getInstanceId() != null)
2139+
.map(VolumeVO::getInstanceId)
2140+
.distinct()
2141+
.collect(Collectors.toList());
2142+
Map<Long, VMInstanceVO> vmMap = new HashMap<>();
2143+
for (VMInstanceVO vm : _vmInstanceDao.listByIds(rootVolumeVmIds)) {
2144+
vmMap.put(vm.getId(), vm);
2145+
}
21292146
for (VolumeVO vol : vols) {
21302147
if (Type.ROOT.equals(vol.getVolumeType())) {
2131-
VMInstanceVO vmInstanceVO = _vmInstanceDao.findById(vol.getInstanceId());
2148+
VMInstanceVO vmInstanceVO = vol.getInstanceId() != null ? vmMap.get(vol.getInstanceId()) : null;
21322149
if (vmInstanceVO != null && vmInstanceVO.getState() == State.Destroyed) {
21332150
logger.debug("ROOT volume [{}] will not be expunged because the VM is [{}], therefore this volume will be expunged with the VM"
21342151
+ " cleanup job.", vol, vmInstanceVO.getState());

0 commit comments

Comments
 (0)