Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

limit repairs to top staked requests in batch #28673

Merged
merged 2 commits into from
Nov 17, 2022

Conversation

jbiseda
Copy link
Contributor

@jbiseda jbiseda commented Oct 29, 2022

Problem

repair requests from staked nodes should be prioritized.

Summary of Changes

For each iteration we will select MAX_REQUESTS_PER_ITERATION requests to process. Accept up to 2 * MAX_REQUESTS_PER_ITERATION packets before load shedding packets in the incoming channel. Select top MAX_REQUESTS_PER_ITERATION requests by stake of requesting node.

Limiting this change to testnet.

Fixes #

@jbiseda jbiseda force-pushed the repair-sort-req-by-stake branch from 0357ea2 to 558c426 Compare October 31, 2022 00:26
@jbiseda jbiseda added the v1.14 label Nov 16, 2022
@jbiseda jbiseda force-pushed the repair-sort-req-by-stake branch from 558c426 to 9ff5fbf Compare November 16, 2022 02:59
@jbiseda jbiseda marked this pull request as ready for review November 16, 2022 08:22
behzadnouri
behzadnouri previously approved these changes Nov 16, 2022
Copy link
Contributor

@behzadnouri behzadnouri left a comment

Choose a reason for hiding this comment

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

lgtm

Comment on lines 475 to 476
for batch in reqs_v {
for packet in &batch {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe instead of nested for loops and double indentation something like:

for packet in reqs_v.iter().flatten() {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed


if decoded_reqs.len() > MAX_REQUESTS_PER_ITERATION {
stats.dropped_requests_low_stake += decoded_reqs.len() - MAX_REQUESTS_PER_ITERATION;
decoded_reqs.sort_by(|(_, _, s1), (_, _, s2)| s2.cmp(s1));
Copy link
Contributor

Choose a reason for hiding this comment

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

decode_reqs.sort_by_key(|(_, _, stake)| Reverse(*stake))

https://doc.rust-lang.org/std/cmp/struct.Reverse.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

.map(|nodes| nodes.get(request.sender()))
.unwrap_or_default()
{
Some(stake) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if epoch_staked_nodes includes any zeros, probably not.
Nevertheless I guess it wouldn't hurt to be explicit here and don't rely on that presumption.
Something like:

let stake = epoch_staked_nodes
  .as_ref()
  .and_then(|stakes| staked.get(request.sender()))
  .unwrap_or_default();
if stake == 0 {
   // ...
} else {
   //...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@mergify mergify bot dismissed behzadnouri’s stale review November 16, 2022 22:21

Pull request has been modified.

@behzadnouri
Copy link
Contributor

please do not backport this until testnet is restarted and is stable.

@jbiseda jbiseda removed the v1.14 label Nov 16, 2022
@jbiseda jbiseda merged commit 17ee334 into solana-labs:master Nov 17, 2022
@jbiseda jbiseda added the v1.14 label Nov 30, 2022
mergify bot pushed a commit that referenced this pull request Nov 30, 2022
mergify bot added a commit that referenced this pull request Nov 30, 2022
limit repairs to top staked requests in batch (#28673)

(cherry picked from commit 17ee334)

Co-authored-by: Jeff Biseda <jbiseda@gmail.com>
jbiseda added a commit to jbiseda/solana that referenced this pull request Dec 9, 2022
mergify bot pushed a commit that referenced this pull request Dec 11, 2022
jbiseda added a commit that referenced this pull request Dec 12, 2022
…ters (backport #29163) (#29206)

apply [limit repairs to top staked... #28673] to non-MainnetBeta clusters (#29163)

(cherry picked from commit 88a8f40)

Co-authored-by: Jeff Biseda <jbiseda@gmail.com>
gnapoli23 pushed a commit to gnapoli23/solana that referenced this pull request Dec 16, 2022
gnapoli23 pushed a commit to gnapoli23/solana that referenced this pull request Dec 16, 2022
nickfrosty pushed a commit to nickfrosty/solana that referenced this pull request Jan 4, 2023
nickfrosty pushed a commit to nickfrosty/solana that referenced this pull request Jan 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants