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

Return 409 Conflict HTTP status instead of 503 on failure to concurre… #8986

Merged
merged 4 commits into from
Aug 14, 2023

Conversation

baba-devv
Copy link
Contributor

Signed-off-by: Mani singh.mani1231@gmail.com

Return 409 Conflict HTTP status instead of 500 on failure to concurrently execute snapshots

Description

[Describe what this change achieves]

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

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.

…ntly execute snapshots

Signed-off-by: Mani <singh.mani1231@gmail.com>

Return 409 Conflict HTTP status instead of 500 on failure to concurrently execute snapshots

Signed-off-by: Mani <singh.mani1231@gmail.com>

another
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@codecov
Copy link

codecov bot commented Jul 29, 2023

Codecov Report

Merging #8986 (7a8057f) into main (8cecd5a) will increase coverage by 0.07%.
Report is 1 commits behind head on main.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main    #8986      +/-   ##
============================================
+ Coverage     71.10%   71.17%   +0.07%     
- Complexity    57415    57474      +59     
============================================
  Files          4775     4775              
  Lines        270700   270700              
  Branches      39566    39566              
============================================
+ Hits         192483   192680     +197     
+ Misses        62066    61878     -188     
+ Partials      16151    16142       -9     
Files Changed Coverage Δ
...src/main/java/org/opensearch/ExceptionsHelper.java 82.71% <ø> (ø)
.../main/java/org/opensearch/OpenSearchException.java 91.11% <ø> (ø)
...rch/core/action/ShardOperationFailedException.java 100.00% <ø> (ø)
.../support/DefaultShardOperationFailedException.java 95.65% <ø> (ø)
...a/org/opensearch/core/common/ParsingException.java 96.66% <ø> (ø)
.../core/common/breaker/CircuitBreakingException.java 100.00% <ø> (ø)
.../opensearch/core/common/io/stream/StreamInput.java 88.84% <ø> (+1.17%) ⬆️
...opensearch/core/common/io/stream/StreamOutput.java 95.18% <ø> (+0.64%) ⬆️
...src/main/java/org/opensearch/core/index/Index.java 100.00% <ø> (ø)
.../java/org/opensearch/core/index/shard/ShardId.java 97.56% <ø> (ø)
... and 39 more

... and 436 files with indirect coverage changes

@baba-devv
Copy link
Contributor Author

@dblock Can you check this PR, its replacement for #5855 ?

Copy link
Member

@saratvemulapalli saratvemulapalli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the change and adding a test.
Seems like a reasonable change to me!

Do we have to update docs and clients for precise error handling?

CHANGELOG.md Outdated Show resolved Hide resolved
@reta
Copy link
Collaborator

reta commented Jul 31, 2023

Do we have to update docs and clients for precise error handling?

The change for public API status code could be considered a breaking change (unless this is a indeed the bug), the documentation definitely needs an update.

@dblock
Copy link
Member

dblock commented Aug 3, 2023

Do we have to update docs and clients for precise error handling?

The change for public API status code could be considered a breaking change (unless this is a indeed the bug), the documentation definitely needs an update.

Yep, we are only considering this for main.

…ntly execute snapshots

Signed-off-by: Mani <singh.mani1231@gmail.com>
@baba-devv
Copy link
Contributor Author

@dblock what should my next steps be for this ?

@opensearch-trigger-bot
Copy link
Contributor

Compatibility status:



> Task :checkCompatibility
Incompatible components: [https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/security-analytics.git]
Compatible components: [https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git]

BUILD SUCCESSFUL in 27m 28s

@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2023

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.remotestore.RemoteIndexPrimaryRelocationIT.testPrimaryRelocationWhileIndexing

Signed-off-by: Daniel (dB.) Doubrovkine <dblock@amazon.com>
@dblock
Copy link
Member

dblock commented Aug 10, 2023

@dblock what should my next steps be for this ?

Will merge as soon as I have a green build.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Daniel (dB.) Doubrovkine <dblock@amazon.com>
@opensearch-trigger-bot
Copy link
Contributor

Compatibility status:



> Task :checkCompatibility
Incompatible components: [https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/security-analytics.git]
Compatible components: [https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git]

BUILD SUCCESSFUL in 37m 59s

@reta reta changed the title Return 409 Conflict HTTP status instead of 500 on failure to concurre… Return 409 Conflict HTTP status instead of 503 on failure to concurre… Aug 14, 2023
@reta reta added the v3.0.0 Issues and PRs related to version 3.0.0 label Aug 14, 2023
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.remotestore.SegmentReplicationUsingRemoteStoreIT.testDropPrimaryDuringReplication

@dblock dblock merged commit 8e57a66 into opensearch-project:main Aug 14, 2023
kaushalmahi12 pushed a commit to kaushalmahi12/OpenSearch that referenced this pull request Sep 12, 2023
…ntly execute snapshots (opensearch-project#8986)

Signed-off-by: Mani <singh.mani1231@gmail.com>

Return 409 Conflict HTTP status instead of 500 on failure to concurrently execute snapshots

Signed-off-by: Mani <singh.mani1231@gmail.com>

another

Signed-off-by: Mani <singh.mani1231@gmail.com>
Signed-off-by: Daniel (dB.) Doubrovkine <dblock@amazon.com>
Co-authored-by: Daniel (dB.) Doubrovkine <dblock@amazon.com>
brusic pushed a commit to brusic/OpenSearch that referenced this pull request Sep 25, 2023
…ntly execute snapshots (opensearch-project#8986)

Signed-off-by: Mani <singh.mani1231@gmail.com>

Return 409 Conflict HTTP status instead of 500 on failure to concurrently execute snapshots

Signed-off-by: Mani <singh.mani1231@gmail.com>

another

Signed-off-by: Mani <singh.mani1231@gmail.com>
Signed-off-by: Daniel (dB.) Doubrovkine <dblock@amazon.com>
Co-authored-by: Daniel (dB.) Doubrovkine <dblock@amazon.com>
Signed-off-by: Ivan Brusic <ivan.brusic@flocksafety.com>
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
…ntly execute snapshots (opensearch-project#8986)

Signed-off-by: Mani <singh.mani1231@gmail.com>

Return 409 Conflict HTTP status instead of 500 on failure to concurrently execute snapshots

Signed-off-by: Mani <singh.mani1231@gmail.com>

another

Signed-off-by: Mani <singh.mani1231@gmail.com>
Signed-off-by: Daniel (dB.) Doubrovkine <dblock@amazon.com>
Co-authored-by: Daniel (dB.) Doubrovkine <dblock@amazon.com>
Signed-off-by: Shivansh Arora <hishiv@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v3.0.0 Issues and PRs related to version 3.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants