jmsperu commented on PR #13074: URL: https://github.com/apache/cloudstack/pull/13074#issuecomment-4768863886
Implemented **Option A** in `a51f335`: - `BackupProvider.handlesChainDeleteResourceAccounting()` (default `false`). - `NASBackupProvider` overrides it `true` and decrements `backup` + `backup_storage` at the single physical-removal choke-point (`deleteBackupFileAndRow`) — which runs for the leaf and every swept delete-pending ancestor, so accounting is exactly-once per actually-removed backup. A tombstoned (`delete-pending`) backup is not decremented until it is swept. - `deleteCheckedBackup` skips its own decrement + `backupDao.remove` for such providers (keeps the `CheckedReservation` limit guard). Unit tests (`NASBackupProviderTest`): the leaf+sweep path decrements both removed backups; the live-children (tombstone) path decrements none. This also fixes the two concrete bugs noted above — the manager no longer destroys the `delete-pending` tombstone, and swept ancestors are no longer leaked. Happy to switch to option (b) (have `deleteBackup` return the removed set and decrement in the manager) if you'd prefer that split. -- 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]
