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(stages, tree): update sync metrics from blockchain tree #3507

Merged
merged 5 commits into from
Jul 2, 2023

Conversation

shekhirin
Copy link
Collaborator

@shekhirin shekhirin commented Jun 30, 2023

Resolves #2604

Currently, reth_sync_checkpoint metric is not updated after the pipeline sync is done.

This PR fixes it by sending new block heights from blockchain tree to reth_stages::MetricsListener, which iterates over all known stages using StageId::ALL and updates the reth_sync_checkpoint gauge value.

@shekhirin shekhirin changed the title feat(stages, blockchain-tree): update sync metrics from blockchain tree feat(stages, tree): update sync metrics from blockchain tree Jun 30, 2023
@codecov
Copy link

codecov bot commented Jun 30, 2023

Codecov Report

Merging #3507 (3487b2b) into main (d14f995) will decrease coverage by 0.03%.
The diff coverage is 15.15%.

Impacted file tree graph

Impacted Files Coverage Δ
bin/reth/src/node/mod.rs 10.27% <0.00%> (-0.10%) ⬇️
crates/consensus/beacon/src/engine/mod.rs 77.76% <0.00%> (-0.53%) ⬇️
crates/interfaces/src/blockchain_tree/mod.rs 31.57% <ø> (ø)
crates/stages/src/metrics/listener.rs 0.00% <0.00%> (ø)
crates/stages/src/metrics/sync_metrics.rs 0.00% <0.00%> (ø)
crates/stages/src/pipeline/builder.rs 78.72% <0.00%> (ø)
crates/storage/provider/src/providers/mod.rs 8.44% <0.00%> (-0.12%) ⬇️
crates/blockchain-tree/src/shareable.rs 38.52% <25.00%> (-2.00%) ⬇️
crates/blockchain-tree/src/blockchain_tree.rs 81.84% <53.84%> (-0.42%) ⬇️

... and 11 files with indirect coverage changes

Flag Coverage Δ
integration-tests 16.26% <0.00%> (-0.03%) ⬇️
unit-tests 64.10% <15.15%> (-0.01%) ⬇️

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

Components Coverage Δ
reth binary 23.02% <0.00%> (-0.03%) ⬇️
blockchain tree 81.25% <40.00%> (-0.44%) ⬇️
pipeline 86.98% <0.00%> (-0.12%) ⬇️
storage (db) 73.89% <0.00%> (-0.05%) ⬇️
trie 95.64% <ø> (ø)
txpool 51.79% <ø> (+0.64%) ⬆️
networking 77.84% <ø> (-0.11%) ⬇️
rpc 58.19% <ø> (+0.03%) ⬆️
consensus 62.58% <0.00%> (-0.20%) ⬇️
revm 34.99% <ø> (ø)
payload builder 6.83% <ø> (ø)
primitives 88.50% <ø> (+0.01%) ⬆️

@shekhirin shekhirin requested a review from mattsse June 30, 2023 16:13
@shekhirin shekhirin marked this pull request as ready for review June 30, 2023 16:13
Comment on lines +52 to +57
MetricEvent::SyncHeight { height } => {
for stage_id in StageId::ALL {
let stage_metrics = self.sync_metrics.get_stage_metrics(stage_id);
stage_metrics.checkpoint.set(height as f64);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

i see, nice - i guess we dont need to do anyhting with the entities processed / entities total?

@gakonst gakonst enabled auto-merge July 2, 2023 13:01
@gakonst gakonst added this pull request to the merge queue Jul 2, 2023
Merged via the queue into main with commit 951fd0a Jul 2, 2023
@gakonst gakonst deleted the alexey/live-sync-metrics branch July 2, 2023 13:52
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.

Stage sync metrics are not updated in blockchain tree
2 participants