fix domain lock set to reflect tag->untagged fallback and limit inheritance

listRowsToLockForLimitCheck previously only locked ancestor domains that
owned an explicit finite resource_limit row for the exact tag, so it
missed domains that inherit a finite limit from an ancestor's row, and
domains that fall back to the untagged limit when no tag-specific limit
is configured (per findCorrectResourceLimitForDomain). Either gap let
concurrent reservations bypass domain-level serialization.

Replace the set-based approximation with an in-memory walk of the
account's domain chain that replicates findCorrectResourceLimitForDomain's
nearest-row-with-fallback semantics, using bulk queries (covered by
existing indexes) to fetch the chain's resource_limit rows and to resolve
the final resource_count rows to lock.

Author: Mark Armstrong

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

@@ -39,12 +39,12 @@ public interface ResourceLimitDao extends GenericDao<ResourceLimitVO, Long> {
     void removeResourceLimitsForNonMatchingTags(Long ownerId, ResourceOwnerType ownerType, List<Resource.ResourceType> types, List<String> tags);
 
     /**
-     * Returns the subset of {@code domainIds} that have an explicit
-     * {@code resource_limit} row whose {@code max} is not
-     * {@link Resource#RESOURCE_UNLIMITED} for the supplied
-     * ({@code type}, {@code tag}). Domains that rely on the global default
-     * are NOT returned — the caller checks
-     * {@code findDefaultResourceLimitForDomain} separately.
+     * 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}.
      */
-    Set<Long> listDomainIdsWithFiniteLimit(Set<Long> domainIds, Resource.ResourceType type, String tag);
+    List<ResourceLimitVO> listByDomainIdsAndTypeAndTag(Set<Long> domainIds, Resource.ResourceType type, String tag);
 }

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

@@ -17,10 +17,8 @@
 package com.cloud.configuration.dao;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 import java.util.Set;
-import java.util.stream.Collectors;
 
 
 import org.apache.commons.collections.CollectionUtils;
@@ -41,8 +39,8 @@ public class ResourceLimitDaoImpl extends GenericDaoBase<ResourceLimitVO, Long>
     private SearchBuilder<ResourceLimitVO> IdTypeTagSearch;
     private SearchBuilder<ResourceLimitVO> IdTypeNullTagSearch;
     private SearchBuilder<ResourceLimitVO> NonMatchingTagsSearch;
-    private SearchBuilder<ResourceLimitVO> DomainsFiniteLimitTagSearch;
-    private SearchBuilder<ResourceLimitVO> DomainsFiniteLimitNullTagSearch;
+    private SearchBuilder<ResourceLimitVO> DomainsLimitTagSearch;
+    private SearchBuilder<ResourceLimitVO> DomainsLimitNullTagSearch;
 
     public ResourceLimitDaoImpl() {
         IdTypeTagSearch = createSearchBuilder();
@@ -66,19 +64,17 @@ public ResourceLimitDaoImpl() {
         NonMatchingTagsSearch.and("tags", NonMatchingTagsSearch.entity().getTag(), SearchCriteria.Op.NIN);
         NonMatchingTagsSearch.done();
 
-        DomainsFiniteLimitTagSearch = createSearchBuilder();
-        DomainsFiniteLimitTagSearch.and("type", DomainsFiniteLimitTagSearch.entity().getType(), SearchCriteria.Op.EQ);
-        DomainsFiniteLimitTagSearch.and("domainIds", DomainsFiniteLimitTagSearch.entity().getDomainId(), SearchCriteria.Op.IN);
-        DomainsFiniteLimitTagSearch.and("tag", DomainsFiniteLimitTagSearch.entity().getTag(), SearchCriteria.Op.EQ);
-        DomainsFiniteLimitTagSearch.and("max", DomainsFiniteLimitTagSearch.entity().getMax(), SearchCriteria.Op.NEQ);
-        DomainsFiniteLimitTagSearch.done();
-
-        DomainsFiniteLimitNullTagSearch = createSearchBuilder();
-        DomainsFiniteLimitNullTagSearch.and("type", DomainsFiniteLimitNullTagSearch.entity().getType(), SearchCriteria.Op.EQ);
-        DomainsFiniteLimitNullTagSearch.and("domainIds", DomainsFiniteLimitNullTagSearch.entity().getDomainId(), SearchCriteria.Op.IN);
-        DomainsFiniteLimitNullTagSearch.and("tag", DomainsFiniteLimitNullTagSearch.entity().getTag(), SearchCriteria.Op.NULL);
-        DomainsFiniteLimitNullTagSearch.and("max", DomainsFiniteLimitNullTagSearch.entity().getMax(), SearchCriteria.Op.NEQ);
-        DomainsFiniteLimitNullTagSearch.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
@@ -171,21 +167,18 @@ public void removeResourceLimitsForNonMatchingTags(Long ownerId, ResourceOwnerTy
     }
 
     @Override
-    public Set<Long> listDomainIdsWithFiniteLimit(Set<Long> domainIds, ResourceType type, String tag) {
+    public List<ResourceLimitVO> listByDomainIdsAndTypeAndTag(Set<Long> domainIds, ResourceType type, String tag) {
         if (CollectionUtils.isEmpty(domainIds)) {
-            return Collections.emptySet();
+            return new ArrayList<>();
         }
         SearchCriteria<ResourceLimitVO> sc = (tag != null)
-                ? DomainsFiniteLimitTagSearch.create()
-                : DomainsFiniteLimitNullTagSearch.create();
+                ? DomainsLimitTagSearch.create()
+                : DomainsLimitNullTagSearch.create();
         sc.setParameters("type", type);
         sc.setParameters("domainIds", domainIds.toArray());
         if (tag != null) {
             sc.setParameters("tag", tag);
         }
-        sc.setParameters("max", (long) Resource.RESOURCE_UNLIMITED);
-        return listBy(sc).stream()
-                .map(ResourceLimitVO::getDomainId)
-                .collect(Collectors.toSet());
+        return listBy(sc);
     }
 }

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

@@ -591,7 +591,7 @@ protected List<ResourceCountVO> lockAccountAndOwnerDomainRows(long accountId, fi
      * 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) explicit limit are
+     * 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
@@ -601,6 +601,25 @@ protected List<ResourceCountVO> lockAccountAndOwnerDomainRows(long accountId, fi
      * <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
@@ -634,18 +653,87 @@ protected Set<Long> listRowsToLockForLimitCheck(long accountId, ResourceType typ
         if (account == null) {
             return rowIds;
         }
-        Set<Long> ancestorDomainIds = _domainDao.getDomainParentIds(account.getDomainId());
-        Set<Long> finiteLimitDomainIds = _resourceLimitDao.listDomainIdsWithFiniteLimit(ancestorDomainIds, type, tag);
 
-        for (Long ancestorDomainId : finiteLimitDomainIds) {
-            ResourceCountVO row = _resourceCountDao.findByOwnerAndTypeAndTag(ancestorDomainId, ResourceOwnerType.Domain, type, tag);
-            if (row != null) {
+        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;