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

Update block arrival histogram values #11424

Merged
merged 12 commits into from
Sep 12, 2022

Conversation

terencechain
Copy link
Member

@terencechain terencechain commented Sep 8, 2022

What does this PR do? Why is it needed?
Update the current block arrival histogram bucket value to align with our expectations. The best case is for the block to arrive within 100ms, then the block that arrives between 100 - 500ms is likely the second best. Any block to arrives passed one slot will be considered too late and will likely to be orphaned anyway

Which issues(s) does this PR fix?
N/A

Other notes for review
The effort to improve upon the block arrival histogram bucket is we want to begin monitoring block arrival time for reorg against late block proposal. The current bucket is not being looked at much

@terencechain terencechain added Ready For Review A pull request ready for code review OK to merge labels Sep 8, 2022
@terencechain terencechain self-assigned this Sep 8, 2022
@terencechain terencechain requested a review from a team as a code owner September 8, 2022 23:24
Buckets: []float64{250, 500, 1000, 1500, 2000, 4000, 8000, 16000},
Buckets: []float64{100, 500, 1000, 4000, 8000, 12000},
Copy link
Member

Choose a reason for hiding this comment

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

I think you need even more granularity around 500 to 1000ms. Here's a current screenshot from mainnet metrics

Screenshot from 2022-09-12 09-17-20

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the chart! I added 250 and 750 back

Copy link
Member

Choose a reason for hiding this comment

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

OK great. Are you sure you want to remove 1500 and 2000? That seems like common latency buckets

Copy link
Member Author

Choose a reason for hiding this comment

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

No, you are right. I was worried that too many buckets might be bad, but then I realized there's not much downside to it. Thanks for the suggestions!

beacon-chain/sync/metrics.go Outdated Show resolved Hide resolved
…labs/prysm into update-block-arrival-histogram
@prylabs-bulldozer prylabs-bulldozer bot merged commit 5a1d260 into develop Sep 12, 2022
@delete-merged-branch delete-merged-branch bot deleted the update-block-arrival-histogram branch September 12, 2022 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants