weizhouapache commented on code in PR #14132:
URL: https://github.com/apache/cloudstack/pull/14132#discussion_r4183137016
##########
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java:
##########
@@ -450,6 +450,7 @@ public AsyncCallFuture<VolumeApiResult>
expungeVolumeAsync(VolumeInfo volume) {
future.complete(result);
return future;
}
+ deletePrimaryOnlySnapshotsBeforeRbdVolumeDelete(vol);
Review Comment:
@Damans227
Yes, primary-only snapshots (those not backed up to secondary storage) will
be gone even if the volume delete then fails. CloudStack Ceph users should be
aware of this behaviour.
Deleting the snapshots before removing the volume has these benefits:
- Resource account: the account's snapshot resource count is decremented.
- Events: the SNAPSHOT.DELETE event and usage record are published.
- Consistency: the database and the storage stay strongly consistent,
because each snapshot is removed through the normal deletion workflow while it
still exists on RBD.
- Chain handling: snapshot chains and parent/child relationships are handled
by the existing logic, with no duplicate code path.
The drawback is extra time, since each snapshot needs its own delete command
to the agent. I think that cost is acceptable.
The alternative is to delete the volume first and then clean up the
snapshots. It saves time, but if the deletion fails, we can't tell what state
the snapshots are in without checking the volume.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
