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>
diff --git a/api/src/main/java/org/apache/cloudstack/context/CallContext.java b/api/src/main/java/org/apache/cloudstack/context/CallContext.java
index 50049b3..ecc1099 100644
--- a/api/src/main/java/org/apache/cloudstack/context/CallContext.java
+++ b/api/src/main/java/org/apache/cloudstack/context/CallContext.java
@@ -92,6 +92,10 @@
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 @@
CallContext callContext = register(parent.getCallingUserId(), parent.getCallingAccountId());
callContext.setStartEventId(parent.getStartEventId());
callContext.setEventResourceType(eventResourceType);
+ callContext.putContextParameters(parent.getContextParameters());
return callContext;
}
diff --git a/server/src/main/java/com/cloud/resourcelimit/CheckedReservation.java b/server/src/main/java/com/cloud/resourcelimit/CheckedReservation.java
index 0075ef1..d0c20a3 100644
--- a/server/src/main/java/com/cloud/resourcelimit/CheckedReservation.java
+++ b/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 @@
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 @@
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 @@
@Override
public void close() throws Exception {
- if (this.reservation != null){
+ if (this.reservation != null) {
+ CallContext.current().removeContextParameter(getContextParameterKey());
reservationDao.remove(reservation.getId());
reservation = null;
}
diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
index 9efeed3..288315d 100644
--- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
+++ b/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 @@
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 @@
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
