Skip to content

Commit 07c5b20

Browse files
sureshanapartidhslove
sureshanaparti
authored and
dhslove
committed
Update the snapshot physical size for the primary storage resource after snapshot creation and during resource count recalculation (apache#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 86f5ff9 commit 07c5b20

5 files changed

Lines changed: 77 additions & 15 deletions

File tree

‎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
@@ -130,4 +130,18 @@ public interface SnapshotDataStoreDao extends GenericDao<SnapshotDataStoreVO, Lo
130130
void updateDisplayForSnapshotStoreRole(long snapshotId, long storeId, DataStoreRole role, boolean display);
131131

132132
int expungeBySnapshotList(List<Long> snapshotIds, Long batchSize);
133+
134+
/**
135+
* Returns the total physical size, in bytes, of all snapshots stored on primary
136+
* storage for the specified account that have not yet been backed up to
137+
* secondary storage.
138+
*
139+
* <p>If no such snapshots are found, this method returns {@code 0}.</p>
140+
*
141+
* @param accountId the ID of the account whose snapshots on primary storage
142+
* should be considered
143+
* @return the total physical size in bytes of matching snapshots on primary
144+
* storage, or {@code 0} if none are found
145+
*/
146+
long getSnapshotsPhysicalSizeOnPrimaryStorageByAccountId(long accountId);
133147
}

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,15 @@ public class SnapshotDataStoreDaoImpl extends GenericDaoBase<SnapshotDataStoreVO
9696
private static final String FIND_SNAPSHOT_IN_ZONE = "SELECT ssr.* FROM " +
9797
"snapshot_store_ref ssr, snapshots s " +
9898
"WHERE ssr.snapshot_id=? AND ssr.snapshot_id = s.id AND s.data_center_id=?;";
99+
100+
private static final String GET_PHYSICAL_SIZE_OF_SNAPSHOTS_ON_PRIMARY_BY_ACCOUNT = "SELECT SUM(s.physical_size) " +
101+
"FROM cloud.snapshot_store_ref s " +
102+
"LEFT JOIN cloud.snapshots ON s.snapshot_id = snapshots.id " +
103+
"WHERE snapshots.account_id = ? " +
104+
"AND snapshots.removed IS NULL " +
105+
"AND s.state = 'Ready' " +
106+
"AND s.store_role = 'Primary' " +
107+
"AND NOT EXISTS (SELECT 1 FROM cloud.snapshot_store_ref i WHERE i.snapshot_id = s.snapshot_id AND i.store_role = 'Image')";
99108

100109
@Override
101110
public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
@@ -142,6 +151,11 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
142151
idStateNinSearch.and(SNAPSHOT_ID, idStateNinSearch.entity().getSnapshotId(), SearchCriteria.Op.EQ);
143152
idStateNinSearch.and(STATE, idStateNinSearch.entity().getState(), SearchCriteria.Op.NOTIN);
144153
idStateNinSearch.done();
154+
155+
idStateNeqSearch = createSearchBuilder();
156+
idStateNeqSearch.and(SNAPSHOT_ID, idStateNeqSearch.entity().getSnapshotId(), SearchCriteria.Op.EQ);
157+
idStateNeqSearch.and(STATE, idStateNeqSearch.entity().getState(), SearchCriteria.Op.NEQ);
158+
idStateNeqSearch.done();
145159

146160
snapshotVOSearch = snapshotDao.createSearchBuilder();
147161
snapshotVOSearch.and(VOLUME_ID, snapshotVOSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
@@ -733,4 +747,23 @@ public int expungeBySnapshotList(final List<Long> snapshotIds, final Long batchS
733747
sc.setParameters("snapshotIds", snapshotIds.toArray());
734748
return batchExpunge(sc, batchSize);
735749
}
750+
751+
@Override
752+
public long getSnapshotsPhysicalSizeOnPrimaryStorageByAccountId(long accountId) {
753+
long snapshotsPhysicalSize = 0;
754+
try (TransactionLegacy transactionLegacy = TransactionLegacy.currentTxn()) {
755+
try (PreparedStatement preparedStatement = transactionLegacy.prepareStatement(GET_PHYSICAL_SIZE_OF_SNAPSHOTS_ON_PRIMARY_BY_ACCOUNT)) {
756+
preparedStatement.setLong(1, accountId);
757+
758+
try (ResultSet resultSet = preparedStatement.executeQuery()) {
759+
if (resultSet.next()) {
760+
snapshotsPhysicalSize = resultSet.getLong(1);
761+
}
762+
}
763+
}
764+
} catch (SQLException e) {
765+
logger.warn("Failed to get the snapshots physical size for the account [{}] due to [{}].", accountId, e.getMessage(), e);
766+
}
767+
return snapshotsPhysicalSize;
768+
}
736769
}

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

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

12261226
return Transaction.execute((TransactionCallback<Long>) status -> {
1227-
long newResourceCount = 0L;
12281227
List<Long> domainIdList = childDomains.stream().map(DomainVO::getId).collect(Collectors.toList());
12291228
domainIdList.add(domainId);
12301229
List<Long> accountIdList = accounts.stream().map(AccountVO::getId).collect(Collectors.toList());
@@ -1242,6 +1241,7 @@ protected long recalculateDomainResourceCount(final long domainId, final Resourc
12421241
List<ResourceCountVO> resourceCounts = _resourceCountDao.lockRows(rowIdsToLock);
12431242

12441243
long oldResourceCount = 0L;
1244+
long newResourceCount = 0L;
12451245
ResourceCountVO domainRC = null;
12461246

12471247
// calculate project count here
@@ -1263,7 +1263,7 @@ protected long recalculateDomainResourceCount(final long domainId, final Resourc
12631263
if (oldResourceCount != newResourceCount) {
12641264
domainRC.setCount(newResourceCount);
12651265
_resourceCountDao.update(domainRC.getId(), domainRC);
1266-
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.",
1266+
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.",
12671267
oldResourceCount, newResourceCount, type, domainId);
12681268
}
12691269
return newResourceCount;
@@ -1527,16 +1527,17 @@ private long calculatePublicIpForAccount(long accountId) {
15271527
}
15281528

15291529
protected long calculatePrimaryStorageForAccount(long accountId, String tag) {
1530+
long snapshotsPhysicalSizeOnPrimaryStorage = _snapshotDataStoreDao.getSnapshotsPhysicalSizeOnPrimaryStorageByAccountId(accountId);
15301531
if (StringUtils.isEmpty(tag)) {
15311532
List<Long> virtualRouters = _vmDao.findIdsOfAllocatedVirtualRoutersForAccount(accountId);
1532-
return _volumeDao.primaryStorageUsedForAccount(accountId, virtualRouters);
1533+
return snapshotsPhysicalSizeOnPrimaryStorage + _volumeDao.primaryStorageUsedForAccount(accountId, virtualRouters);
15331534
}
15341535
long storage = 0;
15351536
List<VolumeVO> volumes = getVolumesWithAccountAndTag(accountId, tag);
15361537
for (VolumeVO volume : volumes) {
15371538
storage += volume.getSize() == null ? 0L : volume.getSize();
15381539
}
1539-
return storage;
1540+
return snapshotsPhysicalSizeOnPrimaryStorage + storage;
15401541
}
15411542

15421543
@Override
@@ -2296,7 +2297,6 @@ public ConfigKey<?>[] getConfigKeys() {
22962297

22972298
protected class ResourceCountCheckTask extends ManagedContextRunnable {
22982299
public ResourceCountCheckTask() {
2299-
23002300
}
23012301

23022302
@Override

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

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,15 @@ protected boolean isBackupSnapshotToSecondaryForZone(long zoneId) {
290290
return !DataCenter.Type.Edge.equals(zone.getType());
291291
}
292292

293+
private ResourceType getStoreResourceType(long dataCenterId, Snapshot.LocationType locationType) {
294+
ResourceType storeResourceType = ResourceType.secondary_storage;
295+
if (!isBackupSnapshotToSecondaryForZone(dataCenterId) ||
296+
Snapshot.LocationType.PRIMARY.equals(locationType)) {
297+
storeResourceType = ResourceType.primary_storage;
298+
}
299+
return storeResourceType;
300+
}
301+
293302
@Override
294303
public String getConfigComponentName() {
295304
return SnapshotManager.class.getSimpleName();
@@ -680,15 +689,14 @@ public Snapshot backupSnapshotFromVmSnapshot(Long snapshotId, Long vmId, Long vo
680689
_snapshotDao.update(snapshot.getId(), snapshot);
681690
snapshotInfo = this.snapshotFactory.getSnapshot(snapshotId, store);
682691

683-
Long snapshotOwnerId = vm.getAccountId();
692+
long snapshotOwnerId = vm.getAccountId();
684693

685694
try {
686695
SnapshotStrategy snapshotStrategy = _storageStrategyFactory.getSnapshotStrategy(snapshot, SnapshotOperation.BACKUP);
687696
if (snapshotStrategy == null) {
688697
throw new CloudRuntimeException(String.format("Unable to find Snapshot strategy to handle Snapshot [%s]", snapshot));
689698
}
690699
snapshotInfo = snapshotStrategy.backupSnapshot(snapshotInfo);
691-
692700
} catch (Exception e) {
693701
logger.debug("Failed to backup Snapshot from Instance Snapshot", e);
694702
_resourceLimitMgr.decrementResourceCount(snapshotOwnerId, ResourceType.snapshot);
@@ -898,12 +906,11 @@ public boolean deleteSnapshot(long snapshotId, Long zoneId) {
898906
_accountMgr.checkAccess(caller, null, true, snapshotCheck);
899907

900908
SnapshotStrategy snapshotStrategy = _storageStrategyFactory.getSnapshotStrategy(snapshotCheck, zoneId, SnapshotOperation.DELETE);
901-
902909
if (snapshotStrategy == null) {
903910
logger.error("Unable to find snapshot strategy to handle snapshot [{}]", snapshotCheck);
904-
905911
return false;
906912
}
913+
907914
Pair<List<SnapshotDataStoreVO>, List<Long>> storeRefAndZones = getStoreRefsAndZonesForSnapshotDelete(snapshotId, zoneId);
908915
List<SnapshotDataStoreVO> snapshotStoreRefs = storeRefAndZones.first();
909916
List<Long> zoneIds = storeRefAndZones.second();
@@ -1728,8 +1735,9 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationExc
17281735
snapshotStoreRef.getPhysicalSize(), volume.getSize(), snapshot.getClass().getName(), snapshot.getUuid());
17291736
}
17301737

1738+
ResourceType storeResourceType = dataStoreRole == DataStoreRole.Image ? ResourceType.secondary_storage : ResourceType.primary_storage;
17311739
// Correct the resource count of snapshot in case of delta snapshots.
1732-
_resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), ResourceType.secondary_storage, new Long(volume.getSize() - snapshotStoreRef.getPhysicalSize()));
1740+
_resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), storeResourceType, new Long(volume.getSize() - snapshotStoreRef.getPhysicalSize()));
17331741

17341742
if (!payload.getAsyncBackup()) {
17351743
if (backupSnapToSecondary) {
@@ -1747,15 +1755,17 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationExc
17471755
if (logger.isDebugEnabled()) {
17481756
logger.debug("Failed to create snapshot - " + cre.getLocalizedMessage());
17491757
}
1758+
ResourceType storeResourceType = getStoreResourceType(volume.getDataCenterId(), payload.getLocationType());
17501759
_resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), ResourceType.snapshot);
1751-
_resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), ResourceType.secondary_storage, volume.getSize());
1760+
_resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), storeResourceType, new Long(volume.getSize()));
17521761
throw cre;
17531762
} catch (Exception e) {
17541763
if (logger.isDebugEnabled()) {
17551764
logger.debug("Failed to create snapshot", e);
17561765
}
1766+
ResourceType storeResourceType = getStoreResourceType(volume.getDataCenterId(), payload.getLocationType());
17571767
_resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), ResourceType.snapshot);
1758-
_resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), ResourceType.secondary_storage, volume.getSize());
1768+
_resourceLimitMgr.decrementResourceCount(snapshotOwner.getId(), storeResourceType, new Long(volume.getSize()));
17591769
throw new CloudRuntimeException("Failed to create snapshot", e);
17601770
}
17611771
return snapshot;
@@ -2055,6 +2065,7 @@ public Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName,
20552065
Type snapshotType = getSnapshotType(policyId);
20562066
Account owner = _accountMgr.getAccount(volume.getAccountId());
20572067

2068+
ResourceType storeResourceType = getStoreResourceType(volume.getDataCenterId(), locationType);
20582069
try {
20592070
_resourceLimitMgr.checkResourceLimit(owner, ResourceType.snapshot);
20602071
_resourceLimitMgr.checkResourceLimit(owner, ResourceType.secondary_storage, new Long(volume.getSize()).longValue());

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.apache.cloudstack.context.CallContext;
3535
import org.apache.cloudstack.framework.config.ConfigKey;
3636
import org.apache.cloudstack.reservation.dao.ReservationDao;
37+
import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
3738
import org.apache.commons.collections.CollectionUtils;
3839
import org.apache.commons.lang3.StringUtils;
3940
import org.apache.logging.log4j.LogManager;
@@ -129,6 +130,8 @@ public class ResourceLimitManagerImplTest {
129130
UserVmDao userVmDao;
130131
@Mock
131132
EntityManager entityManager;
133+
@Mock
134+
SnapshotDataStoreDao snapshotDataStoreDao;
132135

133136
private CallContext callContext;
134137
private List<String> hostTags = List.of("htag1", "htag2", "htag3");
@@ -900,20 +903,21 @@ public void testCalculatePrimaryStorageForAccount() {
900903
String tag = null;
901904
Mockito.when(vmDao.findIdsOfAllocatedVirtualRoutersForAccount(accountId))
902905
.thenReturn(List.of(1L));
906+
Mockito.when(snapshotDataStoreDao.getSnapshotsPhysicalSizeOnPrimaryStorageByAccountId(accountId)).thenReturn(100L);
903907
Mockito.when(volumeDao.primaryStorageUsedForAccount(Mockito.eq(accountId), Mockito.anyList())).thenReturn(100L);
904-
Assert.assertEquals(100L, resourceLimitManager.calculatePrimaryStorageForAccount(accountId, tag));
908+
Assert.assertEquals(200L, resourceLimitManager.calculatePrimaryStorageForAccount(accountId, tag));
905909

906910
tag = "";
907911
Mockito.when(volumeDao.primaryStorageUsedForAccount(Mockito.eq(accountId), Mockito.anyList())).thenReturn(200L);
908-
Assert.assertEquals(200L, resourceLimitManager.calculatePrimaryStorageForAccount(accountId, tag));
912+
Assert.assertEquals(300L, resourceLimitManager.calculatePrimaryStorageForAccount(accountId, tag));
909913

910914
tag = "tag";
911915
VolumeVO vol = Mockito.mock(VolumeVO.class);
912916
long size = 1024;
913917
Mockito.when(vol.getSize()).thenReturn(size);
914918
List<VolumeVO> vols = List.of(vol, vol);
915919
Mockito.doReturn(vols).when(resourceLimitManager).getVolumesWithAccountAndTag(accountId, tag);
916-
Assert.assertEquals(vols.size() * size, resourceLimitManager.calculatePrimaryStorageForAccount(accountId, tag));
920+
Assert.assertEquals((vols.size() * size) + 100L, resourceLimitManager.calculatePrimaryStorageForAccount(accountId, tag));
917921
}
918922

919923
@Test

0 commit comments

Comments
 (0)