-
Notifications
You must be signed in to change notification settings - Fork 4.5k
limit repairs to top staked requests in batch #28673
Conversation
0357ea2
to
558c426
Compare
558c426
to
9ff5fbf
Compare
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.
lgtm
core/src/serve_repair.rs
Outdated
for batch in reqs_v { | ||
for packet in &batch { |
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.
maybe instead of nested for loops and double indentation something like:
for packet in reqs_v.iter().flatten() {
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.
changed
core/src/serve_repair.rs
Outdated
|
||
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)); |
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.
decode_reqs.sort_by_key(|(_, _, stake)| Reverse(*stake))
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.
done
core/src/serve_repair.rs
Outdated
.map(|nodes| nodes.get(request.sender())) | ||
.unwrap_or_default() | ||
{ | ||
Some(stake) => { |
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.
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 {
//...
}
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.
done
Pull request has been modified.
please do not backport this until testnet is restarted and is stable. |
(cherry picked from commit 17ee334)
… to non-MainnetBeta clusters
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 to2 * MAX_REQUESTS_PER_ITERATION
packets before load shedding packets in the incoming channel. Select topMAX_REQUESTS_PER_ITERATION
requests by stake of requesting node.Limiting this change to testnet.
Fixes #