From de8942644df26fdd97010f8eec1fddf7d97fc8ae Mon Sep 17 00:00:00 2001 From: Koushik Das Date: Wed, 16 Dec 2015 17:53:03 +0530 Subject: [PATCH] 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 --- .../RouterDeploymentDefinition.java | 47 +++++++++++++------ .../RouterDeploymentDefinitionTest.java | 36 ++++++++++++-- 2 files changed, 65 insertions(+), 18 deletions(-) diff --git a/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinition.java b/server/src/org/cloud/network/router/deployment/RouterDeploymentDefinition.java index 2d04a7e633f..19f80b94a43 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 deployVirtualRouter() throws InsufficientCapacityException, ConcurrentOperationException, ResourceUnavailableException { - findOrDeployVirtualRouter(); - return nwHelper.startRouters(this); } + private boolean isRouterDeployed() throws ResourceUnavailableException { + boolean isDeployed = true; + checkPreconditions(); + final List 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(); + 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 destinations = findDestinations(); - - for (final DeployDestination destination : destinations) { - dest = destination; - generateDeploymentPlan(); - executeDeployment(); + // dest has pod=null, for Basic Zone findOrDeployVRs for all Pods + final List 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); diff --git a/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTest.java b/server/test/org/cloud/network/router/deployment/RouterDeploymentDefinitionTest.java index 1570a2e15a9..eff16c16b8d 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(); @@ -524,6 +524,36 @@ public class RouterDeploymentDefinitionTest extends RouterDeploymentDefinitionTe fail(); } + @Test + public void testDeployVirtualRouterSkip() throws ConcurrentOperationException, + InsufficientCapacityException, ResourceUnavailableException { + + // Prepare + final List 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