hsato03 commented on code in PR #8031:
URL: https://github.com/apache/cloudstack/pull/8031#discussion_r1353106386
##########
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##########
@@ -1289,37 +1289,52 @@ public void cleanupStorage(boolean recurring) {
try {
List<VMTemplateStoragePoolVO>
unusedTemplatesInPool = _tmpltMgr.getUnusedTemplatesInPool(pool);
- s_logger.debug("Storage pool garbage collector
found " + unusedTemplatesInPool.size() + " templates to clean up in storage
pool: " + pool.getName());
+ s_logger.debug(String.format("Storage pool
garbage collector found [%s] templates to be cleaned up in storage pool [%s].",
unusedTemplatesInPool.size(), pool.getName()));
for (VMTemplateStoragePoolVO templatePoolVO :
unusedTemplatesInPool) {
if (templatePoolVO.getDownloadState() !=
VMTemplateStorageResourceAssoc.Status.DOWNLOADED) {
- s_logger.debug("Storage pool garbage
collector is skipping template with ID: " + templatePoolVO.getTemplateId() + "
on pool " + templatePoolVO.getPoolId()
- + " because it is not completely
downloaded.");
+ s_logger.debug(String.format("Storage
pool garbage collector is skipping template [%s] clean up on pool [%s] " +
+ "because it is not completely
downloaded.", templatePoolVO.getTemplateId(), templatePoolVO.getPoolId()));
continue;
}
if (!templatePoolVO.getMarkedForGC()) {
templatePoolVO.setMarkedForGC(true);
_vmTemplatePoolDao.update(templatePoolVO.getId(), templatePoolVO);
- s_logger.debug("Storage pool garbage
collector has marked template with ID: " + templatePoolVO.getTemplateId() + "
on pool " + templatePoolVO.getPoolId()
- + " for garbage collection.");
+ s_logger.debug(String.format("Storage
pool garbage collector has marked template [%s] on pool [%s] " +
+ "for garbage collection.",
templatePoolVO.getTemplateId(), templatePoolVO.getPoolId()));
continue;
}
_tmpltMgr.evictTemplateFromStoragePool(templatePoolVO);
}
} catch (Exception e) {
- s_logger.warn("Problem cleaning up primary
storage pool " + pool, e);
+ s_logger.error(String.format("Failed to clean
up primary storage pool [%s] due to: [%s].", pool, e.getMessage()), e);
}
}
}
//destroy snapshots in destroying state in
snapshot_store_ref
List<SnapshotDataStoreVO> ssSnapshots =
_snapshotStoreDao.listByState(ObjectInDataStoreStateMachine.State.Destroying);
for (SnapshotDataStoreVO ssSnapshotVO : ssSnapshots) {
+ SnapshotVO snapshot =
_snapshotDao.findById(ssSnapshotVO.getSnapshotId());
+ if (snapshot == null) {
+ s_logger.warn(String.format("Did not find snapshot
[%s] in destroying state; therefore, it cannot be destroyed.",
ssSnapshotVO.getSnapshotId()));
+ continue;
+ }
+
+ String snapshotUuid = snapshot.getUuid();
try {
-
_snapshotService.deleteSnapshot(snapshotFactory.getSnapshot(ssSnapshotVO.getSnapshotId(),
DataStoreRole.Image));
+ s_logger.debug(String.format("Verifying if
snapshot [%s] is in destroying state in any image data store.", snapshotUuid));
+ SnapshotInfo snapshotInfo =
snapshotFactory.getSnapshot(snapshot.getId(), DataStoreRole.Image);
+
+ if (snapshotInfo != null) {
Review Comment:
You are right. But, snapshotInfo is getting only snapshots from Image data
store role. If a snapshot is in destroying state in a Primary data store role
and the same snapshot in the Image data store role has already been cleaned up,
a NPE will be thrown.
Snapshots from ID 18 & 19 were cleaned up in Image data store but the
snapshot still remains in destroying state in Primary data store.
--
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: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
