lock only domains that have finite limits for checked reservation

engine/schema/src/main/java/com/cloud/configuration/dao/ResourceLimitDao.java

@@ -17,6 +17,7 @@
 package com.cloud.configuration.dao;
 
 import java.util.List;
+import java.util.Set;
 
 import com.cloud.configuration.Resource;
 import com.cloud.configuration.Resource.ResourceOwnerType;
@@ -36,4 +37,14 @@ public interface ResourceLimitDao extends GenericDao<ResourceLimitVO, Long> {
 
     long removeEntriesByOwner(Long ownerId, ResourceOwnerType ownerType);
     void removeResourceLimitsForNonMatchingTags(Long ownerId, ResourceOwnerType ownerType, List<Resource.ResourceType> types, List<String> tags);
+
+    /**
+     * Returns the explicit {@code resource_limit} rows owned by any of
+     * {@code domainIds} for the supplied ({@code type}, {@code tag}),
+     * regardless of their {@code max} value. Domains with no matching row
+     * are simply absent from the result — the caller resolves inheritance
+     * (nearest-ancestor lookup) and the tag -&gt; untagged fallback itself,
+     * mirroring {@code ResourceLimitManagerImpl#findCorrectResourceLimitForDomain}.
+     */
+    List<ResourceLimitVO> listByDomainIdsAndTypeAndTag(Set<Long> domainIds, Resource.ResourceType type, String tag);
 }

engine/schema/src/main/java/com/cloud/configuration/dao/ResourceLimitDaoImpl.java

@@ -18,6 +18,7 @@
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Set;
 
 
 import org.apache.commons.collections.CollectionUtils;
@@ -38,6 +39,8 @@ public class ResourceLimitDaoImpl extends GenericDaoBase<ResourceLimitVO, Long>
     private SearchBuilder<ResourceLimitVO> IdTypeTagSearch;
     private SearchBuilder<ResourceLimitVO> IdTypeNullTagSearch;
     private SearchBuilder<ResourceLimitVO> NonMatchingTagsSearch;
+    private SearchBuilder<ResourceLimitVO> DomainsLimitTagSearch;
+    private SearchBuilder<ResourceLimitVO> DomainsLimitNullTagSearch;
 
     public ResourceLimitDaoImpl() {
         IdTypeTagSearch = createSearchBuilder();
@@ -60,6 +63,18 @@ public ResourceLimitDaoImpl() {
         NonMatchingTagsSearch.and("tagNotNull", NonMatchingTagsSearch.entity().getTag(), SearchCriteria.Op.NNULL);
         NonMatchingTagsSearch.and("tags", NonMatchingTagsSearch.entity().getTag(), SearchCriteria.Op.NIN);
         NonMatchingTagsSearch.done();
+
+        DomainsLimitTagSearch = createSearchBuilder();
+        DomainsLimitTagSearch.and("type", DomainsLimitTagSearch.entity().getType(), SearchCriteria.Op.EQ);
+        DomainsLimitTagSearch.and("domainIds", DomainsLimitTagSearch.entity().getDomainId(), SearchCriteria.Op.IN);
+        DomainsLimitTagSearch.and("tag", DomainsLimitTagSearch.entity().getTag(), SearchCriteria.Op.EQ);
+        DomainsLimitTagSearch.done();
+
+        DomainsLimitNullTagSearch = createSearchBuilder();
+        DomainsLimitNullTagSearch.and("type", DomainsLimitNullTagSearch.entity().getType(), SearchCriteria.Op.EQ);
+        DomainsLimitNullTagSearch.and("domainIds", DomainsLimitNullTagSearch.entity().getDomainId(), SearchCriteria.Op.IN);
+        DomainsLimitNullTagSearch.and("tag", DomainsLimitNullTagSearch.entity().getTag(), SearchCriteria.Op.NULL);
+        DomainsLimitNullTagSearch.done();
     }
 
     @Override
@@ -150,4 +165,20 @@ public void removeResourceLimitsForNonMatchingTags(Long ownerId, ResourceOwnerTy
         }
         remove(sc);
     }
+
+    @Override
+    public List<ResourceLimitVO> listByDomainIdsAndTypeAndTag(Set<Long> domainIds, ResourceType type, String tag) {
+        if (CollectionUtils.isEmpty(domainIds)) {
+            return new ArrayList<>();
+        }
+        SearchCriteria<ResourceLimitVO> sc = (tag != null)
+                ? DomainsLimitTagSearch.create()
+                : DomainsLimitNullTagSearch.create();
+        sc.setParameters("type", type);
+        sc.setParameters("domainIds", domainIds.toArray());
+        if (tag != null) {
+            sc.setParameters("tag", tag);
+        }
+        return listBy(sc);
+    }
 }

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

@@ -20,6 +20,7 @@
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -577,12 +578,162 @@ protected void checkAccountResourceLimit(final Account account, final Project pr
     }
 
     protected List<ResourceCountVO> lockAccountAndOwnerDomainRows(long accountId, final ResourceType type, String tag) {
-        Set<Long> rowIdsToLock = _resourceCountDao.listAllRowsToUpdate(accountId, ResourceOwnerType.Account, type, tag);
+        Set<Long> rowIdsToLock = listRowsToLockForLimitCheck(accountId, type, tag);
+        if (rowIdsToLock.isEmpty()) {
+            return Collections.emptyList();
+        }
         SearchCriteria<ResourceCountVO> sc = ResourceCountSearch.create();
         sc.setParameters("id", rowIdsToLock.toArray());
         return _resourceCountDao.lockRows(sc, null, true);
     }
 
+    /**
+     * Returns the {@code resource_count} row IDs that need {@code FOR UPDATE}
+     * locks for a limit check: always the account row, plus ancestor-domain
+     * rows ONLY when their effective limit for ({@code type}, {@code tag})
+     * is finite. Ancestors with an UNLIMITED (or absent) effective limit are
+     * excluded — locking them would only create cross-tenant InnoDB row-lock
+     * contention with no effect on the check outcome (see
+     * {@link #checkDomainResourceLimit} which short-circuits when the
+     * domain's limit is {@link Resource#RESOURCE_UNLIMITED} and skips the
+     * ROOT domain entirely).
+     *
+     * <p>The account row is always included so two concurrent reservations
+     * against the same account still serialize at the InnoDB level.
+     *
+     * <p>"Effective limit" is resolved exactly like
+     * {@link #findCorrectResourceLimitForDomain}: a bulk query fetches every
+     * explicit tag-specific {@code resource_limit} row owned by any domain
+     * in the chain (a second bulk query for untagged rows only runs if some
+     * domain in the chain turns out to have no tag-specific row of its own
+     * or an ancestor's), then the chain is walked in memory from the
+     * account's domain up to (but excluding) ROOT, taking the nearest
+     * tag-specific row and falling back to the nearest untagged row only
+     * when no tag-specific row exists anywhere in the domain's own upward
+     * path. This correctly locks domains that inherit a finite limit from
+     * an ancestor's row (not just domains that own one), and domains that
+     * fall back to a finite untagged limit because no tag-specific limit is
+     * configured.
+     *
+     * <p>The final domain-row lookup is a single bulk
+     * {@link ResourceCountDao#findByOwnersAndTypeAndTag} call over exactly
+     * the domains found to have a finite effective limit — no more, no
+     * fewer, and no per-domain round trips.
+     *
+     * <p>If the global default for this type is itself finite (only possible
+     * for {@code primary_storage}/{@code secondary_storage} via
+     * {@code domainResourceLimitMap}), every ancestor inherits that finite
+     * default and we fall back to {@link ResourceCountDao#listAllRowsToUpdate}
+     * to lock the full chain. Same fallback applies when a tagged account
+     * row does not yet exist, so the create-on-miss materialization in
+     * {@code listAllRowsToUpdate} still fires.
+     *
+     * <p>This is a READ-PATH helper. The write path
+     * ({@link #updateResourceCountForAccount}) keeps using
+     * {@code listAllRowsToUpdate} directly so counts at unconstrained
+     * ancestors stay accurate for audit/aggregation.
+     */
+    protected Set<Long> listRowsToLockForLimitCheck(long accountId, ResourceType type, String tag) {
+        if (findDefaultResourceLimitForDomain(type) != Resource.RESOURCE_UNLIMITED) {
+            return _resourceCountDao.listAllRowsToUpdate(accountId, ResourceOwnerType.Account, type, tag);
+        }
+
+        Set<Long> rowIds = new HashSet<>();
+
+        ResourceCountVO accountRow = _resourceCountDao.findByOwnerAndTypeAndTag(accountId, ResourceOwnerType.Account, type, tag);
+        if (accountRow != null) {
+            rowIds.add(accountRow.getId());
+        } else if (StringUtils.isNotEmpty(tag)) {
+            // Preserve the tagged-row create-on-miss side effect from
+            // ResourceCountDaoImpl.listAllRowsToUpdate.
+            return _resourceCountDao.listAllRowsToUpdate(accountId, ResourceOwnerType.Account, type, tag);
+        }
+
+        AccountVO account = _accountDao.findByIdIncludingRemoved(accountId);
+        if (account == null) {
+            return rowIds;
+        }
+
+        List<Long> ancestorChain = getOrderedAncestorDomainIds(account.getDomainId());
+        Set<Long> finiteLimitDomainIds = findAncestorDomainsWithFiniteLimit(ancestorChain, type, tag);
+        if (!finiteLimitDomainIds.isEmpty()) {
+            List<ResourceCountVO> domainRows = _resourceCountDao.findByOwnersAndTypeAndTag(
+                    new ArrayList<>(finiteLimitDomainIds), ResourceOwnerType.Domain, type, tag);
+            for (ResourceCountVO row : domainRows) {
+                rowIds.add(row.getId());
+            }
+        }
+
+        return rowIds;
+    }
+
+    /**
+     * Returns the account's domain plus every ancestor up to (but excluding)
+     * ROOT, ordered from the account's own domain outward — the same order
+     * {@link #findCorrectResourceLimitForDomain} and
+     * {@link #checkDomainResourceLimit} walk.
+     */
+    private List<Long> getOrderedAncestorDomainIds(long domainId) {
+        List<Long> chain = new ArrayList<>();
+        Long currentId = domainId;
+        while (currentId != null && currentId != Domain.ROOT_DOMAIN) {
+            chain.add(currentId);
+            DomainVO domain = _domainDao.findById(currentId);
+            currentId = (domain != null) ? domain.getParent() : null;
+        }
+        return chain;
+    }
+
+    /**
+     * Returns the subset of {@code ancestorChain} whose effective limit for
+     * ({@code type}, {@code tag}) is finite, per
+     * {@link #findCorrectResourceLimitForDomain}'s nearest-row-with-fallback
+     * semantics. The untagged fallback query only runs if at least one
+     * domain in the chain actually lacks a tag-specific row.
+     */
+    private Set<Long> findAncestorDomainsWithFiniteLimit(List<Long> ancestorChain, ResourceType type, String tag) {
+        if (ancestorChain.isEmpty()) {
+            return Collections.emptySet();
+        }
+
+        Set<Long> chainSet = new HashSet<>(ancestorChain);
+        Map<Long, ResourceLimitVO> tagRowsByDomain = indexByDomainId(_resourceLimitDao.listByDomainIdsAndTypeAndTag(chainSet, type, tag));
+        Map<Long, ResourceLimitVO> untaggedRowsByDomain = null;
+
+        Set<Long> finiteLimitDomainIds = new HashSet<>();
+        for (int i = 0; i < ancestorChain.size(); i++) {
+            ResourceLimitVO nearestRow = findNearestRowFrom(ancestorChain, i, tagRowsByDomain);
+            if (nearestRow == null && StringUtils.isNotEmpty(tag)) {
+                if (untaggedRowsByDomain == null) {
+                    untaggedRowsByDomain = indexByDomainId(_resourceLimitDao.listByDomainIdsAndTypeAndTag(chainSet, type, null));
+                }
+                nearestRow = findNearestRowFrom(ancestorChain, i, untaggedRowsByDomain);
+            }
+            if (nearestRow != null && nearestRow.getMax().longValue() != Resource.RESOURCE_UNLIMITED) {
+                finiteLimitDomainIds.add(ancestorChain.get(i));
+            }
+        }
+        return finiteLimitDomainIds;
+    }
+
+    private static ResourceLimitVO findNearestRowFrom(List<Long> chain, int startIndex, Map<Long, ResourceLimitVO> rowsByDomain) {
+        for (int i = startIndex; i < chain.size(); i++) {
+            ResourceLimitVO row = rowsByDomain.get(chain.get(i));
+            if (row != null) {
+                return row;
+            }
+        }
+        return null;
+    }
+
+    private static Map<Long, ResourceLimitVO> indexByDomainId(List<ResourceLimitVO> rows) {
+        Map<Long, ResourceLimitVO> map = new HashMap<>();
+        for (ResourceLimitVO row : rows) {
+            map.put(row.getDomainId(), row);
+        }
+        return map;
+    }
+
     @Override
     public long findDefaultResourceLimitForDomain(ResourceType resourceType) {
         Long resourceLimit;