Skip to content

Multi quic#634

Closed
t-nelson wants to merge 4 commits intoanza-xyz:masterfrom
t-nelson:multi-quic
Closed

Multi quic#634
t-nelson wants to merge 4 commits intoanza-xyz:masterfrom
t-nelson:multi-quic

Conversation

@t-nelson
Copy link
Copy Markdown

@t-nelson t-nelson commented Apr 6, 2024

Problem

#601 redux

Summary of Changes

Fixes #

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 7, 2024

Codecov Report

Attention: Patch coverage is 97.68786% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 81.9%. Comparing base (03ef611) to head (f3e1549).
Report is 12 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff            @@
##           master     #634    +/-   ##
========================================
  Coverage    81.8%    81.9%            
========================================
  Files         851      851            
  Lines      230238   230365   +127     
========================================
+ Hits       188520   188677   +157     
+ Misses      41718    41688    -30     

Comment thread local-cluster/src/local_cluster.rs
@t-nelson t-nelson added the CI Pull Request is ready to enter CI label Apr 9, 2024
@anza-team anza-team removed the CI Pull Request is ready to enter CI label Apr 9, 2024
Comment thread net-utils/src/lib.rs
},
std::os::unix::io::AsRawFd,
};
let SocketConfig {
Copy link
Copy Markdown

@lijunwangs lijunwangs Apr 25, 2024

Choose a reason for hiding this comment

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

We were creating multiple UDP Socket before with reuseaddr flag and seems to work. I forgot what makes it necessary for Quic's socket we need to explicitly do reuse port?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

SO_REUSEPORT is what signals the kernel to do 4-tuple hash load balancing across marked sockets. it's how UDP horizontal scaling works on linux. see socket(7) manpage

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What I meant is in the original code it was already does SO_REUSEPORT:

    if reuseaddr {
         // best effort, i.e. ignore errors here, we'll get the failure in caller
         setsockopt(sock_fd, ReusePort, &true).ok();
         setsockopt(sock_fd, ReuseAddr, &true).ok();
     }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The original code makes no sense at all. It was using reuseaddr for udp sockets, which aren't connected and don't need reuseaddr. And it was setting reuseport because of a WSL thing iirc? Just don't look at it let's pretend it never existed.

lijunwangs added a commit that referenced this pull request Jun 21, 2024
…er (#1452)

* net-utils: support SO_REUSEPORT

tpu: use multiple quic endpoints

cluster-info: manage port range by hand...

local-cluster: keep udp tpu socket around for tests

* Missing cargo file

* sort cargo.toml

* divide the concurrent_connections among the endpoints for multiquic

* Change default multiquic endpoint count to 1

* Missing Cargo.lock changes

* revert reuseaddr changes

* revert reuseaddr changes;fmt code

* reverted port range changes

* revert DEFAULT_TPU_ENABLE_UDP change in local_cluster

* Turn tpu_enable_udp to true to prevent concurrent local cluster tests to use the same QUIC ports

* changed QUIC_ENDPOINTS to 10 for testing

* Turn QUIC_ENDPOINTS to 1 for now

---------

Co-authored-by: Trent Nelson <trent@solana.com>
Co-authored-by: Lijun Wang <lijun.wang@oracle.com>
samkim-crypto pushed a commit to samkim-crypto/agave that referenced this pull request Jul 31, 2024
…est master (anza-xyz#1452)

* net-utils: support SO_REUSEPORT

tpu: use multiple quic endpoints

cluster-info: manage port range by hand...

local-cluster: keep udp tpu socket around for tests

* Missing cargo file

* sort cargo.toml

* divide the concurrent_connections among the endpoints for multiquic

* Change default multiquic endpoint count to 1

* Missing Cargo.lock changes

* revert reuseaddr changes

* revert reuseaddr changes;fmt code

* reverted port range changes

* revert DEFAULT_TPU_ENABLE_UDP change in local_cluster

* Turn tpu_enable_udp to true to prevent concurrent local cluster tests to use the same QUIC ports

* changed QUIC_ENDPOINTS to 10 for testing

* Turn QUIC_ENDPOINTS to 1 for now

---------

Co-authored-by: Trent Nelson <trent@solana.com>
Co-authored-by: Lijun Wang <lijun.wang@oracle.com>
@alessandrod
Copy link
Copy Markdown

this was merged

@t-nelson t-nelson deleted the multi-quic branch March 7, 2026 04:00
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.

5 participants