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

Don't over-allocate in HeapBufferedAsyncEntityConsumer in order to consume the response #9993

Merged
merged 1 commit into from
Sep 12, 2023

Conversation

reta
Copy link
Collaborator

@reta reta commented Sep 12, 2023

Description

The HeapBufferedAsyncEntityConsumer over-allocates the response buffer (up to configured limit, by default of 100Mb) in order to consume the response. This may not be an issue in most cases but if there are many requests being sent in parallel, the memory overhead will be noticeable.

Screenshot from 2023-09-12 11-48-11

This is not a memory leak (all resources are properly closed) but the consequences of over committing the heap buffers.

Related Issues

Closes #9866

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.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@reta reta added the bug Something isn't working label Sep 12, 2023
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

…nsume the response

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
@github-actions
Copy link
Contributor

github-actions bot commented Sep 12, 2023

Compatibility status:

Checks if related components are compatible with change 696f412

Incompatible components

Skipped components

Compatible components

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

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

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

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@codecov
Copy link

codecov bot commented Sep 12, 2023

Codecov Report

Merging #9993 (696f412) into main (b90a888) will increase coverage by 0.05%.
Report is 1 commits behind head on main.
The diff coverage is 45.45%.

@@             Coverage Diff              @@
##               main    #9993      +/-   ##
============================================
+ Coverage     71.09%   71.14%   +0.05%     
- Complexity    58083    58125      +42     
============================================
  Files          4824     4824              
  Lines        273897   273904       +7     
  Branches      39910    39912       +2     
============================================
+ Hits         194714   194856     +142     
+ Misses        62830    62745      -85     
+ Partials      16353    16303      -50     
Files Changed Coverage Δ
...dination/UnsafeBootstrapClusterManagerCommand.java 0.00% <0.00%> (ø)
...ch/client/nio/HeapBufferedAsyncEntityConsumer.java 66.66% <71.42%> (+9.09%) ⬆️

... and 496 files with indirect coverage changes

@reta reta merged commit 16b54a2 into opensearch-project:main Sep 12, 2023
11 of 12 checks passed
sarthakaggarwal97 pushed a commit to sarthakaggarwal97/OpenSearch that referenced this pull request Sep 20, 2023
…nsume the response (opensearch-project#9993)

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
brusic pushed a commit to brusic/OpenSearch that referenced this pull request Sep 25, 2023
…nsume the response (opensearch-project#9993)

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
Signed-off-by: Ivan Brusic <ivan.brusic@flocksafety.com>
vikasvb90 pushed a commit to vikasvb90/OpenSearch that referenced this pull request Oct 10, 2023
…nsume the response (opensearch-project#9993)

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
…nsume the response (opensearch-project#9993)

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
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
bug Something isn't working 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.

[Performance] Performance of main branch is worse compared to 2.x
2 participants