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

Bitswap should use one stream per message #765

Open
gammazero opened this issue Dec 18, 2024 · 0 comments
Open

Bitswap should use one stream per message #765

gammazero opened this issue Dec 18, 2024 · 0 comments
Labels
kind/discussion Topical discussion; usually not changes to codebase need/triage Needs initial labeling and prioritization

Comments

@gammazero
Copy link
Contributor

gammazero commented Dec 18, 2024

We should consider changing bitswap to use one stream per message.

@MarcoPolo:

Ideally we'd use one stream per message and close the stream once we're done. I believe the reason we don't do that is that in Yamux the initial send buffer is constant per stream (rather than configurable both within a stream and per connection connection as in QUIC). This means that if our initial message is larger than that initial send capacity, then we have to wait for a round trip to send the whole message. By reusing the stream, the peer can increase our send buffer size in a later message.

It may be worth changing this in Yamux.

The real issue is that the cleanup of peer disconnects is slow. A lot of time is spent on acquiring the lock by many different goroutines.

The benefit of the one stream per message approach is that you avoid the whole issue of maintaining a reference and knowing if a connection is even alive or not.

@gammazero:

Seems worth it to having one stream per message. Reusing the stream is like double-muxing to avoid a buffer sizing issue. Being able to have different messages on the same connection is the point of muxing, and streams is the mechanism for doing that.

The initial yamux stream buffer size is 256Kib. If most of our messages are smaller, then no change to yamux is needed. I suspect we do need a larger buffer, particularly for block transfer. Seems like a change to yamux is worth it, unless the occasional extra round trip is not a significant problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/discussion Topical discussion; usually not changes to codebase need/triage Needs initial labeling and prioritization
Projects
None yet
Development

No branches or pull requests

1 participant