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

feat!: limit protocol streams per-connection #1255

Merged
merged 3 commits into from
Jun 17, 2022

Conversation

achingbrain
Copy link
Member

@achingbrain achingbrain commented Jun 17, 2022

Uses the maxInboundStreams and maxOutboundStreams of the registrar.handle opts to limit the number of concurrent streams open on each connection on a per-protocol basis.

Both values default to 1 so some tuning will be necessary to set appropriate values for some protocols before release.

Also refactors the fetch protocol implementation as it turns out it wasn't closing streams after receiving responses 😬

BREAKING CHANGE: libp2p.dialProtocol returns a stream with a stat.protocol property instead of an object with stream and protocol properties. Protocol handlers also need to specify how many inbound/outbound streams are allowed

Uses the `maxInboundStreams` and `maxOutboundStreams` of the `registrar.handle`
opts to limit the number of concurrent streams open on each connection
on a per-protocol basis.

Both values default to 1 so some tuning will be necessary to set
appropriate values for some protocols.
@achingbrain achingbrain changed the title feat: limit protocol streams per-connection feat!: limit protocol streams per-connection Jun 17, 2022
@@ -79,13 +79,21 @@ const DefaultConfig: Partial<Libp2pInit> = {
host: {
agentVersion: AGENT_VERSION
},
timeout: 30000
timeout: 30000,
maxInboundStreams: 1,
Copy link
Member

Choose a reason for hiding this comment

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

should we also mention this in breaking the change changelog?
We should bring to attention that users will need to override defaults :smiles

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good. The text after "BREAKING CHANGE:" in the squashed commit message (e.g. copy/pasted from the OP in this PR) will be added to the changelog on release by semantic-release.

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.

3 participants