Fix DeleteObject API for unversioned objects with insufficient read q… #19581
Conversation
This file contains 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
poornas
force-pushed
the
unversioned-del
branch
from
6006998
to
6d6c3e7
Compare
poornas
force-pushed
the
unversioned-del
branch
6 times, most recently
from
5ce6611
to
bc8a82e
Compare
harshavardhana
approved these changes
Apr 24, 2024
poornas
force-pushed
the
unversioned-del
branch
from
bc8a82e
to
4efb758
Compare
poornas
force-pushed
the
unversioned-del
branch
from
4efb758
to
f350bf3
Compare
…uorum Since the object is being permanently deleted, lack of read quorum should not matter as long as sufficient disks online to complete the delete with parity requirements. If several pools have the same object with insufficient read quorum, attempt to delete object from all the pools where it exists
poornas
force-pushed
the
unversioned-del
branch
from
f350bf3
to
687eabf
Compare
krisis
approved these changes
Apr 25, 2024
| er.addPartial(bucket, object, opts.VersionID) | ||
|
|
||
| return objInfo, InsufficientWriteQuorum{} | ||
| if opts.Versioned || opts.VersionSuspended || countOnlineDisks(storageDisks) < len(storageDisks)/2+1 { |
There was a problem hiding this comment.
It would be good to handle this without the tryDel boolean. It looks like the current code implicitly assumes that an error other than InsufficientReadQuorum is VersionNotFound, as evidenced by if opts.DeleteMarker { versionFound = false .... If we make the error handling for getObjectInfoAndQuorum more robust, we can avoid this boolean flag to alter control flow. If you feel that this change is out of scope to what you wish to accomplish in this change, that is fine too. We can address that subsequently.
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.
…uorum
Since the object is being permanently deleted, lack of read quorum should not matter as long as sufficient disks online to complete the delete with parity requirements.
If several pools have the same object with insufficient read quorum, attempt to delete object from all the pools where it exists
Community Contribution License
All community contributions in this pull request are licensed to the project maintainers
under the terms of the Apache 2 license.
By creating this pull request I represent that I have the right to license the
contributions to the project maintainers under the Apache 2 license.
Description
Motivation and Context
How to test this PR?
need to simulate drives offline/ inconsistent metadata for a multi pool setup
Types of changes
Checklist:
commit-idorPR #here)