From 622568409c1f87ee742c83a4a902482112d3c66b Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 10 Apr 2024 23:55:54 +0530 Subject: [PATCH 1/3] Speed up resource count calculation --- .../configuration/dao/ResourceCountDao.java | 2 + .../dao/ResourceCountDaoImpl.java | 24 ++++++++++ .../reservation/dao/ReservationDao.java | 2 + .../reservation/dao/ReservationDaoImpl.java | 20 ++++++++ .../ResourceLimitManagerImpl.java | 48 +++++-------------- 5 files changed, 59 insertions(+), 37 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDao.java b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDao.java index 59e64dac8807..8104eca76283 100644 --- a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDao.java +++ b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDao.java @@ -53,6 +53,8 @@ public interface ResourceCountDao extends GenericDao { Set listAllRowsToUpdate(long ownerId, ResourceOwnerType ownerType, ResourceType type, String tag); + boolean incrementCountByIds(Set ids, boolean increment, long delta); + Set listRowsToUpdateForDomain(long domainId, ResourceType type, String tag); long removeEntriesByOwner(long ownerId, ResourceOwnerType ownerType); diff --git a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java index c90422377b8e..83453eeb5a1a 100644 --- a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java @@ -24,6 +24,7 @@ import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.stream.Collectors; import javax.annotation.PostConstruct; import javax.inject.Inject; @@ -65,6 +66,9 @@ public class ResourceCountDaoImpl extends GenericDaoBase @Inject private AccountDao _accountDao; + protected static final String INCREMENT_COUNT_BY_IDS_SQL = "UPDATE `cloud`.`resource_count` SET `count` = `count` + ? WHERE `id` IN (?)"; + protected static final String DECREMENT_COUNT_BY_IDS_SQL = "UPDATE `cloud`.`resource_count` SET `count` = `count` - ? WHERE `id` IN (?)"; + public ResourceCountDaoImpl() { TypeSearch = createSearchBuilder(); TypeSearch.and("type", TypeSearch.entity().getType(), SearchCriteria.Op.EQ); @@ -154,6 +158,26 @@ public boolean updateById(long id, boolean increment, long delta) { return update(resourceCountVO.getId(), resourceCountVO); } + @Override + public boolean incrementCountByIds(Set ids, boolean increment, long delta) { + if (CollectionUtils.isEmpty(ids)) { + return false; + } + String updateSql = increment ? INCREMENT_COUNT_BY_IDS_SQL : DECREMENT_COUNT_BY_IDS_SQL; + + String poolIdsInStr = ids.stream().map(id -> String.valueOf(id)).collect(Collectors.joining(",", "(", ")")); + String sql = updateSql.replace("(?)", poolIdsInStr); + + try (TransactionLegacy txn = TransactionLegacy.currentTxn(); + PreparedStatement pstmt = txn.prepareAutoCloseStatement(sql)) { + pstmt.setLong(1, delta); + pstmt.executeUpdate(); + return txn.commit(); + } catch (SQLException e) { + throw new CloudRuntimeException(e); + } + } + @Override public Set listRowsToUpdateForDomain(long domainId, ResourceType type, String tag) { Set rowIds = new HashSet(); diff --git a/engine/schema/src/main/java/org/apache/cloudstack/reservation/dao/ReservationDao.java b/engine/schema/src/main/java/org/apache/cloudstack/reservation/dao/ReservationDao.java index 4b87c71e2e21..0d4c84d9d5d1 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/reservation/dao/ReservationDao.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/reservation/dao/ReservationDao.java @@ -31,4 +31,6 @@ public interface ReservationDao extends GenericDao { void setResourceId(Resource.ResourceType type, Long resourceId); List getResourceIds(long accountId, Resource.ResourceType type); List getReservationsForAccount(long accountId, Resource.ResourceType type, String tag); + + void removeByIds(List reservationIds); } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/reservation/dao/ReservationDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/reservation/dao/ReservationDaoImpl.java index af0bd22619fc..5770b94a3087 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/reservation/dao/ReservationDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/reservation/dao/ReservationDaoImpl.java @@ -29,6 +29,7 @@ import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; import org.apache.cloudstack.user.ResourceReservation; +import org.apache.commons.collections.CollectionUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -40,6 +41,7 @@ public class ReservationDaoImpl extends GenericDaoBase impl private static final String RESOURCE_ID = "resourceId"; private static final String ACCOUNT_ID = "accountId"; private static final String DOMAIN_ID = "domainId"; + private static final String IDS = "ids"; private final SearchBuilder listResourceByAccountAndTypeSearch; private final SearchBuilder listAccountAndTypeSearch; private final SearchBuilder listAccountAndTypeAndNoTagSearch; @@ -48,6 +50,9 @@ public class ReservationDaoImpl extends GenericDaoBase impl private final SearchBuilder listDomainAndTypeAndNoTagSearch; private final SearchBuilder listResourceByAccountAndTypeAndNoTagSearch; + private final SearchBuilder listIdsSearch; + + public ReservationDaoImpl() { listResourceByAccountAndTypeSearch = createSearchBuilder(); @@ -87,6 +92,10 @@ public ReservationDaoImpl() { listDomainAndTypeAndNoTagSearch.and(RESOURCE_TYPE, listDomainAndTypeAndNoTagSearch.entity().getResourceType(), SearchCriteria.Op.EQ); listDomainAndTypeAndNoTagSearch.and(RESOURCE_TAG, listDomainAndTypeAndNoTagSearch.entity().getTag(), SearchCriteria.Op.NULL); listDomainAndTypeAndNoTagSearch.done(); + + listIdsSearch = createSearchBuilder(); + listIdsSearch.and(IDS, listIdsSearch.entity().getId(), SearchCriteria.Op.IN); + listIdsSearch.done(); } @Override @@ -161,4 +170,15 @@ public List getReservationsForAccount(long accountId, Resource.Re } return listBy(sc); } + + @Override + public void removeByIds(List reservationIds) { + if (CollectionUtils.isEmpty(reservationIds)) { + return; + } + + SearchCriteria sc = listIdsSearch.create(); + sc.setParameters(IDS, reservationIds.toArray()); + remove(sc); + } } diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java index 6181c4059e6d..e252388ee9c7 100644 --- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java +++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java @@ -203,17 +203,12 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim @SuppressWarnings("unchecked") protected void removeResourceReservationIfNeededAndIncrementResourceCount(final long accountId, final ResourceType type, String tag, final long numToIncrement) { + Object obj = CallContext.current().getContextParameter(CheckedReservation.getResourceReservationContextParameterKey(type)); + List reservationIds = (List)obj; // This complains an unchecked casting warning Transaction.execute(new TransactionCallbackWithExceptionNoReturn() { @Override public void doInTransactionWithoutResult(TransactionStatus status) throws CloudRuntimeException { - - Object obj = CallContext.current().getContextParameter(CheckedReservation.getResourceReservationContextParameterKey(type)); - if (obj instanceof List) { - List reservationIds = (List)obj; // This complains an unchecked casting warning - for (Long reservationId : reservationIds) { - reservationDao.remove(reservationId); - } - } + reservationDao.removeByIds(reservationIds); if (!updateResourceCountForAccount(accountId, type, tag, 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); @@ -613,13 +608,11 @@ public long findCorrectResourceLimitForAccountAndDomain(Account account, Domain } @Override - @DB public void checkResourceLimit(final Account account, final ResourceType type, long... count) throws ResourceAllocationException { checkResourceLimitWithTag(account, type, null, count); } @Override - @DB public void checkResourceLimitWithTag(final Account account, final ResourceType type, String tag, long... count) throws ResourceAllocationException { final long numResources = ((count.length == 0) ? 1 : count[0]); Project project = null; @@ -1124,7 +1117,6 @@ public List recalculateResourceCount(Long accountId, Lo return recalculateResourceCount(accountId, domainId, typeId, null); } - @DB protected boolean updateResourceCountForAccount(final long accountId, final ResourceType type, String tag, final boolean increment, final long delta) { if (logger.isDebugEnabled()) { String convertedDelta = String.valueOf(delta); @@ -1135,20 +1127,8 @@ protected boolean updateResourceCountForAccount(final long accountId, final Reso logger.debug("Updating resource Type = " + typeStr + " count for Account = " + accountId + " Operation = " + (increment ? "increasing" : "decreasing") + " Amount = " + convertedDelta); } try { - return Transaction.execute(new TransactionCallback() { - @Override - public Boolean doInTransaction(TransactionStatus status) { - boolean result = true; - List rowsToUpdate = lockAccountAndOwnerDomainRows(accountId, type, tag); - for (ResourceCountVO rowToUpdate : rowsToUpdate) { - if (!_resourceCountDao.updateById(rowToUpdate.getId(), increment, delta)) { - logger.trace("Unable to update resource count for the row " + rowToUpdate); - result = false; - } - } - return result; - } - }); + Set rowIdsToUpdate = _resourceCountDao.listAllRowsToUpdate(accountId, ResourceOwnerType.Account, type, tag); + return _resourceCountDao.incrementCountByIds(rowIdsToUpdate, increment, delta); } catch (Exception ex) { logger.error("Failed to update resource count for account id=" + accountId); return false; @@ -1163,8 +1143,8 @@ public Boolean doInTransaction(TransactionStatus status) { * @param type the resource type to do the recalculation for * @return the resulting new resource count */ - @DB protected long recalculateDomainResourceCount(final long domainId, final ResourceType type, String tag) { + List domainChildren = _domainDao.findImmediateChildrenForParent(domainId); return Transaction.execute(new TransactionCallback() { @Override public Long doInTransaction(TransactionStatus status) { @@ -1173,14 +1153,12 @@ public Long doInTransaction(TransactionStatus status) { ResourceCountVO domainRC = _resourceCountDao.findByOwnerAndTypeAndTag(domainId, ResourceOwnerType.Domain, type, tag); long oldResourceCount = domainRC.getCount(); - List domainChildren = _domainDao.findImmediateChildrenForParent(domainId); - // for each child domain update the resource count - // calculate project count here if (type == ResourceType.project) { newResourceCount += _projectDao.countProjectsForDomain(domainId); } + // for each child domain update the resource count for (DomainVO childDomain : domainChildren) { long childDomainResourceCount = recalculateDomainResourceCount(childDomain.getId(), type, tag); newResourceCount += childDomainResourceCount; // add the child domain count to parent domain count @@ -1191,9 +1169,10 @@ public Long doInTransaction(TransactionStatus status) { long accountResourceCount = recalculateAccountResourceCount(account.getId(), type, tag); newResourceCount += accountResourceCount; // add account's resource count to parent domain count } - _resourceCountDao.setResourceCount(domainId, ResourceOwnerType.Domain, type, tag, newResourceCount); if (oldResourceCount != newResourceCount) { + domainRC.setCount(newResourceCount); + _resourceCountDao.update(domainRC.getId(), domainRC); logger.warn("Discrepency in the resource count has been detected " + "(original count = " + oldResourceCount + " correct count = " + newResourceCount + ") for Type = " + type + " for Domain ID = " + domainId + " is fixed during resource count recalculation."); } @@ -1241,13 +1220,8 @@ protected long recalculateAccountResourceCount(final long accountId, final Resou } if (newCount == null || !newCount.equals(oldCount)) { - Transaction.execute(new TransactionCallbackNoReturn() { - @Override - public void doInTransactionWithoutResult(TransactionStatus status) { - lockAccountAndOwnerDomainRows(accountId, type, tag); - _resourceCountDao.setResourceCount(accountId, ResourceOwnerType.Account, type, tag, (newCount == null) ? 0 : newCount); - } - }); + lockAccountAndOwnerDomainRows(accountId, type, tag); + _resourceCountDao.setResourceCount(accountId, ResourceOwnerType.Account, type, tag, (newCount == null) ? 0 : newCount); } // No need to log message for primary and secondary storage because both are recalculating the From 3bbe7fcdb02ccffa72bf1ddd6fd3b7ecf5f77d8f Mon Sep 17 00:00:00 2001 From: Vishesh Date: Thu, 11 Apr 2024 12:52:58 +0530 Subject: [PATCH 2/3] Refactor resource count calculation --- .../configuration/dao/ResourceCountDao.java | 7 +- .../dao/ResourceCountDaoImpl.java | 57 +++++++--- .../reservation/dao/ReservationDao.java | 1 - .../reservation/dao/ReservationDaoImpl.java | 12 +- .../ResourceLimitManagerImpl.java | 105 ++++++++++-------- 5 files changed, 110 insertions(+), 72 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDao.java b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDao.java index 8104eca76283..b978cc04bfa7 100644 --- a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDao.java +++ b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDao.java @@ -49,11 +49,14 @@ public interface ResourceCountDao extends GenericDao { ResourceCountVO findByOwnerAndTypeAndTag(long ownerId, ResourceOwnerType ownerType, ResourceType type, String tag); + List findByOwnersAndTypeAndTag(List ownerIdList, ResourceOwnerType ownerType, + ResourceType type, String tag); + List listResourceCountByOwnerType(ResourceOwnerType ownerType); Set listAllRowsToUpdate(long ownerId, ResourceOwnerType ownerType, ResourceType type, String tag); - boolean incrementCountByIds(Set ids, boolean increment, long delta); + boolean updateCountByDeltaForIds(List ids, boolean increment, long delta); Set listRowsToUpdateForDomain(long domainId, ResourceType type, String tag); @@ -74,4 +77,6 @@ public interface ResourceCountDao extends GenericDao { long countMemoryAllocatedToAccount(long accountId); void removeResourceCountsForNonMatchingTags(Long ownerId, ResourceOwnerType ownerType, List types, List tags); + + List lockRows(Set ids); } diff --git a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java index 83453eeb5a1a..8c189f47ebcb 100644 --- a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java @@ -57,9 +57,9 @@ public class ResourceCountDaoImpl extends GenericDaoBase private final SearchBuilder TypeSearch; private final SearchBuilder TypeNullTagSearch; private final SearchBuilder NonMatchingTagsSearch; - private final SearchBuilder AccountSearch; private final SearchBuilder DomainSearch; + private final SearchBuilder IdsSearch; @Inject private DomainDao _domainDao; @@ -72,15 +72,15 @@ public class ResourceCountDaoImpl extends GenericDaoBase public ResourceCountDaoImpl() { TypeSearch = createSearchBuilder(); TypeSearch.and("type", TypeSearch.entity().getType(), SearchCriteria.Op.EQ); - TypeSearch.and("accountId", TypeSearch.entity().getAccountId(), SearchCriteria.Op.EQ); - TypeSearch.and("domainId", TypeSearch.entity().getDomainId(), SearchCriteria.Op.EQ); + TypeSearch.and("accountId", TypeSearch.entity().getAccountId(), SearchCriteria.Op.IN); + TypeSearch.and("domainId", TypeSearch.entity().getDomainId(), SearchCriteria.Op.IN); TypeSearch.and("tag", TypeSearch.entity().getTag(), SearchCriteria.Op.EQ); TypeSearch.done(); TypeNullTagSearch = createSearchBuilder(); TypeNullTagSearch.and("type", TypeNullTagSearch.entity().getType(), SearchCriteria.Op.EQ); - TypeNullTagSearch.and("accountId", TypeNullTagSearch.entity().getAccountId(), SearchCriteria.Op.EQ); - TypeNullTagSearch.and("domainId", TypeNullTagSearch.entity().getDomainId(), SearchCriteria.Op.EQ); + TypeNullTagSearch.and("accountId", TypeNullTagSearch.entity().getAccountId(), SearchCriteria.Op.IN); + TypeNullTagSearch.and("domainId", TypeNullTagSearch.entity().getDomainId(), SearchCriteria.Op.IN); TypeNullTagSearch.and("tag", TypeNullTagSearch.entity().getTag(), SearchCriteria.Op.NULL); TypeNullTagSearch.done(); @@ -94,6 +94,10 @@ public ResourceCountDaoImpl() { AccountSearch = createSearchBuilder(); DomainSearch = createSearchBuilder(); + + IdsSearch = createSearchBuilder(); + IdsSearch.and("id", IdsSearch.entity().getId(), SearchCriteria.Op.IN); + IdsSearch.done(); } @PostConstruct @@ -113,6 +117,19 @@ protected void configure() { @Override public ResourceCountVO findByOwnerAndTypeAndTag(long ownerId, ResourceOwnerType ownerType, ResourceType type, String tag) { + List resourceCounts = findByOwnersAndTypeAndTag(List.of(ownerId), ownerType, type, tag); + if (CollectionUtils.isNotEmpty(resourceCounts)) { + return resourceCounts.get(0); + } else { + return null; + } + } + + @Override + public List findByOwnersAndTypeAndTag(List ownerIdList, ResourceOwnerType ownerType, ResourceType type, String tag) { + if (CollectionUtils.isEmpty(ownerIdList)) { + return new ArrayList<>(); + } SearchCriteria sc = tag != null ? TypeSearch.create() : TypeNullTagSearch.create(); sc.setParameters("type", type); if (tag != null) { @@ -120,13 +137,13 @@ public ResourceCountVO findByOwnerAndTypeAndTag(long ownerId, ResourceOwnerType } if (ownerType == ResourceOwnerType.Account) { - sc.setParameters("accountId", ownerId); - return findOneIncludingRemovedBy(sc); + sc.setParameters("accountId", ownerIdList.toArray()); + return listIncludingRemovedBy(sc); } else if (ownerType == ResourceOwnerType.Domain) { - sc.setParameters("domainId", ownerId); - return findOneIncludingRemovedBy(sc); + sc.setParameters("domainId", ownerIdList.toArray()); + return listIncludingRemovedBy(sc); } else { - return null; + return new ArrayList<>(); } } @@ -159,20 +176,22 @@ public boolean updateById(long id, boolean increment, long delta) { } @Override - public boolean incrementCountByIds(Set ids, boolean increment, long delta) { + public boolean updateCountByDeltaForIds(List ids, boolean increment, long delta) { if (CollectionUtils.isEmpty(ids)) { return false; } String updateSql = increment ? INCREMENT_COUNT_BY_IDS_SQL : DECREMENT_COUNT_BY_IDS_SQL; - String poolIdsInStr = ids.stream().map(id -> String.valueOf(id)).collect(Collectors.joining(",", "(", ")")); + String poolIdsInStr = ids.stream().map(String::valueOf).collect(Collectors.joining(",", "(", ")")); String sql = updateSql.replace("(?)", poolIdsInStr); try (TransactionLegacy txn = TransactionLegacy.currentTxn(); - PreparedStatement pstmt = txn.prepareAutoCloseStatement(sql)) { + PreparedStatement pstmt = txn.prepareAutoCloseStatement(sql) + ) { pstmt.setLong(1, delta); pstmt.executeUpdate(); - return txn.commit(); + txn.commit(); + return true; } catch (SQLException e) { throw new CloudRuntimeException(e); } @@ -369,4 +388,14 @@ public void removeResourceCountsForNonMatchingTags(Long ownerId, ResourceOwnerTy } remove(sc); } + + @Override + public List lockRows(Set ids) { + if (CollectionUtils.isEmpty(ids)) { + return new ArrayList<>(); + } + SearchCriteria sc = IdsSearch.create(); + sc.setParameters("id", ids.toArray()); + return lockRows(sc, null, true); + } } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/reservation/dao/ReservationDao.java b/engine/schema/src/main/java/org/apache/cloudstack/reservation/dao/ReservationDao.java index 0d4c84d9d5d1..0433dc8c57d9 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/reservation/dao/ReservationDao.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/reservation/dao/ReservationDao.java @@ -31,6 +31,5 @@ public interface ReservationDao extends GenericDao { void setResourceId(Resource.ResourceType type, Long resourceId); List getResourceIds(long accountId, Resource.ResourceType type); List getReservationsForAccount(long accountId, Resource.ResourceType type, String tag); - void removeByIds(List reservationIds); } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/reservation/dao/ReservationDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/reservation/dao/ReservationDaoImpl.java index 5770b94a3087..8d6e0b6eee0e 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/reservation/dao/ReservationDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/reservation/dao/ReservationDaoImpl.java @@ -49,10 +49,8 @@ public class ReservationDaoImpl extends GenericDaoBase impl private final SearchBuilder listDomainAndTypeSearch; private final SearchBuilder listDomainAndTypeAndNoTagSearch; private final SearchBuilder listResourceByAccountAndTypeAndNoTagSearch; - private final SearchBuilder listIdsSearch; - public ReservationDaoImpl() { listResourceByAccountAndTypeSearch = createSearchBuilder(); @@ -173,12 +171,10 @@ public List getReservationsForAccount(long accountId, Resource.Re @Override public void removeByIds(List reservationIds) { - if (CollectionUtils.isEmpty(reservationIds)) { - return; + if (CollectionUtils.isNotEmpty(reservationIds)) { + SearchCriteria sc = listIdsSearch.create(); + sc.setParameters(IDS, reservationIds.toArray()); + remove(sc); } - - SearchCriteria sc = listIdsSearch.create(); - sc.setParameters(IDS, reservationIds.toArray()); - remove(sc); } } diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java index e252388ee9c7..11ebc6da251e 100644 --- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java +++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java @@ -21,6 +21,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -574,13 +575,6 @@ protected List lockAccountAndOwnerDomainRows(long accountId, fi return _resourceCountDao.lockRows(sc, null, true); } - private List lockDomainRows(long domainId, final ResourceType type, String tag) { - Set rowIdsToLock = _resourceCountDao.listAllRowsToUpdate(domainId, ResourceOwnerType.Domain, type, tag); - SearchCriteria sc = ResourceCountSearch.create(); - sc.setParameters("id", rowIdsToLock.toArray()); - return _resourceCountDao.lockRows(sc, null, true); - } - @Override public long findDefaultResourceLimitForDomain(ResourceType resourceType) { Long resourceLimit = null; @@ -1126,13 +1120,8 @@ protected boolean updateResourceCountForAccount(final long accountId, final Reso String typeStr = StringUtils.isNotEmpty(tag) ? String.format("%s (tag: %s)", type, tag) : type.getName(); logger.debug("Updating resource Type = " + typeStr + " count for Account = " + accountId + " Operation = " + (increment ? "increasing" : "decreasing") + " Amount = " + convertedDelta); } - try { - Set rowIdsToUpdate = _resourceCountDao.listAllRowsToUpdate(accountId, ResourceOwnerType.Account, type, tag); - return _resourceCountDao.incrementCountByIds(rowIdsToUpdate, increment, delta); - } catch (Exception ex) { - logger.error("Failed to update resource count for account id=" + accountId); - return false; - } + Set rowIdsToUpdate = _resourceCountDao.listAllRowsToUpdate(accountId, ResourceOwnerType.Account, type, tag); + return _resourceCountDao.updateCountByDeltaForIds(new ArrayList<>(rowIdsToUpdate), increment, delta); } /** @@ -1144,41 +1133,62 @@ protected boolean updateResourceCountForAccount(final long accountId, final Reso * @return the resulting new resource count */ protected long recalculateDomainResourceCount(final long domainId, final ResourceType type, String tag) { - List domainChildren = _domainDao.findImmediateChildrenForParent(domainId); - return Transaction.execute(new TransactionCallback() { - @Override - public Long doInTransaction(TransactionStatus status) { - long newResourceCount = 0; - lockDomainRows(domainId, type, tag); - ResourceCountVO domainRC = _resourceCountDao.findByOwnerAndTypeAndTag(domainId, ResourceOwnerType.Domain, type, tag); - long oldResourceCount = domainRC.getCount(); - - // calculate project count here - if (type == ResourceType.project) { - newResourceCount += _projectDao.countProjectsForDomain(domainId); - } + List accounts = _accountDao.findActiveAccountsForDomain(domainId); + List childDomains = _domainDao.findImmediateChildrenForParent(domainId); - // for each child domain update the resource count - for (DomainVO childDomain : domainChildren) { - long childDomainResourceCount = recalculateDomainResourceCount(childDomain.getId(), type, tag); - newResourceCount += childDomainResourceCount; // add the child domain count to parent domain count - } + if (CollectionUtils.isNotEmpty(childDomains)) { + for (DomainVO childDomain : childDomains) { + recalculateDomainResourceCount(childDomain.getId(), type, tag); + } + } + if (CollectionUtils.isNotEmpty(accounts)) { + for (AccountVO account : accounts) { + recalculateAccountResourceCount(account.getId(), type, tag); + } + } - List accounts = _accountDao.findActiveAccountsForDomain(domainId); - for (AccountVO account : accounts) { - long accountResourceCount = recalculateAccountResourceCount(account.getId(), type, tag); - newResourceCount += accountResourceCount; // add account's resource count to parent domain count - } + return Transaction.execute((TransactionCallback) status -> { + long newResourceCount = 0L; + List domainIdList = childDomains.stream().map(DomainVO::getId).collect(Collectors.toList()); + domainIdList.add(domainId); + List accountIdList = accounts.stream().map(AccountVO::getId).collect(Collectors.toList()); + List domainRCList = _resourceCountDao.findByOwnersAndTypeAndTag(domainIdList, ResourceOwnerType.Domain, type, tag); + List accountRCList = _resourceCountDao.findByOwnersAndTypeAndTag(accountIdList, ResourceOwnerType.Account, type, tag); - if (oldResourceCount != newResourceCount) { - domainRC.setCount(newResourceCount); - _resourceCountDao.update(domainRC.getId(), domainRC); - logger.warn("Discrepency in the resource count has been detected " + "(original count = " + oldResourceCount + " correct count = " + newResourceCount + ") for Type = " + type - + " for Domain ID = " + domainId + " is fixed during resource count recalculation."); + Set rowIdsToLock = new HashSet<>(); + if (domainRCList != null) { + rowIdsToLock.addAll(domainRCList.stream().map(ResourceCountVO::getId).collect(Collectors.toList())); + } + if (accountRCList != null) { + rowIdsToLock.addAll(accountRCList.stream().map(ResourceCountVO::getId).collect(Collectors.toList())); + } + // lock the resource count rows for current domain, immediate child domain & accounts + List resourceCounts = _resourceCountDao.lockRows(rowIdsToLock); + + long oldResourceCount = 0L; + ResourceCountVO domainRC = null; + + // calculate project count here + if (type == ResourceType.project) { + newResourceCount += _projectDao.countProjectsForDomain(domainId); + } + + for (ResourceCountVO resourceCount : resourceCounts) { + if (resourceCount.getResourceOwnerType() == ResourceOwnerType.Domain && resourceCount.getDomainId() == domainId) { + oldResourceCount = resourceCount.getCount(); + domainRC = resourceCount; + } else { + newResourceCount += resourceCount.getCount(); } + } - return newResourceCount; + if (oldResourceCount != newResourceCount) { + domainRC.setCount(newResourceCount); + _resourceCountDao.update(domainRC.getId(), domainRC); + logger.warn("Discrepency in the resource count has been detected " + "(original count = " + oldResourceCount + " correct count = " + newResourceCount + ") for Type = " + type + + " for Domain ID = " + domainId + " is fixed during resource count recalculation."); } + return newResourceCount; }); } @@ -1217,11 +1227,10 @@ protected long recalculateAccountResourceCount(final long accountId, final Resou final ResourceCountVO accountRC = _resourceCountDao.findByOwnerAndTypeAndTag(accountId, ResourceOwnerType.Account, type, tag); if (accountRC != null) { oldCount = accountRC.getCount(); - } - - if (newCount == null || !newCount.equals(oldCount)) { - lockAccountAndOwnerDomainRows(accountId, type, tag); - _resourceCountDao.setResourceCount(accountId, ResourceOwnerType.Account, type, tag, (newCount == null) ? 0 : newCount); + if (newCount == null || !newCount.equals(oldCount)) { + accountRC.setCount((newCount == null) ? 0 : newCount); + _resourceCountDao.update(accountRC.getId(), accountRC); + } } // No need to log message for primary and secondary storage because both are recalculating the From b1797bf12b40d599d45f4f03e17e1ce971f8a0d7 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Fri, 12 Apr 2024 19:32:50 +0530 Subject: [PATCH 3/3] Start transaction for updateCountByDeltaForIds --- .../com/cloud/configuration/dao/ResourceCountDaoImpl.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java index 8c189f47ebcb..65d7fed2d1a1 100644 --- a/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java @@ -185,12 +185,10 @@ public boolean updateCountByDeltaForIds(List ids, boolean increment, long String poolIdsInStr = ids.stream().map(String::valueOf).collect(Collectors.joining(",", "(", ")")); String sql = updateSql.replace("(?)", poolIdsInStr); - try (TransactionLegacy txn = TransactionLegacy.currentTxn(); - PreparedStatement pstmt = txn.prepareAutoCloseStatement(sql) - ) { + final TransactionLegacy txn = TransactionLegacy.currentTxn(); + try(PreparedStatement pstmt = txn.prepareStatement(sql);) { pstmt.setLong(1, delta); pstmt.executeUpdate(); - txn.commit(); return true; } catch (SQLException e) { throw new CloudRuntimeException(e);