Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ public interface SnapshotDao extends GenericDao<SnapshotVO, Long>, StateDao<Snap

List<SnapshotVO> listAllByStatus(Snapshot.State... status);

List<SnapshotVO> listAllByStatusIncludingRemoved(Snapshot.State... status);

void updateVolumeIds(long oldVolId, long newVolId);

List<SnapshotVO> listByStatusNotIn(long volumeId, Snapshot.State... status);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,13 @@ public List<SnapshotVO> listAllByStatus(Snapshot.State... status) {
return listBy(sc, null);
}

@Override
public List<SnapshotVO> listAllByStatusIncludingRemoved(Snapshot.State... status) {
SearchCriteria<SnapshotVO> sc = StatusSearch.create();
sc.setParameters("status", (Object[])status);
return listIncludingRemovedBy(sc, null);
}

@Override
public List<SnapshotVO> listByIds(Object... ids) {
SearchCriteria<SnapshotVO> sc = snapshotIdsSearch.create();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ public interface SnapshotDataStoreDao extends GenericDao<SnapshotDataStoreVO, Lo

List<SnapshotDataStoreVO> findBySnapshotId(long snapshotId);

List<SnapshotDataStoreVO> 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,13 @@ public List<SnapshotDataStoreVO> findByVolume(long snapshotId, long volumeId, Da

@Override
public List<SnapshotDataStoreVO> findBySnapshotId(long snapshotId) {
SearchCriteria<SnapshotDataStoreVO> sc = searchFilteringStoreIdEqStateEqStoreRoleEqIdEqUpdateCountEqSnapshotIdEqVolumeIdEq.create();
sc.setParameters(SNAPSHOT_ID, snapshotId);
return listBy(sc);
}

@Override
public List<SnapshotDataStoreVO> findBySnapshotIdWithNonDestroyedState(long snapshotId) {
SearchCriteria<SnapshotDataStoreVO> sc = idStateNeqSearch.create();
sc.setParameters(SNAPSHOT_ID, snapshotId);
sc.setParameters(STATE, State.Destroyed);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ public boolean deleteSnapshot(Long snapshotId, Long zoneId) {
}

if (Snapshot.State.Error.equals(snapshotVO.getState())) {
List<SnapshotDataStoreVO> storeRefs = snapshotStoreDao.findBySnapshotId(snapshotId);
List<SnapshotDataStoreVO> storeRefs = snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotId);
List<Long> deletedRefs = new ArrayList<>();
for (SnapshotDataStoreVO ref : storeRefs) {
boolean refZoneIdMatch = false;
Expand Down Expand Up @@ -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<SnapshotDataStoreVO> snapshotStoreRefs = snapshotStoreDao.findBySnapshotId(snapshotVo.getId());
List<SnapshotDataStoreVO> snapshotStoreRefs = snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotVo.getId());
boolean isLastSnapshotRef = CollectionUtils.isEmpty(snapshotStoreRefs) || snapshotStoreRefs.size() == 1;
try {
SnapshotObject snapshotObject = castSnapshotInfoToSnapshotObject(snapshotInfo);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public List<SnapshotInfo> getSnapshots(long snapshotId, Long zoneId) {
if (snapshot == null) { //snapshot may have been removed;
return new ArrayList<>();
}
List<SnapshotDataStoreVO> allSnapshotsAndDataStore = snapshotStoreDao.findBySnapshotId(snapshotId);
List<SnapshotDataStoreVO> allSnapshotsAndDataStore = snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotId);
if (CollectionUtils.isEmpty(allSnapshotsAndDataStore)) {
return new ArrayList<>();
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -207,7 +223,7 @@ public List<SnapshotInfo> listSnapshotOnCache(long snapshotId) {

@Override
public void updateOperationFailed(long snapshotId) throws NoTransitionException {
List<SnapshotDataStoreVO> snapshotStoreRefs = snapshotStoreDao.findBySnapshotId(snapshotId);
List<SnapshotDataStoreVO> snapshotStoreRefs = snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotId);
for (SnapshotDataStoreVO snapshotStoreRef : snapshotStoreRefs) {
SnapshotInfo snapshotInfo = getSnapshot(snapshotStoreRef.getSnapshotId(), snapshotStoreRef.getDataStoreId(), snapshotStoreRef.getRole());
if (snapshotInfo != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -382,16 +382,14 @@ 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);
} catch (ExecutionException e) {
logger.debug("Failed to copy snapshot", e);
throw new CloudRuntimeException("Failed to copy snapshot", e);
}

}

protected Void copySnapshotAsyncCallback(AsyncCallbackDispatcher<SnapshotServiceImpl, CopyCommandResult> callback, CopySnapshotContext<CommandResult> context) {
Expand Down Expand Up @@ -479,7 +477,6 @@ protected Void prepareCopySnapshotZoneAsyncCallback(AsyncCallbackDispatcher<Snap
}

protected Void deleteSnapshotCallback(AsyncCallbackDispatcher<SnapshotServiceImpl, CommandResult> callback, DeleteSnapshotContext<CommandResult> context) {

CommandResult result = callback.getResult();
AsyncCallFuture<SnapshotResult> future = context.future;
SnapshotInfo snapshot = context.snapshot;
Expand Down Expand Up @@ -607,7 +604,7 @@ public void cleanupVolumeDuringSnapshotFailure(Long volumeId, Long snapshotId) {

if (snapshot != null) {
if (snapshot.getState() != Snapshot.State.BackedUp) {
List<SnapshotDataStoreVO> snapshotDataStoreVOs = _snapshotStoreDao.findBySnapshotId(snapshotId);
List<SnapshotDataStoreVO> 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());

Expand Down Expand Up @@ -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);
Expand All @@ -723,7 +719,6 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
}
}
});

}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public static String getSnapshotName(Long snapshotId, String snapshotUuid, Snaps
if (snapshotDetails != null) {
return StorPoolStorageAdaptor.getVolumeNameFromPath(snapshotDetails.getValue(), true);
} else {
List<SnapshotDataStoreVO> snapshots = snapshotStoreDao.findBySnapshotId(snapshotId);
List<SnapshotDataStoreVO> snapshots = snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotId);
if (!CollectionUtils.isEmpty(snapshots)) {
for (SnapshotDataStoreVO snapshotDataStoreVO : snapshots) {
String name = StorPoolStorageAdaptor.getVolumeNameFromPath(snapshotDataStoreVO.getInstallPath(), true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ private boolean deleteSnapshotChain(SnapshotInfo snapshot) {
}

protected boolean areLastSnapshotRef(long snapshotId) {
List<SnapshotDataStoreVO> snapshotStoreRefs = _snapshotStoreDao.findBySnapshotId(snapshotId);
List<SnapshotDataStoreVO> snapshotStoreRefs = _snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotId);
if (CollectionUtils.isEmpty(snapshotStoreRefs) || snapshotStoreRefs.size() == 1) {
return true;
}
Expand Down Expand Up @@ -308,7 +308,7 @@ private boolean deleteSnapshotFromDbIfNeeded(SnapshotVO snapshotVO, Long zoneId)
}

if (Snapshot.State.Error.equals(snapshotVO.getState())) {
List<SnapshotDataStoreVO> storeRefs = _snapshotStoreDao.findBySnapshotId(snapshotId);
List<SnapshotDataStoreVO> storeRefs = _snapshotStoreDao.findBySnapshotIdWithNonDestroyedState(snapshotId);
List<Long> deletedRefs = new ArrayList<>();
for (SnapshotDataStoreVO ref : storeRefs) {
boolean refZoneIdMatch = false;
Expand Down
108 changes: 59 additions & 49 deletions server/src/main/java/com/cloud/storage/StorageManagerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -1867,41 +1867,7 @@ public void cleanupStorage(boolean recurring) {
}
}

//destroy snapshots in destroying state in snapshot_store_ref
List<SnapshotDataStoreVO> 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<VolumeVO> vols = volumeDao.listVolumesToBeDestroyed(new Date(System.currentTimeMillis() - ((long)StorageCleanupDelay.value() << 10)));
Expand Down Expand Up @@ -1941,20 +1907,7 @@ public void cleanupStorage(boolean recurring) {
}
}

// remove snapshots in Error state
List<SnapshotVO> snapshots = _snapshotDao.listAllByStatus(Snapshot.State.Error);
for (SnapshotVO snapshotVO : snapshots) {
try {
List<SnapshotDataStoreVO> 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<VolumeDataStoreVO> volumeDataStores = _volumeDataStoreDao.listByVolumeState(Volume.State.UploadError, Volume.State.UploadAbandoned);
Expand Down Expand Up @@ -2055,6 +2008,63 @@ public void cleanupStorage(boolean recurring) {
}
}

private void cleanupSnapshotsFromStoreRefInDestroyingState() {
List<SnapshotDataStoreVO> 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();
if (logger.isDebugEnabled()) {
logger.debug("Snapshot [{}] is in {} state on {} data store ID: {}.", snapshotUuid, snapshotDataStoreVO.getState(), storeRole, snapshotDataStoreVO.getDataStoreId());
}
Comment on lines +2031 to +2033
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (logger.isDebugEnabled()) {
logger.debug("Snapshot [{}] is in {} state on {} data store ID: {}.", snapshotUuid, snapshotDataStoreVO.getState(), storeRole, snapshotDataStoreVO.getDataStoreId());
}
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) {
if (logger.isDebugEnabled()) {
logger.debug("Snapshot [{}] in {} state found on {} data store [{}], it will be deleted.", snapshotUuid, snapshotDataStoreVO.getState(), storeRole, snapshotInfo.getDataStore().getUuid());
}
Comment on lines +2036 to +2038
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the condition is not needed for the simple getters. Else lambdas can work as well.

Suggested change
if (logger.isDebugEnabled()) {
logger.debug("Snapshot [{}] in {} state found on {} data store [{}], it will be deleted.", snapshotUuid, snapshotDataStoreVO.getState(), storeRole, snapshotInfo.getDataStore().getUuid());
}
logger.debug("Snapshot [{}] in {} state found on {} data store [{}], it will be deleted.", snapshotUuid, snapshotDataStoreVO.getState(), storeRole, snapshotInfo.getDataStore().getUuid());

_snapshotService.deleteSnapshot(snapshotInfo);
} else if (logger.isDebugEnabled()) {
logger.debug("Did not find snapshot [{}] in {} state on {} data store ID: {}.", snapshotUuid, snapshotDataStoreVO.getState(), storeRole, snapshotDataStoreVO.getDataStoreId());
}
Comment on lines +2040 to +2042
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
} else if (logger.isDebugEnabled()) {
logger.debug("Did not find snapshot [{}] in {} state on {} data store ID: {}.", snapshotUuid, snapshotDataStoreVO.getState(), storeRole, snapshotDataStoreVO.getDataStoreId());
}
} 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());
if (logger.isDebugEnabled()) {
logger.debug("Failed to delete snapshot [{}] from storage.", snapshot, e);
}
Comment on lines +2045 to +2047
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (logger.isDebugEnabled()) {
logger.debug("Failed to delete snapshot [{}] from storage.", snapshot, e);
}
logger.debug("Failed to delete snapshot [{}] from storage.", snapshot, e);

}
}

private void removeSnapshotsInErrorStatus() {
List<SnapshotVO> snapshotsInErrorStatus = _snapshotDao.listAllByStatusIncludingRemoved(Snapshot.State.Error);
for (SnapshotVO snapshotVO : snapshotsInErrorStatus) {
try {
List<SnapshotDataStoreVO> 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;
Expand Down
Loading
Loading