Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve handling of failures to delete a WIP layer #1138

Merged
merged 3 commits into from
Feb 17, 2022

Conversation

mtrmac
Copy link
Collaborator

@mtrmac mtrmac commented Feb 16, 2022

  • Always log that our cleanup failed.
  • Try to remove the metadata of a layer if we succeeded writing that metadata.

See individual commit messages for details.

Currently, if the attempts to recover from a failure
themselves fail, we don't record that at all.

That makes diagnosing the situation, or hypothetically
detecting that the cleanup could never work, much
harder.

So, log the errors.

Alternatively, we could include those failures as extra
text in the returned error; that's less likely to be lost
(e.g. a Go caller would have the extra text available, without
setting up extra infrastructure to capture logs), but possibly
harder to follow.

Signed-off-by: Miloslav Trmač <[email protected]>
Account for the "diff != nil" path; try to remove even
the metadata of a layer on a failure saving.

Not that there's _much_ hope to be able to save
the version without the new layer when we weren't able
to save the version with the new layer.

Signed-off-by: Miloslav Trmač <[email protected]>
Copy link
Member

@rhatdan rhatdan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vrothberg vrothberg merged commit d689c91 into containers:main Feb 17, 2022
@mtrmac mtrmac deleted the delete-failures branch February 17, 2022 14:56
mtrmac added a commit to mtrmac/storage that referenced this pull request Feb 22, 2022
Signed-off-by: Miloslav Trmač <[email protected]>
vrothberg added a commit that referenced this pull request Feb 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants