Skip to content

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