From 9550ebe7e60b067fcf61b17d41ad5b10665ba170 Mon Sep 17 00:00:00 2001 From: Aaron Chung Date: Mon, 4 May 2026 22:31:12 -0700 Subject: [PATCH] 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) with empty-list guard and IN-clause SearchBuilder, matching the established pattern in HostDao. --- .../java/com/cloud/vm/dao/VMInstanceDao.java | 2 ++ .../com/cloud/vm/dao/VMInstanceDaoImpl.java | 13 ++++++++++++ .../com/cloud/storage/StorageManagerImpl.java | 21 +++++++++++++++++-- 3 files changed, 34 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 4fd3e729e0d2..fbc4ea669e51 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 @@ List searchRemovedByRemoveDate(final Date startDate, final Date en List listDeleteProtectedVmsByAccountId(long accountId); List listDeleteProtectedVmsByDomainIds(Set domainIds); + + List listByIds(List 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 d8c9b9253c89..1240905404b7 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 @@ -1336,4 +1336,17 @@ public List listDeleteProtectedVmsByDomainIds(Set domainIds) Filter filter = new Filter(VMInstanceVO.class, null, false, 0L, 10L); return listBy(sc, filter); } + + @Override + public List listByIds(List ids) { + if (ids == null || ids.isEmpty()) { + return new ArrayList<>(); + } + SearchBuilder sb = createSearchBuilder(); + sb.and("id", sb.entity().getId(), Op.IN); + sb.done(); + SearchCriteria sc = sb.create(); + sc.setParameters("id", ids.toArray()); + return listBy(sc); + } } diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 2d170049663b..f2a7e24b1a4c 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 void connectHostsToPool(DataStore primaryStore, List hostIds, Scope if (CollectionUtils.isEmpty(hostIds)) { return; } + Map hostMap = new HashMap<>(); + for (HostVO h : _hostDao.listByIds(hostIds)) { + hostMap.put(h.getId(), h); + } CopyOnWriteArrayList poolHostIds = new CopyOnWriteArrayList<>(); ExecutorService executorService = Executors.newFixedThreadPool(Math.max(1, Math.min(hostIds.size(), StoragePoolHostConnectWorkers.value()))); @@ -1856,10 +1860,14 @@ public void connectHostsToPool(DataStore primaryStore, List hostIds, Scope 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() { @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 void cleanupStorage(boolean recurring) { cleanupSecondaryStorage(recurring); List vols = volumeDao.listVolumesToBeDestroyed(new Date(System.currentTimeMillis() - ((long)StorageCleanupDelay.value() << 10))); + List rootVolumeVmIds = vols.stream() + .filter(v -> Type.ROOT.equals(v.getVolumeType()) && v.getInstanceId() != null) + .map(VolumeVO::getInstanceId) + .distinct() + .collect(Collectors.toList()); + Map 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());