> On Aug. 16, 2012, 6:34 a.m., Nitin Mehta wrote:
> > Do we delete DB rows for error conditions for any other resources in CS as 
> > well or do we want to give snapshots a different treatment here ? I am 
> > somehow not very comfortable doing hard delete here.
> 
> mice xia wrote:
>     by performing a global search with key word 'Dao.expunge(', similar hard 
> deletes are found in storage package, like volumes/storagePool that failed to 
> create get expunged, and considering codes before applying this patch are 
> also using expunge, is it the author's original idea to make a distinction? 
> Resources that ever exist get 'Removed', and Resources that never really 
> exist get 'Expunged'.

The current snapshot code immediately marks the snapshot db entry as "removed" 
during the delete snapshot operation, which is not good enough. It should be, 
put the snapshot db entry into "destroyed" state after deleting snapshot. And 
put the snapshot into "Error" state if create snapshot failed.
After a while, when snapshot expunge background thread been triggered, set 
"removed" column as not null, for snapshots in  "destroyed" and "error" state. 
Don't delete db entries directly.


- edison


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6325/#review10394
-----------------------------------------------------------


On Aug. 11, 2012, 1:13 a.m., mice xia wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6325/
> -----------------------------------------------------------
> 
> (Updated Aug. 11, 2012, 1:13 a.m.)
> 
> 
> Review request for cloudstack, Nitin Mehta and edison su.
> 
> 
> Description
> -------
> 
> changes:
> marked failed snapshots as Error and expunge them in a background thread, the 
> cleanup interval is storage.cleanup.interval
> 
> 
> This addresses bug CS-15823.
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/storage/StorageManagerImpl.java c17379f 
>   server/src/com/cloud/storage/dao/SnapshotDao.java 4bf54de 
>   server/src/com/cloud/storage/dao/SnapshotDaoImpl.java ac4bb6f 
>   server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java 6e3f9c1 
> 
> Diff: https://reviews.apache.org/r/6325/diff/
> 
> 
> Testing
> -------
> 
> verified
> 
> 
> Thanks,
> 
> mice xia
> 
>

Reply via email to