Improve /metrics endpoint performance #14453

tjiuming · 2022-02-24T11:58:39Z

Motivation

Avoid more than once PrometheusMetricsGenerator#generate invoke in a period(1 minute), for the purpose of saving memory and reducing CPU usage.
Persist metrics data into file and send to client by jetty's send_file. Avoid huge heap memory allocations, too much mem_copy operations, too much memory resizes, high GC pressure and high CPU usage.

Modifications

Using sliding window in PrometheusMetricsGenerator#generate, invoke PrometheusMetricsGenerator#generate once in a period, cache the result and return to client directly.
Persist metrics data into temp file, send it to client by send_file.
Delete temp files when JVM exits.

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

Dependencies (does it add or upgrade a dependency): (no)
The public API: (no)
The schema: (no)
The default values of configurations: (no)
The wire protocol: (no)
The rest endpoints: (no)
The admin cli options: (no)
Anything that affects deployment: (no)

Documentation

Check the box below or label this PR directly (if you have committer privilege).

Need to update docs?

doc-required

(If you need help on updating docs, create a doc issue)
no-need-doc

(Please explain why)
doc

(If this PR contains doc changes)

tjiuming · 2022-02-24T12:00:02Z

Tests to be completed

tjiuming · 2022-02-25T09:40:03Z

Cancel write metrics data to file, cache buf only.

…prometheus_performance

… usage increasing.

eolivelli

Nice work.
We should make this configurable. In case we have a bug and we want to disable this feature

tjiuming · 2022-02-26T16:11:43Z

After and before(write out 200MB metrics data).

Jason918

Persist metrics data into temp file, send it to client by send_file.

Do you actual use file IOs? If so, please help point it out.

Generally, it seems this is a nice optimization, but do we have any downside of this feature?

tjiuming · 2022-02-27T04:04:51Z

Do you actual use file IOs? If so, please help point it out.

Canceled persist data to temp files, maybe broker has a poor performance disk, so use CompositeDirectByteBuf only.

Generally, it seems this is a nice optimization, but do we have any downside of this feature?

When pulsar has to export more than 50MB metrics data, there will be cause high heap memory usage, high GC pressure and high CPU usage. The pr is for the purpose of fix it.

@Jason918

Jason918 · 2022-02-27T09:41:37Z

When pulsar has to export more than 50MB metrics data, there will be cause high heap memory usage, high GC pressure and high CPU usage. The pr is for the purpose of fix it.

Actually, I am just curious if there is some situation that it's better not use this feature, maybe like metric data is under 50MB?

tjiuming · 2022-02-27T09:59:15Z

Actually, I am just curious if there is some situation that it's better not use this feature, maybe like metric data is under 50MB?

Avoid more than once PrometheusMetricsGenerator#generate invoke in a period(1 minute), for the purpose of saving memory and reducing CPU usage. This can be configurable, I'll add a configuration if needed.

Canceled persist data to temp files, maybe broker has a poor performance disk, so use CompositeDirectByteBuf only. But this, we cannot know how much metrics data will be wrote to clients before it generated. So, it cannot be disabled.

@Jason918

tjiuming · 2022-02-27T10:13:22Z

Nice work. We should make this configurable. In case we have a bug and we want to disable this feature

Thanks.

Avoid more than once PrometheusMetricsGenerator#generate invoke in a period(1 minute), for the purpose of saving memory and reducing CPU usage.

This feature is a precursor. What I want is in the future we can clean all the metrics objects after metrics data generated, it could release a lot of objects and reduce heap memory usage if there are over 1 million topics in a broker.
Many metrics exported as Gauges, so we don't have to depend on rate/irate functions to aggregate these metrics. It can be reduce prometheus's pressure.

…prometheus_performance

tjiuming · 2022-03-19T06:40:38Z

Make cache-metrics-data configurable
@Jason918 @eolivelli Could you please help review?

eolivelli

Overall LGTM

Can you also add the configuration entry to broker.conf and proxy.conf ?

pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java

# Conflicts: # site2/docs/reference-configuration.md

tjiuming · 2022-04-15T08:21:38Z

@eolivelli @Jason918 hi，this PR has blocked for a long time, if there are no more change requests, could you please help approve?

# Conflicts: # pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/prometheus/PrometheusMetricsGenerator.java

conf/proxy.conf

Jason918

LGTM

pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java

…prometheus_performance � Conflicts: � conf/proxy.conf

eolivelli

Lgtm

ariestp316@gmail.com added 2 commits February 24, 2022 19:28

improve /metrics endpoint performance

084f5ee

checkstyle fix

a3b2430

github-actions bot assigned tjiuming Feb 24, 2022

github-actions bot added the doc-not-needed Your PR changes do not impact docs label Feb 24, 2022

tjiuming changed the title ~~Dev/prometheus performance~~ Improve /metrics endpoint performance Feb 24, 2022

codelipenghui added this to the 2.11.0 milestone Feb 24, 2022

cancel write metrics data to file, cache buf only

f5d876a

github-actions bot added doc-required Your PR changes impact docs and you will update later. and removed doc-required Your PR changes impact docs and you will update later. labels Feb 25, 2022

ariestp316@gmail.com added 7 commits February 26, 2022 01:58

add MetricsArray Tests

0602cd8

add tests

7a5b731

Merge branch 'master' of https://github.com/tjiuming/pulsar into dev/…

7566c93

…prometheus_performance

performance improve

98f45aa

use unpooled direct buffers for metrics data in case of direct buffer…

5666442

… usage increasing.

use unpooled direct buffers for metrics data in case of direct buffer…

e787f4c

… usage increasing.

rename MetricsArray to TimeWindow

de84829

eolivelli requested changes Feb 26, 2022

View reviewed changes

Jason918 reviewed Feb 27, 2022

View reviewed changes

ariestp316@gmail.com added 5 commits March 3, 2022 11:28

Merge branch 'master' of https://github.com/tjiuming/pulsar into dev/…

82f3d0e

…prometheus_performance

Merge branch 'master' of https://github.com/tjiuming/pulsar into dev/…

455094f

…prometheus_performance

improve test.

e2c38db

improve test.

6d81a7c

Merge branch 'master' into dev/prometheus_performance

efe9409

ariestp316@gmail.com added 2 commits March 18, 2022 23:41

add doc

6e3f1b1

add doc

d99d022

github-actions bot added doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. and removed doc-not-needed Your PR changes do not impact docs labels Mar 18, 2022

improve tests

2a0420a

tjiuming requested review from Jason918 and eolivelli March 22, 2022 15:05

eolivelli requested changes Mar 22, 2022

View reviewed changes

pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java Outdated Show resolved Hide resolved

ariestp316@gmail.com added 2 commits March 22, 2022 23:09

Merge branch 'master' into dev/prometheus_performance

091cee0

# Conflicts: # site2/docs/reference-configuration.md

review fix

0fe614c

tjiuming requested a review from eolivelli March 23, 2022 02:11

Merge branch 'master' into dev/prometheus_performance

8d9596e

# Conflicts: # pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/prometheus/PrometheusMetricsGenerator.java

Jason918 reviewed Apr 15, 2022

View reviewed changes

conf/proxy.conf Outdated Show resolved Hide resolved

review fix

1605705

tjiuming requested a review from Jason918 April 19, 2022 10:41

Jason918 approved these changes Apr 19, 2022

View reviewed changes

eolivelli reviewed May 11, 2022

View reviewed changes

pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java Outdated Show resolved Hide resolved

dao-jun added 2 commits May 11, 2022 18:20

Merge branch 'master' of https://github.com/tjiuming/pulsar into dev/…

af563aa

…prometheus_performance � Conflicts: � conf/proxy.conf

rename enableExposeBufferMetrics configuration

4573a24

eolivelli approved these changes May 11, 2022

View reviewed changes

codelipenghui approved these changes May 12, 2022

View reviewed changes

codelipenghui merged commit 858aa6d into apache:master May 12, 2022

tjiuming deleted the dev/prometheus_performance branch May 12, 2022 15:43

codelipenghui added type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages area/metrics labels May 16, 2022

codelipenghui mentioned this pull request Sep 28, 2022

[branch-2.9] Group prometheus metrics. #17852

Merged

4 tasks

Improve /metrics endpoint performance #14453

Improve /metrics endpoint performance #14453

Uh oh!

Conversation

tjiuming commented Feb 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

Does this pull request potentially affect one of the following parts:

Documentation

Uh oh!

tjiuming commented Feb 24, 2022

Uh oh!

tjiuming commented Feb 25, 2022

Uh oh!

eolivelli left a comment

Choose a reason for hiding this comment

Uh oh!

tjiuming commented Feb 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jason918 left a comment

Choose a reason for hiding this comment

Uh oh!

tjiuming commented Feb 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jason918 commented Feb 27, 2022

Uh oh!

tjiuming commented Feb 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tjiuming commented Feb 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tjiuming commented Mar 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eolivelli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tjiuming commented Apr 15, 2022

Uh oh!

Uh oh!

Jason918 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

eolivelli left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tjiuming commented Feb 24, 2022 •

edited

Loading

tjiuming commented Feb 26, 2022 •

edited

Loading

tjiuming commented Feb 27, 2022 •

edited

Loading

tjiuming commented Feb 27, 2022 •

edited

Loading

tjiuming commented Feb 27, 2022 •

edited

Loading

tjiuming commented Mar 19, 2022 •

edited

Loading