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 changes to block calls in cat shards, indices and segments based on dynamic limit settings #15986

Merged
merged 12 commits into from
Oct 3, 2024

Conversation

sumitasr
Copy link
Member

@sumitasr sumitasr commented Sep 18, 2024

Add changes to block calls in cat shards, indices and segments based on dynamic limit settings

Description

  • Introduced a new class RequestLimitSettings to maintain dynamic settings for 3 actions - cat indices, shards and segments and method to evaluate if circuitLimitBreached for an action.

    • cat.indices.response.limit.number_of_indices (The limit will be applied on number of indices)
    • cat.shards.response.limit.number_of_shards (The limit will be applied on number of shards)
    • cat.segments.response.limit.number_of_indices (The limit will be applied on number of indices)
  • Introduced isRequestLimitCheckSupported method defaulting to false in AbstractCatAction so that we can override this if an action extending AbstractCatAction can define if it supports limit or circuit breaker checks introduced in RequestLimitSettings class. This value can be passed to respected transport action as param e.g. TransportCatShardsAction

  • If circuit limit breached, CircuitBreakingException will be raised which is mapped to 429 error code.

Related Issues

Resolves #15954

Functional Testing

(Outdated - Exception scenario response format has been modified)

Action 1 : /_cat/shards/

Step 1 - Run OS cluster locally

Step 2 - PUT 3 index

Step 3 - Perform /_cat/shards/

curl -XGET "http://localhost:9200/_cat/shards/"
1index 0 p STARTED 1 4.7kb 127.0.0.1 runTask-0
1index 0 r UNASSIGNED
2index 0 p STARTED 1 4.7kb 127.0.0.1 runTask-0
2index 0 r UNASSIGNED
3index 0 p STARTED 1 4.7kb 127.0.0.1 runTask-0
3index 0 r UNASSIGNED

Step 4 - Update setting cat.shards.limit to 2

curl -X PUT "localhost:9200/_cluster/settings" -H 'Content-Type: application/json' -d'
{
"transient" : {
"cat.shards.response.limit.number_of_shards" : 2
}
}'
{"acknowledged":true,"persistent":{},"transient":{"cat":{"shards":{"response":{"limit":{"number_of_shards":"2"}}}}}}%

Step 5 - Perform /_cat/shards/

curl -XGET "http://localhost:9200/_cat/shards/"

curl -XGET "http://localhost:9200/_cat/shards/"
{"error":{"root_cause":[{"type":"response_limit_breached_exception","reason":"Too many shards requested. Can not request shards beyond {2}"}],"type":"response_limit_breached_exception","reason":"Too many shards requested. Can not request shards beyond {2}"},"status":429}%

Step 6 - Update setting cat.shards.limit to 6

curl -X PUT "localhost:9200/_cluster/settings" -H 'Content-Type: application/json' -d'
{
"transient" : {
"cat.shards.response.limit.number_of_shards" : 6
}
}'

Step 7 - Perform /_cat/shards/

curl -XGET "http://localhost:9200/_cat/shards/"
1index 0 p STARTED 1 4.7kb 127.0.0.1 runTask-0
1index 0 r UNASSIGNED
2index 0 p STARTED 1 4.7kb 127.0.0.1 runTask-0
2index 0 r UNASSIGNED
3index 0 p STARTED 1 4.7kb 127.0.0.1 runTask-0
3index 0 r UNASSIGNED

Action 2 : /_cat/indices/

Step 1 - Using cluster started in previous section

Step 2 - Perform /_cat/indices/

curl -XGET "http://localhost:9200/_cat/indices/"
yellow open 1index RdrZ-a-hQ86Dk9BkSKMceQ 1 1 1 0 4.7kb 4.7kb
yellow open 2index hb2ANDTMT7OQfMGa8dKzGA 1 1 1 0 4.7kb 4.7kb
yellow open 3index SmI1dxl2SECswo67Y37TKg 1 1 1 0 4.7kb 4.7kb

Step 3 - Update settings

curl -X PUT "localhost:9200/_cluster/settings" -H 'Content-Type: application/json' -d'
{
"transient" : {
"cat.indices.response.limit.number_of_indices" : 1
}
}'
{"acknowledged":true,"persistent":{},"transient":{"cat":{"indices":{"response":{"limit":{"number_of_indices":"1"}}}}}}%

Step 4 - Perform /_cat/indices/

curl -XGET "http://localhost:9200/_cat/indices/"
{"error":{"root_cause":[{"type":"response_limit_breached_exception","reason":"Too many indices requested. Can not request indices beyond {1}"}],"type":"response_limit_breached_exception","reason":"Too many indices requested. Can not request indices beyond {1}"},"status":429}%

Step 5 - Update settings
curl -X PUT "localhost:9200/_cluster/settings" -H 'Content-Type: application/json' -d'
{
"transient" : {
"cat.indices.response.limit.number_of_indices" : -1
}
}'
{"acknowledged":true,"persistent":{},"transient":{"cat":{"indices":{"response":{"limit":{"number_of_indices":"-1"}}}}}}%

Step 6 - Perform /_cat/indices/

curl -XGET "http://localhost:9200/_cat/indices/"
yellow open 1index RdrZ-a-hQ86Dk9BkSKMceQ 1 1 1 0 4.7kb 4.7kb
yellow open 2index hb2ANDTMT7OQfMGa8dKzGA 1 1 1 0 4.7kb 4.7kb
yellow open 3index SmI1dxl2SECswo67Y37TKg 1 1 1 0 4.7kb 4.7kb

Action 3 : /_cat/segments/

Step 1 - Using cluster started in previous section

Step 2 - Perform /_cat/segments/

curl -XGET "http://localhost:9200/_cat/segments/"
1index 0 p 127.0.0.1 _0 0 1 0 4.5kb 0 false true 9.12.0 true
2index 0 p 127.0.0.1 _0 0 1 0 4.5kb 0 false true 9.12.0 true
3index 0 p 127.0.0.1 _0 0 1 0 4.5kb 0 false true 9.12.0 true

Step3 - Update settings

curl -X PUT "localhost:9200/_cluster/settings" -H 'Content-Type: application/json' -d'
{
"transient" : {
"cat.segments.response.limit.number_of_indices" : 1
}
}'
{"acknowledged":true,"persistent":{},"transient":{"cat":{"segments":{"response":{"limit":{"number_of_indices":"1"}}}}}}%

Step 4 - Perform /_cat/segments/

curl -XGET "http://localhost:9200/_cat/segments/"
{"error":{"root_cause":[{"type":"response_limit_breached_exception","reason":"Segments from too many indices requested. Can not request indices beyond {1}"}],"type":"response_limit_breached_exception","reason":"Segments from too many indices requested. Can not request indices beyond {1}"},"status":429}%

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.

@sumitasr sumitasr changed the title [DRAFT] Add changes to block non-paginated calls in cat shards, indices and segments [DRAFT] Add changes to block calls in cat shards, indices and segments based on dynamic limit settings Sep 18, 2024
@github-actions github-actions bot added Cluster Manager enhancement Enhancement or improvement to existing feature or request labels Sep 18, 2024
Copy link
Contributor

❌ Gradle check result for 8796ab6: 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?

@sumitasr sumitasr force-pushed the draft_changes_15954 branch 2 times, most recently from 59b896a to ec96cdd Compare September 18, 2024 22:36
Copy link
Contributor

❌ Gradle check result for 59b896a: 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 ec96cdd: 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?

@sumitasr sumitasr force-pushed the draft_changes_15954 branch 3 times, most recently from 1944512 to 515a699 Compare September 19, 2024 12:44
…on dynamic limit settings

Signed-off-by: Sumit Bansal <sumitsb@amazon.com>
Copy link
Contributor

❌ Gradle check result for 973f3be: 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 1944512: 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 515a699: 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 27440d2: 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?

@sumitasr
Copy link
Member Author

❌ Gradle check result for 27440d2: 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?

Reason - failed for task ':server:spotlessJavaCheck'.

Pushed fix post running ./gradlew :server:spotlessApply

Copy link
Contributor

❌ Gradle check result for 454e2dd: 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: Sumit Bansal <sumitsb@amazon.com>
Copy link
Contributor

❌ Gradle check result for 8a2140f: 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?

@sumitasr sumitasr changed the title [DRAFT] Add changes to block calls in cat shards, indices and segments based on dynamic limit settings Add changes to block calls in cat shards, indices and segments based on dynamic limit settings Sep 19, 2024
Copy link
Contributor

❌ Gradle check result for 399fb59: 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 7a7ae4b: SUCCESS

Copy link

codecov bot commented Sep 19, 2024

Codecov Report

Attention: Patch coverage is 57.26496% with 50 lines in your changes missing coverage. Please review.

Project coverage is 71.95%. Comparing base (4e3a6d0) to head (ad61a43).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...common/breaker/ResponseLimitBreachedException.java 0.00% 18 Missing ⚠️
...admin/cluster/shards/TransportCatShardsAction.java 11.11% 8 Missing ⚠️
.../opensearch/rest/action/cat/RestIndicesAction.java 30.00% 7 Missing ⚠️
...opensearch/rest/action/cat/RestSegmentsAction.java 30.00% 7 Missing ⚠️
...ensearch/common/breaker/ResponseLimitSettings.java 88.88% 3 Missing and 3 partials ⚠️
.../action/admin/cluster/shards/CatShardsRequest.java 50.00% 2 Missing ⚠️
.../opensearch/rest/action/cat/AbstractCatAction.java 0.00% 1 Missing ⚠️
...search/rest/action/list/RestIndicesListAction.java 66.66% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #15986      +/-   ##
============================================
+ Coverage     71.91%   71.95%   +0.04%     
- Complexity    64491    64561      +70     
============================================
  Files          5289     5291       +2     
  Lines        301509   301620     +111     
  Branches      43557    43571      +14     
============================================
+ Hits         216818   217040     +222     
+ Misses        66892    66820      -72     
+ Partials      17799    17760      -39     

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

@sumitasr sumitasr marked this pull request as ready for review September 20, 2024 05:00
Signed-off-by: Sumit Bansal <sumitsb@amazon.com>
Copy link
Contributor

❌ Gradle check result for 4e732b1: 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: Sumit Bansal <sumitsb@amazon.com>
Copy link
Contributor

github-actions bot commented Oct 1, 2024

❌ Gradle check result for 09492e1: 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?

@sumitasr
Copy link
Member Author

sumitasr commented Oct 1, 2024

❌ Gradle check result for 09492e1: 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?

Existing flaky tests are failing.

Signed-off-by: Sumit Bansal <sumitsb@amazon.com>
Copy link
Contributor

github-actions bot commented Oct 3, 2024

✅ Gradle check result for ad61a43: SUCCESS

@shwetathareja
Copy link
Member

@sumitasr we will need documentation PR for new settings

@shwetathareja
Copy link
Member

Helping with merge as it is already approved by @backslasht & @rajiv-kv

@shwetathareja shwetathareja added the backport 2.x Backport to 2.x branch label Oct 3, 2024
@sumitasr
Copy link
Member Author

sumitasr commented Oct 3, 2024

@sumitasr we will need documentation PR for new settings

Tracking documentation updates here opensearch-project/documentation-website#8460

@shwetathareja shwetathareja merged commit bf6566e into opensearch-project:main Oct 3, 2024
36 of 46 checks passed
@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-15986-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 bf6566e76c46bd7dc5d838503b8c43ff8cb09757
# Push it to GitHub
git push --set-upstream origin backport/backport-15986-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-15986-to-2.x.

gargharsh3134 pushed a commit to gargharsh3134/OpenSearch that referenced this pull request Oct 8, 2024
…on dynamic limit settings (opensearch-project#15986)

* Add changes to block calls in cat shards, indices and segments based on dynamic limit settings

Signed-off-by: Sumit Bansal <sumitsb@amazon.com>
gbbafna pushed a commit that referenced this pull request Oct 9, 2024
…on dynamic limit settings (#15986) (#16235)

Signed-off-by: Sumit Bansal <sumitsb@amazon.com>
dk2k pushed a commit to dk2k/OpenSearch that referenced this pull request Oct 16, 2024
…on dynamic limit settings (opensearch-project#15986)

* Add changes to block calls in cat shards, indices and segments based on dynamic limit settings

Signed-off-by: Sumit Bansal <sumitsb@amazon.com>
dk2k pushed a commit to dk2k/OpenSearch that referenced this pull request Oct 17, 2024
…on dynamic limit settings (opensearch-project#15986)

* Add changes to block calls in cat shards, indices and segments based on dynamic limit settings

Signed-off-by: Sumit Bansal <sumitsb@amazon.com>
dk2k pushed a commit to dk2k/OpenSearch that referenced this pull request Oct 21, 2024
…on dynamic limit settings (opensearch-project#15986)

* Add changes to block calls in cat shards, indices and segments based on dynamic limit settings

Signed-off-by: Sumit Bansal <sumitsb@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch backport-failed Cluster Manager enhancement Enhancement or improvement to existing feature or request
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

[Paginating ClusterManager Read APIs] Blocking non-paginated calls
5 participants