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 MemoryAllocationExports #434

Merged
merged 3 commits into from
Nov 2, 2018
Merged

Conversation

snago
Copy link
Contributor

@snago snago commented Oct 31, 2018

Counters for total bytes allocated to each memory pool.
Can be used to show allocation rate and promotion rate.

Counters for total bytes allocated to each memory pool.
Can be used to show allocation rate and promotion rate.

Signed-off-by: Robin Karlsson <snago86@gmail.com>

// Visible for testing
void handleMemoryPool(String memoryPool, long before, long after) {
AtomicLong last = getOrCreate(lastMemoryUsage, memoryPool);
Copy link
Contributor

Choose a reason for hiding this comment

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

What exactly is this logic doing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The garbage collection notifications has info about memory in the pool before the GC and after the GC.

handleMemoryPool compares before to the after of the previous GC (or 0 for the first) to catch increase between GCs and then before and after to catch increase during the GC.

Typically eden pools will increase between GCs and old gen pools will increase during GCs, but this is written to be GC algorithm agnostic.

If you're asking about getOrCreate it gets the AtomicLong tracking the last memory usage for the pool (or crates a new one if it's the first time this pool is seen).

Copy link
Contributor

Choose a reason for hiding this comment

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

Could two GCs happen concurrently for different pools?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not that I'm aware of. Each notification contain info about all pools. handleNotification then calls handleMemoryPool for each pool.

But I can't find any guarantees in the documentation that handleNotification won't be called concurrently. To be safe I could make handleNotification synchronized.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you put in a comment explaining this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Let me know if you want me to expand it further.

@snago
Copy link
Contributor Author

snago commented Nov 1, 2018

We're already using a collector like this in production. Here is an example graph:
screenshot_2018-11-01 grafana
The logarithmic scale makes them fit quite nicely in the same graph.

This was created using the following two expressions in grafana:
rate(jvm_memory_pool_allocated_bytes{pool="G1 Eden Space",...}[5m]) for allocation rate
rate(jvm_memory_pool_allocated_bytes{pool="G1 Old Gen",...}[5m]) for promotion rate

Signed-off-by: Robin Karlsson <snago86@gmail.com>
public List<MetricFamilySamples> collect() {
List<MetricFamilySamples> sampleFamilies = new ArrayList<MetricFamilySamples>();
CounterMetricFamily allocated = new CounterMetricFamily(
"jvm_memory_pool_allocated_bytes",
Copy link
Contributor

Choose a reason for hiding this comment

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

Counters should end in _total

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed


// Visible for testing
void handleMemoryPool(String memoryPool, long before, long after) {
AtomicLong last = getOrCreate(lastMemoryUsage, memoryPool);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you put in a comment explaining this?

@Override
public List<MetricFamilySamples> collect() {
List<MetricFamilySamples> sampleFamilies = new ArrayList<MetricFamilySamples>();
CounterMetricFamily allocated = new CounterMetricFamily(
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use a Counter here, rather than having to track this yourself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! That really simplified collect.

I also removed the tests for collect since they were now only testing Counter.collect.

* Fix name
* Explain handleMemoryPool
* Use a Counter internally (and removed tests for collect)

Also, since handleNotification is synchronized now,
lastMemoryUsage can be a regular HashMap.

Signed-off-by: Robin Karlsson <snago86@gmail.com>
@brian-brazil brian-brazil merged commit df64303 into prometheus:master Nov 2, 2018
@brian-brazil
Copy link
Contributor

Thanks!

@snago snago mentioned this pull request Nov 23, 2018
@snago snago deleted the memory-allocation branch January 27, 2019 09:53
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.

None yet

2 participants