TPU Vote using quic -- client side implementation #3473
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Merged
lijunwangs
force-pushed
the
vote_use_quic.client
branch
from
2963099
to
c36b5a1
Compare
lijunwangs
commented
Nov 11, 2024
lijunwangs
commented
Nov 11, 2024
lijunwangs
commented
Nov 11, 2024
lijunwangs
changed the title
bw-solana
reviewed
Nov 15, 2024
behzadnouri
reviewed
Nov 15, 2024
Vote using QUIC send vote packet using the rigth sender removed dup declared functions rebase with master QuicServerParams removed remove_tpu_vote first part of sending votes using quic use quic for vote on client side with connection cache add debug messages turn on quic for vote by default for testing
lijunwangs
force-pushed
the
vote_use_quic.client
branch
from
e474eff
to
b9e663d
Compare
|
@behzadnouri @bw-solana Can you please take another look at this PR? Thanks! |
behzadnouri
approved these changes
Dec 3, 2024
Comment on lines
+58
to
+64
| match client.send_data_async(buf) { | ||
| Ok(_) => Ok(()), | ||
| Err(err) => { | ||
| trace!("Ran into an error when sending vote: {err:?} to {tpu:?}"); | ||
| Err(GossipError::SendError) | ||
| } | ||
| } |
There was a problem hiding this comment.
probably better:
client.send_data_async(buf).map_err(|err| { /* ... */ })
Comment on lines
+126
to
+135
| let _ = send_vote_transaction( | ||
| cluster_info, | ||
| vote_op.tx(), | ||
| Some(tpu_vote_socket), | ||
| &connection_cache, | ||
| ); | ||
| } | ||
| } else { | ||
| // Send to our own tpu vote socket if we cannot find a leader to send to | ||
| let _ = cluster_info.send_transaction(vote_op.tx(), None); | ||
| let _ = send_vote_transaction(cluster_info, vote_op.tx(), None, &connection_cache); |
There was a problem hiding this comment.
should we introduce a metric/counter for the errors here?
There was a problem hiding this comment.
I will address it in another PR: #3894
Will do. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
TPU vote using QUIC.
Summary of Changes
Use Connection cache to encapsulate sending TPU vote via either QUIC or UDP.
Introduced a validator flag to control if to send TPU vote via QUIC -- the default is false -- meaning using UDP. The default should have no impacts on TPU vote sending
Tests:
Tests have been done to turn DEFAULT_VOTE_USE_QUIC to true -- meaning sending TPU votes via QUIC only.
This is done in test PR: #3571. Showing CI results are okay.
Further I disabled votes in gossips -- to ensure only sending votes via TPU vote using QUIC. The GCE cluster launched with this configuration is still functioning as expected. Metrics in connection_cache_vote_quic and quic_streamer_tpu_vote shows votes streams sent and received on the client and server side respectively.
Fixes #