Perform volume and local storage delete in tasks - #11027
Open
jmpesp wants to merge 2 commits into
Open
Conversation
Disk, snapshot, and image deletes have historically been the source of a lot of problems, especially when hardware is flaky. The whole associated delete saga had to complete before any of the endpoints would return an OK message, and they would hang if the saga was in a retry loop, often until an expungement occurred that would unblock said retry loop. Even worse was if the "delete freed regions" step was the one that was blocked, as that would hang _any and every_ delete of a volume until resolved. In accordance with the guidance in RFD 704, we're looking at replacing problematic sagas with one or more background tasks. In particular, the volume delete saga _had_ to go. The whole saga was comprised of nodes that only have forward actions, so it was an ideal candidate for this conversion, and sagas that are like this don't make much sense in the first place. Now all the disk delete saga does is: - soft-delete the disk record - update the virtual provisioning accounting - if the disk was backed by crucible, soft-delete the associated volume Note each of these steps could still fail due to transient database issues, but those issues are less likely than the ones caused by either physical disk related issues or software bugs (like the recent Pantry lock-up). The snapshot and image delete sagas were similarly modified: wherever the volume delete sub saga was embedded, instead soft-delete the volume and let the background task handle it. The disk delete saga also recently grown the ability to clean up local storage backed disks, and this was also moved into a different background task. Occasionally there were issues (like if the `dataset is busy` error was returned) that would cause those local storage backed disk deletes to unwind as well, where a rerun would succeed. Note that merging this commit will cause a behaviour change: today, the disk / snapshot / image delete API calls do not return until all the associated resources are cleaned up (in the case of Crucible backed disks, cleaned up as much as possible). After this commit is merged, it will be possible to delete a disk, quickly create another one, and see an INSUFFICIENT_STORAGE error, because the background task that would have cleaned up either the regions or the local storage allocation hasn't fired yet. It would be incorrect to spin in the disk / snapshot / image delete endpoints as that would lead us into the same problems we had before, so this commit simply returns when the associated saga completes. The relevant clean-up background task is activated after each (now much shorter) delete saga in an attempt to close this window.
Member
|
this CI run makes it look like we apparently have a bunch of tests that relied on disk deletes happening synchronously, and they now consistently fail, at least on CI. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Disk, snapshot, and image deletes have historically been the source of a lot of problems, especially when hardware is flaky. The whole associated delete saga had to complete before any of the endpoints would return an OK message, and they would hang if the saga was in a retry loop, often until an expungement occurred that would unblock said retry loop. Even worse was if the "delete freed regions" step was the one that was blocked, as that would hang any and every delete of a volume until resolved.
In accordance with the guidance in RFD 704, we're looking at replacing problematic sagas with one or more background tasks. In particular, the volume delete saga had to go. The whole saga was comprised of nodes that only have forward actions, so it was an ideal candidate for this conversion, and sagas that are like this don't make much sense in the first place.
Now all the disk delete saga does is:
Note each of these steps could still fail due to transient database issues, but those issues are less likely than the ones caused by either physical disk related issues or software bugs (like the recent Pantry lock-up). The snapshot and image delete sagas were similarly modified: wherever the volume delete sub saga was embedded, instead soft-delete the volume and let the background task handle it.
The disk delete saga also recently grown the ability to clean up local storage backed disks, and this was also moved into a different background task. Occasionally there were issues (like if the
dataset is busyerror was returned) that would cause those local storage backed disk deletes to unwind as well, where a rerun would succeed.Note that merging this commit will cause a behaviour change: today, the disk / snapshot / image delete API calls do not return until all the associated resources are cleaned up (in the case of Crucible backed disks, cleaned up as much as possible). After this commit is merged, it will be possible to delete a disk, quickly create another one, and see an INSUFFICIENT_STORAGE error, because the background task that would have cleaned up either the regions or the local storage allocation hasn't fired yet. It would be incorrect to spin in the disk / snapshot / image delete endpoints as that would lead us into the same problems we had before, so this commit simply returns when the associated saga completes. The relevant clean-up background task is activated after each (now much shorter) delete saga in an attempt to close this window.