Fix snapshot copy resource limit concurrency

This commit is contained in:
Fabricio Duarte 2026-03-18 06:57:02 +05:30 committed by Abhisar Sinha
parent ce52b9dae0
commit 639ceeea61
2 changed files with 20 additions and 20 deletions

View File

@ -1807,15 +1807,17 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement
@DB
private boolean copySnapshotToZone(SnapshotDataStoreVO snapshotDataStoreVO, DataStore srcSecStore,
DataCenterVO dstZone, DataStore dstSecStore, Account account)
DataCenterVO dstZone, DataStore dstSecStore, Account account, boolean shouldCheckResourceLimits)
throws ResourceAllocationException {
final long snapshotId = snapshotDataStoreVO.getSnapshotId();
final long dstZoneId = dstZone.getId();
if (checkAndProcessSnapshotAlreadyExistInStore(snapshotId, dstSecStore)) {
return true;
}
try (CheckedReservation secStorageReservation = new CheckedReservation(account, ResourceType.secondary_storage,
snapshotDataStoreVO.getSize(), reservationDao, _resourceLimitMgr)) {
// Resource limit checks are not performed here at the moment, but they were added in case this method is used
// in the future to copy a standalone snapshot
long requiredSecondaryStorageSpace = shouldCheckResourceLimits ? snapshotDataStoreVO.getSize() : 0L;
try (CheckedReservation secondaryStorageReservation = new CheckedReservation(account, ResourceType.secondary_storage, null, null, null, requiredSecondaryStorageSpace, null, reservationDao, _resourceLimitMgr)) {
SnapshotInfo snapshotOnSecondary = snapshotFactory.getSnapshot(snapshotId, srcSecStore);
String copyUrl = null;
try {
@ -1846,6 +1848,7 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_SNAPSHOT_COPY, account.getId(), dstZoneId, snapshotId, null, null, null, snapshotVO.getSize(),
snapshotVO.getSize(), snapshotVO.getClass().getName(), snapshotVO.getUuid());
}
return true;
} catch (InterruptedException | ExecutionException | ResourceUnavailableException ex) {
logger.debug("Failed to copy snapshot ID: {} to image store: {}", snapshotId, dstSecStore);
@ -1882,13 +1885,6 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement
if (CollectionUtils.isEmpty(snapshotChain)) {
return true;
}
try {
_resourceLimitMgr.checkResourceLimit(account, ResourceType.secondary_storage, size);
} catch (ResourceAllocationException e) {
logger.error(String.format("Unable to allocate secondary storage resources for snapshot chain for %s with size: %d", snapshotVO, size), e);
return false;
}
Collections.reverse(snapshotChain);
if (dstSecStore == null) {
// find all eligible image stores for the destination zone
List<DataStore> dstSecStores = dataStoreMgr.getImageStoresByScopeExcludingReadOnly(new ZoneScope(destZoneId));
@ -1900,15 +1896,21 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement
throw new StorageUnavailableException("Destination zone is not ready, no image store with free capacity", DataCenter.class, destZoneId);
}
}
logger.debug("Copying snapshot chain for snapshot ID: {} on secondary store: {} of zone ID: {}", snapshotVO, dstSecStore, destZone);
for (SnapshotDataStoreVO snapshotDataStoreVO : snapshotChain) {
if (!copySnapshotToZone(snapshotDataStoreVO, srcSecStore, destZone, dstSecStore, account)) {
logger.error("Failed to copy snapshot: {} to zone: {} due to failure to copy snapshot ID: {} from snapshot chain",
snapshotVO, destZone, snapshotDataStoreVO.getSnapshotId());
return false;
try (CheckedReservation secondaryStorageReservation = new CheckedReservation(account, ResourceType.secondary_storage, null, null, null, size, null, reservationDao, _resourceLimitMgr)) {
logger.debug("Copying snapshot chain for snapshot ID: {} on secondary store: {} of zone ID: {}", snapshotVO, dstSecStore, destZone);
Collections.reverse(snapshotChain);
for (SnapshotDataStoreVO snapshotDataStoreVO : snapshotChain) {
if (!copySnapshotToZone(snapshotDataStoreVO, srcSecStore, destZone, dstSecStore, account, false)) {
logger.error("Failed to copy snapshot: {} to zone: {} due to failure to copy snapshot ID: {} from snapshot chain",
snapshotVO, destZone, snapshotDataStoreVO.getSnapshotId());
return false;
}
}
return true;
} catch (ResourceAllocationException e) {
logger.error(String.format("Unable to allocate secondary storage resources for snapshot chain for %s with size: %d", snapshotVO, size), e);
return false;
}
return true;
}
@DB

View File

@ -46,7 +46,6 @@ import com.cloud.dc.dao.DataCenterDao;
import com.cloud.event.ActionEventUtils;
import com.cloud.exception.InvalidParameterValueException;
import com.cloud.exception.PermissionDeniedException;
import com.cloud.exception.ResourceAllocationException;
import com.cloud.exception.ResourceUnavailableException;
import com.cloud.org.Grouping;
import com.cloud.storage.DataStoreRole;
@ -284,12 +283,11 @@ public class SnapshotManagerImplTest {
Mockito.when(result1.isFailed()).thenReturn(false);
AsyncCallFuture<SnapshotResult> future1 = Mockito.mock(AsyncCallFuture.class);
try {
Mockito.doNothing().when(resourceLimitService).checkResourceLimit(Mockito.any(), Mockito.any(), Mockito.anyLong());
Mockito.when(future.get()).thenReturn(result);
Mockito.when(snapshotService.queryCopySnapshot(Mockito.any())).thenReturn(future);
Mockito.when(future1.get()).thenReturn(result1);
Mockito.when(snapshotService.copySnapshot(Mockito.any(SnapshotInfo.class), Mockito.anyString(), Mockito.any(DataStore.class))).thenReturn(future1);
} catch (ResourceAllocationException | ResourceUnavailableException | ExecutionException | InterruptedException e) {
} catch (ResourceUnavailableException | ExecutionException | InterruptedException e) {
Assert.fail(e.getMessage());
}
List<Long> addedZone = new ArrayList<>();