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

Implement forwarding via TpuConnection #23817

Merged
merged 1 commit into from
Mar 25, 2022

Conversation

ryleung-solana
Copy link
Contributor

Problem

We want to forward transactions using Quic.

Summary of Changes

Changes packet forwarding to use the TpuConnection and the connection cache, so it can be switched between UDP and Quic.

Fixes #

client/src/udp_client.rs Outdated Show resolved Hide resolved
client/src/quic_client.rs Outdated Show resolved Hide resolved
streamer/src/sendmmsg.rs Outdated Show resolved Hide resolved
core/src/banking_stage.rs Show resolved Hide resolved
client/src/connection_cache.rs Outdated Show resolved Hide resolved
client/src/quic_client.rs Outdated Show resolved Hide resolved
client/src/tpu_connection.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@t-nelson t-nelson left a comment

Choose a reason for hiding this comment

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

i think the nominal changes are looking pretty good. just a couple nits there. this PR got a little out of hand with the extraneous stuff again. PR numbers are cheap, use them! 😉

@@ -13,9 +14,15 @@ use {
// Should be non-zero
static MAX_CONNECTIONS: usize = 64;

#[derive(Clone)]
enum Connection {
Copy link
Contributor

Choose a reason for hiding this comment

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

try to avoid doing these refactors in unrelated change sets. it makes review unnecessarily time consuming

client/src/quic_client.rs Outdated Show resolved Hide resolved

fn par_serialize_and_send_transaction_batch(
&self,
transaction_batch: &[VersionedTransaction],
transactions: &[VersionedTransaction],
Copy link
Contributor

Choose a reason for hiding this comment

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

renames, also separate

client/src/tpu_connection.rs Outdated Show resolved Hide resolved
@@ -90,7 +90,7 @@ impl VotingService {

let mut measure = Measure::start("vote_tx_send-ms");
let target_address = target_address.unwrap_or_else(|| cluster_info.my_contact_info().tpu);
let _ = get_connection(&target_address).serialize_and_send_transaction(vote_op.tx());
let _ = serialize_and_send_transaction(vote_op.tx(), &target_address);
Copy link
Contributor

Choose a reason for hiding this comment

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

there's a merge conflict here against changes that Pankaj and Justin put in yesterday

core/src/banking_stage.rs Show resolved Hide resolved
1000
);

if let Err(err) = res {
Copy link
Contributor

Choose a reason for hiding this comment

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

could res.is_err() here and avoid de/restructuring


measure.stop();
inc_new_counter_info!(
"banking_stage-forward-us",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"banking_stage-forward-us",
"banking_stage-forward-send-us",

@ryleung-solana ryleung-solana merged commit 6b85c21 into solana-labs:master Mar 25, 2022
@sakridge sakridge added the v1.10 label Mar 25, 2022
mergify bot pushed a commit that referenced this pull request Mar 25, 2022
sakridge pushed a commit that referenced this pull request Mar 28, 2022
(cherry picked from commit 6b85c21)

Co-authored-by: ryleung-solana <91908731+ryleung-solana@users.noreply.github.com>
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.

4 participants