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

stats for staked/unstaked repair requests #28215

Merged
merged 3 commits into from
Oct 5, 2022

Conversation

jbiseda
Copy link
Contributor

@jbiseda jbiseda commented Oct 4, 2022

Problem

need stats to break down repair requests by staked/unstaked senders

Summary of Changes

minimal changes to track staked/unstaked sender of all requests which pass the load shedding filter.

See also #27708 #27771

Fixes #

@jbiseda jbiseda marked this pull request as ready for review October 4, 2022 09:22
if budget_exhausted {
stats.dropped_requests_outbound_bandwidth += 1;
continue;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The issue I see here is that this is still paying for deserializing packets and looking up pubkeys in epoch_staked_nodes hashmap even though the outbound budget is exhausted. This seems to me pretty wasteful just for the sake of collecting metrics.

The other thing, as previously mentioned: #27771 (comment)
I still don't see why stake vs unstaked breakdown of load matters?
If we cannot handle staked nodes requests, then why should we allocate any budget to unstaked nodes?

Copy link
Contributor

Choose a reason for hiding this comment

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

and, if we want to prioritize requests by stake, then again, we don't care about stake vs unstaked breakdown

behzadnouri
behzadnouri previously approved these changes Oct 4, 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

@jbiseda jbiseda added the v1.14 label Oct 4, 2022
@mergify mergify bot dismissed behzadnouri’s stale review October 4, 2022 21:49

Pull request has been modified.

@jbiseda jbiseda merged commit e3e888c into solana-labs:master Oct 5, 2022
mergify bot pushed a commit that referenced this pull request Oct 5, 2022
mergify bot added a commit that referenced this pull request Oct 5, 2022
stats for staked/unstaked repair requests (#28215)

(cherry picked from commit e3e888c)

Co-authored-by: Jeff Biseda <jbiseda@gmail.com>
jbiseda added a commit to jbiseda/solana that referenced this pull request Oct 10, 2022
solana-labs#28230)

stats for staked/unstaked repair requests (solana-labs#28215)

(cherry picked from commit e3e888c)

Co-authored-by: Jeff Biseda <jbiseda@gmail.com>
@jbiseda jbiseda added the v1.13 label Oct 26, 2022
mergify bot pushed a commit that referenced this pull request Oct 26, 2022
(cherry picked from commit e3e888c)

# Conflicts:
#	core/src/serve_repair.rs
mergify bot added a commit that referenced this pull request Oct 26, 2022
* stats for staked/unstaked repair requests (#28215)

(cherry picked from commit e3e888c)

# Conflicts:
#	core/src/serve_repair.rs

* resolve merge conflicts

Co-authored-by: Jeff Biseda <jbiseda@gmail.com>
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

2 participants