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

repurpose a dead metric in broadcast stage #4963

Merged
merged 2 commits into from
Feb 16, 2025

Conversation

alexpyattaev
Copy link

Problem

get_peers_elapsed metric in broadcast stage was unused

Summary of Changes

  • repurpose it to measure the time used for transmission to the quic endpoint channel

Some notes:

  • we are not actually measuring the time for packets to land on the wire, but rather time needed to ship them over to quic endpoint. To address this we probably should use a bounded channel to stream to the quic endpoint so we get a better idea of the time needed to send data out.

@alexpyattaev alexpyattaev marked this pull request as ready for review February 13, 2025 13:49
behzadnouri
behzadnouri previously approved these changes Feb 13, 2025
@@ -445,6 +445,8 @@ pub fn broadcast_shreds(
quic_endpoint_sender: &AsyncSender<(SocketAddr, Bytes)>,
) -> Result<()> {
let mut result = Ok(());

Choose a reason for hiding this comment

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

Please do follow below guideline in general.
https://google.github.io/styleguide/cppguide.html#Vertical_Whitespace

Choose a reason for hiding this comment

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

the change in this file is probably better to be reverted.

Copy link
Author

Choose a reason for hiding this comment

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

done

@alexpyattaev alexpyattaev merged commit 4b7ea3a into anza-xyz:master Feb 16, 2025
47 checks passed
@alexpyattaev alexpyattaev deleted the dead_metrics branch February 16, 2025 05:09
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.

2 participants