From 81ca4c9c0ad91ac22b5d42482cd50dc328be4909 Mon Sep 17 00:00:00 2001 From: Aaron Chung Date: Mon, 4 May 2026 22:28:17 -0700 Subject: [PATCH] Batch network lookups per NIC in NetworkOrchestrator to eliminate N+1 queries during migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three methods called _networksDao.findById(nic.getNetworkId()) per NIC in a loop: setHypervisorHostname, prepareNicForMigration, prepareAllNicsForMigration. Now batch-loads all networks for a VM's NICs in a single WHERE id IN (...) query. - Add NetworkDao.listByIds(List) for batch ID lookup - Add getNetworkMapForNics helper returning Map - Replace per-NIC findById with map lookup at 3 sites - Add null guard for deleted networks (pre-existing NPE risk on all 3 sites) - Site 2 (prepare) left unchanged — findById only on error path, not N+1 --- .../orchestration/NetworkOrchestrator.java | 34 +++++++++++++++++-- .../com/cloud/network/dao/NetworkDao.java | 2 ++ .../com/cloud/network/dao/NetworkDaoImpl.java | 13 +++++++ .../com/cloud/vpc/dao/MockNetworkDaoImpl.java | 5 +++ 4 files changed, 51 insertions(+), 3 deletions(-) diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java index 6fc673cc2c9b..2a5220654689 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java @@ -2085,13 +2085,31 @@ public void finalizeUpdateInSequence(Network network, boolean success) { } } + private Map batchLoadNetworksForNics(List nics) { + List networkIds = new ArrayList<>(nics.size()); + for (NicVO nic : nics) { + networkIds.add(nic.getNetworkId()); + } + Map result = new HashMap<>(networkIds.size()); + if (!networkIds.isEmpty()) { + for (NetworkVO network : _networksDao.listByIds(networkIds)) { + result.put(network.getId(), network); + } + } + return result; + } + @Override public void setHypervisorHostname(VirtualMachineProfile vm, DeployDestination dest, boolean migrationSuccessful) throws ResourceUnavailableException { String hypervisorHostName = VirtualMachineManager.getHypervisorHostname(dest.getHost().getName()); if (StringUtils.isNotEmpty(hypervisorHostName)) { final List nics = _nicDao.listByVmId(vm.getId()); + final Map networksById = batchLoadNetworksForNics(nics); for (final NicVO nic : nics) { - final NetworkVO network = _networksDao.findById(nic.getNetworkId()); + final NetworkVO network = networksById.get(nic.getNetworkId()); + if (network == null) { + continue; + } final Integer networkRate = _networkModel.getNetworkRate(network.getId(), vm.getId()); final NicProfile profile = new NicProfile(nic, network, nic.getBroadcastUri(), nic.getIsolationUri(), networkRate, _networkModel.isSecurityGroupSupportedInNetwork(network), _networkModel.getNetworkTag(vm.getHypervisorType(), network)); @@ -2266,8 +2284,13 @@ public void prepareNicForMigration(final VirtualMachineProfile vm, final DeployD } final List nics = _nicDao.listByVmId(vm.getId()); final ReservationContext context = new ReservationContextImpl(UUID.randomUUID().toString(), null, null); + final Map networksById = batchLoadNetworksForNics(nics); for (final NicVO nic : nics) { - final NetworkVO network = _networksDao.findById(nic.getNetworkId()); + final NetworkVO network = networksById.get(nic.getNetworkId()); + if (network == null) { + logger.warn("Network {} not found for nic {} during migration prep, skipping", nic.getNetworkId(), nic); + continue; + } final Integer networkRate = _networkModel.getNetworkRate(network.getId(), vm.getId()); final NetworkGuru guru = AdapterBase.getAdapterByName(networkGurus, network.getGuruName()); @@ -2311,9 +2334,14 @@ public void prepareNicForMigration(final VirtualMachineProfile vm, final DeployD public void prepareAllNicsForMigration(final VirtualMachineProfile vm, final DeployDestination dest) { final List nics = _nicDao.listByVmId(vm.getId()); final ReservationContext context = new ReservationContextImpl(UUID.randomUUID().toString(), null, null); + final Map networksById = batchLoadNetworksForNics(nics); Long guestNetworkId = null; for (final NicVO nic : nics) { - final NetworkVO network = _networksDao.findById(nic.getNetworkId()); + final NetworkVO network = networksById.get(nic.getNetworkId()); + if (network == null) { + logger.warn("Network {} not found for nic {} during migration prep, skipping", nic.getNetworkId(), nic); + continue; + } if (network.getTrafficType().equals(TrafficType.Guest) && network.getGuestType().equals(GuestType.Isolated)) { guestNetworkId = network.getId(); } diff --git a/engine/schema/src/main/java/com/cloud/network/dao/NetworkDao.java b/engine/schema/src/main/java/com/cloud/network/dao/NetworkDao.java index fdca6e43f00f..4ef1489a25c9 100644 --- a/engine/schema/src/main/java/com/cloud/network/dao/NetworkDao.java +++ b/engine/schema/src/main/java/com/cloud/network/dao/NetworkDao.java @@ -32,6 +32,8 @@ public interface NetworkDao extends GenericDao, StateDao listByOwner(long ownerId); + List listByIds(List ids); + List listByGuestType(GuestType type); List listBy(long accountId, long offeringId, long dataCenterId); diff --git a/engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java b/engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java index 5887498e094c..1999a58fc845 100644 --- a/engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java @@ -563,6 +563,19 @@ public List listByOwner(final long ownerId) { return listBy(sc); } + @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); + } + @Override public void addDomainToNetwork(final long networkId, final long domainId, final Boolean subdomainAccess) { addDomainToNetworknetwork(networkId, domainId, subdomainAccess); diff --git a/server/src/test/java/com/cloud/vpc/dao/MockNetworkDaoImpl.java b/server/src/test/java/com/cloud/vpc/dao/MockNetworkDaoImpl.java index 8a0bec56df7b..4e77d2c89bbe 100644 --- a/server/src/test/java/com/cloud/vpc/dao/MockNetworkDaoImpl.java +++ b/server/src/test/java/com/cloud/vpc/dao/MockNetworkDaoImpl.java @@ -280,4 +280,9 @@ public List listByNetworkDomainsAndAccountIds(Set uniqueNtwkD public List listByNetworkDomainsAndDomainIds(Set uniqueNtwkDomains, Set domainIds) { return List.of(); } + + @Override + public List listByIds(List ids) { + return null; + } }