From 238c55fb6e40220cbbf60b241a902cdc7ccf8d37 Mon Sep 17 00:00:00 2001 From: Koushik Das Date: Mon, 10 Dec 2012 13:56:51 +0530 Subject: [PATCH] CLOUDSTACK-596 : DeployVM command takes a lot of time to return job id Issue happens while deploying VM in advanced zone and 'networkids' parameter is not passed to deployVM command. In this case CS tries to identify a default guest network to be used for deploying VM. This logic is not optimized and latency increases with increase in user accounts and guest networks. Optimized logic for getting default network. Signed-off-by: Koushik Das Signed-off-by: Abhinandan Prateek --- .../com/cloud/network/NetworkManagerImpl.java | 13 +-------- .../src/com/cloud/network/dao/NetworkDao.java | 2 ++ .../com/cloud/network/dao/NetworkDaoImpl.java | 27 +++++++++++++++++-- .../src/com/cloud/vm/UserVmManagerImpl.java | 7 ++--- .../com/cloud/vpc/dao/MockNetworkDaoImpl.java | 9 +++++++ 5 files changed, 39 insertions(+), 19 deletions(-) diff --git a/server/src/com/cloud/network/NetworkManagerImpl.java b/server/src/com/cloud/network/NetworkManagerImpl.java index b3fdf32aa7f..3f8b08519ad 100755 --- a/server/src/com/cloud/network/NetworkManagerImpl.java +++ b/server/src/com/cloud/network/NetworkManagerImpl.java @@ -4540,18 +4540,7 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag @Override public List listNetworksForAccount(long accountId, long zoneId, Network.GuestType type) { - List accountNetworks = new ArrayList(); - List zoneNetworks = _networksDao.listByZone(zoneId); - - for (NetworkVO network : zoneNetworks) { - if (!isNetworkSystem(network)) { - if (network.getGuestType() == Network.GuestType.Shared || !_networksDao.listBy(accountId, network.getId()).isEmpty()) { - if (type == null || type == network.getGuestType()) { - accountNetworks.add(network); - } - } - } - } + List accountNetworks = _networksDao.listNetworksByAccount(accountId, zoneId, type, false); return accountNetworks; } diff --git a/server/src/com/cloud/network/dao/NetworkDao.java b/server/src/com/cloud/network/dao/NetworkDao.java index a2e37b7b33b..4079955c2e9 100644 --- a/server/src/com/cloud/network/dao/NetworkDao.java +++ b/server/src/com/cloud/network/dao/NetworkDao.java @@ -108,4 +108,6 @@ public interface NetworkDao extends GenericDao { long countVpcNetworks(long vpcId); + List listNetworksByAccount(long accountId, long zoneId, Network.GuestType type, boolean isSystem); + } diff --git a/server/src/com/cloud/network/dao/NetworkDaoImpl.java b/server/src/com/cloud/network/dao/NetworkDaoImpl.java index cbfec895b32..8228393f93f 100644 --- a/server/src/com/cloud/network/dao/NetworkDaoImpl.java +++ b/server/src/com/cloud/network/dao/NetworkDaoImpl.java @@ -72,8 +72,8 @@ public class NetworkDaoImpl extends GenericDaoBase implements N final SearchBuilder SourceNATSearch; final GenericSearchBuilder CountByZoneAndURI; final GenericSearchBuilder VpcNetworksCount; - - + final SearchBuilder OfferingAccountNetworkSearch; + ResourceTagsDaoImpl _tagsDao = ComponentLocator.inject(ResourceTagsDaoImpl.class); NetworkAccountDaoImpl _accountsDao = ComponentLocator.inject(NetworkAccountDaoImpl.class); NetworkDomainDaoImpl _domainsDao = ComponentLocator.inject(NetworkDomainDaoImpl.class); @@ -202,6 +202,17 @@ public class NetworkDaoImpl extends GenericDaoBase implements N VpcNetworksCount.select(null, Func.COUNT, VpcNetworksCount.entity().getId()); VpcNetworksCount.done(); + OfferingAccountNetworkSearch = createSearchBuilder(); + OfferingAccountNetworkSearch.select(null, Func.DISTINCT, OfferingAccountNetworkSearch.entity().getId()); + SearchBuilder ntwkOfferingJoin = _ntwkOffDao.createSearchBuilder(); + ntwkOfferingJoin.and("isSystem", ntwkOfferingJoin.entity().isSystemOnly(), Op.EQ); + OfferingAccountNetworkSearch.join("ntwkOfferingSearch", ntwkOfferingJoin, OfferingAccountNetworkSearch.entity().getNetworkOfferingId(), ntwkOfferingJoin.entity().getId(), JoinBuilder.JoinType.LEFT); + SearchBuilder ntwkAccountJoin = _accountsDao.createSearchBuilder(); + ntwkAccountJoin.and("accountId", ntwkAccountJoin.entity().getAccountId(), Op.EQ); + OfferingAccountNetworkSearch.join("ntwkAccountSearch", ntwkAccountJoin, OfferingAccountNetworkSearch.entity().getId(), ntwkAccountJoin.entity().getNetworkId(), JoinBuilder.JoinType.INNER); + OfferingAccountNetworkSearch.and("zoneId", OfferingAccountNetworkSearch.entity().getDataCenterId(), Op.EQ); + OfferingAccountNetworkSearch.and("type", OfferingAccountNetworkSearch.entity().getGuestType(), Op.EQ); + OfferingAccountNetworkSearch.done(); } @Override @@ -551,4 +562,16 @@ public class NetworkDaoImpl extends GenericDaoBase implements N sc.setParameters("vpcId", vpcId); return customSearch(sc, null).get(0); } + + @Override + public List listNetworksByAccount(long accountId, long zoneId, Network.GuestType type, boolean isSystem) { + SearchCriteria sc = OfferingAccountNetworkSearch.create(); + sc.setJoinParameters("ntwkOfferingSearch", "isSystem", isSystem); + sc.setJoinParameters("ntwkAccountSearch", "accountId", accountId); + sc.setParameters("zoneId", zoneId); + sc.setParameters("type", type); + + List networks = search(sc, null); + return networks; + } } diff --git a/server/src/com/cloud/vm/UserVmManagerImpl.java b/server/src/com/cloud/vm/UserVmManagerImpl.java index 837e3f7d146..eeb1c78a44f 100755 --- a/server/src/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/com/cloud/vm/UserVmManagerImpl.java @@ -2262,7 +2262,6 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager if (requiredOfferings.get(0).getState() == NetworkOffering.State.Enabled) { // get Virtual networks List virtualNetworks = _networkMgr.listNetworksForAccount(owner.getId(), zone.getId(), Network.GuestType.Isolated); - if (virtualNetworks.isEmpty()) { long physicalNetworkId = _networkMgr.findPhysicalNetworkId(zone.getId(), requiredOfferings.get(0).getTags(), requiredOfferings.get(0).getTrafficType()); // Validate physical network @@ -2278,7 +2277,7 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager } else if (virtualNetworks.size() > 1) { throw new InvalidParameterValueException("More than 1 default Isolated networks are found for account " + owner + "; please specify networkIds"); } else { - defaultNetwork = virtualNetworks.get(0); + defaultNetwork = _networkDao.findById(virtualNetworks.get(0).getId()); } } else { throw new InvalidParameterValueException("Required network offering id=" + requiredOfferings.get(0).getId() + " is not in " + NetworkOffering.State.Enabled); @@ -3701,7 +3700,6 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager if (requiredOfferings.get(0).getState() == NetworkOffering.State.Enabled) { // get Virtual networks List virtualNetworks = _networkMgr.listNetworksForAccount(newAccount.getId(), zone.getId(), Network.GuestType.Isolated); - if (virtualNetworks.isEmpty()) { long physicalNetworkId = _networkMgr.findPhysicalNetworkId(zone.getId(), requiredOfferings.get(0).getTags(), requiredOfferings.get(0).getTrafficType()); // Validate physical network @@ -3709,7 +3707,6 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager if (physicalNetwork == null) { throw new InvalidParameterValueException("Unable to find physical network with id: "+physicalNetworkId + " and tag: " +requiredOfferings.get(0).getTags()); } - s_logger.debug("Creating network for account " + newAccount + " from the network offering id=" + requiredOfferings.get(0).getId() + " as a part of deployVM process"); Network newNetwork = _networkMgr.createGuestNetwork(requiredOfferings.get(0).getId(), @@ -3720,7 +3717,7 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager throw new InvalidParameterValueException("More than 1 default Isolated networks are found " + "for account " + newAccount + "; please specify networkIds"); } else { - defaultNetwork = virtualNetworks.get(0); + defaultNetwork = _networkDao.findById(virtualNetworks.get(0).getId()); } } else { throw new InvalidParameterValueException("Required network offering id=" + requiredOfferings.get(0).getId() + " is not in " + NetworkOffering.State.Enabled); diff --git a/server/test/com/cloud/vpc/dao/MockNetworkDaoImpl.java b/server/test/com/cloud/vpc/dao/MockNetworkDaoImpl.java index 2a675b3a217..509d9c72f73 100644 --- a/server/test/com/cloud/vpc/dao/MockNetworkDaoImpl.java +++ b/server/test/com/cloud/vpc/dao/MockNetworkDaoImpl.java @@ -342,4 +342,13 @@ public class MockNetworkDaoImpl extends GenericDaoBase implemen return 0; } + /* (non-Javadoc) + * @see com.cloud.network.dao.NetworkDao#listNetworksByAccount(long, long, com.cloud.network.Network.GuestType, boolean) + */ + @Override + public List listNetworksByAccount(long accountId, long zoneId, GuestType type, boolean isSystem) { + // TODO Auto-generated method stub + return null; + } + }