From a0ef9e7674e6633bf6f3e16781ffb059a7262d77 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Fri, 29 Jul 2022 17:10:27 +0530 Subject: [PATCH 1/2] Updated resource counter to include correct size after volume creation/resize and other improvements - Recalculate resource counters for root domain in the periodic task - Update correct size in the primary_storage resource counter after volume creation/resize - Some code improvements --- .../com/cloud/configuration/Resource.java | 5 +- .../storage/volume/VolumeObject.java | 31 ++++++++++++ .../api/query/dao/AccountJoinDaoImpl.java | 49 ++++++++++--------- .../api/query/dao/DomainJoinDaoImpl.java | 49 ++++++++++--------- .../ResourceLimitManagerImpl.java | 25 +++++----- .../cloud/storage/VolumeApiServiceImpl.java | 3 ++ 6 files changed, 100 insertions(+), 62 deletions(-) diff --git a/api/src/main/java/com/cloud/configuration/Resource.java b/api/src/main/java/com/cloud/configuration/Resource.java index 76f2930e6150..fefeeb15ef55 100644 --- a/api/src/main/java/com/cloud/configuration/Resource.java +++ b/api/src/main/java/com/cloud/configuration/Resource.java @@ -18,9 +18,10 @@ public interface Resource { - public static final short RESOURCE_UNLIMITED = -1; + short RESOURCE_UNLIMITED = -1; + String UNLIMITED = "Unlimited"; - public enum ResourceType { // Primary and Secondary storage are allocated_storage and not the physical storage. + enum ResourceType { // Primary and Secondary storage are allocated_storage and not the physical storage. user_vm("user_vm", 0, ResourceOwnerType.Account, ResourceOwnerType.Domain), public_ip("public_ip", 1, ResourceOwnerType.Account, ResourceOwnerType.Domain), volume("volume", 2, ResourceOwnerType.Account, ResourceOwnerType.Domain), diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java index 3875d8155c05..063b532271ce 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java @@ -20,6 +20,7 @@ import javax.inject.Inject; +import com.cloud.configuration.Resource.ResourceType; import com.cloud.dc.VsphereStoragePolicyVO; import com.cloud.dc.dao.VsphereStoragePolicyDao; import com.cloud.service.dao.ServiceOfferingDetailsDao; @@ -28,6 +29,7 @@ import com.cloud.storage.VolumeDetailVO; import com.cloud.storage.dao.VMTemplateDao; import com.cloud.storage.dao.VolumeDetailsDao; +import com.cloud.user.ResourceLimitService; import com.cloud.vm.VmDetailConstants; import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao; @@ -88,6 +90,8 @@ public class VolumeObject implements VolumeInfo { @Inject ObjectInDataStoreManager objectInStoreMgr; @Inject + ResourceLimitService resourceLimitMgr; + @Inject VMInstanceDao vmInstanceDao; @Inject DiskOfferingDao diskOfferingDao; @@ -678,6 +682,32 @@ protected void updateVolumeInfo(VolumeObjectTO newVolume, VolumeVO volumeVo, boo s_logger.debug(String.format("Updated %s from %s to %s ", volumeVo.getVolumeDescription(), previousValues, newValues)); } + protected void updateResourceCount(VolumeObjectTO newVolume, VolumeVO volumeVo) { + if (newVolume == null || newVolume.getSize() == null || volumeVo == null || volumeVo.getSize() == null) { + return; + } + + Long newVolumeSize = newVolume.getSize(); + if (newVolumeSize == volumeVo.getSize()) { + // Volume size already updated, check with earlier vo in the object + if (volumeVO.getSize() != null && volumeVO.getSize() != newVolumeSize) { + Long oldVolumeSize = volumeVO.getSize(); + if (oldVolumeSize < newVolumeSize) { + resourceLimitMgr.incrementResourceCount(volumeVo.getAccountId(), ResourceType.primary_storage, volumeVo.isDisplayVolume(), newVolumeSize - oldVolumeSize); + } else { + resourceLimitMgr.decrementResourceCount(volumeVo.getAccountId(), ResourceType.primary_storage, volumeVo.isDisplayVolume(), oldVolumeSize - newVolumeSize); + } + } + } else { + Long oldVolumeSize = volumeVo.getSize(); + if (oldVolumeSize < newVolumeSize) { + resourceLimitMgr.incrementResourceCount(volumeVo.getAccountId(), ResourceType.primary_storage, volumeVo.isDisplayVolume(), newVolumeSize - oldVolumeSize); + } else { + resourceLimitMgr.decrementResourceCount(volumeVo.getAccountId(), ResourceType.primary_storage, volumeVo.isDisplayVolume(), oldVolumeSize - newVolumeSize); + } + } + } + protected void handleProcessEventCopyCmdAnswerNotPrimaryStore(VolumeObjectTO newVolume) { VolumeDataStoreVO volStore = volumeStoreDao.findByStoreVolume(dataStore.getId(), getId()); @@ -709,6 +739,7 @@ protected void handleProcessEventAnswer(CreateObjectAnswer createObjectAnswer, b VolumeObjectTO newVolume = (VolumeObjectTO)createObjectAnswer.getData(); VolumeVO volumeVo = volumeDao.findById(getId()); updateVolumeInfo(newVolume, volumeVo, true, setFormat); + updateResourceCount(newVolume, volumeVo); } protected void handleProcessEventAnswer(DownloadAnswer downloadAnswer) { diff --git a/server/src/main/java/com/cloud/api/query/dao/AccountJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/AccountJoinDaoImpl.java index c980ffd3ab60..38d3e8f0b210 100644 --- a/server/src/main/java/com/cloud/api/query/dao/AccountJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/AccountJoinDaoImpl.java @@ -34,6 +34,7 @@ import com.cloud.api.query.ViewResponseHelper; import com.cloud.api.query.vo.AccountJoinVO; import com.cloud.api.query.vo.UserAccountJoinVO; +import com.cloud.configuration.Resource; import com.cloud.configuration.Resource.ResourceType; import com.cloud.user.Account; import com.cloud.user.AccountManager; @@ -85,9 +86,9 @@ public AccountResponse newAccountResponse(ResponseView view, EnumSet domains = _domainDao.findImmediateChildrenForParent(Domain.ROOT_DOMAIN); + List accounts = _accountDao.findActiveAccountsForDomain(Domain.ROOT_DOMAIN); - // recalculateDomainResourceCount will take care of re-calculation of resource counts for sub-domains - // and accounts of the sub-domains also. so just loop through immediate children of root domain - for (Domain domain : domains) { - for (ResourceType type : ResourceCount.ResourceType.values()) { - if (type.supportsOwner(ResourceOwnerType.Domain)) { + for (ResourceType type : ResourceCount.ResourceType.values()) { + if (type.supportsOwner(ResourceOwnerType.Domain)) { + // recalculateDomainResourceCount will take care of re-calculation of resource counts for root and sub-domains + // and accounts of the sub-domains also. so just loop through immediate children of root domain + recalculateDomainResourceCount(Domain.ROOT_DOMAIN, type); + for (Domain domain : domains) { recalculateDomainResourceCount(domain.getId(), type); } } - } - // run through the accounts in the root domain - List accounts = _accountDao.findActiveAccountsForDomain(Domain.ROOT_DOMAIN); - for (AccountVO account : accounts) { - for (ResourceType type : ResourceCount.ResourceType.values()) { - if (type.supportsOwner(ResourceOwnerType.Account)) { + if (type.supportsOwner(ResourceOwnerType.Account)) { + // run through the accounts in the root domain + for (AccountVO account : accounts) { recalculateAccountResourceCount(account.getId(), type); } } diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index e6726f6977b8..e5a4aaa5c2b8 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -1439,6 +1439,9 @@ private VolumeVO orchestrateResizeVolume(long volumeId, long currentSize, long n final VolumeVO volumeNow = _volsDao.findById(volumeId); if (currentSize == volumeNow.getSize() && currentSize != newSize) { volume.setSize(newSize); + } else if (volumeNow.getSize() != newSize) { + // consider the updated size as the new size + newSize = volumeNow.getSize(); } _volsDao.update(volume.getId(), volume); From 8fde79714727d8da11e4876554079a8a116acc2a Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Wed, 3 Aug 2022 16:57:53 +0200 Subject: [PATCH 2/2] review and sonarcloud issues --- .../storage/volume/VolumeObject.java | 24 ++++++------------- .../api/query/dao/AccountJoinDaoImpl.java | 2 +- .../api/query/dao/DomainJoinDaoImpl.java | 4 ++-- .../ResourceLimitManagerImpl.java | 12 +++++++--- 4 files changed, 19 insertions(+), 23 deletions(-) diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java index 063b532271ce..8c23d61f6971 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java @@ -682,28 +682,18 @@ protected void updateVolumeInfo(VolumeObjectTO newVolume, VolumeVO volumeVo, boo s_logger.debug(String.format("Updated %s from %s to %s ", volumeVo.getVolumeDescription(), previousValues, newValues)); } - protected void updateResourceCount(VolumeObjectTO newVolume, VolumeVO volumeVo) { - if (newVolume == null || newVolume.getSize() == null || volumeVo == null || volumeVo.getSize() == null) { + protected void updateResourceCount(VolumeObjectTO newVolume, VolumeVO oldVolume) { + if (newVolume == null || newVolume.getSize() == null || oldVolume == null || oldVolume.getSize() == null) { return; } - Long newVolumeSize = newVolume.getSize(); - if (newVolumeSize == volumeVo.getSize()) { - // Volume size already updated, check with earlier vo in the object - if (volumeVO.getSize() != null && volumeVO.getSize() != newVolumeSize) { - Long oldVolumeSize = volumeVO.getSize(); - if (oldVolumeSize < newVolumeSize) { - resourceLimitMgr.incrementResourceCount(volumeVo.getAccountId(), ResourceType.primary_storage, volumeVo.isDisplayVolume(), newVolumeSize - oldVolumeSize); - } else { - resourceLimitMgr.decrementResourceCount(volumeVo.getAccountId(), ResourceType.primary_storage, volumeVo.isDisplayVolume(), oldVolumeSize - newVolumeSize); - } - } - } else { - Long oldVolumeSize = volumeVo.getSize(); + long newVolumeSize = newVolume.getSize(); + long oldVolumeSize = oldVolume.getSize(); + if (newVolumeSize != oldVolumeSize) { if (oldVolumeSize < newVolumeSize) { - resourceLimitMgr.incrementResourceCount(volumeVo.getAccountId(), ResourceType.primary_storage, volumeVo.isDisplayVolume(), newVolumeSize - oldVolumeSize); + resourceLimitMgr.incrementResourceCount(oldVolume.getAccountId(), ResourceType.primary_storage, oldVolume.isDisplayVolume(), newVolumeSize - oldVolumeSize); } else { - resourceLimitMgr.decrementResourceCount(volumeVo.getAccountId(), ResourceType.primary_storage, volumeVo.isDisplayVolume(), oldVolumeSize - newVolumeSize); + resourceLimitMgr.decrementResourceCount(oldVolume.getAccountId(), ResourceType.primary_storage, oldVolume.isDisplayVolume(), oldVolumeSize - newVolumeSize); } } } diff --git a/server/src/main/java/com/cloud/api/query/dao/AccountJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/AccountJoinDaoImpl.java index 38d3e8f0b210..790758c627fd 100644 --- a/server/src/main/java/com/cloud/api/query/dao/AccountJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/AccountJoinDaoImpl.java @@ -224,7 +224,7 @@ public void setResourceLimits(AccountJoinVO account, boolean fullView, ResourceL long secondaryStorageLimit = ApiDBUtils.findCorrectResourceLimit(account.getSecondaryStorageLimit(), account.getId(), ResourceType.secondary_storage); String secondaryStorageLimitDisplay = (fullView || secondaryStorageLimit == -1) ? Resource.UNLIMITED : String.valueOf(secondaryStorageLimit / ResourceType.bytesToGiB); float secondaryStorageTotal = (account.getSecondaryStorageTotal() == null) ? 0 : (account.getSecondaryStorageTotal() / (ResourceType.bytesToGiB * 1f)); - String secondaryStorageAvail = (fullView || secondaryStorageLimit == -1) ? Resource.UNLIMITED : String.valueOf((secondaryStorageLimit / ResourceType.bytesToGiB) + String secondaryStorageAvail = (fullView || secondaryStorageLimit == -1) ? Resource.UNLIMITED : String.valueOf(( (double)secondaryStorageLimit / ResourceType.bytesToGiB) - secondaryStorageTotal); response.setSecondaryStorageLimit(secondaryStorageLimitDisplay); diff --git a/server/src/main/java/com/cloud/api/query/dao/DomainJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/DomainJoinDaoImpl.java index 49bf196919dc..56f5417da3fa 100644 --- a/server/src/main/java/com/cloud/api/query/dao/DomainJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/DomainJoinDaoImpl.java @@ -123,7 +123,7 @@ public void setResourceLimits(DomainJoinVO domain, boolean fullView, ResourceLim long ipLimit = ApiDBUtils.findCorrectResourceLimitForDomain(domain.getIpLimit(), ResourceType.public_ip, domain.getId()); String ipLimitDisplay = (fullView || ipLimit == -1) ? Resource.UNLIMITED : String.valueOf(ipLimit); long ipTotal = (domain.getIpTotal() == null) ? 0 : domain.getIpTotal(); - String ipAvail = ((fullView || ipLimit == -1)) ? Resource.UNLIMITED : String.valueOf(ipLimit - ipTotal); + String ipAvail = (fullView || ipLimit == -1) ? Resource.UNLIMITED : String.valueOf(ipLimit - ipTotal); response.setIpLimit(ipLimitDisplay); response.setIpTotal(ipTotal); response.setIpAvailable(ipAvail); @@ -201,7 +201,7 @@ public void setResourceLimits(DomainJoinVO domain, boolean fullView, ResourceLim long secondaryStorageLimit = ApiDBUtils.findCorrectResourceLimitForDomain(domain.getSecondaryStorageLimit(), ResourceType.secondary_storage, domain.getId()); String secondaryStorageLimitDisplay = (fullView || secondaryStorageLimit == -1) ? Resource.UNLIMITED : String.valueOf(secondaryStorageLimit / ResourceType.bytesToGiB); float secondaryStorageTotal = (domain.getSecondaryStorageTotal() == null) ? 0 : (domain.getSecondaryStorageTotal() / (ResourceType.bytesToGiB * 1f)); - String secondaryStorageAvail = (fullView || secondaryStorageLimit == -1) ? Resource.UNLIMITED : String.valueOf((secondaryStorageLimit / ResourceType.bytesToGiB) - secondaryStorageTotal); + String secondaryStorageAvail = (fullView || secondaryStorageLimit == -1) ? Resource.UNLIMITED : String.valueOf(( (double)secondaryStorageLimit / ResourceType.bytesToGiB) - secondaryStorageTotal); response.setSecondaryStorageLimit(secondaryStorageLimitDisplay); response.setSecondaryStorageTotal(secondaryStorageTotal); response.setSecondaryStorageAvailable(secondaryStorageAvail); diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java index 4c4ca951c0de..3afc47f418b9 100644 --- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java +++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java @@ -498,7 +498,7 @@ public long findDefaultResourceLimitForDomain(ResourceType resourceType) { Long resourceLimit = null; resourceLimit = domainResourceLimitMap.get(resourceType); if (resourceLimit != null && (resourceType == ResourceType.primary_storage || resourceType == ResourceType.secondary_storage)) { - if (resourceLimit != Long.valueOf(Resource.RESOURCE_UNLIMITED)) { + if (! Long.valueOf(Resource.RESOURCE_UNLIMITED).equals(resourceLimit)) { resourceLimit = resourceLimit * ResourceType.bytesToGiB; } } else { @@ -849,6 +849,14 @@ public Boolean doInTransaction(TransactionStatus status) { } } + /** + * This will take care of re-calculation of resource counts for root and sub-domains + * and accounts of the sub-domains also. so just loop through immediate children of root domain + * + * @param domainId the domain level to start at + * @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) { return Transaction.execute(new TransactionCallback() { @@ -1118,8 +1126,6 @@ protected void runInContext() { for (ResourceType type : ResourceCount.ResourceType.values()) { if (type.supportsOwner(ResourceOwnerType.Domain)) { - // recalculateDomainResourceCount will take care of re-calculation of resource counts for root and sub-domains - // and accounts of the sub-domains also. so just loop through immediate children of root domain recalculateDomainResourceCount(Domain.ROOT_DOMAIN, type); for (Domain domain : domains) { recalculateDomainResourceCount(domain.getId(), type);