Skip to content

Bump collation in_peers to 300 for the experimental collator protocol#11762

Queued
tdimitrov wants to merge 11 commits intomasterfrom
tsv-collator-revamp-connection-limit
Queued

Bump collation in_peers to 300 for the experimental collator protocol#11762
tdimitrov wants to merge 11 commits intomasterfrom
tsv-collator-revamp-connection-limit

Conversation

@tdimitrov
Copy link
Copy Markdown
Contributor

Sync the values of CONNECTED_PEERS_PARA_LIMIT in experimental collator protocol and MAX_AUTHORITY_INCOMING_STREAMS in peer_set.

@tdimitrov
Copy link
Copy Markdown
Contributor Author

/cmd prdoc --audience node_dev --bump patch

@tdimitrov tdimitrov added T0-node This PR/Issue is related to the topic “node”. T8-polkadot This PR/Issue is related to/affects the Polkadot network. A4-backport-stable2603 Pull request must be backported to the stable2603 release branch labels Apr 14, 2026
github-actions bot and others added 2 commits April 14, 2026 18:24
Comment thread polkadot/node/network/collator-protocol/src/validator_side_experimental/common.rs Outdated

/// Maximum allowed incoming connection streams for validator nodes on the collation protocol
/// when the experimental collator protocol is enabled.
pub const MAX_AUTHORITY_INCOMING_STREAMS_EXPERIMENTAL_COLLATOR_PROTO: u32 = 310;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

jost noting: without some testing it's difficult to know how much bigger this hard limit needs to be than the 300 soft limit

peerset_protocol_names: &PeerSetProtocolNames,
metrics: NotificationMetrics,
peer_store_handle: Arc<dyn PeerStoreProvider>,
experimental_collator_protocol: bool,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I just realized this is a breaking change but the alternative is to bump MAX_AUTHORITY_INCOMING_STREAMS for both implementations which I prefer not to do.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you could also add a new function and keep the old one untouched

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

but do we really need this backported?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

you could also add a new function and keep the old one untouched

We could, but is it really worth it?

but do we really need this backported?

Yes, we'll be deploying from stable2603, so all revamp changes should be there.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, we'll be deploying from stable2603, so all revamp changes should be there.

I think we're going to do a one-off stable2604 release soon

We could, but is it really worth it?

what's the alternative? bump it to 310 for all? I think that could be fine also

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Didn't know about the new release. In that case we can skip backporting.

what's the alternative?
well 😊 hack the CI checks and merge breaking change. Won't be the first time we do it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what's the alternative? bump it to 310 for all? I think that could be fine also

Yeah. I'd go for this to keep things simple. I don't see any good reason why not.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

peerset_protocol_names: &PeerSetProtocolNames,
metrics: NotificationMetrics,
peer_store_handle: Arc<dyn PeerStoreProvider>,
experimental_collator_protocol: bool,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what's the alternative? bump it to 310 for all? I think that could be fine also

Yeah. I'd go for this to keep things simple. I don't see any good reason why not.

@github-actions
Copy link
Copy Markdown
Contributor

Review required! Latest push from author must always be reviewed

@tdimitrov tdimitrov requested review from alindima and sandreim April 15, 2026 11:28
@tdimitrov tdimitrov removed the T8-polkadot This PR/Issue is related to/affects the Polkadot network. label Apr 15, 2026
@tdimitrov tdimitrov added this pull request to the merge queue Apr 17, 2026
Any commits made after this event will not be merged.
github-merge-queue bot pushed a commit that referenced this pull request Apr 17, 2026
…ol (#11762)

Sync the values of `CONNECTED_PEERS_PARA_LIMIT` in experimental collator
protocol and `MAX_AUTHORITY_INCOMING_STREAMS` in peer_set.

---------

Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@tdimitrov tdimitrov removed this pull request from the merge queue due to a manual request Apr 17, 2026
@tdimitrov tdimitrov added this pull request to the merge queue Apr 17, 2026
Any commits made after this event will not be merged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A4-backport-stable2603 Pull request must be backported to the stable2603 release branch T0-node This PR/Issue is related to the topic “node”.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants