WIP: Add metrics to TxPool #2321
Conversation
acerone85
requested review from
xgreenx,
Dentosal and
MitchTurner
as code owners
|
base branch should be |
|
please consider branching this from |
acerone85
force-pushed
the
1937_tx_pool_metrics
branch
from
3c473bf
to
080f21f
Compare
acerone85
force-pushed
the
1937_tx_pool_metrics
branch
from
7f54be5
to
f5f97e9
Compare
acerone85
changed the base branch from
release/v0.37.0
to
807_add_more_metrics
crates/metrics/src/txpool_metrics.rs
Outdated
| vec![ | ||
| 1.0, 2.0, 3.0, 4.0, 5.0, 10.0, 20.0, 30.0, 40.0, 50.0, 60.0, 120.0, | ||
| 240.0, 480.0, 960.0, 1920.0, 3840.0, | ||
| ] | ||
| .into_iter(), | ||
| ); | ||
| let select_transaction_time_nanoseconds = Histogram::new( | ||
| vec![ | ||
| 10.0, 20.0, 30.0, 40.0, 50.0, 100.0, 200.0, 500.0, 1000.0, 2000.0, | ||
| 5000.0, 10000.0, 50000.0, 100000.0, 500000.0, | ||
| ] | ||
| .into_iter(), |
There was a problem hiding this comment.
We should move these to buckets.rs.
| @@ -228,6 +228,16 @@ where | |||
| Ok(can_store_transaction) | |||
| } | |||
|
|
|||
| fn record_transaction_time_in_txpool(tx: &StorageData) { | |||
There was a problem hiding this comment.
Also, we need to remember to record these metrics only if config.metrics is true.
There was a problem hiding this comment.
This is done in ed18ad1, although the workflow could be improved. Maybe having MetricsRecorder struct that wraps TxPoolMetrics and exposes the functions to record the metrics could work
…7_tx_pool_metrics
…ion spends in the rayon thread pool
|
Looking good. What things are left here? |
I can do some minor improvements and add the changelog, but otherwise I think the only thing left here is to babysit the PR until https://github.com/FuelLabs/fuel-core/tree/807_add_more_metrics is merged and then this can be merged to master |
…7_tx_pool_metrics
If TxPool has a lot of events, it will interrupt the work of the timer. ### Before requesting review - [x] I have reviewed the code myself --------- Co-authored-by: AurelienFT <32803821+AurelienFT@users.noreply.github.com> Co-authored-by: AurelienFT <aurefook@gmail.com>
This PR disables the flaky `test_poa_multiple_producers` test. Follow-up issue: #2351 ### Before requesting review - [X] I have reviewed the code myself - [X] I have created follow-up issues caused by this PR and linked them here --------- Co-authored-by: green <xgreenx9999@gmail.com>
Linked Issues/PRs
Closes #1937
Description
TODO:
config.metrics = trueChecklist
Before requesting review
After merging, notify other teams
[Add or remove entries as needed]