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(metrics): Track mempool actions and size bucketed by weight #6972

Closed
wants to merge 2 commits into from

Conversation

str4d
Copy link
Contributor

@str4d str4d commented Jun 15, 2023

Motivation

It is useful to have insight into the composition of the mempool along the axes used for the default block construction algorithm. The algorithm from the now-deployed ZIP 317 takes into account both a transaction's weight, and the number of "unpaid actions" it contains.

Solution

This PR adds the following new metrics:

  • (gauge) zcash.mempool.actions.unpaid { "bk" = ["< 0.2", "< 0.4", "< 0.6", "< 0.8", "< 1"] }
  • (gauge) zcash.mempool.actions.paid
  • (gauge) zcash.mempool.size.weighted { "bk" = ["< 1", "1", "> 1", "> 2", "> 3"] }

These are proposed as common Zcash metrics (with a zcash. prefix). The corresponding zcashd PR is zcash/zcash#6632.

Reviewer Checklist

  • Will the PR name make sense to users?
    • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
  • Are the PR labels correct?
  • Does the code do what the ticket and PR says?
    • Does it change concurrent code, unsafe code, or consensus rules?
  • How do you know it works? Does it have tests?

@str4d str4d requested review from a team as code owners June 15, 2023 13:58
@str4d str4d requested review from upbqdn and removed request for a team June 15, 2023 13:59
@str4d
Copy link
Contributor Author

str4d commented Jun 15, 2023

zcash/zcash#6632 updates our default Grafana dashboard to use these new metrics. This PR can either be tested against that dashboard, or those changes could be pulled into the zebrad default dashboard as appropriate.

Copy link
Member

@upbqdn upbqdn left a comment

Choose a reason for hiding this comment

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

Thank you, str4d. I noticed some tests needed adjustments. I fixed them right away.

I can't think of a suitable way to import the dashboards from zcashd. I guess other engineers will know.

@str4d
Copy link
Contributor Author

str4d commented Jun 15, 2023

Thank you, str4d. I noticed some tests needed adjustments. I fixed them right away.

Ahh, I checked with cargo check --tests locally before pushing, but didn't check cargo check --tests --all-features. Thanks!

I can't think of a suitable way to import the dashboards from zcashd. I guess other engineers will know.

The zcashd dashboard should mostly Just Work, as I think it's mostly relying on metrics that zebrad also has. As for importing that specific part into zebrad's Grafana dashboard, that can be done with some JSON munging, but it might be easier to just create the new panels inside Grafana matching what I did for the zcashd one.

@upbqdn upbqdn changed the title metrics: Track mempool actions and size bucketed by weight add(metrics): Track mempool actions and size bucketed by weight Jun 15, 2023
@upbqdn upbqdn added A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement P-Low ❄️ A-diagnostics Area: Diagnosing issues or monitoring performance A-mempool Area: Memory pool transactions labels Jun 15, 2023
@upbqdn
Copy link
Member

upbqdn commented Jun 15, 2023

I opened #6976 to handle the dashboards. I think we can merge this PR first and add the dashboards later, adjusting the code if there are issues.

@teor2345
Copy link
Contributor

Due to #4529, we'll need to push this branch to the zebra repository to get all our CI to run on it.

@mpguerra
Copy link
Contributor

Due to #4529, we'll need to push this branch to the zebra repository to get all our CI to run on it.

Done in #7019

@mpguerra mpguerra closed this Jun 20, 2023
@mergify
Copy link
Contributor

mergify bot commented Jun 20, 2023

⚠️ The sha of the head commit of this PR conflicts with #7019. Mergify cannot evaluate rules on this PR. ⚠️

@str4d str4d deleted the metrics-mempool-composition branch June 20, 2023 11:30
mergify bot pushed a commit that referenced this pull request Jul 18, 2023
… of #6972, credit @str4d) (#7019)

* metrics: Track mempool actions and size bucketed by weight

* Fix tests

* draft fix tests

* fix `fix_arbitrary_generated_action_overflows`

* add some docs

* manually derive arbitrary

* remove unused import

---------

Co-authored-by: Jack Grigg <jack@electriccoin.co>
Co-authored-by: Marek <mail@marek.onl>
Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Diagnosing issues or monitoring performance A-mempool Area: Memory pool transactions A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants