streamer: set fixed RX window for all connections by alexpyattaev · Pull Request #9143 · anza-xyz/agave

alexpyattaev · 2025-11-19T10:20:11Z

Problem

Streamer RX window management logic is unnecessarily complex
We control MAX_DATA and MAX_STREAMS independently even though controlling one of them is sufficient
Since most our logic related to streamer operates in streams, setting RX window in bytes is adding unnecessary complications to convert from streams to bytes

Summary of Changes

Allow for 8MB MAX_DATA for all connections (enough to reach 200Mbps over 320ms RTT, reasonable connections will never be limited by this)
Ensure the SWQOS efficiently throttles connections based on MAX_STREAMS setting

alexpyattaev · 2025-11-19T10:31:13Z

 }

 impl StakedNodes {
-    /// Calculate the stake stats: return the new (total_stake, min_stake and max_stake) tuple


This can be simplified now that min_stake and max_stake are not used anymore.

alexpyattaev · 2025-11-19T10:32:11Z


    let config = Arc::get_mut(&mut server_config.transport).unwrap();

-    // QUIC_MAX_CONCURRENT_STREAMS doubled, which was found to improve reliability


This was doing nothing since we override these settings after handshake anyway.

alexpyattaev · 2025-11-19T10:32:52Z

        self.total_stake
    }

-    #[inline]


min_stake and max_stake were only used in receive window calculations

alexpyattaev · 2025-11-19T10:33:15Z

    use super::*;

    #[test]
-    fn test_cacluate_receive_window_ratio_for_staked_node() {


testing removed code

alexpyattaev · 2025-11-19T10:34:38Z

    );
    stats.total_connections.fetch_add(1, Ordering::Relaxed);

+    connection.set_receive_window(CONNECTION_RECEIVE_WINDOW_BYTES);


this is the key change - we set RX window to const after connection is fully confirmed.

minor: since we now disable streams in the default transport config in configure_server(), it should be safe to set the window value to CONNECTION_RECEIVE_WINDOW_BYTES there too. One less place that tweaks connection settings. wdyt?

Moved to server config.

alexpyattaev

Had to apply more changes to SimpleQos as it did not have own logic to set max_streams. Removing throttling from it completely is not possible since our throttling limits are very low (20 streams / s is normal), so just MAX_STREAMS is not granular enough to achieve enforcement we want for voting.

alexpyattaev · 2025-11-19T12:02:49Z

For SimpleQos we either set max_streams to some constant, or compute based on RTT. Computing based on RTT seems less brittle.

codecov-commenter · 2025-11-19T12:30:45Z

Codecov Report

❌ Patch coverage is 95.23810% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 82.6%. Comparing base (e8cbabe) to head (7abf156).

Additional details and impacted files

@@            Coverage Diff            @@
##           master    #9143     +/-   ##
=========================================
- Coverage    82.6%    82.6%   -0.1%     
=========================================
  Files         890      890             
  Lines      320845   320773     -72     
=========================================
- Hits       265285   265147    -138     
- Misses      55560    55626     +66

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

stablebits · 2025-11-19T13:05:38Z

+/// connection to require more bandwidth. This prevents MAX_DATA from affecting
+/// the bitrate achieved by a single connection. Actual throttling is achieved based
+/// on the number of concurrent streams.
+const CONNECTION_RECEIVE_WINDOW_BYTES: VarInt = VarInt::from_u32(8 * 1024 * 1024);


I'd remove the "unreasonable to expect a single connection..." part and expose this setting via config. No need to enforce strict limits.

Then we will have to explain to users how to choose this value. Our users build from source anyway, can customize it if they really want to.

KirillLykov · 2025-11-21T12:35:02Z

I agree that we should used stake parametrization for only one of these MAX_DATA or MAX_STREAMS. Reasoning in terms of streams (transactions) might be simpler for users. Although we can have many small transactions and limiting streams might be a bit more restrictive but since the idea that we don't restrict users when we have some bandwidth for them, doesn't sound like a blocking thing.
I'm sort of neutral to which one to pick. The only thing is that in the discussions I found argument that in quinn implementation moving stream is more expensive than RW. No idea how much more expensive.

Have we checked in some stress tests of isolated streamer + mock-client that this change doesn't lead to any surprises if any?

stablebits · 2025-11-21T17:15:01Z

-                if let Ok(receive_window) = receive_window {
-                    connection.set_receive_window(receive_window);
-                }
                connection.set_max_concurrent_uni_streams(max_uni_streams);


Why not apply RTT-based scaling like we introduce in simple_qos? In this case, we should also ensure that senders don’t receive less than before -- at least until we have auto-tuning, and maybe even afterward unless these defaults turn out to be inadequate.

In a separate PR. One small change at a time.

stablebits · 2025-11-21T17:21:43Z

+        // for very low values of max_streams_per_second, prevent connections from having zero
+        // streams in flight
+        let max_streams_in_flight = max_streams_in_flight.max(STREAMS_IN_FLIGHT_MARGIN);
+        connection.set_max_concurrent_uni_streams(VarInt::from_u32(max_streams_in_flight));


It looks like the receive window remained to be set to PACKET_DATA_SIZE (default option in configure_server(), thus limiting votes TPS to 1 per RTT.

No, the receive window is set for both QoS modules in the common code, see async fn handle_connection<Q, C>(...)

Yes, I wanted to say the 1transaction/RTT problem exists in the current (before this PR) code.

Yes, it does exist today indeed. And we're fixing it =)

alexpyattaev · 2025-11-21T18:30:42Z

The only thing is that in the discussions I found argument that in quinn implementation moving stream is more expensive than RW. No idea how much more expensive.

Let us fix the QoS logic first to be correct, then we focus on making it performant.

Have we checked in some stress tests of isolated streamer + mock-client that this change doesn't lead to any surprises if any?

Yes, this was tested against mock_clients. no TPS change compared to the case before the change.

mergify · 2025-11-29T12:28:51Z

Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis.

(cherry picked from commit a155621) # Conflicts: # streamer/src/nonblocking/simple_qos.rs # streamer/src/nonblocking/swqos.rs # streamer/src/streamer.rs

…9143) (#9330) * streamer: set fixed RX window for all connections (#9143) (cherry picked from commit a155621) # Conflicts: # streamer/src/nonblocking/simple_qos.rs # streamer/src/nonblocking/swqos.rs # streamer/src/streamer.rs * resolve conflicts --------- Co-authored-by: Alex Pyattaev <alex.pyattaev@anza.xyz>

alexpyattaev force-pushed the streamer_simplify_window_sizing branch from d5b96d6 to 306d3a1 Compare November 19, 2025 10:28

alexpyattaev marked this pull request as ready for review November 19, 2025 10:29

alexpyattaev requested a review from stablebits November 19, 2025 10:29

alexpyattaev commented Nov 19, 2025

View reviewed changes

alexpyattaev requested a review from lijunwangs November 19, 2025 10:36

alexpyattaev commented Nov 19, 2025

View reviewed changes

stablebits reviewed Nov 19, 2025

View reviewed changes

Comment thread streamer/src/nonblocking/simple_qos.rs

stablebits reviewed Nov 19, 2025

View reviewed changes

Comment thread streamer/src/nonblocking/simple_qos.rs

alexpyattaev requested a review from KirillLykov November 19, 2025 21:59

stablebits reviewed Nov 21, 2025

View reviewed changes

alexpyattaev requested a review from stablebits November 21, 2025 18:10

streamer: set fixed RX window for all connections

7abf156

alexpyattaev force-pushed the streamer_simplify_window_sizing branch from fa4887e to 7abf156 Compare November 21, 2025 18:27

stablebits approved these changes Nov 23, 2025

View reviewed changes

alexpyattaev added this pull request to the merge queue Nov 23, 2025

Merged via the queue into anza-xyz:master with commit a155621 Nov 23, 2025
47 checks passed

alexpyattaev deleted the streamer_simplify_window_sizing branch November 23, 2025 08:53

AvhiMaz pushed a commit to AvhiMaz/agave that referenced this pull request Nov 28, 2025

streamer: set fixed RX window for all connections (anza-xyz#9143)

27d6bbd

alexpyattaev added the v3.1 Backport to v3.1 branch label Nov 29, 2025

mergify Bot mentioned this pull request Nov 29, 2025

v3.1: streamer: set fixed RX window for all connections (backport of #9143) #9330

Merged

alexpyattaev mentioned this pull request Jan 9, 2026

solana-quic-definitions: remove useless consts anza-xyz/solana-sdk#294

Closed


		let config = Arc::get_mut(&mut server_config.transport).unwrap();

		// QUIC_MAX_CONCURRENT_STREAMS doubled, which was found to improve reliability

Conversation

alexpyattaev commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Summary of Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexpyattaev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov-commenter commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

KirillLykov commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexpyattaev Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stablebits Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexpyattaev commented Nov 21, 2025

Uh oh!

Uh oh!

mergify Bot commented Nov 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

alexpyattaev commented Nov 19, 2025 •

edited

Loading

codecov-commenter commented Nov 19, 2025 •

edited

Loading

KirillLykov commented Nov 21, 2025 •

edited

Loading

alexpyattaev Nov 21, 2025 •

edited

Loading

stablebits Nov 21, 2025 •

edited

Loading