DaanHoogland commented on code in PR #7977:
URL: https://github.com/apache/cloudstack/pull/7977#discussion_r1341349169
##########
server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java:
##########
@@ -911,35 +917,40 @@ protected long recalculateDomainResourceCount(final long
domainId, final Resourc
@Override
public Long doInTransaction(TransactionStatus status) {
long newResourceCount = 0;
- lockDomainRows(domainId, type);
ResourceCountVO domainRC =
_resourceCountDao.findByOwnerAndType(domainId, ResourceOwnerType.Domain, type);
long oldResourceCount = domainRC.getCount();
- List<DomainVO> domainChildren =
_domainDao.findImmediateChildrenForParent(domainId);
- // for each child domain update the resource count
+ try {
+ _resourceCountDao.acquireInLockTable(domainRC.getId());
- // calculate project count here
- if (type == ResourceType.project) {
- newResourceCount +=
_projectDao.countProjectsForDomain(domainId);
- }
+ List<DomainVO> domainChildren =
_domainDao.findImmediateChildrenForParent(domainId);
+ // for each child domain update the resource count
Review Comment:
```suggestion
```
##########
server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java:
##########
@@ -578,12 +572,18 @@ public void checkResourceLimit(final Account account,
final ResourceType type, l
Transaction.execute(new
TransactionCallbackWithExceptionNoReturn<ResourceAllocationException>() {
@Override
public void doInTransactionWithoutResult(TransactionStatus status)
throws ResourceAllocationException {
- // Lock all rows first so nobody else can read it
- lockAccountAndOwnerDomainRows(account.getId(), type);
- // Check account limits
- checkAccountResourceLimit(account, projectFinal, type,
numResources);
- // check all domains in the account's domain hierarchy
- checkDomainResourceLimit(account, projectFinal, type,
numResources);
+ List<ResourceCountVO> resourceCounts =
getAccountAndOwnerDomainResourceCounts(account.getId(), type);
+ for (ResourceCountVO resourceCountVO : resourceCounts) {
+ try {
+
_resourceCountDao.acquireInLockTable(resourceCountVO.getId());
+ // Check account limits
Review Comment:
```suggestion
```
##########
server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java:
##########
@@ -911,35 +917,40 @@ protected long recalculateDomainResourceCount(final long
domainId, final Resourc
@Override
public Long doInTransaction(TransactionStatus status) {
long newResourceCount = 0;
- lockDomainRows(domainId, type);
ResourceCountVO domainRC =
_resourceCountDao.findByOwnerAndType(domainId, ResourceOwnerType.Domain, type);
long oldResourceCount = domainRC.getCount();
- List<DomainVO> domainChildren =
_domainDao.findImmediateChildrenForParent(domainId);
- // for each child domain update the resource count
+ try {
+ _resourceCountDao.acquireInLockTable(domainRC.getId());
- // calculate project count here
- if (type == ResourceType.project) {
- newResourceCount +=
_projectDao.countProjectsForDomain(domainId);
- }
+ List<DomainVO> domainChildren =
_domainDao.findImmediateChildrenForParent(domainId);
+ // for each child domain update the resource count
- for (DomainVO childDomain : domainChildren) {
- long childDomainResourceCount =
recalculateDomainResourceCount(childDomain.getId(), type);
- newResourceCount += childDomainResourceCount; // add the
child domain count to parent domain count
- }
- List<AccountVO> accounts =
_accountDao.findActiveAccountsForDomain(domainId);
- for (AccountVO account : accounts) {
- long accountResourceCount =
recalculateAccountResourceCount(account.getId(), type);
- newResourceCount += accountResourceCount; // add account's
resource count to parent domain count
- }
- _resourceCountDao.setResourceCount(domainId,
ResourceOwnerType.Domain, type, newResourceCount);
+ // calculate project count here
+ if (type == ResourceType.project) {
+ newResourceCount +=
_projectDao.countProjectsForDomain(domainId);
+ }
- if (oldResourceCount != newResourceCount) {
- s_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.");
- }
+ for (DomainVO childDomain : domainChildren) {
+ long childDomainResourceCount =
recalculateDomainResourceCount(childDomain.getId(), type);
+ newResourceCount += childDomainResourceCount; // add
the child domain count to parent domain count
Review Comment:
maybe better to rename `newResourceCount` to `parentResourceCount` and
remove the comment.
##########
server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java:
##########
@@ -911,35 +917,40 @@ protected long recalculateDomainResourceCount(final long
domainId, final Resourc
@Override
public Long doInTransaction(TransactionStatus status) {
long newResourceCount = 0;
- lockDomainRows(domainId, type);
ResourceCountVO domainRC =
_resourceCountDao.findByOwnerAndType(domainId, ResourceOwnerType.Domain, type);
long oldResourceCount = domainRC.getCount();
- List<DomainVO> domainChildren =
_domainDao.findImmediateChildrenForParent(domainId);
- // for each child domain update the resource count
+ try {
+ _resourceCountDao.acquireInLockTable(domainRC.getId());
- // calculate project count here
- if (type == ResourceType.project) {
- newResourceCount +=
_projectDao.countProjectsForDomain(domainId);
- }
+ List<DomainVO> domainChildren =
_domainDao.findImmediateChildrenForParent(domainId);
+ // for each child domain update the resource count
- for (DomainVO childDomain : domainChildren) {
- long childDomainResourceCount =
recalculateDomainResourceCount(childDomain.getId(), type);
- newResourceCount += childDomainResourceCount; // add the
child domain count to parent domain count
- }
- List<AccountVO> accounts =
_accountDao.findActiveAccountsForDomain(domainId);
- for (AccountVO account : accounts) {
- long accountResourceCount =
recalculateAccountResourceCount(account.getId(), type);
- newResourceCount += accountResourceCount; // add account's
resource count to parent domain count
- }
- _resourceCountDao.setResourceCount(domainId,
ResourceOwnerType.Domain, type, newResourceCount);
+ // calculate project count here
+ if (type == ResourceType.project) {
+ newResourceCount +=
_projectDao.countProjectsForDomain(domainId);
+ }
- if (oldResourceCount != newResourceCount) {
- s_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.");
- }
+ for (DomainVO childDomain : domainChildren) {
+ long childDomainResourceCount =
recalculateDomainResourceCount(childDomain.getId(), type);
+ newResourceCount += childDomainResourceCount; // add
the child domain count to parent domain count
+ }
+ List<AccountVO> accounts =
_accountDao.findActiveAccountsForDomain(domainId);
+ for (AccountVO account : accounts) {
+ long accountResourceCount =
recalculateAccountResourceCount(account.getId(), type);
+ newResourceCount += accountResourceCount; // add
account's resource count to parent domain count
Review Comment:
```suggestion
newResourceCount += accountResourceCount;
```
##########
server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java:
##########
@@ -911,35 +917,40 @@ protected long recalculateDomainResourceCount(final long
domainId, final Resourc
@Override
public Long doInTransaction(TransactionStatus status) {
long newResourceCount = 0;
- lockDomainRows(domainId, type);
ResourceCountVO domainRC =
_resourceCountDao.findByOwnerAndType(domainId, ResourceOwnerType.Domain, type);
long oldResourceCount = domainRC.getCount();
- List<DomainVO> domainChildren =
_domainDao.findImmediateChildrenForParent(domainId);
- // for each child domain update the resource count
+ try {
+ _resourceCountDao.acquireInLockTable(domainRC.getId());
- // calculate project count here
- if (type == ResourceType.project) {
- newResourceCount +=
_projectDao.countProjectsForDomain(domainId);
- }
+ List<DomainVO> domainChildren =
_domainDao.findImmediateChildrenForParent(domainId);
+ // for each child domain update the resource count
- for (DomainVO childDomain : domainChildren) {
- long childDomainResourceCount =
recalculateDomainResourceCount(childDomain.getId(), type);
- newResourceCount += childDomainResourceCount; // add the
child domain count to parent domain count
- }
- List<AccountVO> accounts =
_accountDao.findActiveAccountsForDomain(domainId);
- for (AccountVO account : accounts) {
- long accountResourceCount =
recalculateAccountResourceCount(account.getId(), type);
- newResourceCount += accountResourceCount; // add account's
resource count to parent domain count
- }
- _resourceCountDao.setResourceCount(domainId,
ResourceOwnerType.Domain, type, newResourceCount);
+ // calculate project count here
Review Comment:
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
