From 0cdb67fdc7ab7252e8abf1cc45f2d8a2d24bb044 Mon Sep 17 00:00:00 2001
From: alena <alena@cloud.com>
Date: Thu, 18 Aug 2011 10:13:03 -0700
Subject: [PATCH] bug 11167: no need to lock account when create security group
 to ensure that the group name is unique for account. If group already exists
 in the db, and we try to persist it again, mysql constraint (groupName,
 accountId) will fail and exception will be thrown. status 11167: resolved
 fixed

---
 .../security/SecurityGroupManagerImpl.java    | 29 +++++--------------
 .../com/cloud/user/AccountManagerImpl.java    | 14 ++++-----
 2 files changed, 14 insertions(+), 29 deletions(-)

diff --git a/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java b/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java
index 8b40666833a..e1c99130ce8 100755
--- a/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java
+++ b/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java
@@ -764,33 +764,18 @@ public class SecurityGroupManagerImpl implements SecurityGroupManager, SecurityG
         return createSecurityGroup(cmd.getSecurityGroupName(), cmd.getDescription(), owner.getDomainId(), owner.getAccountId(), owner.getAccountName());
     }
 
-    @DB
     @Override
     public SecurityGroupVO createSecurityGroup(String name, String description, Long domainId, Long accountId, String accountName) {
-        final Transaction txn = Transaction.currentTxn();
-        AccountVO account = null;
-        txn.start();
-        try {
-            account = _accountDao.acquireInLockTable(accountId); // to ensure duplicate group names are not created.
-            if (account == null) {
-                s_logger.warn("Failed to acquire lock on account");
-                return null;
-            }
-            SecurityGroupVO group = _securityGroupDao.findByAccountAndName(accountId, name);
-            if (group == null) {
-                group = new SecurityGroupVO(name, description, domainId, accountId);
-                group = _securityGroupDao.persist(group);
-            }
-
+        SecurityGroupVO group = _securityGroupDao.findByAccountAndName(accountId, name);
+        if (group == null) {
+            group = new SecurityGroupVO(name, description, domainId, accountId);
+            group = _securityGroupDao.persist(group);
             s_logger.debug("Created security group " + group + " for account id=" + accountId);
-            return group;
-        } finally {
-            if (account != null) {
-                _accountDao.releaseFromLockTable(accountId);
-            }
-            txn.commit();
+        } else {
+            s_logger.debug("Returning existing security group " + group + " for account id=" + accountId);
         }
 
+        return group;
     }
 
     @Override
diff --git a/server/src/com/cloud/user/AccountManagerImpl.java b/server/src/com/cloud/user/AccountManagerImpl.java
index 7c6f0e5dd2e..48cdcfe655a 100755
--- a/server/src/com/cloud/user/AccountManagerImpl.java
+++ b/server/src/com/cloud/user/AccountManagerImpl.java
@@ -54,10 +54,9 @@ import com.cloud.api.commands.UpdateResourceLimitCmd;
 import com.cloud.api.commands.UpdateUserCmd;
 import com.cloud.configuration.Config;
 import com.cloud.configuration.ConfigurationManager;
-import com.cloud.configuration.ResourceCount;
+import com.cloud.configuration.ResourceCount.ResourceType;
 import com.cloud.configuration.ResourceCountVO;
 import com.cloud.configuration.ResourceLimitVO;
-import com.cloud.configuration.ResourceCount.ResourceType;
 import com.cloud.configuration.dao.ConfigurationDao;
 import com.cloud.configuration.dao.ResourceCountDao;
 import com.cloud.configuration.dao.ResourceLimitDao;
@@ -68,11 +67,9 @@ import com.cloud.domain.DomainVO;
 import com.cloud.domain.dao.DomainDao;
 import com.cloud.event.ActionEvent;
 import com.cloud.event.EventTypes;
-import com.cloud.event.UsageEventVO;
 import com.cloud.event.dao.UsageEventDao;
 import com.cloud.exception.AgentUnavailableException;
 import com.cloud.exception.ConcurrentOperationException;
-import com.cloud.exception.InsufficientCapacityException;
 import com.cloud.exception.InvalidParameterValueException;
 import com.cloud.exception.OperationTimedoutException;
 import com.cloud.exception.PermissionDeniedException;
@@ -109,6 +106,7 @@ import com.cloud.utils.component.ComponentLocator;
 import com.cloud.utils.component.Inject;
 import com.cloud.utils.component.Manager;
 import com.cloud.utils.concurrency.NamedThreadFactory;
+import com.cloud.utils.db.DB;
 import com.cloud.utils.db.Filter;
 import com.cloud.utils.db.GlobalLock;
 import com.cloud.utils.db.SearchBuilder;
@@ -122,7 +120,6 @@ import com.cloud.vm.ReservationContextImpl;
 import com.cloud.vm.UserVmManager;
 import com.cloud.vm.UserVmVO;
 import com.cloud.vm.VMInstanceVO;
-import com.cloud.vm.VirtualMachine;
 import com.cloud.vm.VirtualMachineManager;
 import com.cloud.vm.dao.InstanceGroupDao;
 import com.cloud.vm.dao.UserVmDao;
@@ -1223,6 +1220,7 @@ public class AccountManagerImpl implements AccountManager, AccountService, Manag
 
     @Override
     @ActionEvent(eventType = EventTypes.EVENT_ACCOUNT_CREATE, eventDescription = "creating Account")
+    @DB
     public UserAccount createAccount(CreateAccountCmd cmd) {
         Long accountId = null;
         String username = cmd.getUsername();
@@ -1327,10 +1325,12 @@ public class AccountManagerImpl implements AccountManager, AccountService, Manag
         if (s_logger.isDebugEnabled()) {
             s_logger.debug("Creating user: " + username + ", account: " + accountName + " (id:" + accountId + "), domain: " + domainId + " timezone:" + timezone);
         }
-
+        
+        Transaction txn = Transaction.currentTxn();
+        txn.start();
         UserVO dbUser = _userDao.persist(user);
-
         _networkGroupMgr.createDefaultSecurityGroup(accountId);
+        txn.commit();
 
         if (!user.getPassword().equals(dbUser.getPassword())) {
             throw new CloudRuntimeException("The user " + username + " being creating is using a password that is different than what's in the db");