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(pd): raises default peering limits #3063

Merged
merged 1 commit into from
Sep 20, 2023
Merged

Conversation

conorsch
Copy link
Contributor

Makes a few changes to pd, designed to improve peering behavior on testnets:

  • raises the default values for inbound/outbound peers
  • limits the number of peers discovered during join to 5, randomized

We originally decided to add node discovery logic to aid in peering. There's a downside, however, to reusing all peers of the bootstrap node: the CI deployment nodes get swamped, and the network remains centralized on the nodes that Penumbra Labs run. Instead, let's just take a handful, at random, and trust the p2p gossip to discover more over time.

Refs #3056.

Makes a few changes to `pd`, designed to improve peering behavior on
testnets:

  * raises the default values for inbound/outbound peers
  * limits the number of peers discovered during join to 5, randomized

We originally decided to add node discovery logic to aid in peering.
There's a downside, however, to reusing all peers of the bootstrap node:
the CI deployment nodes get swamped, and the network remains centralized
on the nodes that Penumbra Labs run. Instead, let's just take a handful,
at random, and trust the p2p gossip to discover more over time.

Refs #3056.
@conorsch conorsch temporarily deployed to smoke-test September 19, 2023 22:47 — with GitHub Actions Inactive
@conorsch conorsch requested a review from zbuc September 19, 2023 22:48
Copy link
Member

@zbuc zbuc left a comment

Choose a reason for hiding this comment

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

LGTM!

@conorsch conorsch merged commit 081fe52 into main Sep 20, 2023
8 checks passed
@conorsch conorsch deleted the 3056-max-peers-selection branch September 20, 2023 19:49
conorsch added a commit that referenced this pull request Oct 5, 2023
When joining a network via `pd testnet join`, pd already inspects
the peers known to the target bootstrap node, and includes those peers
in the generated CometBFT config. Technically it's not correct to assume
that these peers are seeds, but limiting to a threshold (#3063) ensures
that only a few nodes will be miscategorized this way.

The new behavior introduced here is that `pd testnet join` will inspect
monikers of peers learned from the bootstrap node, and if the string
"seed" appears in the moniker, guarantee that peer lands in the `seeds`
field of the CometBFT config.

To complement this change, we update the deployment logic to create a
seed node, and ensure that seed node is excluded from the loadbalanced
RPC, to avoid hitting it via the join request (in which case it won't
report many peers, because seed nodes drop peer connections after
serving address info).

Refs #3056.
conorsch added a commit that referenced this pull request Oct 5, 2023
When joining a network via `pd testnet join`, pd already inspects
the peers known to the target bootstrap node, and includes those peers
in the generated CometBFT config. Technically it's not correct to assume
that these peers are seeds, but limiting to a threshold (#3063) ensures
that only a few nodes will be miscategorized this way.

The new behavior introduced here is that `pd testnet join` will inspect
monikers of peers learned from the bootstrap node, and if the string
"seed" appears in the moniker, guarantee that peer lands in the `seeds`
field of the CometBFT config.

To complement this change, we update the deployment logic to create a
seed node, and ensure that seed node is excluded from the loadbalanced
RPC, to avoid hitting it via the join request (in which case it won't
report many peers, because seed nodes drop peer connections after
serving address info).

Refs #3056.
conorsch added a commit that referenced this pull request Oct 10, 2023
When joining a network via `pd testnet join`, pd already inspects
the peers known to the target bootstrap node, and includes those peers
in the generated CometBFT config. Technically it's not correct to assume
that these peers are seeds, but limiting to a threshold (#3063) ensures
that only a few nodes will be miscategorized this way.

The new behavior introduced here is that `pd testnet join` will inspect
monikers of peers learned from the bootstrap node, and if the string
"seed" appears in the moniker, guarantee that peer lands in the `seeds`
field of the CometBFT config.

To complement this change, we update the deployment logic to create a
seed node, and ensure that seed node is excluded from the loadbalanced
RPC, to avoid hitting it via the join request (in which case it won't
report many peers, because seed nodes drop peer connections after
serving address info).

Refs #3056.
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.

2 participants