Prevent deletion of snapshot backing index #5069

Vishalks · 2022-11-04T06:17:57Z

Description

The changes prevent deletion of snapshots that are backing searchable snapshot indexes

Issues Resolved

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff
Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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.

…dexes Signed-off-by: Vishal Sarda <[email protected]>

Signed-off-by: Vishal Sarda <[email protected]>

github-actions · 2022-11-04T06:35:49Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/6346/
CommitID: e9f450a
Please examine the workflow log, locate, and copy-paste the failure below, then iterate to green.
Is the failure a flaky test unrelated to your change?

github-actions · 2022-11-04T06:57:34Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/6347/
CommitID: bf32979
Please examine the workflow log, locate, and copy-paste the failure below, then iterate to green.
Is the failure a flaky test unrelated to your change?

Signed-off-by: Vishal Sarda <[email protected]>

github-actions · 2022-11-04T08:46:36Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/6352/
CommitID: 3b43e06
Please examine the workflow log, locate, and copy-paste the failure below, then iterate to green.
Is the failure a flaky test unrelated to your change?

Signed-off-by: Vishal Sarda <[email protected]>

github-actions · 2022-11-04T19:36:51Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/6394/
CommitID: eabebd8
Please examine the workflow log, locate, and copy-paste the failure below, then iterate to green.
Is the failure a flaky test unrelated to your change?

github-actions · 2022-11-04T19:58:12Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/6395/
CommitID: cca5ccc
Please examine the workflow log, locate, and copy-paste the failure below, then iterate to green.
Is the failure a flaky test unrelated to your change?

Signed-off-by: Vishal Sarda <[email protected]>

github-actions · 2022-11-04T20:12:44Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/6397/
CommitID: 0228972
Please examine the workflow log, locate, and copy-paste the failure below, then iterate to green.
Is the failure a flaky test unrelated to your change?

server/src/main/java/org/opensearch/snapshots/SnapshotsService.java

andrross · 2022-11-07T17:35:14Z

Can you add some tests of the new behavior?

Vishalks · 2022-11-08T12:48:27Z

Can you add some tests of the new behavior?

Already working on it while I get any review comments on the PR.

Signed-off-by: Vishal Sarda <[email protected]>

github-actions · 2022-11-09T19:58:13Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/6713/
CommitID: dbbe321
Please examine the workflow log, locate, and copy-paste the failure below, then iterate to green.
Is the failure a flaky test unrelated to your change?

Signed-off-by: Vishal Sarda <[email protected]>

github-actions · 2022-11-14T22:17:13Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/6871/
CommitID: e09a8b9
Please examine the workflow log, locate, and copy-paste the failure below, then iterate to green.
Is the failure a flaky test unrelated to your change?

Signed-off-by: Vishal Sarda <[email protected]>

github-actions · 2022-11-19T00:30:27Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/7092/
CommitID: fb6d33d
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?

github-actions · 2022-11-19T00:36:58Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/7093/
CommitID: 081a207

codecov-commenter · 2022-11-19T00:39:12Z

Codecov Report

Merging #5069 (575ff53) into main (933e8c3) will decrease coverage by 0.00%.
The diff coverage is 86.36%.

@@             Coverage Diff              @@
##               main    #5069      +/-   ##
============================================
- Coverage     71.07%   71.07%   -0.01%     
- Complexity    58135    58141       +6     
============================================
  Files          4704     4705       +1     
  Lines        277266   277288      +22     
  Branches      40147    40151       +4     
============================================
  Hits         197080   197080              
- Misses        64001    64021      +20     
- Partials      16185    16187       +2

Impacted Files	Coverage Δ
...arch/snapshots/SnapshotInUseDeletionException.java	`40.00% <40.00%> (ø)`
.../main/java/org/opensearch/OpenSearchException.java	`92.35% <100.00%> (-0.20%)`	⬇️
...n/java/org/opensearch/snapshots/SnapshotUtils.java	`87.30% <100.00%> (+3.96%)`	⬆️
...ava/org/opensearch/snapshots/SnapshotsService.java	`47.82% <100.00%> (-2.18%)`	⬇️
...rch/test/junit/listeners/ReproduceInfoPrinter.java	`9.52% <0.00%> (-66.67%)`	⬇️
...cluster/coordination/PendingClusterStateStats.java	`20.00% <0.00%> (-48.00%)`	⬇️
...opensearch/persistent/PersistentTasksExecutor.java	`22.22% <0.00%> (-44.45%)`	⬇️
...adcast/BroadcastShardOperationFailedException.java	`55.55% <0.00%> (-44.45%)`	⬇️
.../admin/cluster/reroute/ClusterRerouteResponse.java	`60.00% <0.00%> (-40.00%)`	⬇️
...luster/routing/allocation/RoutingExplanations.java	`62.06% <0.00%> (-37.94%)`	⬇️
... and 493 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

github-actions · 2022-11-19T00:40:41Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/7094/
CommitID: 6b601ee

github-actions · 2022-11-19T00:42:33Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.index.ShardIndexingPressureConcurrentExecutionTests.testReplicaThreadedUpdateToShardLimitsAndRejections

URL: https://build.ci.opensearch.org/job/gradle-check/7095/
CommitID: 8434fbd
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

kotwanikunal

LGTM.

dblock

Do we have similar behavior elsewhere where we return a 403 when some resource is holding the one being deleted?

By returning FORBIDDEN (401) we are breaking the contract in which FORBIDDEN means "server doesn't want to tell you why this is not allowed". We are explicitly telling the client that this is not allowed. Should this be a 400? Or a 405 (operation not allowed?), or some kind of other not permitted error?

server/src/main/java/org/opensearch/snapshots/SnapshotDeletionException.java

dblock

I'll hit approve. Leaving it to @andrross to final-comment, if you think we're making a mistake, then let's discuss. I am not sure anymore.

Signed-off-by: Vishal Sarda <[email protected]>

github-actions · 2022-11-21T23:57:25Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.smoketest.SmokeTestMultiNodeClientYamlTestSuiteIT.test {yaml=pit/10_basic/Delete all}

URL: https://build.ci.opensearch.org/job/gradle-check/7185/
CommitID: 6481b11
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

server/src/test/java/org/opensearch/snapshots/SnapshotUtilsTests.java

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

server/src/main/java/org/opensearch/snapshots/SnapshotUtils.java

server/src/main/java/org/opensearch/snapshots/SnapshotDeletionException.java

Signed-off-by: Vishal Sarda <[email protected]>

github-actions · 2022-11-22T23:27:29Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/7249/
CommitID: 8341095
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?

github-actions · 2022-11-22T23:35:50Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/7250/
CommitID: eb73220
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: Vishal Sarda <[email protected]>

github-actions · 2022-11-23T00:24:56Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      3 org.opensearch.index.ShardIndexingPressureConcurrentExecutionTests.testReplicaThreadedUpdateToShardLimitsAndRejections

URL: https://build.ci.opensearch.org/job/gradle-check/7253/
CommitID: e5ec3aa
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

CHANGELOG.md

Signed-off-by: Vishal Sarda <[email protected]>

github-actions · 2022-11-23T02:50:48Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/7259/
CommitID: 575ff53

opensearch-trigger-bot · 2022-11-23T16:01:29Z

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:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-5069-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 08ac17fd7b9a604ecffee0a8e0d666f4dd372dba
# Push it to GitHub
git push --set-upstream origin backport/backport-5069-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-2.x

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

dblock · 2022-11-23T16:32:56Z

Is this a breaking change for 2.x? Was the user experience broken when these snapshots were deleted in 2.4.0?

@Vishalks this will need a manual backport

andrross · 2022-11-23T16:35:40Z

Was the user experience broken when these snapshots were deleted in 2.4.0?

@dblock Yes, it was broken. Searchable snapshots are still behind a feature flag so we have some latitude for changing behavior here.

Vishalks · 2022-11-23T17:23:33Z

Is this a breaking change for 2.x? Was the user experience broken when these snapshots were deleted in 2.4.0?

@Vishalks this will need a manual backport

Starting the backport

Prevent deletion of snapshots that are backing searchable snapshot indexes Signed-off-by: Vishal Sarda <[email protected]> (cherry picked from commit 08ac17f) Signed-off-by: Vishal Sarda <[email protected]>

Vishalks added 3 commits November 4, 2022 11:39

Prevent deletion of snapshots that are backing searchable snapshot in…

5d30a56

…dexes Signed-off-by: Vishal Sarda <[email protected]>

Merge branch 'main' into 4967_prevent_deletion_of_snapshot_backing_index

e9f450a

Adding changelog and applying gradle precommit

bf32979

Signed-off-by: Vishal Sarda <[email protected]>

Adding javadoc

3b43e06

Signed-off-by: Vishal Sarda <[email protected]>

Vishalks added 2 commits November 5, 2022 00:39

Fixing gradle check error

eabebd8

Signed-off-by: Vishal Sarda <[email protected]>

Merge branch 'main' into 4967_prevent_deletion_of_snapshot_backing_index

cca5ccc

Signed-off-by: Vishal Sarda <[email protected]>

Fixing tests

0228972

Signed-off-by: Vishal Sarda <[email protected]>

Vishalks marked this pull request as ready for review November 4, 2022 20:03

Vishalks requested review from a team and reta as code owners November 4, 2022 20:03

kotwanikunal changed the title ~~4967 prevent deletion of snapshot backing index~~ Prevent deletion of snapshot backing index Nov 4, 2022

kotwanikunal reviewed Nov 4, 2022

View reviewed changes

server/src/main/java/org/opensearch/snapshots/SnapshotsService.java Outdated Show resolved Hide resolved

server/src/main/java/org/opensearch/snapshots/SnapshotsService.java Outdated Show resolved Hide resolved

kotwanikunal reviewed Nov 4, 2022

View reviewed changes

server/src/main/java/org/opensearch/snapshots/SnapshotsService.java Outdated Show resolved Hide resolved

WIP

dbbe321

Signed-off-by: Vishal Sarda <[email protected]>

Merge branch 'main' into 4967_prevent_deletion_of_snapshot_backing_index

e09a8b9

Signed-off-by: Vishal Sarda <[email protected]>

Vishalks added 4 commits November 18, 2022 16:03

Adding unit and integration tests, moving validation function to utils

fb6d33d

Signed-off-by: Vishal Sarda <[email protected]>

Merge branch 'main' into 4967_prevent_deletion_of_snapshot_backing_index

081a207

removing unwanted characters

6b601ee

Signed-off-by: Vishal Sarda <[email protected]>

Remove stale entries from changelog

8434fbd

Signed-off-by: Vishal Sarda <[email protected]>

kotwanikunal requested a review from andrross November 20, 2022 04:41

kotwanikunal approved these changes Nov 20, 2022

View reviewed changes

dblock requested changes Nov 21, 2022

View reviewed changes

server/src/main/java/org/opensearch/snapshots/SnapshotDeletionException.java Outdated Show resolved Hide resolved

dblock mentioned this pull request Nov 21, 2022

[BUG] Fix flaky test org.opensearch.index.ShardIndexingPressureConcurrentExecutionTests.testReplicaThreadedUpdateToShardLimitsAndRejections #4212

Closed

dblock approved these changes Nov 21, 2022

View reviewed changes

Changing HTTP status from 401 to 409

6481b11

Signed-off-by: Vishal Sarda <[email protected]>

andrross reviewed Nov 22, 2022

View reviewed changes

dblock reviewed Nov 22, 2022

View reviewed changes

server/src/main/java/org/opensearch/snapshots/SnapshotDeletionException.java Outdated Show resolved Hide resolved

Vishalks added 2 commits November 22, 2022 15:06

PR review implementation

8341095

Signed-off-by: Vishal Sarda <[email protected]>

PR review implementation

eb73220

Signed-off-by: Vishal Sarda <[email protected]>

Gradle spotless

e5ec3aa

Signed-off-by: Vishal Sarda <[email protected]>

andrross reviewed Nov 23, 2022

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

Updating CHANGELOG

575ff53

Signed-off-by: Vishal Sarda <[email protected]>

andrross approved these changes Nov 23, 2022

View reviewed changes

andrross merged commit 08ac17f into opensearch-project:main Nov 23, 2022

andrross added the backport 2.x Backport to 2.x branch label Nov 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent deletion of snapshot backing index #5069

Prevent deletion of snapshot backing index #5069

Vishalks commented Nov 4, 2022 •

edited

Loading

github-actions bot commented Nov 4, 2022

github-actions bot commented Nov 4, 2022

github-actions bot commented Nov 4, 2022

github-actions bot commented Nov 4, 2022

github-actions bot commented Nov 4, 2022

github-actions bot commented Nov 4, 2022

andrross commented Nov 7, 2022

Vishalks commented Nov 8, 2022

github-actions bot commented Nov 9, 2022

github-actions bot commented Nov 14, 2022

github-actions bot commented Nov 19, 2022

github-actions bot commented Nov 19, 2022

codecov-commenter commented Nov 19, 2022 •

edited

Loading

github-actions bot commented Nov 19, 2022

github-actions bot commented Nov 19, 2022

kotwanikunal left a comment

dblock left a comment •

edited

Loading

dblock left a comment

github-actions bot commented Nov 21, 2022

github-actions bot commented Nov 22, 2022

github-actions bot commented Nov 22, 2022

github-actions bot commented Nov 23, 2022

github-actions bot commented Nov 23, 2022

opensearch-trigger-bot bot commented Nov 23, 2022

dblock commented Nov 23, 2022

andrross commented Nov 23, 2022 •

edited

Loading

Vishalks commented Nov 23, 2022

Prevent deletion of snapshot backing index #5069

Prevent deletion of snapshot backing index #5069

Conversation

Vishalks commented Nov 4, 2022 • edited Loading

Description

Issues Resolved

Check List

github-actions bot commented Nov 4, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Nov 4, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Nov 4, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Nov 4, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Nov 4, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Nov 4, 2022

Gradle Check (Jenkins) Run Completed with:

andrross commented Nov 7, 2022

Vishalks commented Nov 8, 2022

github-actions bot commented Nov 9, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Nov 14, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Nov 19, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Nov 19, 2022

Gradle Check (Jenkins) Run Completed with:

codecov-commenter commented Nov 19, 2022 • edited Loading

Codecov Report

github-actions bot commented Nov 19, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Nov 19, 2022

Gradle Check (Jenkins) Run Completed with:

kotwanikunal left a comment

Choose a reason for hiding this comment

dblock left a comment • edited Loading

Choose a reason for hiding this comment

dblock left a comment

Choose a reason for hiding this comment

github-actions bot commented Nov 21, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Nov 22, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Nov 22, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Nov 23, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Nov 23, 2022

Gradle Check (Jenkins) Run Completed with:

opensearch-trigger-bot bot commented Nov 23, 2022

dblock commented Nov 23, 2022

andrross commented Nov 23, 2022 • edited Loading

Vishalks commented Nov 23, 2022

Vishalks commented Nov 4, 2022 •

edited

Loading

codecov-commenter commented Nov 19, 2022 •

edited

Loading

dblock left a comment •

edited

Loading

andrross commented Nov 23, 2022 •

edited

Loading