[opt](cloud) Check object existence before explicit object deletion#62973
Open
wyxxxcat wants to merge 4 commits intoapache:masterfrom
Open
[opt](cloud) Check object existence before explicit object deletion#62973wyxxxcat wants to merge 4 commits intoapache:masterfrom
wyxxxcat wants to merge 4 commits intoapache:masterfrom
Conversation
### What problem does this PR solve? Issue Number: close #xxx Related PR: #xxx Problem Summary: S3-compatible object deletes can succeed even when the target object does not exist, so recycler file deletion could silently ignore missing files. Check object existence in the object storage client before explicit file deletion and propagate NOT_FOUND to the accessor. ### Release note None ### Check List (For Author) - Test: No need to test (requested to only change code and not write or run tests) - Behavior changed: Yes (explicit file deletion now returns an error when the object is missing) - Does this need documentation: No
### What problem does this PR solve? Issue Number: close #xxx Related PR: #xxx Problem Summary: The object existence check before explicit file deletion should be configurable because it adds an extra HEAD request. Add a default-on cloud config and keep the strict delete behavior enabled by default. ### Release note None ### Check List (For Author) - Test: No need to test (requested to only change code and not write or run tests) - Behavior changed: No (default behavior remains checking object existence before explicit file deletion) - Does this need documentation: No
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary: The configurable object existence check before S3 object deletion needs unit coverage to verify the HEAD request is issued when enabled and skipped when disabled.
### Release note
None
### Check List (For Author)
- Test: Unit Test
- ./run-cloud-ut.sh --run --filter=s3_accessor_client_test:S3ObjClientTest.DeleteObjectCheckExistsConfigTest
- Behavior changed: No
- Does this need documentation: No
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
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.
What problem does this PR solve?
S3
DeleteObjectis idempotent and can return success even when the target object does not exist. Because of this, explicit file deletion in cloud recycler could silently ignore missing objects.This change makes explicit object deletion verify object existence before deleting by default, so missing files are surfaced as errors instead of being treated as successful deletes.
What changed?
S3ObjClient::delete_objectandAzureObjClient::delete_object.S3Accessor::delete_fileto propagateNOT_FOUNDinstead of treating it as success.ObjClientOptions::check_exists_before_deleteso explicit batch file deletes can request existence validation.enable_delete_file_check_object_exists, defaulting totrue, to allow disabling the extraHEADrequest when needed.DCHECKwhen the pre-deleteHEADcheck fails.Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)