- 
                Notifications
    You must be signed in to change notification settings 
- Fork 64
apollo_dashboard: prettify CONSENSUS_DECISIONS_REACHED_AS_PROPOSER panel #9777
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions
crates/apollo_dashboard/src/panels/consensus.rs line 439 at r1 (raw file):
Panel::new( "Consensus Decisions Reached As Proposer", "The number of rounds with decision reached where this node is the proposer (10m window)",
what does the 10 min window mean here? it's a counter, it's not like it will forget all the value after 10 min
crates/apollo_dashboard/src/panels/consensus.rs line 446 at r1 (raw file):
PanelType::TimeSeries, ) .with_log_query("\"Building proposal\" OR \"BATCHER_FIN_PROPOSER\"")
It actually matches this lig:
| "DECISION_REACHED: Decision reached for round {} with proposer {}. {:?}", | 
This isn't for proposal built but for proposal decision reached.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @matanl-starkware)
crates/apollo_dashboard/src/panels/consensus.rs line 460 at r1 (raw file):
get_panel_consensus_round_above_zero(), get_panel_consensus_block_number_diff_from_sync(), get_panel_consensus_decisions_reached_as_proposer(),
actually @ShahakShama asked for this to be high up on the row
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @guyf-starkware and @ShahakShama)
crates/apollo_dashboard/src/panels/consensus.rs line 439 at r1 (raw file):
Previously, guyf-starkware wrote…
what does the 10 min window mean here? it's a counter, it's not like it will forget all the value after 10 min
The concept of showing an increase instead of a raw value serves 2 reasons:
- The actual value has no real meaning. If you see "300,000", what does it tells you? Is it OK or not?
 With increase, you get the "change over the last period", from which you can understand whether it makes sense or not.
- After a pod reset, the counter goes to 0. Then different nodes may have very large difference, which makes the panel useless.
The 10m value is a common value for many panels.
crates/apollo_dashboard/src/panels/consensus.rs line 446 at r1 (raw file):
Previously, guyf-starkware wrote…
It actually matches this lig:
"DECISION_REACHED: Decision reached for round {} with proposer {}. {:?}", This isn't for proposal built but for proposal decision reached.
I didn't understand what you're saying.
I tried to think "what may be an interesting query in regards to proposer reaching decisions".
The purpose is not to match all cases where the counter is being increased.
You may suggest an alternative query if you believe this is incorrect.
crates/apollo_dashboard/src/panels/consensus.rs line 460 at r1 (raw file):
Previously, guyf-starkware wrote…
actually @ShahakShama asked for this to be high up on the row
I overrule his decision.
This panel is not particularly important, and it's better to place it near other panels that display similar metrics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @matanl-starkware)
crates/apollo_dashboard/src/panels/consensus.rs line 446 at r1 (raw file):
Previously, matanl-starkware (Matan Lior) wrote…
I didn't understand what you're saying.
I tried to think "what may be an interesting query in regards to proposer reaching decisions".
The purpose is not to match all cases where the counter is being increased.You may suggest an alternative query if you believe this is incorrect.
The link I gave you is the exact log message that should get logged when this metric is increased. Did you want to search for other message?
The query for this line is:
textPayload =~ "DECISION_REACHED:.*proposer 0x0*<VALIDATOR ID>"
CAST(REGEXP_EXTRACT(textPayload, "height: (\\\\d+)"), "INT64") > [BLOCK_NUMBER]'
Where validator_id is  your ID (to see block proposed by you and accepted) and BLOCK_NUMBER is the block number you care about.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @guyf-starkware)
crates/apollo_dashboard/src/panels/consensus.rs line 446 at r1 (raw file):
Previously, guyf-starkware wrote…
The link I gave you is the exact log message that should get logged when this metric is increased. Did you want to search for other message?
The query for this line is:textPayload =~ "DECISION_REACHED:.*proposer 0x0*<VALIDATOR ID>" CAST(REGEXP_EXTRACT(textPayload, "height: (\\\\d+)"), "INT64") > [BLOCK_NUMBER]'Where
validator_idis your ID (to see block proposed by you and accepted) andBLOCK_NUMBERis the block number you care about.
I didn't look for "the places where the counter is incremented". That alone is not interesting enough.
Usually, when there's a problem, you would search for things "around" that might point to the problem.
Therefore, I've also added Building Proposal, so you can see if there are a pair of building-deciding couples.
Bear in mind that the query is automatically filtered for the time range you've zoomed in on Grafana. So, if you search for a specific time range, simply zoom and then click the link button.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matanl-starkware reviewed 2 of 2 files at r1, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @matanl-starkware)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@guyf-starkware reviewed all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @matanl-starkware)
No description provided.