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.

Sign up for GitHub

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

Jump to bottom

[Circuit-Breaker] Add memory circuit breaker configuration #1347

Merged
merged 1 commit into from
Apr 12, 2022
Merged

[Circuit-Breaker] Add memory circuit breaker configuration #1347

merged 1 commit into from
Apr 12, 2022

Conversation

zuochengding
Copy link
Contributor

@zuochengding zuochengding commented Mar 15, 2022 •
edited
Loading

Add responseMaxHeapPercentage, memoryCircuitBreakerEnabled flags in opensearch_dashboards.yml

Signed-off-by: Zuocheng Ding zding817@gmail.com

Description

This PR is going to add the capability for OSD to control the memory circuit breaker limits.
Comment out some code due to the dependency from opensearch-js client. Tested through local modification in elasticsearch

Dependency : opensearch-project/opensearch-js#207

Issues Resolved

opensearch-project/opensearch-js#202

Check List

  • New functionality includes testing.
    • All tests pass
      • yarn test:jest
      • yarn test:jest_integration
      • yarn test:ftr
  • New functionality has been documented.
  • Commits are signed per the DCO using --signoff

@zuochengding zuochengding requested a review from a team as a code owner March 15, 2022 20:33
ananzh
ananzh reviewed Mar 18, 2022
View reviewed changes
Copy link
Member

@ananzh ananzh left a comment

Choose a reason for hiding this comment

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

just one comment. thanks zuocheng

@zuochengding zuochengding force-pushed the memory-circuit-breaker branch 3 times, most recently from 3110cb2 to f9d7351 Compare March 21, 2022 22:32
tmarkley
tmarkley previously requested changes Mar 23, 2022
View reviewed changes
src/core/server/opensearch/client/client_config.ts Outdated Show resolved Hide resolved
src/core/server/opensearch/client/client_config.ts Outdated Show resolved Hide resolved
src/core/server/opensearch/opensearch_config.ts Outdated Show resolved Hide resolved
config/opensearch_dashboards.yml Outdated Show resolved Hide resolved
config/opensearch_dashboards.yml Outdated Show resolved Hide resolved
@tmarkley
Copy link
Contributor

Are we adding documentation for this on the OpenSearch Dashboards side of things?

@zuochengding zuochengding force-pushed the memory-circuit-breaker branch 2 times, most recently from 11f36b7 to 6a6abdf Compare March 24, 2022 17:03
ananzh
ananzh previously approved these changes Mar 28, 2022
View reviewed changes
Copy link
Member

@ananzh ananzh left a comment

Choose a reason for hiding this comment

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

LGTM

@kavilla
Copy link
Member

kavilla commented Mar 29, 2022

@zuochengding can we convert this to a draft until this ready to consume.

@zuochengding
Copy link
Contributor Author

zuochengding commented Mar 30, 2022 •
edited
Loading

@zuochengding can we convert this to a draft until this ready to consume.

Updated the PR with opensearch-js v1.1.0 consumed. This new revision will be the latest PR.

@kavilla kavilla linked an issue Apr 5, 2022 that may be closed by this pull request
[OSJS] Consume circuit breaker #1425
Closed
mihirsoni
mihirsoni previously approved these changes Apr 5, 2022
View reviewed changes
Copy link
Contributor

@mihirsoni mihirsoni left a comment

Choose a reason for hiding this comment

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

LGTM !!

ananzh
ananzh previously approved these changes Apr 5, 2022
View reviewed changes
@kavilla kavilla dismissed tmarkley’s stale review April 7, 2022 12:37

Thanks for blocking, code was merged and npm package was released so now this PR is in a valid state.

@zuochengding
[Circuit-Breaker] Add memory circuit breaker configuration
210f36b 
Add opensearch.memoryCircuitBreaker.enabled, opensearch.memoryCircuitBreaker.maxPercentage flags in opensearch_dashboards.yml

Signed-off-by: Zuocheng Ding <zding817@gmail.com>
@zuochengding zuochengding dismissed stale reviews from ananzh and mihirsoni via 210f36b April 8, 2022 17:15
@zuochengding zuochengding merged commit 7bc0f85 into opensearch-project:main Apr 12, 2022
tmarkley pushed a commit to tmarkley/opensearch-build that referenced this pull request Apr 14, 2022
Adds OpenSearch Dashboards circuit breaker vars
85d658d 
  * Adds new `opensearch.memoryCircuitBreaker.enabled` and
  `opensearch.memoryCircuitBreaker.maxPercentage` variables
  for the memory circuit breaker limits.
  * Issue: opensearch-project/opensearch-js#202
  * OpenSearch Dashboards PR: opensearch-project/OpenSearch-Dashboards#1347

Signed-off-by: Tommy Markley <markleyt@amazon.com>
@tmarkley tmarkley mentioned this pull request Apr 14, 2022
Merged
1 task
tmarkley pushed a commit to tmarkley/OpenSearch-Dashboards that referenced this pull request Apr 14, 2022
Cleans up changes from memory circuit breaker PR
0bcd407 
* Cleans up whitespace, reduces delta of the changes.

Follow-up from opensearch-project#1347

Signed-off-by: Tommy Markley <markleyt@amazon.com>
@tmarkley tmarkley mentioned this pull request Apr 14, 2022
Merged
1 task
peterzhuamazon pushed a commit to opensearch-project/opensearch-build that referenced this pull request Apr 14, 2022
Adds OpenSearch Dashboards circuit breaker vars (#1968)
94df5ef 
* Adds new `opensearch.memoryCircuitBreaker.enabled` and
  `opensearch.memoryCircuitBreaker.maxPercentage` variables
  for the memory circuit breaker limits.
  * Issue: opensearch-project/opensearch-js#202
  * OpenSearch Dashboards PR: opensearch-project/OpenSearch-Dashboards#1347

Signed-off-by: Tommy Markley <markleyt@amazon.com>
kavilla pushed a commit that referenced this pull request Apr 14, 2022
Cleans up changes from memory circuit breaker PR (#1463)
e6911a0 
* Cleans up whitespace, reduces delta of the changes.

Follow-up from #1347

Signed-off-by: Tommy Markley <markleyt@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tmarkley tmarkley tmarkley left review comments

@kavilla kavilla kavilla approved these changes

@ananzh ananzh ananzh approved these changes

@mihirsoni mihirsoni mihirsoni left review comments

Assignees
No one assigned
Labels
v2.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[OSJS] Consume circuit breaker
5 participants
@zuochengding @tmarkley @kavilla @mihirsoni @ananzh