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

adds quic receiver to shred-fetch-stage #31576

Merged
merged 1 commit into from
Jun 12, 2023

Conversation

behzadnouri
Copy link
Contributor

@behzadnouri behzadnouri commented May 10, 2023

Problem

Working towards migrating turbine to QUIC.

Summary of Changes

Added QUIC receiver to shred-fetch-stage.

@t-nelson
Copy link
Contributor

is this something that can wait for 1.16 to be cut? really need to keep branch diffs to a minimum for the time being

@behzadnouri
Copy link
Contributor Author

is this something that can wait for 1.16 to be cut? really need to keep branch diffs to a minimum for the time being

I actually need this to be deployed to testnet asap,
so that we can test performance on an actual cluster and optimize where needed.

@behzadnouri
Copy link
Contributor Author

behzadnouri commented May 12, 2023

Can I have this reviewed/merged?
I also have #31610 depending on this change.

Also these PRs only add a code-path which is inactive for all shreds. The goal is to incrementally rollout and monitor for small percent of shreds on testnet and debug/optimize based on the outcome.

@codecov
Copy link

codecov bot commented May 12, 2023

Codecov Report

Merging #31576 (9cb1fc2) into master (8937fd9) will increase coverage by 0.0%.
The diff coverage is 94.1%.

@@           Coverage Diff           @@
##           master   #31576   +/-   ##
=======================================
  Coverage    81.8%    81.9%           
=======================================
  Files         763      763           
  Lines      207690   207735   +45     
=======================================
+ Hits       170067   170156   +89     
+ Misses      37623    37579   -44     

@t-nelson
Copy link
Contributor

we teed this feature set up for discussion on the thursday priorities call. general consensus today was that holding quic turbine for 1.16 would fair better for 1.16 stabilization

Copy link
Contributor

@steviez steviez left a comment

Choose a reason for hiding this comment

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

Overall, the code looks good. I see merit to Trent's point about not wanting to introduce more changes between v1.14 and v1.16 given the already massive diff between the two. However, I also see merit to Behzad's point about wanting to get some basic plumbing in to build on top of to enable initial testing without having to deal with rebase hell later.

In order to try to meet in the middle, maybe something like a5f290a would be agreeable? That is, push the code in but not explicitly spawn the server by default for now.

core/src/shred_fetch_stage.rs Show resolved Hide resolved
@behzadnouri
Copy link
Contributor Author

In order to try to meet in the middle, maybe something like a5f290a would be agreeable? That is, push the code in but not explicitly spawn the server by default for now.

We actually need to start testing QUIC on testnet for propagating a small percentage of slots.

Working towards migrating turbine to QUIC.
Copy link
Contributor

@steviez steviez left a comment

Choose a reason for hiding this comment

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

With v1.16 branch cut, I believe we're good to go on these.

I see there have been some conversations in Discord and a SIMD, but assuming you're looking to push this in and test in parallel while those get hashed out ?

@behzadnouri behzadnouri merged commit aed4ecb into solana-labs:master Jun 12, 2023
@behzadnouri behzadnouri deleted the turbine-quic-server branch June 12, 2023 13:16
jeffwashington pushed a commit to HaoranYi/solana that referenced this pull request Jun 13, 2023
Working towards migrating turbine to QUIC.
wen-coding added a commit to wen-coding/solana that referenced this pull request Aug 15, 2023
Working towards migrating turbine to QUIC.
wen-coding pushed a commit to wen-coding/solana that referenced this pull request Aug 15, 2023
Working towards migrating turbine to QUIC.
wen-coding added a commit to wen-coding/solana that referenced this pull request Aug 15, 2023
Working towards migrating turbine to QUIC.
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.

3 participants