This is an automated email from the ASF dual-hosted git repository.
nvazquez pushed a commit to branch 4.20
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.20 by this push:
new b8a44e94951 lock only domains that have finite limits for checked
reservation (#14169)
b8a44e94951 is described below
commit b8a44e949510d8fd76fbfaf9ceea235a548b57d6
Author: sb-abhish3k <[email protected]>
AuthorDate: Wed Oct 7 00:39:19 2026 +0530
lock only domains that have finite limits for checked reservation (#14169)
* lock on only domains that have finite limits for checked reservation
* 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.
---------
Co-authored-by: Mark Armstrong <[email protected]>
---
.../cloud/configuration/dao/ResourceLimitDao.java | 11 +
.../configuration/dao/ResourceLimitDaoImpl.java | 31 +++
.../resourcelimit/ResourceLimitManagerImpl.java | 153 +++++++++++-
.../ResourceLimitManagerImplTest.java | 256 +++++++++++++++++++++
4 files changed, 450 insertions(+), 1 deletion(-)
diff --git
a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceLimitDao.java
b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceLimitDao.java
index 7cdc2aacc3b..6cbfadf7ac5 100644
---
a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceLimitDao.java
+++
b/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 -> untagged fallback itself,
+ * mirroring {@code
ResourceLimitManagerImpl#findCorrectResourceLimitForDomain}.
+ */
+ List<ResourceLimitVO> listByDomainIdsAndTypeAndTag(Set<Long> domainIds,
Resource.ResourceType type, String tag);
}
diff --git
a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceLimitDaoImpl.java
b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceLimitDaoImpl.java
index 96523ba9bea..c649b038d97 100644
---
a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceLimitDaoImpl.java
+++
b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceLimitDaoImpl.java
@@ -18,6 +18,7 @@ package com.cloud.configuration.dao;
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 class ResourceLimitDaoImpl extends
GenericDaoBase<ResourceLimitVO, Long>
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 class ResourceLimitDaoImpl extends
GenericDaoBase<ResourceLimitVO, Long>
}
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);
+ }
}
diff --git
a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
index 6d2ec103ca2..210c70e607a 100644
--- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
+++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
@@ -20,6 +20,7 @@ import static com.cloud.utils.NumbersUtil.toHumanReadableSize;
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 @@ public class ResourceLimitManagerImpl extends
ManagerBase implements ResourceLim
}
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;
diff --git
a/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java
b/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java
index bdc6620c490..78d69365abb 100644
---
a/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java
+++
b/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java
@@ -19,9 +19,12 @@ package com.cloud.resourcelimit;
import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collections;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import org.apache.cloudstack.api.response.AccountResponse;
import org.apache.cloudstack.api.response.DomainResponse;
@@ -574,6 +577,259 @@ public class ResourceLimitManagerImplTest extends
TestCase {
}
}
+ private ResourceCountVO mockCountRow(long id) {
+ ResourceCountVO row = Mockito.mock(ResourceCountVO.class);
+ Mockito.when(row.getId()).thenReturn(id);
+ return row;
+ }
+
+ private ResourceLimitVO mockLimitRow(long domainId, long max) {
+ ResourceLimitVO row = Mockito.mock(ResourceLimitVO.class);
+ Mockito.when(row.getDomainId()).thenReturn(domainId);
+ Mockito.when(row.getMax()).thenReturn(max);
+ return row;
+ }
+
+ /**
+ * Mocks domainDao.findById() so that walking getParent() from
+ * chainWithRootLast[0] visits chainWithRootLast in order, ending at
+ * Domain.ROOT_DOMAIN (the last element).
+ */
+ private void mockDomainParentChain(long... chainWithRootLast) {
+ for (int i = 0; i < chainWithRootLast.length - 1; i++) {
+ DomainVO domain = Mockito.mock(DomainVO.class);
+ Mockito.when(domain.getParent()).thenReturn(chainWithRootLast[i +
1]);
+
Mockito.when(domainDao.findById(chainWithRootLast[i])).thenReturn(domain);
+ }
+ }
+
+ @Test
+ public void
testListRowsToLockForLimitCheckOnlyAccountDomainHasFiniteLimit() {
+ // Account-domain has a finite limit; zone-domain (parent) has none
(unlimited).
+ // Expected lock set = {account row, account-domain row}.
+ long accountId = 1L;
+ long accountDomainId = 7L;
+ long zoneDomainId = 3L;
+ Resource.ResourceType type = Resource.ResourceType.volume;
+ AccountVO account = Mockito.mock(AccountVO.class);
+ Mockito.when(account.getDomainId()).thenReturn(accountDomainId);
+
Mockito.when(accountDao.findByIdIncludingRemoved(accountId)).thenReturn(account);
+ mockDomainParentChain(accountDomainId, zoneDomainId,
Domain.ROOT_DOMAIN);
+
+ ResourceCountVO accountRow = mockCountRow(100L);
+ ResourceCountVO accountDomainRow = mockCountRow(200L);
+ Mockito.when(resourceCountDao.findByOwnerAndTypeAndTag(accountId,
Resource.ResourceOwnerType.Account, type, null))
+ .thenReturn(accountRow);
+
+ Set<Long> chainSet = new HashSet<>(Arrays.asList(accountDomainId,
zoneDomainId));
+ ResourceLimitVO accountDomainLimitRow = mockLimitRow(accountDomainId,
10L);
+ Mockito.when(resourceLimitDao.listByDomainIdsAndTypeAndTag(chainSet,
type, null))
+ .thenReturn(Collections.singletonList(accountDomainLimitRow));
+
Mockito.when(resourceCountDao.findByOwnersAndTypeAndTag(Mockito.anyList(),
Mockito.eq(Resource.ResourceOwnerType.Domain), Mockito.eq(type),
Mockito.isNull()))
+ .thenReturn(Collections.singletonList(accountDomainRow));
+
+ Set<Long> rowIds =
resourceLimitManager.listRowsToLockForLimitCheck(accountId, type, null);
+
+ Assert.assertEquals(new HashSet<>(Arrays.asList(100L, 200L)), rowIds);
+ Mockito.verify(resourceCountDao,
Mockito.never()).listAllRowsToUpdate(Mockito.anyLong(), Mockito.any(),
Mockito.any(), Mockito.any());
+ Mockito.verify(resourceCountDao, Mockito.times(1))
+
.findByOwnersAndTypeAndTag(Mockito.eq(Collections.singletonList(accountDomainId)),
Mockito.eq(Resource.ResourceOwnerType.Domain), Mockito.eq(type),
Mockito.isNull());
+ }
+
+ @Test
+ public void testListRowsToLockForLimitCheckMultipleFiniteAncestors() {
+ // Both ancestor-domain rows have finite limits; both must be locked.
+ long accountId = 1L;
+ long accountDomainId = 7L;
+ long zoneDomainId = 3L;
+ Resource.ResourceType type = Resource.ResourceType.cpu;
+ AccountVO account = Mockito.mock(AccountVO.class);
+ Mockito.when(account.getDomainId()).thenReturn(accountDomainId);
+
Mockito.when(accountDao.findByIdIncludingRemoved(accountId)).thenReturn(account);
+ mockDomainParentChain(accountDomainId, zoneDomainId,
Domain.ROOT_DOMAIN);
+
+ ResourceCountVO accountRow = mockCountRow(100L);
+ ResourceCountVO accountDomainRow = mockCountRow(200L);
+ ResourceCountVO zoneDomainRow = mockCountRow(300L);
+ Mockito.when(resourceCountDao.findByOwnerAndTypeAndTag(accountId,
Resource.ResourceOwnerType.Account, type, null))
+ .thenReturn(accountRow);
+
+ Set<Long> chainSet = new HashSet<>(Arrays.asList(accountDomainId,
zoneDomainId));
+ ResourceLimitVO accountDomainLimitRow = mockLimitRow(accountDomainId,
10L);
+ ResourceLimitVO zoneDomainLimitRow = mockLimitRow(zoneDomainId, 20L);
+ Mockito.when(resourceLimitDao.listByDomainIdsAndTypeAndTag(chainSet,
type, null))
+ .thenReturn(Arrays.asList(accountDomainLimitRow,
zoneDomainLimitRow));
+
Mockito.when(resourceCountDao.findByOwnersAndTypeAndTag(Mockito.anyList(),
Mockito.eq(Resource.ResourceOwnerType.Domain), Mockito.eq(type),
Mockito.isNull()))
+ .thenReturn(Arrays.asList(accountDomainRow, zoneDomainRow));
+
+ Set<Long> rowIds =
resourceLimitManager.listRowsToLockForLimitCheck(accountId, type, null);
+
+ Assert.assertEquals(new HashSet<>(Arrays.asList(100L, 200L, 300L)),
rowIds);
+ }
+
+ @Test
+ public void
testListRowsToLockForLimitCheckInheritsFiniteLimitFromAncestorRow() {
+ // Account-domain has NO explicit row of its own, but inherits a
finite limit from
+ // its parent (zone-domain)'s explicit row. Both must be locked: the
zone-domain
+ // because it owns the finite row, and the account-domain because its
own effective
+ // limit (via inheritance) is also finite.
+ long accountId = 1L;
+ long accountDomainId = 7L;
+ long zoneDomainId = 3L;
+ Resource.ResourceType type = Resource.ResourceType.cpu;
+ AccountVO account = Mockito.mock(AccountVO.class);
+ Mockito.when(account.getDomainId()).thenReturn(accountDomainId);
+
Mockito.when(accountDao.findByIdIncludingRemoved(accountId)).thenReturn(account);
+ mockDomainParentChain(accountDomainId, zoneDomainId,
Domain.ROOT_DOMAIN);
+
+ ResourceCountVO accountRow = mockCountRow(100L);
+ ResourceCountVO accountDomainRow = mockCountRow(200L);
+ ResourceCountVO zoneDomainRow = mockCountRow(300L);
+ Mockito.when(resourceCountDao.findByOwnerAndTypeAndTag(accountId,
Resource.ResourceOwnerType.Account, type, null))
+ .thenReturn(accountRow);
+
+ Set<Long> chainSet = new HashSet<>(Arrays.asList(accountDomainId,
zoneDomainId));
+ // Only the zone-domain owns an explicit row; the account-domain has
none.
+ ResourceLimitVO zoneDomainLimitRow = mockLimitRow(zoneDomainId, 20L);
+ Mockito.when(resourceLimitDao.listByDomainIdsAndTypeAndTag(chainSet,
type, null))
+ .thenReturn(Collections.singletonList(zoneDomainLimitRow));
+
Mockito.when(resourceCountDao.findByOwnersAndTypeAndTag(Mockito.anyList(),
Mockito.eq(Resource.ResourceOwnerType.Domain), Mockito.eq(type),
Mockito.isNull()))
+ .thenReturn(Arrays.asList(accountDomainRow, zoneDomainRow));
+
+ Set<Long> rowIds =
resourceLimitManager.listRowsToLockForLimitCheck(accountId, type, null);
+
+ Assert.assertEquals(new HashSet<>(Arrays.asList(100L, 200L, 300L)),
rowIds);
+ }
+
+ @Test
+ public void testListRowsToLockForLimitCheckAllUnlimited() {
+ // No ancestor has a finite limit; only the account row is locked.
+ long accountId = 1L;
+ long accountDomainId = 7L;
+ Resource.ResourceType type = Resource.ResourceType.cpu;
+ AccountVO account = Mockito.mock(AccountVO.class);
+ Mockito.when(account.getDomainId()).thenReturn(accountDomainId);
+
Mockito.when(accountDao.findByIdIncludingRemoved(accountId)).thenReturn(account);
+ mockDomainParentChain(accountDomainId, Domain.ROOT_DOMAIN);
+
+ ResourceCountVO accountRow = mockCountRow(100L);
+ Mockito.when(resourceCountDao.findByOwnerAndTypeAndTag(accountId,
Resource.ResourceOwnerType.Account, type, null))
+ .thenReturn(accountRow);
+
+ Set<Long> chainSet = Collections.singleton(accountDomainId);
+ Mockito.when(resourceLimitDao.listByDomainIdsAndTypeAndTag(chainSet,
type, null))
+ .thenReturn(Collections.emptyList());
+
+ Set<Long> rowIds =
resourceLimitManager.listRowsToLockForLimitCheck(accountId, type, null);
+
+ Assert.assertEquals(Collections.singleton(100L), rowIds);
+ Mockito.verify(resourceCountDao,
Mockito.never()).findByOwnersAndTypeAndTag(Mockito.any(), Mockito.any(),
Mockito.any(), Mockito.any());
+ }
+
+ @Test
+ public void testListRowsToLockForLimitCheckFiniteGlobalDefaultFallsBack() {
+ // primary_storage with a finite global default → fall back to
listAllRowsToUpdate
+ // to preserve upstream behavior (every ancestor inherits the finite
default).
+ long accountId = 1L;
+ Resource.ResourceType type = Resource.ResourceType.primary_storage;
+ Set<Long> fullChain = new HashSet<>(Arrays.asList(100L, 200L, 300L,
400L));
+
+
Mockito.doReturn(1024L).when(resourceLimitManager).findDefaultResourceLimitForDomain(type);
+ Mockito.when(resourceCountDao.listAllRowsToUpdate(accountId,
Resource.ResourceOwnerType.Account, type, null))
+ .thenReturn(fullChain);
+
+ Set<Long> rowIds =
resourceLimitManager.listRowsToLockForLimitCheck(accountId, type, null);
+
+ Assert.assertEquals(fullChain, rowIds);
+ Mockito.verify(resourceLimitDao,
Mockito.never()).listByDomainIdsAndTypeAndTag(Mockito.anySet(), Mockito.any(),
Mockito.any());
+ }
+
+ @Test
+ public void
testListRowsToLockForLimitCheckTaggedAccountRowMissingFallsBack() {
+ // Tagged limit, account row not yet materialized → fall back to
listAllRowsToUpdate
+ // so the create-on-miss tagged-row materialization still fires.
+ long accountId = 1L;
+ Resource.ResourceType type = Resource.ResourceType.cpu;
+ String tag = hostTags.get(0);
+ Set<Long> fullChain = new HashSet<>(Arrays.asList(101L, 201L));
+
+ Mockito.when(resourceCountDao.findByOwnerAndTypeAndTag(accountId,
Resource.ResourceOwnerType.Account, type, tag))
+ .thenReturn(null);
+ Mockito.when(resourceCountDao.listAllRowsToUpdate(accountId,
Resource.ResourceOwnerType.Account, type, tag))
+ .thenReturn(fullChain);
+
+ Set<Long> rowIds =
resourceLimitManager.listRowsToLockForLimitCheck(accountId, type, tag);
+
+ Assert.assertEquals(fullChain, rowIds);
+ Mockito.verify(resourceCountDao,
Mockito.times(1)).listAllRowsToUpdate(accountId,
Resource.ResourceOwnerType.Account, type, tag);
+ Mockito.verify(resourceLimitDao,
Mockito.never()).listByDomainIdsAndTypeAndTag(Mockito.anySet(), Mockito.any(),
Mockito.any());
+ }
+
+ @Test
+ public void
testListRowsToLockForLimitCheckTagFallsBackToUntaggedWhenNoTagRowExists() {
+ // No domain in the chain has ANY explicit tag-specific row, but the
account-domain
+ // has a finite UNTAGGED limit. findCorrectResourceLimitForDomain
falls back to that
+ // untagged limit for this domain, so it must still get its (tagged)
resource_count
+ // row locked.
+ long accountId = 1L;
+ long accountDomainId = 7L;
+ Resource.ResourceType type = Resource.ResourceType.cpu;
+ String tag = hostTags.get(0);
+ AccountVO account = Mockito.mock(AccountVO.class);
+ Mockito.when(account.getDomainId()).thenReturn(accountDomainId);
+
Mockito.when(accountDao.findByIdIncludingRemoved(accountId)).thenReturn(account);
+ mockDomainParentChain(accountDomainId, Domain.ROOT_DOMAIN);
+
+ ResourceCountVO accountRow = mockCountRow(100L);
+ ResourceCountVO accountDomainRow = mockCountRow(200L);
+ Mockito.when(resourceCountDao.findByOwnerAndTypeAndTag(accountId,
Resource.ResourceOwnerType.Account, type, tag))
+ .thenReturn(accountRow);
+
+ Set<Long> chainSet = Collections.singleton(accountDomainId);
+ Mockito.when(resourceLimitDao.listByDomainIdsAndTypeAndTag(chainSet,
type, tag))
+ .thenReturn(Collections.emptyList());
+ ResourceLimitVO accountDomainLimitRow = mockLimitRow(accountDomainId,
10L);
+ Mockito.when(resourceLimitDao.listByDomainIdsAndTypeAndTag(chainSet,
type, null))
+ .thenReturn(Collections.singletonList(accountDomainLimitRow));
+
Mockito.when(resourceCountDao.findByOwnersAndTypeAndTag(Mockito.eq(Collections.singletonList(accountDomainId)),
Mockito.eq(Resource.ResourceOwnerType.Domain), Mockito.eq(type),
Mockito.eq(tag)))
+ .thenReturn(Collections.singletonList(accountDomainRow));
+
+ Set<Long> rowIds =
resourceLimitManager.listRowsToLockForLimitCheck(accountId, type, tag);
+
+ Assert.assertEquals(new HashSet<>(Arrays.asList(100L, 200L)), rowIds);
+ }
+
+ @Test
+ public void
testListRowsToLockForLimitCheckExplicitUnlimitedTagRowSkipsUntaggedFallback() {
+ // An explicit tag-specific row exists (even though it's UNLIMITED), so
+ // findCorrectResourceLimitForDomain does NOT fall back to the
untagged limit for
+ // this domain. The untagged finite-limit lookup must never be
consulted, and the
+ // domain must not be locked.
+ long accountId = 1L;
+ long accountDomainId = 7L;
+ Resource.ResourceType type = Resource.ResourceType.cpu;
+ String tag = hostTags.get(0);
+ AccountVO account = Mockito.mock(AccountVO.class);
+ Mockito.when(account.getDomainId()).thenReturn(accountDomainId);
+
Mockito.when(accountDao.findByIdIncludingRemoved(accountId)).thenReturn(account);
+ mockDomainParentChain(accountDomainId, Domain.ROOT_DOMAIN);
+
+ ResourceCountVO accountRow = mockCountRow(100L);
+ Mockito.when(resourceCountDao.findByOwnerAndTypeAndTag(accountId,
Resource.ResourceOwnerType.Account, type, tag))
+ .thenReturn(accountRow);
+
+ Set<Long> chainSet = Collections.singleton(accountDomainId);
+ ResourceLimitVO unlimitedTagRow = mockLimitRow(accountDomainId,
Resource.RESOURCE_UNLIMITED);
+ Mockito.when(resourceLimitDao.listByDomainIdsAndTypeAndTag(chainSet,
type, tag))
+ .thenReturn(Collections.singletonList(unlimitedTagRow));
+
+ Set<Long> rowIds =
resourceLimitManager.listRowsToLockForLimitCheck(accountId, type, tag);
+
+ Assert.assertEquals(Collections.singleton(100L), rowIds);
+ Mockito.verify(resourceLimitDao,
Mockito.never()).listByDomainIdsAndTypeAndTag(chainSet, type, null);
+ Mockito.verify(resourceCountDao,
Mockito.never()).findByOwnersAndTypeAndTag(Mockito.any(), Mockito.any(),
Mockito.any(), Mockito.any());
+ }
+
@Test
public void testRemoveResourceLimitAndCountForNonMatchingTags() {
resourceLimitManager.removeResourceLimitAndCountForNonMatchingTags(1L,
Resource.ResourceOwnerType.Account, hostTags, storageTags);
