[Metrics] Fix block height metric #2193
Conversation
dailinsubjam
requested review from
elliedavidson and
bfish713
as code owners
crates/task-impls/src/consensus.rs
Outdated
| .last_synced_block_height | ||
| .set(usize::try_from(leaf.get_height()).unwrap_or(0)); | ||
|
|
||
| if !is_ancestor { |
There was a problem hiding this comment.
Can we just update this metric below where we send the decide event? I.e. add this in the if block starting on line 677 (if new_dcide_reached)
There was a problem hiding this comment.
Actually I think this makes more sense. But what I don't understand is that, if I update metrics at line 677 the synced block height will be 2 views higher than updating at line 613, why they are different?
There was a problem hiding this comment.
Oh, my understanding is that leafA we called on 677 is the current newest pending leaf, we made a decide on this view which equals leafA.view(), but the decided one is leafB on view - 2 (in most cases) due to the consensus protocol. Therefore we'd better still do the update on line 613. Is this understanding correct?
This PR:
Fix the block height metrics by assigning the most recent leaf (rather than its old ancestor).