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

Introduce optional memory ballast #45

Merged
merged 1 commit into from
Jun 25, 2019
Merged

Introduce optional memory ballast #45

merged 1 commit into from
Jun 25, 2019

Conversation

owais
Copy link
Contributor

@owais owais commented Jun 24, 2019

This change add an optional memory ballast to help with ease GC pressure
in high thoughput scenarios

  • Added new CLI flag called --mem-ballast-size-mib which represents
    the size of memory ballast to use in MiB. Omitting the value or setting
    it to zero does not set the ballast.

  • Updated process telemetry code to account for the ballast and report
    memory usage statistics after subtracting the ballast size. I'm not sure
    if this makes sense in every scenario. If not, we can add another flag
    to disable the behaviour. Another option is to add one more metric that
    just exports the memory ballast size as a static number and then let
    metric consumers to adjust the numbers.

https://blog.twitch.tv/go-memory-ballast-how-i-learnt-to-stop-worrying-and-love-the-heap-26c2462549a2

golang/go#23044

Effects

We've seen 30-50% increase in throughput and were able to considerably reduce the size of our collector cluster at Omnition.

image

This change add an optional memory ballast to help with ease GC pressure
in high thoughput scenarios

- Added new CLI flag called `--mem-ballast-size-mib` which represents
the size of memory ballast to use in MiB. Omitting the value or setting
it to zero does not set the ballast.

- Updated process telemetry code to account for the ballast and report
memory usage statistics after subtracting the ballast size. I'm not sure
if this makes sense in every scenario. If not, we can add another flag
to disable the behaviour. Another option is to add one more metric that
just exports the memory ballast size as a static number and then let
metric consumers to adjust the numbers.

https://blog.twitch.tv/go-memory-ballast-how-i-learnt-to-stop-worrying-and-love-the-heap-26c2462549a2

golang/go#23044
@tigrannajaryan
Copy link
Member

@owais FYI, build failure is a known bug: #43
I submitted a patch to Jaeger, waiting for them to accept it. For now you can re-run the build when you see this particular issue.
If they don't accept the patch quickly we will have to introduce workarounds into our code.

Copy link
Contributor

@pjanotti pjanotti left a comment

Choose a reason for hiding this comment

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

LGTM

@pjanotti pjanotti merged commit 6566e78 into open-telemetry:master Jun 25, 2019
@flands flands added this to the 0.1.0 milestone Jun 28, 2019
@owais owais deleted the add-mem-ballast branch June 28, 2019 17:42
MovieStoreGuy pushed a commit to atlassian-forks/opentelemetry-collector that referenced this pull request Nov 11, 2021
hughesjj pushed a commit to hughesjj/opentelemetry-collector that referenced this pull request Apr 27, 2023
* Initial installer script for collector and fluentd

* Initial config with hostmetrics for linux hosts

* Allow release to test stage without signing

* Add default apt/yum repo definition files

* Migrate tests to pytest
Troels51 pushed a commit to Troels51/opentelemetry-collector that referenced this pull request Jul 5, 2024
* Work on Flush-Close api

* Add Flush & Close

* Add flush and close interface

* Remove return type from Flush-Close

* Fix docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants