Partially resolved issue #4687 by changing the improper status code for concurrent snapshot executions #5855

baba-devv · 2023-01-13T18:34:03Z

Description

Partially resolved issue #4687 by changing the improper status code for concurrent snapshot execution exception.

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.

…y_Requests). Signed-off-by: Mani <[email protected]>

…ptionChange

github-actions · 2023-01-13T19:04:39Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/9391/
CommitID: 1395a14

codecov-commenter · 2023-01-13T19:09:24Z

Codecov Report

Merging #5855 (fd1572d) into main (a3aab67) will increase coverage by 0.09%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main    #5855      +/-   ##
============================================
+ Coverage     71.00%   71.09%   +0.09%     
- Complexity    57166    57248      +82     
============================================
  Files          4763     4763              
  Lines        270003   270003              
  Branches      39508    39508              
============================================
+ Hits         191713   191967     +254     
+ Misses        62139    61866     -273     
- Partials      16151    16170      +19

Files Changed	Coverage Δ
...napshots/ConcurrentSnapshotExecutionException.java	`42.85% <100.00%> (+42.85%)`	⬆️

... and 498 files with indirect coverage changes

baba-devv · 2023-01-15T10:16:13Z

@Bukhtawar Hi, for issue #4687, should I make a PR by resolving Repository exception status as well ?
Also, any comments on this one ?
cc: @andrross

Bukhtawar · 2023-01-15T13:49:47Z

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

    public RestStatus status() {
-        return RestStatus.SERVICE_UNAVAILABLE;
+        return RestStatus.TOO_MANY_REQUESTS;
    }


Thanks can you please also add a test?

I am wondering shouldn't this be 409 (Conflict) instead of 429?

What's the underlying error? Is it concurrent snapshots being taken whereas this isn't supported? In that case it should definitely not be a 429, that implies the server is unable to handle traffic, not a predictable conflicting behavior - 409 seems more correct in that case.

"Thrown when a user tries to multiple conflicting snapshot/restore operations at the same time." -> 409

The only question I had was that since it is concurrent & not specifically conflicting, should I change that to 409 ?
@dblock @shwetathareja

@baba-devv : Since it is concurrent, it is resulting in conflict and hence rejected by server.

@baba-devv yes in my opinion it should be 409

…ptionChange

…rch-project#8883) * Updating Version.java from server/ to buildSrc/ Signed-off-by: Varun Jain <[email protected]> * Adding Changelog Signed-off-by: Varun Jain <[email protected]> * Path update Signed-off-by: Varun Jain <[email protected]> * Changelog Update Signed-off-by: Varun Jain <[email protected]> * Removing Changelog from commit Signed-off-by: Varun Jain <[email protected]> --------- Signed-off-by: Varun Jain <[email protected]>

…dex (opensearch-project#8792) * Add support to restore only unassigned shards of an index --------- Signed-off-by: Sachin Kale <[email protected]> Signed-off-by: Sachin Kale <[email protected]> Co-authored-by: Sachin Kale <[email protected]>

…arch-project#8929) Signed-off-by: Andriy Redko <[email protected]> (cherry picked from commit 212dba4)

opensearch-project#8901) * Fix flakiness in MasterServiceTests.testThrottlingForMultipleTaskTypes The test configured a [timeout duration of zero][1] for certain tasks and asserted that all tasks were throttled or timed out. This is not a valid assertion because it is possible for a task to complete before the [asynchronous timeout operation runs][2], which means the task would complete successfully. The fix is to adjust the assertion to allow for successful tasks in this case. [1]: https://github.com/opensearch-project/OpenSearch/blob/60985bc300d9eafd36c1ab25d46235e1c925c565/server/src/test/java/org/opensearch/cluster/service/MasterServiceTests.java#L941 [2]: https://github.com/opensearch-project/OpenSearch/blob/9fc3f4096958159ec9b53012fc7ced19fd793e1b/server/src/main/java/org/opensearch/common/util/concurrent/PrioritizedOpenSearchThreadPoolExecutor.java#L266 Signed-off-by: Andrew Ross <[email protected]> * Add a deterministic test case for timeout Signed-off-by: Andrew Ross <[email protected]> --------- Signed-off-by: Andrew Ross <[email protected]>

…node replication (opensearch-project#8912) * Fix SegmentReplicationIT.testReplicahasDiffFilesThanPrimary This test is now failing for node-node replication. On the primary shard the prepareSegmentReplication method should cancel any ongoing replication if it is running and start a new sync. Thisis incorrectly using Map.compute which will not replace the existing handler entry in the allocationIdToHandlers map. It will only cancel the existing source handler. As a result this can leave the copyState map with an entry and hold an index commit while the test is cleaning up. The copyState is only cleared when a handler is cancelled directly or from a cluster state update. Signed-off-by: Marc Handalian <[email protected]> * PR feedback. Signed-off-by: Marc Handalian <[email protected]> --------- Signed-off-by: Marc Handalian <[email protected]>

The intent of opensearch-project#8825 was to retry only specified tests. The wrong parameter was configured though: ['filter' should be set][1], not 'classRetry'. [1]: https://github.com/gradle/test-retry-gradle-plugin/blob/main/README.adoc#filtering Signed-off-by: Andrew Ross <[email protected]>

…roject#8940) This commit rote refactors MediaTypeParserRegistry to MediaTypeRegistry to make the class naming align with the intention of the logic. The MediaTypeRegistry is a mechanism for downstream extensions to register concrete MediaTypes thus having Parser in the name is unneeded. Signed-off-by: Nicholas Walter Knize <[email protected]>

…invocation of segment replication (opensearch-project#8937) * [Segment Replication] Use deterministic mechanism to have concurrent invocation of segment replication Signed-off-by: Suraj Singh <[email protected]> * Clean up Signed-off-by: Suraj Singh <[email protected]> --------- Signed-off-by: Suraj Singh <[email protected]>

* Register MediaTypes through SPI This commit provides a new SPI interface MediaContentProvider. Modules, Plugins, Extensions, implement this interface and provide the concrete MediaType implementations (and MIME aliases) through getMediaTypes and getAdditionalMediaTypes, respectively. This enables downstream extensions (e.g., serverless or cloud native implementations) to register their own custom MediaType and define the serialization format that is registered when the classloader loads the MediaTypeRegistry instead of having to register the types explicitly in application code. Signed-off-by: Nicholas Walter Knize <[email protected]> * pass the MediaTypeProvider classloader to the SPI ServiceLoader Signed-off-by: Nicholas Walter Knize <[email protected]> --------- Signed-off-by: Nicholas Walter Knize <[email protected]>

…ch-project#8943) Signed-off-by: Suraj Singh <[email protected]>

Signed-off-by: Sarthak Aggarwal <[email protected]>

…ect#8914) Signed-off-by: Gaurav Bafna <[email protected]>

…ch-project#8963) Signed-off-by: Andriy Redko <[email protected]>

… assemble task fails without them. (opensearch-project#8904) Signed-off-by: Evan Kielley <[email protected]>

…" (opensearch-project#8968) This reverts commit c43743d. Signed-off-by: Andriy Redko <[email protected]>

…ributions bundled with JRE (opensearch-project#8195) Signed-off-by: Andriy Redko <[email protected]>

…er (opensearch-project#8967) * Replace deprecated Query#rewrite(IndexReader with rewrite(IndexSearcher Query#rewrite(IndexReader reader) is deprecated in Lucene 9.7 and removed in Lucene 10 in favor of Query#rewrite(IndexSearcher searcher). The latter provides LeafCollector hooks to optimize for concurrent queries. This commit cuts over usage of rewrite(IndexReader) to rewrite(IndexSearcher) for Lucene 10 compatibility and upstream query optimizations. Signed-off-by: Nicholas Walter Knize <[email protected]> * remove stray collector.finish Signed-off-by: Nicholas Walter Knize <[email protected]> --------- Signed-off-by: Nicholas Walter Knize <[email protected]>

…erRegistry (opensearch-project#8826) XContentFactory is tightly coupled to concrete XContentType. This commit builds on the MediaType abstractions, specifically MediaTypeParserRegistry, to decouple contentType introspection (e.g., determining type from byte streams) from concrete XContentTypes. This enables downstream extensions (e.g., serverless or cloud native implementations) to register their own custom XContentType and define the serialization format for proper content introspection. This also removes the tight coupling of :libs:opensearch-x-content with abstract interface contracts further enabling modularity. Signed-off-by: Nicholas Walter Knize <[email protected]>

Signed-off-by: Sarthak Aggarwal <[email protected]>

…" (opensearch-project#8968) This reverts commit c43743d. Signed-off-by: Andriy Redko <[email protected]>

…eStorePromotedAsPrimaryCommitCommit (opensearch-project#8931) Signed-off-by: Poojita Raj <[email protected]> Return 409 Conflict HTTP status instead of 500 on failure to concurrently execute snapshots Signed-off-by: Mani <[email protected]>

Signed-off-by: Sarthak Aggarwal <[email protected]>

…" (opensearch-project#8968) This reverts commit c43743d. Signed-off-by: Andriy Redko <[email protected]>

…ptionChange

github-actions · 2023-07-29T08:26:49Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.http.SearchRestCancellationIT.testAutomaticCancellationDuringFetchPhase

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

github-actions · 2023-07-29T08:26:50Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.repositories.azure.AzureBlobContainerRetriesTests.testReadRangeBlobWithRetries

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

andrross · 2023-08-03T18:42:35Z

Superseded by #8986

baba-devv added 2 commits January 13, 2023 00:59

Changed status of ConcurrentSnapshotException from 5xx -> 429(Too_Man…

05874f3

…y_Requests). Signed-off-by: Mani <[email protected]>

Merge branch 'main' of github.com:baba-devv/OpenSearch into Mani-Exce…

1395a14

…ptionChange

Bukhtawar reviewed Jan 15, 2023

View reviewed changes

baba-devv added 2 commits January 16, 2023 22:44

Merge branch 'main' of github.com:baba-devv/OpenSearch into Mani-Exce…

114e6bc

…ptionChange

Merge branch 'main' of github.com:baba-devv/OpenSearch into Mani-Exce…

dda7f85

…ptionChange

vibrantvarun and others added 23 commits July 29, 2023 13:09

Add 2.9.1 to BWC and known versions (opensearch-project#8923) (opense…

64b3b39

…arch-project#8929) Signed-off-by: Andriy Redko <[email protected]> (cherry picked from commit 212dba4)

Remove testUpdateSegmentsWhileRefreshing flaky test failure (opensear…

594627f

…ch-project#8943) Signed-off-by: Suraj Singh <[email protected]>

amending tests to include all codecs (opensearch-project#8907)

bfe5462

Signed-off-by: Sarthak Aggarwal <[email protected]>

Add metadata prefix to Remote Translog Metadata file (opensearch-proj…

91b0be2

…ect#8914) Signed-off-by: Gaurav Bafna <[email protected]>

[BUG] CompletionSuggestSearchIT.testSkipDuplicates is flaky (opensear…

f7ece2d

…ch-project#8963) Signed-off-by: Andriy Redko <[email protected]>

Added empty build.gradle files for arm64 and s390x because the gradle…

dd78796

… assemble task fails without them. (opensearch-project#8904) Signed-off-by: Evan Kielley <[email protected]>

Revert "amending tests to include all codecs (opensearch-project#8907)…

d492e57

…" (opensearch-project#8968) This reverts commit c43743d. Signed-off-by: Andriy Redko <[email protected]>

[distribution/archives] [Linux] [x64] Provide the variant of the dist…

d19ba04

…ributions bundled with JRE (opensearch-project#8195) Signed-off-by: Andriy Redko <[email protected]>

amending tests to include all codecs (opensearch-project#8907)

4c4d2e8

Signed-off-by: Sarthak Aggarwal <[email protected]>

Revert "amending tests to include all codecs (opensearch-project#8907)…

77c4bcf

…" (opensearch-project#8968) This reverts commit c43743d. Signed-off-by: Andriy Redko <[email protected]>

amending tests to include all codecs (opensearch-project#8907)

15d08fa

Signed-off-by: Sarthak Aggarwal <[email protected]>

Revert "amending tests to include all codecs (opensearch-project#8907)…

7a73a7d

…" (opensearch-project#8968) This reverts commit c43743d. Signed-off-by: Andriy Redko <[email protected]>

baba-devv force-pushed the Mani-ExceptionChange branch from d5b8dcc to 7a73a7d Compare July 29, 2023 07:40

Merge branch 'main' of github.com:baba-devv/OpenSearch into Mani-Exce…

fd1572d

…ptionChange

baba-devv mentioned this pull request Jul 29, 2023

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

Merged

6 tasks

andrross closed this Aug 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Partially resolved issue #4687 by changing the improper status code for concurrent snapshot executions #5855

Partially resolved issue #4687 by changing the improper status code for concurrent snapshot executions #5855

baba-devv commented Jan 13, 2023

github-actions bot commented Jan 13, 2023

codecov-commenter commented Jan 13, 2023 •

edited by codecov bot

Loading

baba-devv commented Jan 15, 2023

Bukhtawar Jan 15, 2023

shwetathareja May 9, 2023 •

edited

Loading

dblock May 9, 2023 •

edited

Loading

baba-devv May 9, 2023

shwetathareja May 10, 2023 •

edited

Loading

shwetathareja May 11, 2023

github-actions bot commented Jul 29, 2023

github-actions bot commented Jul 29, 2023

andrross commented Aug 3, 2023

Partially resolved issue #4687 by changing the improper status code for concurrent snapshot executions #5855

Partially resolved issue #4687 by changing the improper status code for concurrent snapshot executions #5855

Conversation

baba-devv commented Jan 13, 2023

Description

Issues Resolved

Check List

github-actions bot commented Jan 13, 2023

Gradle Check (Jenkins) Run Completed with:

codecov-commenter commented Jan 13, 2023 • edited by codecov bot Loading

Codecov Report

baba-devv commented Jan 15, 2023

Bukhtawar Jan 15, 2023

Choose a reason for hiding this comment

shwetathareja May 9, 2023 • edited Loading

Choose a reason for hiding this comment

dblock May 9, 2023 • edited Loading

Choose a reason for hiding this comment

baba-devv May 9, 2023

Choose a reason for hiding this comment

shwetathareja May 10, 2023 • edited Loading

Choose a reason for hiding this comment

shwetathareja May 11, 2023

Choose a reason for hiding this comment

github-actions bot commented Jul 29, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Jul 29, 2023

Gradle Check (Jenkins) Run Completed with:

andrross commented Aug 3, 2023

codecov-commenter commented Jan 13, 2023 •

edited by codecov bot

Loading

shwetathareja May 9, 2023 •

edited

Loading

dblock May 9, 2023 •

edited

Loading

shwetathareja May 10, 2023 •

edited

Loading