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

Forwarder: forward_packets w/o metrics #30925

Merged
merged 3 commits into from
Apr 12, 2023

Conversation

apfitzge
Copy link
Contributor

@apfitzge apfitzge commented Mar 28, 2023

Problem

Separate forwarding from metrics updates - banking stage workers will not use the same metrics structs.

Summary of Changes

This is a combination of and is blocked by #30920, #30921, #30922, #30923

Fixes #

@apfitzge apfitzge force-pushed the refactor/forwarder-5 branch from ded6b09 to 2ed673c Compare April 6, 2023 17:51
@codecov
Copy link

codecov bot commented Apr 6, 2023

Codecov Report

Merging #30925 (9a3294d) into master (bf986d6) will increase coverage by 0.0%.
The diff coverage is 76.1%.

@@           Coverage Diff            @@
##           master   #30925    +/-   ##
========================================
  Coverage    81.5%    81.5%            
========================================
  Files         727      728     +1     
  Lines      205166   205477   +311     
========================================
+ Hits       167295   167557   +262     
- Misses      37871    37920    +49     

/// Forwards all valid, unprocessed packets in the iterator, up to a rate limit.
/// Returns whether forwarding succeeded, the number of attempted forwarded packets
/// if any, the time spent forwarding, and the leader pubkey if any.
fn forward_packets<'a>(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Put this fn higher since in the worker PR it will become pub(crate)

let conn = self.connection_cache.get_connection(addr);
conn.send_data_batch_async(packet_vec)
}
ForwardOption::NotForward => panic!("should not forward"),
Copy link
Contributor Author

@apfitzge apfitzge Apr 7, 2023

Choose a reason for hiding this comment

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

only called within forward_packets which exits early on ForwardOption::NotForward due to get_leader_and_addr checks.

@apfitzge apfitzge marked this pull request as ready for review April 7, 2023 18:34
@apfitzge apfitzge requested a review from lijunwangs April 7, 2023 18:34
@apfitzge
Copy link
Contributor Author

apfitzge commented Apr 7, 2023

@lijunwangs, I'm thinking maybe this PR (includes changes from #30923) is better as a standalone - makes it more clear why the change is happening.

@apfitzge
Copy link
Contributor Author

apfitzge commented Apr 7, 2023

Planning to follow-up on this PR with some actual tests of the forwarder. The ones that currently exist are ignored ☹️

@apfitzge apfitzge requested review from tao-stones and ryoqun April 11, 2023 17:31
Copy link
Contributor

@lijunwangs lijunwangs left a comment

Choose a reason for hiding this comment

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

Looks good to me

@apfitzge apfitzge merged commit 01659ed into solana-labs:master Apr 12, 2023
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Apr 14, 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