diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotDataFactory.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotDataFactory.java index d318707a29ac..32434829c074 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotDataFactory.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotDataFactory.java @@ -30,6 +30,8 @@ public interface SnapshotDataFactory { SnapshotInfo getSnapshot(long snapshotId, long storeId, DataStoreRole role); + SnapshotInfo getSnapshotIncludingRemoved(long snapshotId, long storeId, DataStoreRole role); + SnapshotInfo getSnapshotWithRoleAndZone(long snapshotId, DataStoreRole role, long zoneId); SnapshotInfo getSnapshotOnPrimaryStore(long snapshotId); diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDao.java b/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDao.java index 171634fb1044..737e4a5a53ca 100755 --- a/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDao.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDao.java @@ -47,6 +47,8 @@ public interface SnapshotDao extends GenericDao, StateDao listAllByStatus(Snapshot.State... status); + List listAllByStatusIncludingRemoved(Snapshot.State... status); + void updateVolumeIds(long oldVolId, long newVolId); List listByStatusNotIn(long volumeId, Snapshot.State... status); diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDaoImpl.java index f5fc9c47d036..238ae54e07f3 100755 --- a/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDaoImpl.java @@ -252,6 +252,13 @@ public List listAllByStatus(Snapshot.State... status) { return listBy(sc, null); } + @Override + public List listAllByStatusIncludingRemoved(Snapshot.State... status) { + SearchCriteria sc = StatusSearch.create(); + sc.setParameters("status", (Object[])status); + return listIncludingRemovedBy(sc, null); + } + @Override public List listByIds(Object... ids) { SearchCriteria sc = snapshotIdsSearch.create(); 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 4cd29b465eeb..ef0a5d0ebff0 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 @@ -56,6 +56,8 @@ public interface SnapshotDataStoreDao extends GenericDao findBySnapshotId(long snapshotId); + List findBySnapshotIdWithNonDestroyedState(long snapshotId); + void duplicateCacheRecordsOnRegionStore(long storeId); // delete the snapshot entry on primary data store to make sure that next snapshot will be full snapshot 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 5bf67eb38819..ba76a6b3f411 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 @@ -340,6 +340,13 @@ public List findByVolume(long snapshotId, long volumeId, Da @Override public List findBySnapshotId(long snapshotId) { + SearchCriteria sc = searchFilteringStoreIdEqStateEqStoreRoleEqIdEqUpdateCountEqSnapshotIdEqVolumeIdEq.create(); + sc.setParameters(SNAPSHOT_ID, snapshotId); + return listBy(sc); + } + + @Override + public List findBySnapshotIdWithNonDestroyedState(long snapshotId) { SearchCriteria sc = idStateNeqSearch.create(); sc.setParameters(SNAPSHOT_ID, snapshotId); sc.setParameters(STATE, State.Destroyed); diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java index f5cfaf072743..4fd76da0e7cf 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java @@ -270,7 +270,7 @@ public boolean deleteSnapshot(Long snapshotId, Long zoneId) { } if (Snapshot.State.Error.equals(snapshotVO.getState())) { - List storeRefs = snapshotStoreDao.findBySnapshotId(snapshotId); + List storeRefs = snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotId); List deletedRefs = new ArrayList<>(); for (SnapshotDataStoreVO ref : storeRefs) { boolean refZoneIdMatch = false; @@ -351,7 +351,7 @@ protected boolean deleteSnapshotInfos(SnapshotVO snapshotVo, Long zoneId) { protected Boolean deleteSnapshotInfo(SnapshotInfo snapshotInfo, SnapshotVO snapshotVo) { DataStore dataStore = snapshotInfo.getDataStore(); String storageToString = String.format("%s {uuid: \"%s\", name: \"%s\"}", dataStore.getRole().name(), dataStore.getUuid(), dataStore.getName()); - List snapshotStoreRefs = snapshotStoreDao.findBySnapshotId(snapshotVo.getId()); + List snapshotStoreRefs = snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotVo.getId()); boolean isLastSnapshotRef = CollectionUtils.isEmpty(snapshotStoreRefs) || snapshotStoreRefs.size() == 1; try { SnapshotObject snapshotObject = castSnapshotInfoToSnapshotObject(snapshotInfo); diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotDataFactoryImpl.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotDataFactoryImpl.java index cd314b0be638..5c0a613d82d6 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotDataFactoryImpl.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotDataFactoryImpl.java @@ -94,7 +94,7 @@ public List getSnapshots(long snapshotId, Long zoneId) { if (snapshot == null) { //snapshot may have been removed; return new ArrayList<>(); } - List allSnapshotsAndDataStore = snapshotStoreDao.findBySnapshotId(snapshotId); + List allSnapshotsAndDataStore = snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotId); if (CollectionUtils.isEmpty(allSnapshotsAndDataStore)) { return new ArrayList<>(); } @@ -118,7 +118,23 @@ public SnapshotInfo getSnapshot(long snapshotId, long storeId, DataStoreRole rol if (snapshot == null) { return null; } - SnapshotDataStoreVO snapshotStore = snapshotStoreDao.findByStoreSnapshot(role, storeId, snapshotId); + return getSnapshotOnStore(snapshot, storeId, role); + } + + @Override + public SnapshotInfo getSnapshotIncludingRemoved(long snapshotId, long storeId, DataStoreRole role) { + SnapshotVO snapshot = snapshotDao.findByIdIncludingRemoved(snapshotId); + if (snapshot == null) { + return null; + } + return getSnapshotOnStore(snapshot, storeId, role); + } + + private SnapshotInfo getSnapshotOnStore(SnapshotVO snapshot, long storeId, DataStoreRole role) { + if (snapshot == null) { + return null; + } + SnapshotDataStoreVO snapshotStore = snapshotStoreDao.findByStoreSnapshot(role, storeId, snapshot.getId()); if (snapshotStore == null) { return null; } @@ -207,7 +223,7 @@ public List listSnapshotOnCache(long snapshotId) { @Override public void updateOperationFailed(long snapshotId) throws NoTransitionException { - List snapshotStoreRefs = snapshotStoreDao.findBySnapshotId(snapshotId); + List snapshotStoreRefs = snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotId); for (SnapshotDataStoreVO snapshotStoreRef : snapshotStoreRefs) { SnapshotInfo snapshotInfo = getSnapshot(snapshotStoreRef.getSnapshotId(), snapshotStoreRef.getDataStoreId(), snapshotStoreRef.getRole()); if (snapshotInfo != null) { diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java index 2173aba3f056..d43a5371380c 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java @@ -382,8 +382,7 @@ public SnapshotInfo backupSnapshot(SnapshotInfo snapshot) { if (res.isFailed()) { throw new CloudRuntimeException(res.getResult()); } - SnapshotInfo destSnapshot = res.getSnapshot(); - return destSnapshot; + return res.getSnapshot(); } catch (InterruptedException e) { logger.debug("failed copy snapshot", e); throw new CloudRuntimeException("Failed to copy snapshot", e); @@ -391,7 +390,6 @@ public SnapshotInfo backupSnapshot(SnapshotInfo snapshot) { logger.debug("Failed to copy snapshot", e); throw new CloudRuntimeException("Failed to copy snapshot", e); } - } protected Void copySnapshotAsyncCallback(AsyncCallbackDispatcher callback, CopySnapshotContext context) { @@ -479,7 +477,6 @@ protected Void prepareCopySnapshotZoneAsyncCallback(AsyncCallbackDispatcher callback, DeleteSnapshotContext context) { - CommandResult result = callback.getResult(); AsyncCallFuture future = context.future; SnapshotInfo snapshot = context.snapshot; @@ -607,7 +604,7 @@ public void cleanupVolumeDuringSnapshotFailure(Long volumeId, Long snapshotId) { if (snapshot != null) { if (snapshot.getState() != Snapshot.State.BackedUp) { - List snapshotDataStoreVOs = _snapshotStoreDao.findBySnapshotId(snapshotId); + List snapshotDataStoreVOs = _snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotId); for (SnapshotDataStoreVO snapshotDataStoreVO : snapshotDataStoreVOs) { logger.debug("Remove snapshot {}, status {} on snapshot_store_ref table with id: {}", snapshot, snapshotDataStoreVO.getState(), snapshotDataStoreVO.getId()); @@ -712,7 +709,6 @@ public void doInTransactionWithoutResult(TransactionStatus status) { SnapshotObject srcSnapshot = (SnapshotObject)snapshot; srcSnapshot.processEvent(Event.DestroyRequested); srcSnapshot.processEvent(Event.OperationSuccessed); - srcSnapshot.processEvent(Snapshot.Event.OperationFailed); _snapshotDetailsDao.removeDetail(srcSnapshot.getId(), AsyncJob.Constants.MS_ID); @@ -723,7 +719,6 @@ public void doInTransactionWithoutResult(TransactionStatus status) { } } }); - } @Override diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java index 280d3bfbeb57..2bfa3eda11b1 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/StorageSystemSnapshotStrategy.java @@ -540,7 +540,6 @@ public void postSnapshotCreation(SnapshotInfo snapshot) { logger.warn("Failed to clean up snapshot '" + snapshot.getId() + "' on primary storage: " + e.getMessage()); } } - } private VMSnapshot takeHypervisorSnapshot(VolumeInfo volumeInfo) { diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/ObjectInDataStoreManagerImpl.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/ObjectInDataStoreManagerImpl.java index 570a47a752c6..e2eb4813973b 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/ObjectInDataStoreManagerImpl.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/ObjectInDataStoreManagerImpl.java @@ -101,6 +101,9 @@ public ObjectInDataStoreManagerImpl() { stateMachines.addTransition(State.Destroying, Event.DestroyRequested, State.Destroying); stateMachines.addTransition(State.Destroying, Event.OperationSuccessed, State.Destroyed); stateMachines.addTransition(State.Destroying, Event.OperationFailed, State.Destroying); + stateMachines.addTransition(State.Destroyed, Event.DestroyRequested, State.Destroyed); + stateMachines.addTransition(State.Destroyed, Event.OperationSuccessed, State.Destroyed); + stateMachines.addTransition(State.Destroyed, Event.OperationFailed, State.Destroyed); stateMachines.addTransition(State.Failed, Event.DestroyRequested, State.Destroying); // TODO: further investigate why an extra event is sent when it is // already Ready for DownloadListener diff --git a/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/util/StorPoolHelper.java b/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/util/StorPoolHelper.java index f13d296af3b0..e6b70c7123e1 100644 --- a/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/util/StorPoolHelper.java +++ b/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/util/StorPoolHelper.java @@ -102,7 +102,7 @@ public static String getSnapshotName(Long snapshotId, String snapshotUuid, Snaps if (snapshotDetails != null) { return StorPoolStorageAdaptor.getVolumeNameFromPath(snapshotDetails.getValue(), true); } else { - List snapshots = snapshotStoreDao.findBySnapshotId(snapshotId); + List snapshots = snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotId); if (!CollectionUtils.isEmpty(snapshots)) { for (SnapshotDataStoreVO snapshotDataStoreVO : snapshots) { String name = StorPoolStorageAdaptor.getVolumeNameFromPath(snapshotDataStoreVO.getInstallPath(), true); diff --git a/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/snapshot/StorPoolSnapshotStrategy.java b/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/snapshot/StorPoolSnapshotStrategy.java index 5ec86df91e17..072e53ab4093 100644 --- a/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/snapshot/StorPoolSnapshotStrategy.java +++ b/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/snapshot/StorPoolSnapshotStrategy.java @@ -240,7 +240,7 @@ private boolean deleteSnapshotChain(SnapshotInfo snapshot) { } protected boolean areLastSnapshotRef(long snapshotId) { - List snapshotStoreRefs = _snapshotStoreDao.findBySnapshotId(snapshotId); + List snapshotStoreRefs = _snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotId); if (CollectionUtils.isEmpty(snapshotStoreRefs) || snapshotStoreRefs.size() == 1) { return true; } @@ -308,7 +308,7 @@ private boolean deleteSnapshotFromDbIfNeeded(SnapshotVO snapshotVO, Long zoneId) } if (Snapshot.State.Error.equals(snapshotVO.getState())) { - List storeRefs = _snapshotStoreDao.findBySnapshotId(snapshotId); + List storeRefs = _snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotId); List deletedRefs = new ArrayList<>(); for (SnapshotDataStoreVO ref : storeRefs) { boolean refZoneIdMatch = false; diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index d23856552ee3..8b5e0b24f484 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -1867,41 +1867,7 @@ public void cleanupStorage(boolean recurring) { } } - //destroy snapshots in destroying state in snapshot_store_ref - List ssSnapshots = _snapshotStoreDao.listByState(ObjectInDataStoreStateMachine.State.Destroying); - for (SnapshotDataStoreVO snapshotDataStoreVO : ssSnapshots) { - String snapshotUuid = null; - SnapshotVO snapshot = null; - final String storeRole = snapshotDataStoreVO.getRole().toString().toLowerCase(); - if (logger.isDebugEnabled()) { - snapshot = _snapshotDao.findById(snapshotDataStoreVO.getSnapshotId()); - if (snapshot == null) { - logger.warn(String.format("Did not find snapshot [ID: %d] for which store reference is in destroying state; therefore, it cannot be destroyed.", snapshotDataStoreVO.getSnapshotId())); - continue; - } - snapshotUuid = snapshot.getUuid(); - } - - try { - if (logger.isDebugEnabled()) { - logger.debug(String.format("Verifying if snapshot [%s] is in destroying state in %s data store ID: %d.", snapshotUuid, storeRole, snapshotDataStoreVO.getDataStoreId())); - } - SnapshotInfo snapshotInfo = snapshotFactory.getSnapshot(snapshotDataStoreVO.getSnapshotId(), snapshotDataStoreVO.getDataStoreId(), snapshotDataStoreVO.getRole()); - if (snapshotInfo != null) { - if (logger.isDebugEnabled()) { - logger.debug(String.format("Snapshot [%s] in destroying state found in %s data store [%s]; therefore, it will be destroyed.", snapshotUuid, storeRole, snapshotInfo.getDataStore().getUuid())); - } - _snapshotService.deleteSnapshot(snapshotInfo); - } else if (logger.isDebugEnabled()) { - logger.debug(String.format("Did not find snapshot [%s] in destroying state in %s data store ID: %d.", snapshotUuid, storeRole, snapshotDataStoreVO.getDataStoreId())); - } - } catch (Exception e) { - logger.error("Failed to delete snapshot [{}] from storage due to: [{}].", snapshot, e.getMessage()); - if (logger.isDebugEnabled()) { - logger.debug("Failed to delete snapshot [{}] from storage.", snapshot, e); - } - } - } + cleanupSnapshotsFromStoreRefInDestroyingState(); cleanupSecondaryStorage(recurring); List vols = volumeDao.listVolumesToBeDestroyed(new Date(System.currentTimeMillis() - ((long)StorageCleanupDelay.value() << 10))); @@ -1941,20 +1907,7 @@ public void cleanupStorage(boolean recurring) { } } - // remove snapshots in Error state - List snapshots = _snapshotDao.listAllByStatus(Snapshot.State.Error); - for (SnapshotVO snapshotVO : snapshots) { - try { - List storeRefs = _snapshotStoreDao.findBySnapshotId(snapshotVO.getId()); - for (SnapshotDataStoreVO ref : storeRefs) { - _snapshotStoreDao.expunge(ref.getId()); - } - _snapshotDao.expunge(snapshotVO.getId()); - } catch (Exception e) { - logger.error("Unable to destroy snapshot [{}] due to: [{}].", snapshotVO, e.getMessage()); - logger.debug("Unable to destroy snapshot [{}].", snapshotVO, e); - } - } + removeSnapshotsInErrorStatus(); // destroy uploaded volumes in abandoned/error state List volumeDataStores = _volumeDataStoreDao.listByVolumeState(Volume.State.UploadError, Volume.State.UploadAbandoned); @@ -2055,6 +2008,56 @@ public void cleanupStorage(boolean recurring) { } } + private void cleanupSnapshotsFromStoreRefInDestroyingState() { + List storeRefSnapshotsInDestroyingState = _snapshotStoreDao.listByState(ObjectInDataStoreStateMachine.State.Destroying); + for (SnapshotDataStoreVO snapshotDataStoreVO : storeRefSnapshotsInDestroyingState) { + SnapshotVO snapshot = _snapshotDao.findById(snapshotDataStoreVO.getSnapshotId()); + if (snapshot == null) { + logger.warn("Did not find snapshot [ID: {}] for which store reference is in destroying state; therefore, it cannot be destroyed.", snapshotDataStoreVO.getSnapshotId()); + continue; + } + deleteSnapshot(snapshot, snapshotDataStoreVO); + } + } + + private void deleteSnapshot(SnapshotVO snapshot, SnapshotDataStoreVO snapshotDataStoreVO) { + if (snapshot == null || snapshotDataStoreVO == null) { + return; + } + + try { + final String snapshotUuid = snapshot.getUuid(); + final String storeRole = snapshotDataStoreVO.getRole().toString().toLowerCase(); + logger.debug("Snapshot [{}] is in {} state on {} data store ID: {}.", snapshotUuid, snapshotDataStoreVO.getState(), storeRole, snapshotDataStoreVO.getDataStoreId()); + SnapshotInfo snapshotInfo = snapshotFactory.getSnapshotIncludingRemoved(snapshotDataStoreVO.getSnapshotId(), snapshotDataStoreVO.getDataStoreId(), snapshotDataStoreVO.getRole()); + if (snapshotInfo != null) { + logger.debug("Snapshot [{}] in {} state found on {} data store [{}], it will be deleted.", snapshotUuid, snapshotDataStoreVO.getState(), storeRole, snapshotInfo.getDataStore().getUuid()); + _snapshotService.deleteSnapshot(snapshotInfo); + } else { + logger.debug("Did not find snapshot [{}] in {} state on {} data store ID: {}.", snapshotUuid, snapshotDataStoreVO.getState(), storeRole, snapshotDataStoreVO.getDataStoreId()); + } + } catch (Exception e) { + logger.error("Failed to delete snapshot [{}] from storage due to: [{}].", snapshot, e.getMessage(), e); + } + } + + private void removeSnapshotsInErrorStatus() { + List snapshotsInErrorStatus = _snapshotDao.listAllByStatusIncludingRemoved(Snapshot.State.Error); + for (SnapshotVO snapshotVO : snapshotsInErrorStatus) { + try { + List storeRefSnapshotsInErrorStatus = _snapshotStoreDao.findBySnapshotId(snapshotVO.getId()); + for (SnapshotDataStoreVO snapshotDataStoreVO : storeRefSnapshotsInErrorStatus) { + deleteSnapshot(snapshotVO, snapshotDataStoreVO); + _snapshotStoreDao.expunge(snapshotDataStoreVO.getId()); + } + _snapshotDao.expunge(snapshotVO.getId()); + } catch (Exception e) { + logger.error("Unable to destroy snapshot [{}] due to: [{}].", snapshotVO, e.getMessage()); + logger.debug("Unable to destroy snapshot [{}].", snapshotVO, e); + } + } + } + protected boolean isVolumeSuspectedDestroyDuplicateOfVmVolume(VolumeVO gcVolume) { if (gcVolume.getPath() == null) { return false; 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 b4b4220615e2..e7606572a07c 100755 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -721,7 +721,7 @@ private void postCreateRecurringSnapshotForPolicy(long userId, long volumeId, lo protected Pair, List> getStoreRefsAndZonesForSnapshotDelete(long snapshotId, Long zoneId) { List snapshotStoreRefs = new ArrayList<>(); - List allSnapshotStoreRefs = _snapshotStoreDao.findBySnapshotId(snapshotId); + List allSnapshotStoreRefs = _snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotId); List zoneIds = new ArrayList<>(); if (zoneId != null) { DataCenterVO zone = dataCenterDao.findById(zoneId); @@ -1503,22 +1503,22 @@ protected void backupSnapshotToSecondary(boolean asyncBackup, SnapshotStrategy s if (asyncBackup) { backupSnapshotExecutor.schedule(new BackupSnapshotTask(snapshotOnPrimary, snapshotBackupRetries - 1, snapshotStrategy, zoneIds), 0, TimeUnit.SECONDS); } else { - SnapshotInfo backupedSnapshot = snapshotStrategy.backupSnapshot(snapshotOnPrimary); - if (backupedSnapshot != null) { + SnapshotInfo backedUpSnapshot = snapshotStrategy.backupSnapshot(snapshotOnPrimary); + if (backedUpSnapshot != null) { snapshotStrategy.postSnapshotCreation(snapshotOnPrimary); } } } protected class BackupSnapshotTask extends ManagedContextRunnable { - SnapshotInfo snapshot; + SnapshotInfo snapshotOnPrimary; int attempts; SnapshotStrategy snapshotStrategy; List zoneIds; - public BackupSnapshotTask(SnapshotInfo snap, int maxRetries, SnapshotStrategy strategy, List zoneIds) { - snapshot = snap; + public BackupSnapshotTask(SnapshotInfo snapshot, int maxRetries, SnapshotStrategy strategy, List zoneIds) { + snapshotOnPrimary = snapshot; attempts = maxRetries; snapshotStrategy = strategy; this.zoneIds = zoneIds; @@ -1529,19 +1529,18 @@ protected void runInContext() { try { logger.debug("Value of attempts is " + (snapshotBackupRetries - attempts)); - SnapshotInfo backupedSnapshot = snapshotStrategy.backupSnapshot(snapshot); - - if (backupedSnapshot != null) { - snapshotStrategy.postSnapshotCreation(snapshot); - copyNewSnapshotToZones(snapshot.getId(), snapshot.getDataCenterId(), zoneIds); + SnapshotInfo backedUpSnapshot = snapshotStrategy.backupSnapshot(snapshotOnPrimary); + if (backedUpSnapshot != null) { + snapshotStrategy.postSnapshotCreation(snapshotOnPrimary); + copyNewSnapshotToZones(snapshotOnPrimary.getId(), snapshotOnPrimary.getDataCenterId(), zoneIds); } } catch (final Exception e) { if (attempts >= 0) { - logger.debug("Backing up of snapshot failed, for snapshot {}, left with {} more attempts", snapshot, attempts); - backupSnapshotExecutor.schedule(new BackupSnapshotTask(snapshot, --attempts, snapshotStrategy, zoneIds), snapshotBackupRetryInterval, TimeUnit.SECONDS); + logger.debug("Backing up of snapshot failed, for snapshot {}, left with {} more attempts", snapshotOnPrimary, attempts); + backupSnapshotExecutor.schedule(new BackupSnapshotTask(snapshotOnPrimary, --attempts, snapshotStrategy, zoneIds), snapshotBackupRetryInterval, TimeUnit.SECONDS); } else { - logger.debug("Done with {} attempts in backing up of snapshot {}", snapshotBackupRetries, snapshot.getSnapshotVO()); - snapshotSrv.cleanupOnSnapshotBackupFailure(snapshot); + logger.debug("Done with {} attempts in backing up of snapshot {}", snapshotBackupRetries, snapshotOnPrimary.getSnapshotVO()); + snapshotSrv.cleanupOnSnapshotBackupFailure(snapshotOnPrimary); } } } @@ -1762,7 +1761,7 @@ public Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName, public void markVolumeSnapshotsAsDestroyed(Volume volume) { List snapshots = _snapshotDao.listByVolumeId(volume.getId()); for (SnapshotVO snapshot: snapshots) { - List snapshotDataStoreVOs = _snapshotStoreDao.findBySnapshotId(snapshot.getId()); + List snapshotDataStoreVOs = _snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshot.getId()); if (CollectionUtils.isEmpty(snapshotDataStoreVOs)) { snapshot.setState(Snapshot.State.Destroyed); _snapshotDao.update(snapshot.getId(), snapshot); diff --git a/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerImplTest.java b/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerImplTest.java index e6c2a0d0f3cf..86fdcfecc137 100644 --- a/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerImplTest.java +++ b/server/src/test/java/com/cloud/storage/snapshot/SnapshotManagerImplTest.java @@ -139,7 +139,7 @@ public void testGetStoreRefsAndZonesForSnapshotDeleteMultiZones() { Mockito.when(ref1.getDataStoreId()).thenReturn(2L); Mockito.when(ref1.getRole()).thenReturn(DataStoreRole.Image); List snapshotStoreList = List.of(ref, ref1); - Mockito.when(snapshotStoreDao.findBySnapshotId(snapshotId)).thenReturn(snapshotStoreList); + Mockito.when(snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotId)).thenReturn(snapshotStoreList); Mockito.when(dataStoreManager.getStoreZoneId(1L, DataStoreRole.Image)).thenReturn(100L); Mockito.when(dataStoreManager.getStoreZoneId(2L, DataStoreRole.Image)).thenReturn(101L); Pair, List> pair = snapshotManager.getStoreRefsAndZonesForSnapshotDelete(snapshotId, null); @@ -164,7 +164,7 @@ public void testGetStoreRefsAndZonesForSnapshotDeleteSingle() { Mockito.when(ref2.getDataStoreId()).thenReturn(3L); Mockito.when(ref2.getRole()).thenReturn(DataStoreRole.Image); List snapshotStoreList = List.of(ref, ref1, ref2); - Mockito.when(snapshotStoreDao.findBySnapshotId(snapshotId)).thenReturn(snapshotStoreList); + Mockito.when(snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotId)).thenReturn(snapshotStoreList); Mockito.when(dataStoreManager.getStoreZoneId(1L, DataStoreRole.Image)).thenReturn(zoneId); Mockito.when(dataStoreManager.getStoreZoneId(2L, DataStoreRole.Primary)).thenReturn(zoneId); Mockito.when(dataStoreManager.getStoreZoneId(3L, DataStoreRole.Image)).thenReturn(2L);