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

Leader qos part 2 - add stage to find packet's sender stake #23690

Merged
merged 3 commits into from Mar 18, 2022

Conversation

tao-stones
Copy link
Contributor

@tao-stones tao-stones commented Mar 15, 2022

Problem

Packet Sender's stake can be helpful information for Leader's prioritization algo.

Summary of Changes

  • Add find_packet_sender_stake_stage, insert it between streamer and sigverify_stage.
  • The new stage finds packet senders' stakes from gossip, set it to packet.meta.sender_stake field.

Fixes #

core/src/cluster_nodes.rs Outdated Show resolved Hide resolved
core/src/cluster_nodes.rs Outdated Show resolved Hide resolved
core/src/tpu.rs Outdated Show resolved Hide resolved
core/src/tpu.rs Outdated Show resolved Hide resolved
core/src/cluster_nodes.rs Outdated Show resolved Hide resolved
core/src/cluster_nodes.rs Outdated Show resolved Hide resolved
core/src/cluster_nodes.rs Outdated Show resolved Hide resolved
Comment on lines +115 to +117
let mut send_batches_time = Measure::start("send_batches_time");
if let Err(e) = sender.send(batches) {
info!("Sender error: {:?}", e);
}
send_batches_time.stop();
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: One thing we've started doing is using Measure::this()

let (bank_start, poh_recorder_lock_time) = Measure::this(
|_| poh_recorder.lock().unwrap().bank_start(),
(),
"poh_recorder_lock",
);
to make some of these measurement code blocks clearer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I find using Measure::this on simple function with one or no param is cleaner than if function takes multiple params. Not huge fan of adding another indentation tho; nor the mixed use of Measure::this and "Measure::start` in same code block.

@codecov
Copy link

codecov bot commented Mar 17, 2022

Codecov Report

Merging #23690 (f999ad2) into master (2da4e3e) will decrease coverage by 0.0%.
The diff coverage is 82.1%.

@@            Coverage Diff            @@
##           master   #23690     +/-   ##
=========================================
- Coverage    81.8%    81.7%   -0.1%     
=========================================
  Files         581      582      +1     
  Lines      158312   158437    +125     
=========================================
+ Hits       129518   129534     +16     
- Misses      28794    28903    +109     

@tao-stones
Copy link
Contributor Author

@carllin do you think can give this PR a green light, or there are rooms to improve?

@tao-stones tao-stones merged commit c478fe2 into solana-labs:master Mar 18, 2022
@tao-stones tao-stones deleted the leader-qos-part-2 branch March 18, 2022 00:31
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.

None yet

4 participants