incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "edison su" <edison...@citrix.com>
Subject Re: Review Request: CS-15823 Failed snapshot should be marked as Error and cleanup asynchronously
Date Thu, 16 Aug 2012 17:49:08 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message