This is an automated email from the ASF dual-hosted git repository.
bstoyanov 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 83ce0067b82 Update the snapshot physical size for the primary storage
resource after snapshot creation and during resource count recalculation
(#12481)
83ce0067b82 is described below
commit 83ce0067b82bc39c7c91667c6ac4d2dd144ce450
Author: Suresh Kumar Anaparti <[email protected]>
AuthorDate: Wed Jan 28 16:37:57 2026 +0530
Update the snapshot physical size for the primary storage resource after
snapshot creation and during resource count recalculation (#12481)
* Update snapshot size for the primary storage resource after snapshot
creation and during resource count recalculation
* Update snapshot physical size
* review
* review
---
.../command/user/snapshot/CreateSnapshotCmd.java | 3 +--
.../storage/datastore/db/SnapshotDataStoreDao.java | 14 ++++++++++
.../datastore/db/SnapshotDataStoreDaoImpl.java | 29 ++++++++++++++++++++-
.../resourcelimit/ResourceLimitManagerImpl.java | 10 ++++----
.../storage/snapshot/SnapshotManagerImpl.java | 30 +++++++++++++---------
.../ResourceLimitManagerImplTest.java | 10 +++++---
6 files changed, 73 insertions(+), 23 deletions(-)
diff --git
a/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotCmd.java
b/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotCmd.java
index bd541b69183..078d4517f95 100644
---
a/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotCmd.java
+++
b/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotCmd.java
@@ -244,8 +244,7 @@ public class CreateSnapshotCmd extends BaseAsyncCreateCmd {
}
private Snapshot.LocationType getLocationType() {
-
- if (Snapshot.LocationType.values() == null ||
Snapshot.LocationType.values().length == 0 || locationType == null) {
+ if (locationType == null) {
return null;
}
diff --git
a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java
b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java
index ef0a5d0ebff..96df4928773 100644
---
a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java
+++
b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java
@@ -110,4 +110,18 @@ StateDao<ObjectInDataStoreStateMachine.State,
ObjectInDataStoreStateMachine.Even
void updateDisplayForSnapshotStoreRole(long snapshotId, long storeId,
DataStoreRole role, boolean display);
int expungeBySnapshotList(List<Long> snapshotIds, Long batchSize);
+
+ /**
+ * Returns the total physical size, in bytes, of all snapshots stored on
primary
+ * storage for the specified account that have not yet been backed up to
+ * secondary storage.
+ *
+ * <p>If no such snapshots are found, this method returns {@code 0}.</p>
+ *
+ * @param accountId the ID of the account whose snapshots on primary
storage
+ * should be considered
+ * @return the total physical size in bytes of matching snapshots on
primary
+ * storage, or {@code 0} if none are found
+ */
+ long getSnapshotsPhysicalSizeOnPrimaryStorageByAccountId(long accountId);
}
diff --git
a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java
b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java
index ba76a6b3f41..c68316dd1fe 100644
---
a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java
+++
b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java
@@ -78,6 +78,15 @@ public class SnapshotDataStoreDaoImpl extends
GenericDaoBase<SnapshotDataStoreVO
" order by created %s " +
" limit 1";
+ private static final String
GET_PHYSICAL_SIZE_OF_SNAPSHOTS_ON_PRIMARY_BY_ACCOUNT = "SELECT
SUM(s.physical_size) " +
+ "FROM cloud.snapshot_store_ref s " +
+ "LEFT JOIN cloud.snapshots ON s.snapshot_id = snapshots.id " +
+ "WHERE snapshots.account_id = ? " +
+ "AND snapshots.removed IS NULL " +
+ "AND s.state = 'Ready' " +
+ "AND s.store_role = 'Primary' " +
+ "AND NOT EXISTS (SELECT 1 FROM cloud.snapshot_store_ref i WHERE
i.snapshot_id = s.snapshot_id AND i.store_role = 'Image')";
+
@Override
public boolean configure(String name, Map<String, Object> params) throws
ConfigurationException {
super.configure(name, params);
@@ -118,7 +127,6 @@ public class SnapshotDataStoreDaoImpl extends
GenericDaoBase<SnapshotDataStoreVO
stateSearch.and(STATE, stateSearch.entity().getState(),
SearchCriteria.Op.IN);
stateSearch.done();
-
idStateNeqSearch = createSearchBuilder();
idStateNeqSearch.and(SNAPSHOT_ID,
idStateNeqSearch.entity().getSnapshotId(), SearchCriteria.Op.EQ);
idStateNeqSearch.and(STATE, idStateNeqSearch.entity().getState(),
SearchCriteria.Op.NEQ);
@@ -578,4 +586,23 @@ public class SnapshotDataStoreDaoImpl extends
GenericDaoBase<SnapshotDataStoreVO
sc.setParameters("snapshotIds", snapshotIds.toArray());
return batchExpunge(sc, batchSize);
}
+
+ @Override
+ public long getSnapshotsPhysicalSizeOnPrimaryStorageByAccountId(long
accountId) {
+ long snapshotsPhysicalSize = 0;
+ try (TransactionLegacy transactionLegacy =
TransactionLegacy.currentTxn()) {
+ try (PreparedStatement preparedStatement =
transactionLegacy.prepareStatement(GET_PHYSICAL_SIZE_OF_SNAPSHOTS_ON_PRIMARY_BY_ACCOUNT))
{
+ preparedStatement.setLong(1, accountId);
+
+ try (ResultSet resultSet = preparedStatement.executeQuery()) {
+ if (resultSet.next()) {
+ snapshotsPhysicalSize = resultSet.getLong(1);
+ }
+ }
+ }
+ } catch (SQLException e) {
+ logger.warn("Failed to get the snapshots physical size for the
account [{}] due to [{}].", accountId, e.getMessage(), e);
+ }
+ return snapshotsPhysicalSize;
+ }
}
diff --git
a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
index b890b72f758..d4d91b6de7b 100644
--- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
+++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
@@ -1171,7 +1171,6 @@ public class ResourceLimitManagerImpl extends ManagerBase
implements ResourceLim
}
return Transaction.execute((TransactionCallback<Long>) status -> {
- long newResourceCount = 0L;
List<Long> domainIdList =
childDomains.stream().map(DomainVO::getId).collect(Collectors.toList());
domainIdList.add(domainId);
List<Long> accountIdList =
accounts.stream().map(AccountVO::getId).collect(Collectors.toList());
@@ -1189,6 +1188,7 @@ public class ResourceLimitManagerImpl extends ManagerBase
implements ResourceLim
List<ResourceCountVO> resourceCounts =
_resourceCountDao.lockRows(rowIdsToLock);
long oldResourceCount = 0L;
+ long newResourceCount = 0L;
ResourceCountVO domainRC = null;
// calculate project count here
@@ -1210,7 +1210,7 @@ public class ResourceLimitManagerImpl extends ManagerBase
implements ResourceLim
if (oldResourceCount != newResourceCount) {
domainRC.setCount(newResourceCount);
_resourceCountDao.update(domainRC.getId(), domainRC);
- logger.warn("Discrepency in the resource count has been
detected (original count = {} correct count = {}) for Type = {} for Domain ID =
{} is fixed during resource count recalculation.",
+ logger.warn("Discrepancy in the resource count has been
detected (original count = {} correct count = {}) for Type = {} for Domain ID =
{} is fixed during resource count recalculation.",
oldResourceCount, newResourceCount, type, domainId);
}
return newResourceCount;
@@ -1436,16 +1436,17 @@ public class ResourceLimitManagerImpl extends
ManagerBase implements ResourceLim
}
protected long calculatePrimaryStorageForAccount(long accountId, String
tag) {
+ long snapshotsPhysicalSizeOnPrimaryStorage =
_snapshotDataStoreDao.getSnapshotsPhysicalSizeOnPrimaryStorageByAccountId(accountId);
if (StringUtils.isEmpty(tag)) {
List<Long> virtualRouters =
_vmDao.findIdsOfAllocatedVirtualRoutersForAccount(accountId);
- return _volumeDao.primaryStorageUsedForAccount(accountId,
virtualRouters);
+ return snapshotsPhysicalSizeOnPrimaryStorage +
_volumeDao.primaryStorageUsedForAccount(accountId, virtualRouters);
}
long storage = 0;
List<VolumeVO> volumes = getVolumesWithAccountAndTag(accountId, tag);
for (VolumeVO volume : volumes) {
storage += volume.getSize() == null ? 0L : volume.getSize();
}
- return storage;
+ return snapshotsPhysicalSizeOnPrimaryStorage + storage;
}
@Override
@@ -2143,7 +2144,6 @@ public class ResourceLimitManagerImpl extends ManagerBase
implements ResourceLim
protected class ResourceCountCheckTask extends ManagedContextRunnable {
public ResourceCountCheckTask() {
-
}
@Override
diff --git
a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java
b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java
index e7606572a07..19cde4da0f1 100755
--- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java
+++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java
@@ -276,6 +276,15 @@ public class SnapshotManagerImpl extends
MutualExclusiveIdsManagerBase implement
return !DataCenter.Type.Edge.equals(zone.getType());
}
+ private ResourceType getStoreResourceType(long dataCenterId,
Snapshot.LocationType locationType) {
+ ResourceType storeResourceType = ResourceType.secondary_storage;
+ if (!isBackupSnapshotToSecondaryForZone(dataCenterId) ||
+ Snapshot.LocationType.PRIMARY.equals(locationType)) {
+ storeResourceType = ResourceType.primary_storage;
+ }
+ return storeResourceType;
+ }
+
@Override
public String getConfigComponentName() {
return SnapshotManager.class.getSimpleName();
@@ -614,7 +623,7 @@ public class SnapshotManagerImpl extends
MutualExclusiveIdsManagerBase implement
_snapshotDao.update(snapshot.getId(), snapshot);
snapshotInfo = this.snapshotFactory.getSnapshot(snapshotId, store);
- Long snapshotOwnerId = vm.getAccountId();
+ long snapshotOwnerId = vm.getAccountId();
try {
SnapshotStrategy snapshotStrategy =
_storageStrategyFactory.getSnapshotStrategy(snapshot, SnapshotOperation.BACKUP);
@@ -622,7 +631,6 @@ public class SnapshotManagerImpl extends
MutualExclusiveIdsManagerBase implement
throw new CloudRuntimeException(String.format("Unable to find
Snapshot strategy to handle Snapshot [%s]", snapshot));
}
snapshotInfo = snapshotStrategy.backupSnapshot(snapshotInfo);
-
} catch (Exception e) {
logger.debug("Failed to backup Snapshot from Instance Snapshot",
e);
_resourceLimitMgr.decrementResourceCount(snapshotOwnerId,
ResourceType.snapshot);
@@ -771,12 +779,11 @@ public class SnapshotManagerImpl extends
MutualExclusiveIdsManagerBase implement
_accountMgr.checkAccess(caller, null, true, snapshotCheck);
SnapshotStrategy snapshotStrategy =
_storageStrategyFactory.getSnapshotStrategy(snapshotCheck, zoneId,
SnapshotOperation.DELETE);
-
if (snapshotStrategy == null) {
logger.error("Unable to find snapshot strategy to handle snapshot
[{}]", snapshotCheck);
-
return false;
}
+
Pair<List<SnapshotDataStoreVO>, List<Long>> storeRefAndZones =
getStoreRefsAndZonesForSnapshotDelete(snapshotId, zoneId);
List<SnapshotDataStoreVO> snapshotStoreRefs = storeRefAndZones.first();
List<Long> zoneIds = storeRefAndZones.second();
@@ -1472,8 +1479,9 @@ public class SnapshotManagerImpl extends
MutualExclusiveIdsManagerBase implement
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_SNAPSHOT_CREATE,
snapshot.getAccountId(), snapshot.getDataCenterId(), snapshotId,
snapshot.getName(), null, null,
snapshotStoreRef.getPhysicalSize(), volume.getSize(),
snapshot.getClass().getName(), snapshot.getUuid());
+ ResourceType storeResourceType = dataStoreRole ==
DataStoreRole.Image ? ResourceType.secondary_storage :
ResourceType.primary_storage;
// Correct the resource count of snapshot in case of delta
snapshots.
-
_resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(),
ResourceType.secondary_storage, new Long(volume.getSize() -
snapshotStoreRef.getPhysicalSize()));
+
_resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(),
storeResourceType, new Long(volume.getSize() -
snapshotStoreRef.getPhysicalSize()));
if (!payload.getAsyncBackup() && backupSnapToSecondary) {
copyNewSnapshotToZones(snapshotId,
snapshot.getDataCenterId(), payload.getZoneIds());
@@ -1485,15 +1493,17 @@ public class SnapshotManagerImpl extends
MutualExclusiveIdsManagerBase implement
if (logger.isDebugEnabled()) {
logger.debug("Failed to create snapshot" +
cre.getLocalizedMessage());
}
+ ResourceType storeResourceType =
getStoreResourceType(volume.getDataCenterId(), payload.getLocationType());
_resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(),
ResourceType.snapshot);
- _resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(),
ResourceType.secondary_storage, new Long(volume.getSize()));
+ _resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(),
storeResourceType, new Long(volume.getSize()));
throw cre;
} catch (Exception e) {
if (logger.isDebugEnabled()) {
logger.debug("Failed to create snapshot", e);
}
+ ResourceType storeResourceType =
getStoreResourceType(volume.getDataCenterId(), payload.getLocationType());
_resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(),
ResourceType.snapshot);
- _resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(),
ResourceType.secondary_storage, new Long(volume.getSize()));
+ _resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(),
storeResourceType, new Long(volume.getSize()));
throw new CloudRuntimeException("Failed to create snapshot", e);
}
return snapshot;
@@ -1695,11 +1705,7 @@ public class SnapshotManagerImpl extends
MutualExclusiveIdsManagerBase implement
Type snapshotType = getSnapshotType(policyId);
Account owner = _accountMgr.getAccount(volume.getAccountId());
- ResourceType storeResourceType = ResourceType.secondary_storage;
- if (!isBackupSnapshotToSecondaryForZone(volume.getDataCenterId()) ||
- Snapshot.LocationType.PRIMARY.equals(locationType)) {
- storeResourceType = ResourceType.primary_storage;
- }
+ ResourceType storeResourceType =
getStoreResourceType(volume.getDataCenterId(), locationType);
try {
_resourceLimitMgr.checkResourceLimit(owner, ResourceType.snapshot);
_resourceLimitMgr.checkResourceLimit(owner, storeResourceType,
volume.getSize());
diff --git
a/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java
b/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java
index 34030626d22..53ccc830dd2 100644
---
a/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java
+++
b/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java
@@ -28,6 +28,7 @@ import org.apache.cloudstack.api.response.DomainResponse;
import org.apache.cloudstack.api.response.TaggedResourceLimitAndCountResponse;
import org.apache.cloudstack.framework.config.ConfigKey;
import org.apache.cloudstack.reservation.dao.ReservationDao;
+import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
@@ -118,6 +119,8 @@ public class ResourceLimitManagerImplTest extends TestCase {
VolumeDao volumeDao;
@Mock
UserVmDao userVmDao;
+ @Mock
+ SnapshotDataStoreDao snapshotDataStoreDao;
private List<String> hostTags = List.of("htag1", "htag2", "htag3");
private List<String> storageTags = List.of("stag1", "stag2");
@@ -840,12 +843,13 @@ public class ResourceLimitManagerImplTest extends
TestCase {
String tag = null;
Mockito.when(vmDao.findIdsOfAllocatedVirtualRoutersForAccount(accountId))
.thenReturn(List.of(1L));
+
Mockito.when(snapshotDataStoreDao.getSnapshotsPhysicalSizeOnPrimaryStorageByAccountId(accountId)).thenReturn(100L);
Mockito.when(volumeDao.primaryStorageUsedForAccount(Mockito.eq(accountId),
Mockito.anyList())).thenReturn(100L);
- Assert.assertEquals(100L,
resourceLimitManager.calculatePrimaryStorageForAccount(accountId, tag));
+ Assert.assertEquals(200L,
resourceLimitManager.calculatePrimaryStorageForAccount(accountId, tag));
tag = "";
Mockito.when(volumeDao.primaryStorageUsedForAccount(Mockito.eq(accountId),
Mockito.anyList())).thenReturn(200L);
- Assert.assertEquals(200L,
resourceLimitManager.calculatePrimaryStorageForAccount(accountId, tag));
+ Assert.assertEquals(300L,
resourceLimitManager.calculatePrimaryStorageForAccount(accountId, tag));
tag = "tag";
VolumeVO vol = Mockito.mock(VolumeVO.class);
@@ -853,7 +857,7 @@ public class ResourceLimitManagerImplTest extends TestCase {
Mockito.when(vol.getSize()).thenReturn(size);
List<VolumeVO> vols = List.of(vol, vol);
Mockito.doReturn(vols).when(resourceLimitManager).getVolumesWithAccountAndTag(accountId,
tag);
- Assert.assertEquals(vols.size() * size,
resourceLimitManager.calculatePrimaryStorageForAccount(accountId, tag));
+ Assert.assertEquals((vols.size() * size) + 100L,
resourceLimitManager.calculatePrimaryStorageForAccount(accountId, tag));
}
@Test
