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

Improve calculation of max uni_stream for a given QUIC connection #26340

Closed
pgarg66 opened this issue Jun 30, 2022 · 4 comments · Fixed by #28705
Closed

Improve calculation of max uni_stream for a given QUIC connection #26340

pgarg66 opened this issue Jun 30, 2022 · 4 comments · Fixed by #28705

Comments

@pgarg66
Copy link
Contributor

pgarg66 commented Jun 30, 2022

Problem

The calculation of maximum number of uni_streams that a QUIC connection can open is error prone. For an unstaked peer, it's hardcoded to 128. However, a staked peer might be limited to an even lower number to that.

Reference: #25993 (comment)
Reference: #26299 (comment)

Proposed Solution

Calculation of max uni_streams for staked connections is summing up all the stake across the cluster. However, not every node may try to establish a connection with the node. We could potentially tweak the calculation to account for only the nodes that have had connections. Another tweak could be to use number of currently active streams in this calculation. Lastly, the streams are expected to be short lived (the streams are used to send one transaction each). So, we can try to prune the streams by timing out the streams that have been opened for some time.

@pgarg66
Copy link
Contributor Author

pgarg66 commented Jul 13, 2022

I tried simulating the impact of this issue on a dev cluster.

The cluster has 5 nodes, with stake being equally distributed. So, by default, each node can establish up to about 20K uni streams with a peer node.

The following graph is for the default case (20K streams per peer node)
image

To simulate a condition where we have 50 nodes with similar stake, I changed the uni stream count to 2K per peer. The following graph corresponds to the change.
image

If I change the hardcoded limit to 20K (instead of computing the limit by the total stake/node's stake), it behaves just like the default scenario above.
image

The current max uni_stream calculation is impacting the forwarding of transactions. We need to make it more dynamic.

@pgarg66
Copy link
Contributor Author

pgarg66 commented Jul 29, 2022

This PR also helps this issue: #26802

@ryleung-solana
Copy link
Contributor

@pgarg66 just wanted to follow up on this. Is there anything left to do on this? The server seems to already compute the max allowed uni streams and set this on the quinn connection, and of course the client already limits the chunk size as well. Is there any other work that needs to be done on this, or can we close this out?

@pgarg66
Copy link
Contributor Author

pgarg66 commented Oct 28, 2022

@pgarg66 just wanted to follow up on this. Is there anything left to do on this? The server seems to already compute the max allowed uni streams and set this on the quinn connection, and of course the client already limits the chunk size as well. Is there any other work that needs to be done on this, or can we close this out?

The peers with low stake might still be limited to lower stream count compared to unstaked peer. I think the equation to compute max allowed streams needs to be reevaluated. The comments referenced in the issue description have more details on it.

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 a pull request may close this issue.

3 participants
@pgarg66 @ryleung-solana and others