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 support to configure min GC intervals for soft and hard limits #12450

Merged
merged 1 commit into from
Feb 24, 2025

Conversation

bogdandrutu
Copy link
Member

@bogdandrutu bogdandrutu commented Feb 21, 2025

When servers are in a high memory usage mode, the amount of times GC is called creates a high CPU usage which combined with a high ingest rate limits capability to offload existing queued data.

Configuring the minimum interval even for hard limit, allows the system to spend some CPU cycles between GCs to offload old data from queues/batch processor. The most amount of data I've seen accumulated are blocked in the batch processor queue (incoming channel) not in the exporter queue.

Copy link

codecov bot commented Feb 21, 2025

Codecov Report

Attention: Patch coverage is 88.88889% with 5 lines in your changes missing coverage. Please review.

Project coverage is 92.19%. Comparing base (cffb7b6) to head (1254bcb).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
internal/memorylimiter/config.go 42.85% 4 Missing ⚠️
internal/memorylimiter/memorylimiter.go 97.22% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (88.88%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12450      +/-   ##
==========================================
+ Coverage   92.10%   92.19%   +0.09%     
==========================================
  Files         465      465              
  Lines       25180    25194      +14     
==========================================
+ Hits        23191    23228      +37     
+ Misses       1592     1570      -22     
+ Partials      397      396       -1     

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

@bogdandrutu bogdandrutu requested a review from jmacd February 22, 2025 03:37
@atoulme
Copy link
Contributor

atoulme commented Feb 24, 2025

This PR addresses a very interesting problem. Note GC kicks in whenever memory doubles, so it can be punitive right now to run in a high throughput environment. In our case, we leaned on the GOGC environment variable. We found that after removing the memory ballast extension, our GC would kick too often. We have set GOGC to 400 as result.

It looks like this change addresses the same issue.

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
@bogdandrutu bogdandrutu added this pull request to the merge queue Feb 24, 2025
Merged via the queue into open-telemetry:main with commit 2b5fa0e Feb 24, 2025
54 of 55 checks passed
@bogdandrutu bogdandrutu deleted the add-support-for-gc branch February 24, 2025 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants