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

ethdb/pebble: fix pebble metrics registration #29699

Merged

Conversation

magicxyyz
Copy link
Contributor

Pebble gauge metrics are stuck when opening a database for the second time (within the same metrics namespace, after closing first one).

Gauge metrics are created with metrics.NewRegisteredGauge and it fails silently if metric name already exists. The Meter metrics work despite that metrics.NewRegisteredMeter is used, because it calls metrics.GetOrRegisterMeter internally.

This PR fixes the issue by using metrics.GetOrRegister* functions instead.

@rjl493456442
Copy link
Member

Sorry, what's the issue fixed by this pull request? As you said, GetOrRegisterMeter call GetOrRegister internally and these two should be equivalent, isn't it?

@fjl fjl added this to the 1.14.1 milestone May 6, 2024
@karalabe karalabe merged commit 3e896c8 into ethereum:master May 6, 2024
2 checks passed
@magicxyyz magicxyyz deleted the fix-pebble-metrics-registration branch May 6, 2024 13:33
@magicxyyz
Copy link
Contributor Author

magicxyyz commented May 6, 2024

Sorry, what's the issue fixed by this pull request? As you said, GetOrRegisterMeter call GetOrRegister internally and these two should be equivalent, isn't it?

metrics.NewRegisteredMeter and metrics.GetOrRegisterMeter are equivalent, but metrics.NewRegisteredGauge and metrics.GetOrRegisterGauge are not - the former fails silently if metric name already exists and that causes the issue mentioned in this PR's description:

Pebble gauge metrics are stuck when opening a database for the second time (within the same metrics namespace, after closing first one).

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

4 participants