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

feat(storage): database/transaction/cursor metrics #5149

Merged
merged 23 commits into from
Oct 27, 2023

Conversation

shekhirin
Copy link
Collaborator

@shekhirin shekhirin commented Oct 23, 2023

image

@shekhirin shekhirin added C-enhancement New feature or request A-db Related to the database A-observability Related to tracing, metrics, logs and other observability tools labels Oct 23, 2023
@codecov
Copy link

codecov bot commented Oct 24, 2023

Codecov Report

Merging #5149 (4bb82f6) into main (4dc15c3) will increase coverage by 36.08%.
Report is 28 commits behind head on main.
The diff coverage is 85.83%.

❗ Current head 4bb82f6 differs from pull request most recent head 093710b. Consider uploading reports for the commit 093710b to get more accurate results

Impacted file tree graph

Files Coverage Δ
crates/storage/db/src/implementation/mdbx/mod.rs 97.51% <100.00%> (+90.72%) ⬆️
crates/storage/db/src/lib.rs 53.94% <ø> (+38.15%) ⬆️
bin/reth/src/stage/run.rs 1.36% <0.00%> (+1.36%) ⬆️
crates/storage/db/src/abstraction/mock.rs 0.00% <0.00%> (ø)
crates/storage/db/src/metrics/listener.rs 90.90% <90.90%> (ø)
...rates/storage/db/src/implementation/mdbx/cursor.rs 83.66% <94.20%> (+42.59%) ⬆️
bin/reth/src/prometheus_exporter.rs 6.87% <68.75%> (+6.87%) ⬆️
crates/storage/db/src/implementation/mdbx/tx.rs 92.77% <89.47%> (+14.88%) ⬆️
bin/reth/src/node/mod.rs 29.96% <59.37%> (-33.61%) ⬇️
crates/storage/db/src/metrics/mod.rs 84.52% <84.52%> (ø)

... and 476 files with indirect coverage changes

Flag Coverage Δ
integration-tests ?
unit-tests 62.15% <85.83%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
reth binary 29.96% <61.22%> (+4.18%) ⬆️
blockchain tree 80.82% <ø> (+52.36%) ⬆️
pipeline 88.26% <ø> (+83.21%) ⬆️
storage (db) 70.82% <89.89%> (+40.84%) ⬆️
trie 94.93% <ø> (+72.39%) ⬆️
txpool 45.41% <ø> (+4.03%) ⬆️
networking 71.25% <ø> (+40.35%) ⬆️
rpc 40.89% <ø> (+14.41%) ⬆️
consensus 62.95% <ø> (+37.88%) ⬆️
revm 23.36% <ø> (+13.51%) ⬆️
payload builder 3.39% <ø> (-10.77%) ⬇️
primitives 81.34% <ø> (+52.17%) ⬆️

@shekhirin shekhirin marked this pull request as ready for review October 24, 2023 14:45
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

hmm, what's the motivation for using a channel for metrics updates? I guess for more sophisticated reporting that requires more work than just increasing or decreasing metric values like histograms etc?

crates/storage/db/src/implementation/mdbx/tx.rs Outdated Show resolved Hide resolved
crates/storage/db/src/metrics/mod.rs Outdated Show resolved Hide resolved
@shekhirin shekhirin marked this pull request as draft October 24, 2023 18:20
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

this looks great!

would love to see some additional docs for the new metrics type, otherwise lgtm

@shekhirin shekhirin marked this pull request as ready for review October 26, 2023 12:06
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

changes lgtm

not sure about the operation specific metrics

crates/storage/db/src/implementation/mdbx/cursor.rs Outdated Show resolved Hide resolved
@shekhirin shekhirin added this pull request to the merge queue Oct 27, 2023
Merged via the queue into main with commit a9fa281 Oct 27, 2023
22 checks passed
@shekhirin shekhirin deleted the alexey/database-metrics branch October 27, 2023 17:57
@shekhirin shekhirin mentioned this pull request Oct 30, 2023
Arindam2407 pushed a commit to Arindam2407/reth that referenced this pull request Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-db Related to the database A-observability Related to tracing, metrics, logs and other observability tools C-enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants