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

Add support for async deletion in S3BlobContainer #15621

Merged

Conversation

ashking94
Copy link
Member

@ashking94 ashking94 commented Sep 3, 2024

Description

In this PR, we are introducing the deletion using the S3 asyc client for repository-s3. We have also added a setting to control the async deletion in Snapshot code with default being false. We have also added multiple tests around the new code to ensure we are handling the edge cases.

Check List

  • Functionality includes testing.
  • [ ] API changes companion pull request created, if applicable.
  • [ ] Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Ashish Singh <ssashish@amazon.com>
Copy link
Contributor

github-actions bot commented Sep 3, 2024

❌ Gradle check result for 7822739: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

…epository

Signed-off-by: Ashish Singh <ssashish@amazon.com>
Copy link
Contributor

github-actions bot commented Sep 3, 2024

❕ Gradle check result for 639257a: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

Signed-off-by: Ashish Singh <ssashish@amazon.com>
Copy link
Contributor

github-actions bot commented Sep 5, 2024

❌ Gradle check result for 1f5a9cb: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

…epository

Signed-off-by: Ashish Singh <ssashish@amazon.com>
Signed-off-by: Ashish Singh <ssashish@amazon.com>
Copy link
Contributor

github-actions bot commented Sep 9, 2024

✅ Gradle check result for ffc81c0: SUCCESS

Copy link

codecov bot commented Sep 9, 2024

Codecov Report

Attention: Patch coverage is 85.92593% with 19 lines in your changes missing coverage. Please review.

Project coverage is 71.88%. Comparing base (3937ccb) to head (5209835).
Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
...ch/repositories/blobstore/BlobStoreRepository.java 57.14% 10 Missing and 2 partials ⚠️
...bstore/AsyncMultiStreamEncryptedBlobContainer.java 0.00% 4 Missing ⚠️
...rg/opensearch/repositories/s3/S3BlobContainer.java 97.01% 1 Missing and 1 partial ⚠️
...rch/repositories/s3/async/S3AsyncDeleteHelper.java 97.14% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #15621      +/-   ##
============================================
- Coverage     71.90%   71.88%   -0.02%     
+ Complexity    64392    64333      -59     
============================================
  Files          5278     5281       +3     
  Lines        300877   301004     +127     
  Branches      43478    43486       +8     
============================================
+ Hits         216351   216387      +36     
- Misses        66747    66873     +126     
+ Partials      17779    17744      -35     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ashking94 ashking94 force-pushed the async-deletion-s3-repository branch 2 times, most recently from 5013993 to 69f66d1 Compare September 10, 2024 10:44
Copy link
Contributor

❕ Gradle check result for 90548d2: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

Copy link
Contributor

✅ Gradle check result for 5013993: SUCCESS

@ashking94 ashking94 force-pushed the async-deletion-s3-repository branch 2 times, most recently from 1fcdf52 to 59b9c64 Compare September 10, 2024 13:31
@ashking94 ashking94 marked this pull request as ready for review September 10, 2024 13:31
@opensearch-project opensearch-project deleted a comment from github-actions bot Sep 11, 2024
@opensearch-project opensearch-project deleted a comment from github-actions bot Sep 11, 2024
@opensearch-project opensearch-project deleted a comment from github-actions bot Sep 11, 2024
@opensearch-project opensearch-project deleted a comment from github-actions bot Sep 11, 2024
@opensearch-project opensearch-project deleted a comment from github-actions bot Sep 11, 2024
Copy link
Contributor

❌ Gradle check result for 55f87d6: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Copy link
Contributor

❌ Gradle check result for 08154aa: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: Ashish Singh <ssashish@amazon.com>
Copy link
Contributor

❌ Gradle check result for 96cd6f7: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@ashking94
Copy link
Member Author

❌ Gradle check result for 96cd6f7: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

> Task :server:compileTestJava
/var/jenkins/workspace/gradle-check/search/server/src/test/java/org/opensearch/cluster/routing/allocation/AllocationCommandsTests.java:751: error: incompatible types: cannot infer type arguments for HashSet<>
            new HashSet<>(
                       ^
    reason: inference variable E has incompatible bounds
      equality constraints: DiscoveryNodeRole
      lower bounds: T#1,E,Object,T#2,DiscoveryNodeRole
  where E,T#1,T#2 are type-variables:
    E extends Object declared in class HashSet
    T#1 extends Object declared in method <T#1>randomSubsetOf(Collection<T#1>)
    T#2 extends Object declared in method <T#2>asList(T#2...)

Copy link
Contributor

❕ Gradle check result for 96cd6f7: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

…epository

Signed-off-by: Ashish Singh <ssashish@amazon.com>
Copy link
Contributor

✅ Gradle check result for 5209835: SUCCESS

@ashking94 ashking94 merged commit 2b670cc into opensearch-project:main Sep 20, 2024
34 of 35 checks passed
@ashking94 ashking94 added the backport 2.x Backport to 2.x branch label Sep 20, 2024
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch/backport-2.x
# Create a new branch
git switch --create backport/backport-15621-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 2b670cc11f52ada701cbf867ce8ad4527d8b6072
# Push it to GitHub
git push --set-upstream origin backport/backport-15621-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-15621-to-2.x.

ashking94 added a commit to ashking94/OpenSearch that referenced this pull request Sep 20, 2024
…#15621)

* Add support for async deletion in S3BlobContainer

Signed-off-by: Ashish Singh <ssashish@amazon.com>

* Move helper methods to helper class

Signed-off-by: Ashish Singh <ssashish@amazon.com>

* Minor refactor

Signed-off-by: Ashish Singh <ssashish@amazon.com>

* Add UTs

Signed-off-by: Ashish Singh <ssashish@amazon.com>

* Add more tests

Signed-off-by: Ashish Singh <ssashish@amazon.com>

* Integrate async deletion in the snapshot interactions

Signed-off-by: Ashish Singh <ssashish@amazon.com>

---------

Signed-off-by: Ashish Singh <ssashish@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants