Skip to content

Commit 83ce006

Browse files
sureshanapartisureshanaparti
authored
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
1 parent ff7ec0c commit 83ce006

File tree

6 files changed

+73
-23
lines changed

6 files changed

+73
-23
lines changed

‎api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotCmd.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -244,8 +244,7 @@ public void execute() {
244244
}
245245

246246
private Snapshot.LocationType getLocationType() {
247-
248-
if (Snapshot.LocationType.values() == null || Snapshot.LocationType.values().length == 0 || locationType == null) {
247+
if (locationType == null) {
249248
return null;
250249
}
251250

‎engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,4 +110,18 @@ public interface SnapshotDataStoreDao extends GenericDao<SnapshotDataStoreVO, Lo
110110
void updateDisplayForSnapshotStoreRole(long snapshotId, long storeId, DataStoreRole role, boolean display);
111111

112112
int expungeBySnapshotList(List<Long> snapshotIds, Long batchSize);
113+
114+
/**
115+
* Returns the total physical size, in bytes, of all snapshots stored on primary
116+
* storage for the specified account that have not yet been backed up to
117+
* secondary storage.
118+
*
119+
* <p>If no such snapshots are found, this method returns {@code 0}.</p>
120+
*
121+
* @param accountId the ID of the account whose snapshots on primary storage
122+
* should be considered
123+
* @return the total physical size in bytes of matching snapshots on primary
124+
* storage, or {@code 0} if none are found
125+
*/
126+
long getSnapshotsPhysicalSizeOnPrimaryStorageByAccountId(long accountId);
113127
}

‎engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,15 @@ public class SnapshotDataStoreDaoImpl extends GenericDaoBase<SnapshotDataStoreVO
7878
" order by created %s " +
7979
" limit 1";
8080

81+
private static final String GET_PHYSICAL_SIZE_OF_SNAPSHOTS_ON_PRIMARY_BY_ACCOUNT = "SELECT SUM(s.physical_size) " +
82+
"FROM cloud.snapshot_store_ref s " +
83+
"LEFT JOIN cloud.snapshots ON s.snapshot_id = snapshots.id " +
84+
"WHERE snapshots.account_id = ? " +
85+
"AND snapshots.removed IS NULL " +
86+
"AND s.state = 'Ready' " +
87+
"AND s.store_role = 'Primary' " +
88+
"AND NOT EXISTS (SELECT 1 FROM cloud.snapshot_store_ref i WHERE i.snapshot_id = s.snapshot_id AND i.store_role = 'Image')";
89+
8190
@Override
8291
public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
8392
super.configure(name, params);
@@ -118,7 +127,6 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
118127
stateSearch.and(STATE, stateSearch.entity().getState(), SearchCriteria.Op.IN);
119128
stateSearch.done();
120129

121-
122130
idStateNeqSearch = createSearchBuilder();
123131
idStateNeqSearch.and(SNAPSHOT_ID, idStateNeqSearch.entity().getSnapshotId(), SearchCriteria.Op.EQ);
124132
idStateNeqSearch.and(STATE, idStateNeqSearch.entity().getState(), SearchCriteria.Op.NEQ);
@@ -578,4 +586,23 @@ public int expungeBySnapshotList(final List<Long> snapshotIds, final Long batchS
578586
sc.setParameters("snapshotIds", snapshotIds.toArray());
579587
return batchExpunge(sc, batchSize);
580588
}
589+
590+
@Override
591+
public long getSnapshotsPhysicalSizeOnPrimaryStorageByAccountId(long accountId) {
592+
long snapshotsPhysicalSize = 0;
593+
try (TransactionLegacy transactionLegacy = TransactionLegacy.currentTxn()) {
594+
try (PreparedStatement preparedStatement = transactionLegacy.prepareStatement(GET_PHYSICAL_SIZE_OF_SNAPSHOTS_ON_PRIMARY_BY_ACCOUNT)) {
595+
preparedStatement.setLong(1, accountId);
596+
597+
try (ResultSet resultSet = preparedStatement.executeQuery()) {
598+
if (resultSet.next()) {
599+
snapshotsPhysicalSize = resultSet.getLong(1);
600+
}
601+
}
602+
}
603+
} catch (SQLException e) {
604+
logger.warn("Failed to get the snapshots physical size for the account [{}] due to [{}].", accountId, e.getMessage(), e);
605+
}
606+
return snapshotsPhysicalSize;
607+
}
581608
}

‎server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1171,7 +1171,6 @@ protected long recalculateDomainResourceCount(final long domainId, final Resourc
11711171
}
11721172

11731173
return Transaction.execute((TransactionCallback<Long>) status -> {
1174-
long newResourceCount = 0L;
11751174
List<Long> domainIdList = childDomains.stream().map(DomainVO::getId).collect(Collectors.toList());
11761175
domainIdList.add(domainId);
11771176
List<Long> accountIdList = accounts.stream().map(AccountVO::getId).collect(Collectors.toList());
@@ -1189,6 +1188,7 @@ protected long recalculateDomainResourceCount(final long domainId, final Resourc
11891188
List<ResourceCountVO> resourceCounts = _resourceCountDao.lockRows(rowIdsToLock);
11901189

11911190
long oldResourceCount = 0L;
1191+
long newResourceCount = 0L;
11921192
ResourceCountVO domainRC = null;
11931193

11941194
// calculate project count here
@@ -1210,7 +1210,7 @@ protected long recalculateDomainResourceCount(final long domainId, final Resourc
12101210
if (oldResourceCount != newResourceCount) {
12111211
domainRC.setCount(newResourceCount);
12121212
_resourceCountDao.update(domainRC.getId(), domainRC);
1213-
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.",
1213+
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.",
12141214
oldResourceCount, newResourceCount, type, domainId);
12151215
}
12161216
return newResourceCount;
@@ -1436,16 +1436,17 @@ private long calculatePublicIpForAccount(long accountId) {
14361436
}
14371437

14381438
protected long calculatePrimaryStorageForAccount(long accountId, String tag) {
1439+
long snapshotsPhysicalSizeOnPrimaryStorage = _snapshotDataStoreDao.getSnapshotsPhysicalSizeOnPrimaryStorageByAccountId(accountId);
14391440
if (StringUtils.isEmpty(tag)) {
14401441
List<Long> virtualRouters = _vmDao.findIdsOfAllocatedVirtualRoutersForAccount(accountId);
1441-
return _volumeDao.primaryStorageUsedForAccount(accountId, virtualRouters);
1442+
return snapshotsPhysicalSizeOnPrimaryStorage + _volumeDao.primaryStorageUsedForAccount(accountId, virtualRouters);
14421443
}
14431444
long storage = 0;
14441445
List<VolumeVO> volumes = getVolumesWithAccountAndTag(accountId, tag);
14451446
for (VolumeVO volume : volumes) {
14461447
storage += volume.getSize() == null ? 0L : volume.getSize();
14471448
}
1448-
return storage;
1449+
return snapshotsPhysicalSizeOnPrimaryStorage + storage;
14491450
}
14501451

14511452
@Override
@@ -2143,7 +2144,6 @@ public ConfigKey<?>[] getConfigKeys() {
21432144

21442145
protected class ResourceCountCheckTask extends ManagedContextRunnable {
21452146
public ResourceCountCheckTask() {
2146-
21472147
}
21482148

21492149
@Override

‎server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java‎

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,15 @@ protected boolean isBackupSnapshotToSecondaryForZone(long zoneId) {
276276
return !DataCenter.Type.Edge.equals(zone.getType());
277277
}
278278

279+
private ResourceType getStoreResourceType(long dataCenterId, Snapshot.LocationType locationType) {
280+
ResourceType storeResourceType = ResourceType.secondary_storage;
281+
if (!isBackupSnapshotToSecondaryForZone(dataCenterId) ||
282+
Snapshot.LocationType.PRIMARY.equals(locationType)) {
283+
storeResourceType = ResourceType.primary_storage;
284+
}
285+
return storeResourceType;
286+
}
287+
279288
@Override
280289
public String getConfigComponentName() {
281290
return SnapshotManager.class.getSimpleName();
@@ -614,15 +623,14 @@ public Snapshot backupSnapshotFromVmSnapshot(Long snapshotId, Long vmId, Long vo
614623
_snapshotDao.update(snapshot.getId(), snapshot);
615624
snapshotInfo = this.snapshotFactory.getSnapshot(snapshotId, store);
616625

617-
Long snapshotOwnerId = vm.getAccountId();
626+
long snapshotOwnerId = vm.getAccountId();
618627

619628
try {
620629
SnapshotStrategy snapshotStrategy = _storageStrategyFactory.getSnapshotStrategy(snapshot, SnapshotOperation.BACKUP);
621630
if (snapshotStrategy == null) {
622631
throw new CloudRuntimeException(String.format("Unable to find Snapshot strategy to handle Snapshot [%s]", snapshot));
623632
}
624633
snapshotInfo = snapshotStrategy.backupSnapshot(snapshotInfo);
625-
626634
} catch (Exception e) {
627635
logger.debug("Failed to backup Snapshot from Instance Snapshot", e);
628636
_resourceLimitMgr.decrementResourceCount(snapshotOwnerId, ResourceType.snapshot);
@@ -771,12 +779,11 @@ public boolean deleteSnapshot(long snapshotId, Long zoneId) {
771779
_accountMgr.checkAccess(caller, null, true, snapshotCheck);
772780

773781
SnapshotStrategy snapshotStrategy = _storageStrategyFactory.getSnapshotStrategy(snapshotCheck, zoneId, SnapshotOperation.DELETE);
774-
775782
if (snapshotStrategy == null) {
776783
logger.error("Unable to find snapshot strategy to handle snapshot [{}]", snapshotCheck);
777-
778784
return false;
779785
}
786+
780787
Pair<List<SnapshotDataStoreVO>, List<Long>> storeRefAndZones = getStoreRefsAndZonesForSnapshotDelete(snapshotId, zoneId);
781788
List<SnapshotDataStoreVO> snapshotStoreRefs = storeRefAndZones.first();
782789
List<Long> zoneIds = storeRefAndZones.second();
@@ -1472,8 +1479,9 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationExc
14721479
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_SNAPSHOT_CREATE, snapshot.getAccountId(), snapshot.getDataCenterId(), snapshotId, snapshot.getName(), null, null,
14731480
snapshotStoreRef.getPhysicalSize(), volume.getSize(), snapshot.getClass().getName(), snapshot.getUuid());
14741481

1482+
ResourceType storeResourceType = dataStoreRole == DataStoreRole.Image ? ResourceType.secondary_storage : ResourceType.primary_storage;
14751483
// Correct the resource count of snapshot in case of delta snapshots.
1476-
_resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), ResourceType.secondary_storage, new Long(volume.getSize() - snapshotStoreRef.getPhysicalSize()));
1484+
_resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), storeResourceType, new Long(volume.getSize() - snapshotStoreRef.getPhysicalSize()));
14771485

14781486
if (!payload.getAsyncBackup() && backupSnapToSecondary) {
14791487
copyNewSnapshotToZones(snapshotId, snapshot.getDataCenterId(), payload.getZoneIds());
@@ -1485,15 +1493,17 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationExc
14851493
if (logger.isDebugEnabled()) {
14861494
logger.debug("Failed to create snapshot" + cre.getLocalizedMessage());
14871495
}
1496+
ResourceType storeResourceType = getStoreResourceType(volume.getDataCenterId(), payload.getLocationType());
14881497
_resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), ResourceType.snapshot);
1489-
_resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), ResourceType.secondary_storage, new Long(volume.getSize()));
1498+
_resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), storeResourceType, new Long(volume.getSize()));
14901499
throw cre;
14911500
} catch (Exception e) {
14921501
if (logger.isDebugEnabled()) {
14931502
logger.debug("Failed to create snapshot", e);
14941503
}
1504+
ResourceType storeResourceType = getStoreResourceType(volume.getDataCenterId(), payload.getLocationType());
14951505
_resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), ResourceType.snapshot);
1496-
_resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), ResourceType.secondary_storage, new Long(volume.getSize()));
1506+
_resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), storeResourceType, new Long(volume.getSize()));
14971507
throw new CloudRuntimeException("Failed to create snapshot", e);
14981508
}
14991509
return snapshot;
@@ -1695,11 +1705,7 @@ public Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName,
16951705
Type snapshotType = getSnapshotType(policyId);
16961706
Account owner = _accountMgr.getAccount(volume.getAccountId());
16971707

1698-
ResourceType storeResourceType = ResourceType.secondary_storage;
1699-
if (!isBackupSnapshotToSecondaryForZone(volume.getDataCenterId()) ||
1700-
Snapshot.LocationType.PRIMARY.equals(locationType)) {
1701-
storeResourceType = ResourceType.primary_storage;
1702-
}
1708+
ResourceType storeResourceType = getStoreResourceType(volume.getDataCenterId(), locationType);
17031709
try {
17041710
_resourceLimitMgr.checkResourceLimit(owner, ResourceType.snapshot);
17051711
_resourceLimitMgr.checkResourceLimit(owner, storeResourceType, volume.getSize());

‎server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.apache.cloudstack.api.response.TaggedResourceLimitAndCountResponse;
2929
import org.apache.cloudstack.framework.config.ConfigKey;
3030
import org.apache.cloudstack.reservation.dao.ReservationDao;
31+
import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
3132
import org.apache.commons.collections.CollectionUtils;
3233
import org.apache.commons.lang3.StringUtils;
3334
import org.apache.logging.log4j.LogManager;
@@ -118,6 +119,8 @@ public class ResourceLimitManagerImplTest extends TestCase {
118119
VolumeDao volumeDao;
119120
@Mock
120121
UserVmDao userVmDao;
122+
@Mock
123+
SnapshotDataStoreDao snapshotDataStoreDao;
121124

122125
private List<String> hostTags = List.of("htag1", "htag2", "htag3");
123126
private List<String> storageTags = List.of("stag1", "stag2");
@@ -840,20 +843,21 @@ public void testCalculatePrimaryStorageForAccount() {
840843
String tag = null;
841844
Mockito.when(vmDao.findIdsOfAllocatedVirtualRoutersForAccount(accountId))
842845
.thenReturn(List.of(1L));
846+
Mockito.when(snapshotDataStoreDao.getSnapshotsPhysicalSizeOnPrimaryStorageByAccountId(accountId)).thenReturn(100L);
843847
Mockito.when(volumeDao.primaryStorageUsedForAccount(Mockito.eq(accountId), Mockito.anyList())).thenReturn(100L);
844-
Assert.assertEquals(100L, resourceLimitManager.calculatePrimaryStorageForAccount(accountId, tag));
848+
Assert.assertEquals(200L, resourceLimitManager.calculatePrimaryStorageForAccount(accountId, tag));
845849

846850
tag = "";
847851
Mockito.when(volumeDao.primaryStorageUsedForAccount(Mockito.eq(accountId), Mockito.anyList())).thenReturn(200L);
848-
Assert.assertEquals(200L, resourceLimitManager.calculatePrimaryStorageForAccount(accountId, tag));
852+
Assert.assertEquals(300L, resourceLimitManager.calculatePrimaryStorageForAccount(accountId, tag));
849853

850854
tag = "tag";
851855
VolumeVO vol = Mockito.mock(VolumeVO.class);
852856
long size = 1024;
853857
Mockito.when(vol.getSize()).thenReturn(size);
854858
List<VolumeVO> vols = List.of(vol, vol);
855859
Mockito.doReturn(vols).when(resourceLimitManager).getVolumesWithAccountAndTag(accountId, tag);
856-
Assert.assertEquals(vols.size() * size, resourceLimitManager.calculatePrimaryStorageForAccount(accountId, tag));
860+
Assert.assertEquals((vols.size() * size) + 100L, resourceLimitManager.calculatePrimaryStorageForAccount(accountId, tag));
857861
}
858862

859863
@Test

0 commit comments

Comments
 (0)