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 99f348e0f94 Batch network lookups per NIC in NetworkOrchestrator to
eliminate N+1 queries during migration (#13354)
99f348e0f94 is described below
commit 99f348e0f94681a35a03fcb885af61d0c61305f2
Author: Vishesh <[email protected]>
AuthorDate: Mon Oct 19 04:00:47 2026 +0530
Batch network lookups per NIC in NetworkOrchestrator to eliminate N+1
queries during migration (#13354)
* Batch network lookups per NIC in NetworkOrchestrator to eliminate N+1
queries during migration
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<Long>) for batch ID lookup
- Add getNetworkMapForNics helper returning Map<Long, NetworkVO>
- 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
* Address comments
---------
Co-authored-by: Aaron Chung <[email protected]>
---
.../engine/orchestration/NetworkOrchestrator.java | 29 +++++++++++++++++++---
.../java/com/cloud/network/dao/NetworkDao.java | 2 ++
.../java/com/cloud/network/dao/NetworkDaoImpl.java | 13 ++++++++++
.../com/cloud/network/dao/NetworkDaoImplTest.java | 24 ++++++++++++++++++
.../java/com/cloud/vpc/dao/MockNetworkDaoImpl.java | 5 ++++
5 files changed, 70 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 20b9bf125a4..9b1609c669a 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,26 @@ public class NetworkOrchestrator extends ManagerBase
implements NetworkOrchestra
}
}
+ private Map<Long, NetworkVO> batchLoadNetworksForNics(List<NicVO> nics) {
+ List<Long> networkIds =
nics.stream().map(NicVO::getNetworkId).distinct().collect(Collectors.toList());
+ Map<Long, NetworkVO> result = new HashMap<>(networkIds.size());
+ 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<NicVO> nics = _nicDao.listByVmId(vm.getId());
+ final Map<Long, NetworkVO> 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 +2279,13 @@ public class NetworkOrchestrator extends ManagerBase
implements NetworkOrchestra
}
final List<NicVO> nics = _nicDao.listByVmId(vm.getId());
final ReservationContext context = new
ReservationContextImpl(UUID.randomUUID().toString(), null, null);
+ final Map<Long, NetworkVO> 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());
@@ -2316,9 +2334,14 @@ public class NetworkOrchestrator extends ManagerBase
implements NetworkOrchestra
public void prepareAllNicsForMigration(final VirtualMachineProfile vm,
final DeployDestination dest) {
final List<NicVO> nics = _nicDao.listByVmId(vm.getId());
final ReservationContext context = new
ReservationContextImpl(UUID.randomUUID().toString(), null, null);
+ final Map<Long, NetworkVO> 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 fdca6e43f00..4ef1489a25c 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<NetworkVO,
Long>, StateDao<State,
List<NetworkVO> listByOwner(long ownerId);
+ List<NetworkVO> listByIds(List<Long> ids);
+
List<NetworkVO> listByGuestType(GuestType type);
List<NetworkVO> 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 5887498e094..1999a58fc84 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 class NetworkDaoImpl extends
GenericDaoBase<NetworkVO, Long>implements Ne
return listBy(sc);
}
+ @Override
+ public List<NetworkVO> listByIds(List<Long> ids) {
+ if (ids == null || ids.isEmpty()) {
+ return new ArrayList<>();
+ }
+ SearchBuilder<NetworkVO> sb = createSearchBuilder();
+ sb.and("id", sb.entity().getId(), Op.IN);
+ sb.done();
+ SearchCriteria<NetworkVO> 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/engine/schema/src/test/java/com/cloud/network/dao/NetworkDaoImplTest.java
b/engine/schema/src/test/java/com/cloud/network/dao/NetworkDaoImplTest.java
index ab5f4352105..b92066dbd94 100644
--- a/engine/schema/src/test/java/com/cloud/network/dao/NetworkDaoImplTest.java
+++ b/engine/schema/src/test/java/com/cloud/network/dao/NetworkDaoImplTest.java
@@ -26,11 +26,13 @@ import com.cloud.utils.db.TransactionLegacy;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnitRunner;
+import java.util.Collections;
import java.util.List;
@RunWith(MockitoJUnitRunner.class)
@@ -68,4 +70,26 @@ public class NetworkDaoImplTest {
txn.close();
}
}
+
+ @Test
+ public void listByIdsTestNullOrEmptyIdsReturnsEmptyList() {
+ NetworkDaoImpl networkDaoImplSpy = Mockito.spy(NetworkDaoImpl.class);
+
+ Assert.assertTrue(networkDaoImplSpy.listByIds(null).isEmpty());
+
Assert.assertTrue(networkDaoImplSpy.listByIds(Collections.emptyList()).isEmpty());
+ Mockito.verify(networkDaoImplSpy,
Mockito.never()).listBy(Mockito.any(SearchCriteria.class));
+ }
+
+ @Test
+ public void listByIdsTestBuildsInClause() {
+ NetworkDaoImpl networkDaoImplSpy = Mockito.spy(NetworkDaoImpl.class);
+
Mockito.doReturn(listNetworkVoMock).when(networkDaoImplSpy).listBy(Mockito.any(SearchCriteria.class));
+
+ List<NetworkVO> result = networkDaoImplSpy.listByIds(List.of(1L, 2L,
3L));
+
+ Assert.assertEquals(listNetworkVoMock, result);
+ ArgumentCaptor<SearchCriteria> scCaptor =
ArgumentCaptor.forClass(SearchCriteria.class);
+ Mockito.verify(networkDaoImplSpy).listBy(scCaptor.capture());
+ Assert.assertEquals("networks.id IN (?,?,?)",
scCaptor.getValue().getWhereClause().trim());
+ }
}
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 8a0bec56df7..b1d7e82839e 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 class MockNetworkDaoImpl extends
GenericDaoBase<NetworkVO, Long> implemen
public List<NetworkVO> listByNetworkDomainsAndDomainIds(Set<String>
uniqueNtwkDomains, Set<Long> domainIds) {
return List.of();
}
+
+ @Override
+ public List<NetworkVO> listByIds(List<Long> ids) {
+ return List.of();
+ }
}
