Repository: cloudstack Updated Branches: refs/heads/master ecac28ba4 -> 237bd46d5
CLOUDSTACK-9180: Optimize concurrent VM deployment operation on same network Check if VR needs to be allocated for a given network and only acquire lock if required Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/de894264 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/de894264 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/de894264 Branch: refs/heads/master Commit: de8942644df26fdd97010f8eec1fddf7d97fc8ae Parents: 94a1448 Author: Koushik Das <kous...@apache.org> Authored: Wed Dec 16 17:53:03 2015 +0530 Committer: Koushik Das <kous...@apache.org> Committed: Wed Dec 16 17:53:03 2015 +0530 ---------------------------------------------------------------------- .../deployment/RouterDeploymentDefinition.java | 49 +++++++++++++------- .../RouterDeploymentDefinitionTest.java | 36 ++++++++++++-- 2 files changed, 66 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/de894264/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinition.java ---------------------------------------------------------------------- diff --git a/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinition.java b/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinition.java index 2d04a7e..19f80b9 100644 --- a/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinition.java +++ b/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinition.java @@ -193,28 +193,45 @@ public class RouterDeploymentDefinition { } public List<DomainRouterVO> deployVirtualRouter() throws InsufficientCapacityException, ConcurrentOperationException, ResourceUnavailableException { - findOrDeployVirtualRouter(); - return nwHelper.startRouters(this); } + private boolean isRouterDeployed() throws ResourceUnavailableException { + boolean isDeployed = true; + checkPreconditions(); + final List<DeployDestination> destinations = findDestinations(); + for (final DeployDestination destination : destinations) { + dest = destination; + generateDeploymentPlan(); + planDeploymentRouters(); + if (getNumberOfRoutersToDeploy() > 0 && prepareDeployment()) { + isDeployed = false; + break; + } + } + return isDeployed; + } + @DB protected void findOrDeployVirtualRouter() throws ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException { - try { - lock(); - checkPreconditions(); - - // dest has pod=null, for Basic Zone findOrDeployVRs for all Pods - final List<DeployDestination> destinations = findDestinations(); - - for (final DeployDestination destination : destinations) { - dest = destination; - generateDeploymentPlan(); - executeDeployment(); + if (!isRouterDeployed()) { + try { + lock(); + // reset router list that got populated during isRouterDeployed() + routers.clear(); + checkPreconditions(); + + // dest has pod=null, for Basic Zone findOrDeployVRs for all Pods + final List<DeployDestination> destinations = findDestinations(); + for (final DeployDestination destination : destinations) { + dest = destination; + generateDeploymentPlan(); + executeDeployment(); + } + } finally { + unlock(); } - } finally { - unlock(); } } @@ -378,7 +395,7 @@ public class RouterDeploymentDefinition { final PhysicalNetworkServiceProvider provider = physicalProviderDao.findByServiceProvider(physicalNetworkId, type.toString()); if (provider == null) { - throw new CloudRuntimeException(String.format("Cannot find service provider %s in physical network %s", type.toString(), physicalNetworkId)); + throw new CloudRuntimeException(String.format("Cannot find service provider %s in physical network %s", type.toString(), physicalNetworkId)); } vrProvider = vrProviderDao.findByNspIdAndType(provider.getId(), type); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/de894264/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTest.java ---------------------------------------------------------------------- diff --git a/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTest.java b/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTest.java index 1570a2e..eff16c1 100644 --- a/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTest.java +++ b/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTest.java @@ -513,9 +513,9 @@ public class RouterDeploymentDefinitionTest extends RouterDeploymentDefinitionTe } finally { // Assert verify(deploymentUT, times(1)).lock(); - verify(deploymentUT, times(1)).checkPreconditions(); - verify(deploymentUT, times(1)).findDestinations(); - verify(deploymentUT, times(2)).generateDeploymentPlan(); + verify(deploymentUT, times(2)).checkPreconditions(); + verify(deploymentUT, times(2)).findDestinations(); + verify(deploymentUT, times(3)).generateDeploymentPlan(); verify(deploymentUT, times(2)).executeDeployment(); //verify(deploymentUT, times(2)).planDeploymentRouters(); verify(deploymentUT, times(1)).unlock(); @@ -525,6 +525,36 @@ public class RouterDeploymentDefinitionTest extends RouterDeploymentDefinitionTe } @Test + public void testDeployVirtualRouterSkip() throws ConcurrentOperationException, + InsufficientCapacityException, ResourceUnavailableException { + + // Prepare + final List<DeployDestination> mockDestinations = new ArrayList<>(); + mockDestinations.add(mock(DeployDestination.class)); + mockDestinations.add(mock(DeployDestination.class)); + + final RouterDeploymentDefinition deploymentUT = spy(deployment); + doNothing().when(deploymentUT).checkPreconditions(); + doReturn(mockDestinations).when(deploymentUT).findDestinations(); + doNothing().when(deploymentUT).planDeploymentRouters(); + doNothing().when(deploymentUT).generateDeploymentPlan(); + doReturn(0).when(deploymentUT).getNumberOfRoutersToDeploy(); + + // Execute + try { + deploymentUT.findOrDeployVirtualRouter(); + } finally { + // Assert + verify(deploymentUT, times(0)).lock(); // lock shouldn't be acquired when VR already present + verify(deploymentUT, times(1)).checkPreconditions(); + verify(deploymentUT, times(1)).findDestinations(); + verify(deploymentUT, times(2)).generateDeploymentPlan(); + verify(deploymentUT, times(0)).executeDeployment(); // no need to deploy VR as already present + verify(deploymentUT, times(0)).unlock(); // same as lock + } + } + + @Test public void testGetNumberOfRoutersToDeploy() { // Prepare deployment.routers = new ArrayList<>(); // Empty list
