server: clear resource reservation and increment resource count in a transaction (#7724)

This PR addresses rare case of potential overlap of resource reservation and resource count.
For different resource types there could be some delay between incrementing of the resource count and clearing of the earlier done reservation. This may result in failures when there are parallel deployments happening.

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

Author: shwstppr

api/src/main/java/org/apache/cloudstack/context/CallContext.java

@@ -92,6 +92,10 @@ public void putContextParameter(Object key, Object value) {
         context.put(key, value);
     }
 
+    public void removeContextParameter(Object key) {
+        context.remove(key);
+    }
+
     /**
      * @param key any not null key object
      * @return the value of the key from context map
@@ -234,6 +238,7 @@ public static CallContext register(CallContext parent, ApiCommandResourceType ev
         CallContext callContext = register(parent.getCallingUserId(), parent.getCallingAccountId());
         callContext.setStartEventId(parent.getStartEventId());
         callContext.setEventResourceType(eventResourceType);
+        callContext.putContextParameters(parent.getContextParameters());
         return callContext;
     }
 

server/src/main/java/com/cloud/resourcelimit/CheckedReservation.java

@@ -18,17 +18,19 @@
 //
 package com.cloud.resourcelimit;
 
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.reservation.ReservationVO;
+import org.apache.cloudstack.reservation.dao.ReservationDao;
+import org.apache.cloudstack.user.ResourceReservation;
+import org.apache.log4j.Logger;
+import org.jetbrains.annotations.NotNull;
+
 import com.cloud.configuration.Resource.ResourceType;
 import com.cloud.exception.ResourceAllocationException;
 import com.cloud.user.Account;
 import com.cloud.user.ResourceLimitService;
 import com.cloud.utils.db.GlobalLock;
-import org.apache.cloudstack.user.ResourceReservation;
 import com.cloud.utils.exception.CloudRuntimeException;
-import org.apache.cloudstack.reservation.ReservationVO;
-import org.apache.cloudstack.reservation.dao.ReservationDao;
-import org.apache.log4j.Logger;
-import org.jetbrains.annotations.NotNull;
 
 
 public class CheckedReservation  implements AutoCloseable, ResourceReservation {
@@ -42,6 +44,10 @@ public class CheckedReservation  implements AutoCloseable, ResourceReservation {
     private Long amount;
     private ResourceReservation reservation;
 
+    private String getContextParameterKey() {
+        return String.format("%s-%s", ResourceReservation.class.getSimpleName(), resourceType.getName());
+    }
+
     /**
      * - check if adding a reservation is allowed
      * - create DB entry for reservation
@@ -70,6 +76,7 @@ public CheckedReservation(Account account, ResourceType resourceType, Long amoun
                     resourceLimitService.checkResourceLimit(account,resourceType,amount);
                     ReservationVO reservationVO = new ReservationVO(account.getAccountId(), account.getDomainId(), resourceType, amount);
                     this.reservation = reservationDao.persist(reservationVO);
+                    CallContext.current().putContextParameter(getContextParameterKey(), reservationVO.getId());
                 } catch (NullPointerException npe) {
                     throw new CloudRuntimeException("not enough means to check limits", npe);
                 } finally {
@@ -97,7 +104,8 @@ protected void setQuotaLimitLock(GlobalLock quotaLimitLock) {
 
     @Override
     public void close() throws Exception {
-        if (this.reservation != null){
+        if (this.reservation != null) {
+            CallContext.current().removeContextParameter(getContextParameterKey());
             reservationDao.remove(reservation.getId());
             reservation = null;
         }

server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java

@@ -16,6 +16,8 @@
 // under the License.
 package com.cloud.resourcelimit;
 
+import static com.cloud.utils.NumbersUtil.toHumanReadableSize;
+
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.EnumMap;
@@ -103,13 +105,11 @@
 import com.cloud.utils.db.TransactionCallbackWithExceptionNoReturn;
 import com.cloud.utils.db.TransactionStatus;
 import com.cloud.utils.exception.CloudRuntimeException;
-import com.cloud.vm.VirtualMachineManager;
 import com.cloud.vm.VirtualMachine.State;
+import com.cloud.vm.VirtualMachineManager;
 import com.cloud.vm.dao.UserVmDao;
 import com.cloud.vm.dao.VMInstanceDao;
 
-import static com.cloud.utils.NumbersUtil.toHumanReadableSize;
-
 @Component
 public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLimitService, Configurable {
     public static final Logger s_logger = Logger.getLogger(ResourceLimitManagerImpl.class);
@@ -173,6 +173,23 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim
     Map<ResourceType, Long> domainResourceLimitMap = new EnumMap<ResourceType, Long>(ResourceType.class);
     Map<ResourceType, Long> projectResourceLimitMap = new EnumMap<ResourceType, Long>(ResourceType.class);
 
+    protected void removeResourceReservationIfNeededAndIncrementResourceCount(final long accountId, final ResourceType type, final long numToIncrement) {
+        Transaction.execute(new TransactionCallbackWithExceptionNoReturn<CloudRuntimeException>() {
+            @Override
+            public void doInTransactionWithoutResult(TransactionStatus status) throws CloudRuntimeException {
+
+                Object obj = CallContext.current().getContextParameter(String.format("%s-%s", ResourceReservation.class.getSimpleName(), type.getName()));
+                if (obj instanceof Long) {
+                    reservationDao.remove((long)obj);
+                }
+                if (!updateResourceCountForAccount(accountId, type, true, numToIncrement)) {
+                    // we should fail the operation (resource creation) when failed to update the resource count
+                    throw new CloudRuntimeException("Failed to increment resource count of type " + type + " for account id=" + accountId);
+                }
+            }
+        });
+    }
+
     @Override
     public boolean start() {
         if (_resourceCountCheckInterval > 0) {
@@ -270,12 +287,8 @@ public void incrementResourceCount(long accountId, ResourceType type, Long... de
             return;
         }
 
-        long numToIncrement = (delta.length == 0) ? 1 : delta[0].longValue();
-
-        if (!updateResourceCountForAccount(accountId, type, true, numToIncrement)) {
-            // we should fail the operation (resource creation) when failed to update the resource count
-            throw new CloudRuntimeException("Failed to increment resource count of type " + type + " for account id=" + accountId);
-        }
+        final long numToIncrement = (delta.length == 0) ? 1 : delta[0].longValue();
+        removeResourceReservationIfNeededAndIncrementResourceCount(accountId, type, numToIncrement);
     }
 
     @Override