CLOUDSTACK-7828.Avoid marking IPs already in Allocated as Allocated again. Use row lock to ensure that prev state is either Allocating or Free. This will inturn avoid logging duplicate events

sedukull · sedukull · commit c916f30574eb · 2014-11-10T21:22:47.000+05:30
diff --git a/server/src/com/cloud/network/IpAddressManagerImpl.java b/server/src/com/cloud/network/IpAddressManagerImpl.java
@@ -807,32 +807,32 @@ public IPAddressVO doInTransaction(TransactionStatus status) throws Insufficient
     @Override
     public void markPublicIpAsAllocated(final IPAddressVO addr) {
 
-        assert (addr.getState() == IpAddress.State.Allocating || addr.getState() == IpAddress.State.Free) : "Unable to transition from state " + addr.getState() + " to "
-                + IpAddress.State.Allocated;
         Transaction.execute(new TransactionCallbackNoReturn() {
             @Override
             public void doInTransactionWithoutResult(TransactionStatus status) {
-        Account owner = _accountMgr.getAccount(addr.getAllocatedToAccountId());
-
-        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);
+            Account owner = _accountMgr.getAccount(addr.getAllocatedToAccountId());
+            synchronized (this) {
+                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);
+                            }
+                        }
+                    }
+                }
             }
         }
-            }
         });
     }
 
@@ -922,7 +922,6 @@ public PublicIp doInTransaction(TransactionStatus status) throws InsufficientAdd
     public boolean applyIpAssociations(Network network, boolean continueOnError) throws ResourceUnavailableException {
         List<IPAddressVO> userIps = _ipAddressDao.listByAssociatedNetwork(network.getId(), null);
         boolean success = true;
-
         // CloudStack will take a lazy approach to associate an acquired public IP to a network service provider as
         // it will not know what service an acquired IP will be used for. An IP is actually associated with a provider when first
         // rule is applied. Similarly when last rule on the acquired IP is revoked, IP is not associated with any provider
@@ -941,7 +940,6 @@ public boolean applyIpAssociations(Network network, boolean continueOnError) thr
                 }
             }
         }
-
         return success;
     }
 
