Parallelize stale blobs deletion during snapshot delete #3796

piyushdaftary · 2022-07-07T08:36:27Z

Signed-off-by: Piyush Daftary [email protected]

Description

Currently during snapshot delete, deletion of unlinked shard level blob is single threaded using SNAPSHOT threadpool. Hence if there is huge number of unlinked shard level blob flies, it will take considerable amount of time to clean them.

Hence I propose to make unlinked shard level blob deletion multi threaded delete the same way we do for cleaning up of stale indices, to speedup the overall snapshot deletion process.

Issues Resolved

#2156

Check List

[Y] New functionality includes testing.
- [Y] All tests pass
New functionality has been documented.
- New functionality has javadoc added
[Y] Commits are signed per the DCO using --signoff

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.

piyushdaftary · 2022-07-07T08:51:23Z

@sohami @AmiStrn @dreamer-89 : Thanks for reviewing the earlier PR : #2159 .
Somehow the PR is closed and I am unable to open it. Thus creating this new PR addressing the comments given by you in the previous PR . Please review.

github-actions · 2022-07-07T09:01:02Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/223/
CommitID: 04f8da289207c49c66d870b4692a1786f69991c0

dreamer-89 · 2022-07-07T16:04:26Z

@sohami @AmiStrn @dreamer-89 : Thanks for reviewing the earlier PR : #2159 . Somehow the PR is closed and I am unable to open it. Thus creating this new PR addressing the comments given by you in the previous PR . Please review.

@piyushdaftary : I re-opened the PR #2159 which already has review comments and useful discussion. Please feel free to close this one and continue on PR 2159.

piyushdaftary · 2022-07-07T20:31:24Z

@dreamer-89 : Unable to update the old PR. Thus Created this new PR.
To ensure earlier discussion is not lost, I have mentioned the old PR #2159 here.

nknize

🎉 nice! This is a great performance boost. Just have some suggestions around testing and a question around the newly introduced parameter (javadocs and user guidance); I think we can do a little better there?

nknize · 2022-07-07T21:51:24Z

server/src/internalClusterTest/java/org/opensearch/snapshots/RepositoriesIT.java

+        int numberOfFiles = numberOfFiles(repositoryPath);
+
+        logger.info("--> adding some more documents to test index");
+        for (int j = 0; j < 10; ++j) {


can we randomize this using randomIntBetween() to inject some entropy in the number of threads? Do you have a general feel for the number of documents per number of threads so we can use a reasonable range?

@nknize : The number of documents per thread varies a lot depending on number of documents deleted (stale) or updated between the snapshot being deleted and other remaining snapshots in the repository. I am planning to make batch size to be randomIntBetween(1, 1000) and number of document to be in range between 1 to 10000000. WDYT ?

server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java

server/src/internalClusterTest/java/org/opensearch/snapshots/RepositoriesIT.java

server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java

github-actions · 2022-07-15T05:56:38Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/672/
CommitID: dd4d3f1d546a016e6a6eae47a2e634ee353656b8

github-actions · 2022-07-15T06:29:56Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/674/
CommitID: 53cdf185a0b939149a1df89aad241777a207dd9e

github-actions · 2022-07-15T06:52:04Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/676/
CommitID: 4ce3769089db10083741aef06f520002b29e30ab

reta

LGTM on my side

server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java

Signed-off-by: Piyush Daftary <[email protected]>

github-actions · 2022-07-16T02:02:50Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/709/
CommitID: c9d84ae

codecov-commenter · 2022-07-16T02:03:48Z

Codecov Report

Merging #3796 (2d07bc5) into main (e724b2e) will increase coverage by 0.11%.
The diff coverage is 57.64%.

@@             Coverage Diff              @@
##               main    #3796      +/-   ##
============================================
+ Coverage     70.46%   70.58%   +0.11%     
- Complexity    56600    56672      +72     
============================================
  Files          4557     4563       +6     
  Lines        272737   272781      +44     
  Branches      40040    40043       +3     
============================================
+ Hits         192188   192540     +352     
+ Misses        64324    63925     -399     
- Partials      16225    16316      +91

Impacted Files	Coverage Δ
...n/cluster/health/TransportClusterHealthAction.java	`47.75% <ø> (ø)`
...g/opensearch/cluster/ClusterStateTaskListener.java	`100.00% <ø> (ø)`
.../opensearch/cluster/MasterNodeChangePredicate.java	`0.00% <0.00%> (ø)`
...ava/org/opensearch/cluster/NotMasterException.java	`0.00% <0.00%> (ø)`
...rch/cluster/coordination/NoMasterBlockService.java	`0.00% <0.00%> (ø)`
...ter/coordination/UnsafeBootstrapMasterCommand.java	`0.00% <0.00%> (ø)`
...rg/opensearch/common/settings/ClusterSettings.java	`91.89% <ø> (ø)`
...rch/common/settings/ConsistentSettingsService.java	`67.85% <ø> (ø)`
.../opensearch/common/util/concurrent/BaseFuture.java	`62.71% <0.00%> (ø)`
...java/org/opensearch/discovery/DiscoveryModule.java	`90.27% <ø> (ø)`
... and 532 more

Signed-off-by: Piyush Daftary <[email protected]>

github-actions · 2022-07-19T15:48:19Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/796/
CommitID: fb5b074

server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java

…delete_batch_size Signed-off-by: Piyush Daftary <[email protected]>

github-actions · 2022-07-19T20:03:32Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/804/
CommitID: 2d07bc5

andrross · 2022-07-21T19:35:28Z

@reta @nknize You still have outstanding changes requested on this PR. Can you confirm this looks good?

Changes have been addressed

* Parallelize stale blobs deletion during snapshot delete Signed-off-by: Piyush Daftary <[email protected]> * Adding test which throws exception Signed-off-by: Piyush Daftary <[email protected]> * Adusting identation for spotlessJavaCheck Signed-off-by: Piyush Daftary <[email protected]> * Adding more description to MAX_SHARD_BLOB_DELETE_BATCH_SIZE Signed-off-by: Piyush Daftary <[email protected]> * Renaming max_shard_blob_delete_batch_size to max_snapshot_shard_blob_delete_batch_size Signed-off-by: Piyush Daftary <[email protected]> (cherry picked from commit 1c787e8)

nknize

Thanks all for pushing this forward. Sorry for the delay @piyushdaftary and thanks for addressing the questions. Adding my LGTM post merge for completeness!

* Parallelize stale blobs deletion during snapshot delete Signed-off-by: Piyush Daftary <[email protected]> * Adding test which throws exception Signed-off-by: Piyush Daftary <[email protected]> * Adusting identation for spotlessJavaCheck Signed-off-by: Piyush Daftary <[email protected]> * Adding more description to MAX_SHARD_BLOB_DELETE_BATCH_SIZE Signed-off-by: Piyush Daftary <[email protected]> * Renaming max_shard_blob_delete_batch_size to max_snapshot_shard_blob_delete_batch_size Signed-off-by: Piyush Daftary <[email protected]> (cherry picked from commit 1c787e8) Co-authored-by: piyush <[email protected]>

piyushdaftary requested review from a team and reta as code owners July 7, 2022 08:36

piyushdaftary mentioned this pull request Jul 7, 2022

Parallelize stale blobs deletion during snapshot delete #2159

Closed

2 tasks

dblock requested review from andrross and kartg July 7, 2022 20:55

nknize previously requested changes Jul 7, 2022

View reviewed changes

reta previously requested changes Jul 8, 2022

View reviewed changes

server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java Outdated Show resolved Hide resolved

reta reviewed Jul 8, 2022

View reviewed changes

server/src/internalClusterTest/java/org/opensearch/snapshots/RepositoriesIT.java Show resolved Hide resolved

AmiStrn reviewed Jul 12, 2022

View reviewed changes

server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java Show resolved Hide resolved

piyushdaftary force-pushed the issue-2156 branch from dd4d3f1 to 53cdf18 Compare July 15, 2022 06:12

reta reviewed Jul 15, 2022

View reviewed changes

dreamer-89 reviewed Jul 15, 2022

View reviewed changes

server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java Outdated Show resolved Hide resolved

piyushdaftary added 3 commits July 15, 2022 18:38

Parallelize stale blobs deletion during snapshot delete

df49ac3

Signed-off-by: Piyush Daftary <[email protected]>

Adding test which throws exception

b73f391

Signed-off-by: Piyush Daftary <[email protected]>

Adusting identation for spotlessJavaCheck

c9d84ae

Signed-off-by: Piyush Daftary <[email protected]>

piyushdaftary force-pushed the issue-2156 branch from 4ce3769 to c9d84ae Compare July 16, 2022 01:39

Adding more description to MAX_SHARD_BLOB_DELETE_BATCH_SIZE

fb5b074

Signed-off-by: Piyush Daftary <[email protected]>

piyushdaftary requested review from reta, nknize and dreamer-89 July 19, 2022 15:53

dreamer-89 approved these changes Jul 19, 2022

View reviewed changes

kartg reviewed Jul 19, 2022

View reviewed changes

Renaming max_shard_blob_delete_batch_size to max_snapshot_shard_blob_…

2d07bc5

…delete_batch_size Signed-off-by: Piyush Daftary <[email protected]>

piyushdaftary requested a review from kartg July 19, 2022 19:45

kartg approved these changes Jul 19, 2022

View reviewed changes

andrross added the backport 2.x Backport to 2.x branch label Jul 21, 2022

andrross merged commit 1c787e8 into opensearch-project:main Jul 22, 2022

opensearch-trigger-bot bot mentioned this pull request Jul 22, 2022

[Backport 2.x] Parallelize stale blobs deletion during snapshot delete #3990

Merged

nknize reviewed Jul 22, 2022

View reviewed changes

kartg mentioned this pull request Aug 30, 2022

Adding @dreamer-89 to Opensearch maintainers. #4342

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parallelize stale blobs deletion during snapshot delete #3796

Parallelize stale blobs deletion during snapshot delete #3796

piyushdaftary commented Jul 7, 2022

piyushdaftary commented Jul 7, 2022

github-actions bot commented Jul 7, 2022

dreamer-89 commented Jul 7, 2022 •

edited

Loading

piyushdaftary commented Jul 7, 2022

nknize left a comment

nknize Jul 7, 2022

piyushdaftary Jul 12, 2022 •

edited

Loading

github-actions bot commented Jul 15, 2022

github-actions bot commented Jul 15, 2022

github-actions bot commented Jul 15, 2022

reta left a comment

github-actions bot commented Jul 16, 2022

codecov-commenter commented Jul 16, 2022 •

edited

Loading

github-actions bot commented Jul 19, 2022

github-actions bot commented Jul 19, 2022

andrross commented Jul 21, 2022

nknize left a comment

Parallelize stale blobs deletion during snapshot delete #3796

Parallelize stale blobs deletion during snapshot delete #3796

Conversation

piyushdaftary commented Jul 7, 2022

Description

Issues Resolved

Check List

piyushdaftary commented Jul 7, 2022

github-actions bot commented Jul 7, 2022

Gradle Check (Jenkins) Run Completed with:

dreamer-89 commented Jul 7, 2022 • edited Loading

piyushdaftary commented Jul 7, 2022

nknize left a comment

Choose a reason for hiding this comment

nknize Jul 7, 2022

Choose a reason for hiding this comment

piyushdaftary Jul 12, 2022 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Jul 15, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jul 15, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jul 15, 2022

Gradle Check (Jenkins) Run Completed with:

reta left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 16, 2022

Gradle Check (Jenkins) Run Completed with:

codecov-commenter commented Jul 16, 2022 • edited Loading

Codecov Report

github-actions bot commented Jul 19, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jul 19, 2022

Gradle Check (Jenkins) Run Completed with:

andrross commented Jul 21, 2022

nknize left a comment

Choose a reason for hiding this comment

dreamer-89 commented Jul 7, 2022 •

edited

Loading

piyushdaftary Jul 12, 2022 •

edited

Loading

codecov-commenter commented Jul 16, 2022 •

edited

Loading