rhtyd commented on a change in pull request #1762: CLOUDSTACK-9595 Transactions 
are not getting retried in case of datab?
URL: https://github.com/apache/cloudstack/pull/1762#discussion_r158483275
 
 

 ##########
 File path: server/src/com/cloud/network/IpAddressManagerImpl.java
 ##########
 @@ -836,35 +835,51 @@ public IPAddressVO doInTransaction(TransactionStatus 
status) throws Insufficient
     @DB
     @Override
     public void markPublicIpAsAllocated(final IPAddressVO addr) {
-
-        Transaction.execute(new TransactionCallbackNoReturn() {
-            @Override
-            public void doInTransactionWithoutResult(TransactionStatus status) 
{
-                Account owner = 
_accountMgr.getAccount(addr.getAllocatedToAccountId());
-                synchronized (this) {
+        synchronized (_allocatedLock) {
+            Transaction.execute(new TransactionCallbackNoReturn() {
+                @Override
+                public void doInTransactionWithoutResult(TransactionStatus 
status) {
+                    Account owner = 
_accountMgr.getAccount(addr.getAllocatedToAccountId());
                     if (_ipAddressDao.lockRow(addr.getId(), true) != null) {
                         IPAddressVO userIp = 
_ipAddressDao.findById(addr.getId());
                         if (userIp.getState() == IpAddress.State.Allocating || 
addr.getState() == IpAddress.State.Free) {
                             addr.setState(IpAddress.State.Allocated);
-                            _ipAddressDao.update(addr.getId(), addr);
-                            // Save usage event
-                            if (owner.getAccountId() != 
Account.ACCOUNT_ID_SYSTEM) {
-                                VlanVO vlan = 
_vlanDao.findById(addr.getVlanId());
-                                String guestType = 
vlan.getVlanType().toString();
-                                if (!isIpDedicated(addr)) {
-                                    
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NET_IP_ASSIGN, 
owner.getId(), addr.getDataCenterId(), addr.getId(),
-                                            addr.getAddress().toString(),
-                                            addr.isSourceNat(), guestType, 
addr.getSystem(), addr.getClass().getName(), addr.getUuid());
-                                }
-                                if (updateIpResourceCount(addr)) {
-                                    
_resourceLimitMgr.incrementResourceCount(owner.getId(), ResourceType.public_ip);
+                            if (_ipAddressDao.update(addr.getId(), addr)) {
+                                // Save usage event
+                                if (owner.getAccountId() != 
Account.ACCOUNT_ID_SYSTEM) {
+                                    VlanVO vlan = 
_vlanDao.findById(addr.getVlanId());
+                                    String guestType = 
vlan.getVlanType().toString();
+                                    if (!isIpDedicated(addr)) {
+                                        
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NET_IP_ASSIGN, 
owner.getId(), addr.getDataCenterId(), addr.getId(),
+                                                addr.getAddress().toString(),
+                                                addr.isSourceNat(), guestType, 
addr.getSystem(), addr.getClass().getName(), addr.getUuid());
+                                    }
+                                    if (updateIpResourceCount(addr)) {
+                                        
_resourceLimitMgr.incrementResourceCount(owner.getId(), ResourceType.public_ip);
+                                    }
                                 }
                             }
                         }
                     }
                 }
-            }
-        });
+            });
+        }
+    }
+
+    @DB
+    private void markPublicIpAsAllocating(final IPAddressVO addr) {
+        synchronized (_allocatingLock) {
+            Transaction.execute(new TransactionCallbackNoReturn() {
+                @Override
+                public void doInTransactionWithoutResult(TransactionStatus 
status) {
+
+                    if (_ipAddressDao.lockRow(addr.getId(), true) != null) {
 
 Review comment:
   @yvsubhash as explained I could reproduce in an environment a case where 
both cpvm and ssvm got the same public IP. Should we add a check to make sure 
that you're only marking ip as allocating when they are Free?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to