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(p2p): Seed connectivity tuning options: max-incoming-connection-time, incoming-connection-window #532

Merged
merged 16 commits into from
Dec 20, 2022

Conversation

lklimek
Copy link
Collaborator

@lklimek lklimek commented Dec 16, 2022

Issue being fixed or feature implemented

Clients connecting to seed nodes never disconnect, causing seed to reject new clients due to lack of "free slots".

What was done?

  1. Added configuration option max-incoming-connection-time that will drop incoming connections from non-persistent peers after configured time.
  2. Exposed incoming-connection-window setting in config, to allow blocking immediate reconnect attempts

These settings, together with max-outgoing-connections, max-connections and persistent-peers, makes it possible to create seed node configuration that will disconnect clients from seed once it retrieves initial peer list.

Configuration of seed nodes

Suggested configuration of a seed node is as follows:

  1. Set some trusted peers as persistent-peers (for example, 5 persistent peers) (optional)
  2. Set max-outgoing-connections to a number slightly bigger than num of persistent peers (for example, 10)
  3. Set max-connections to some value much larger than max-outgoing-connections (example: 50)
  4. Set max-incoming-connection-time to a few seconds (example: "20s")
  5. Set incoming-connection-window to a few minutes (example: "1m")

Note: numbers above require tuning on the actual network, they are just some kind of guess

How Has This Been Tested?

E2E

Using e2e framework, created a network of 20 nodes, and configured seed with the following settings:

  • persistent_peers = ["validator01"]
  • p2p_max_connections = 4
  • p2p_max_outgoing_connections = 2
  • p2p_max_incoming_connection_time = "5s"
  • p2p_incoming_connection_window = "10s"

The network started successfully.

DevNet

Devnet test scenario

Configuration of seed node

In section [p2p], set the following settings:

max-outgoing-connections = 2
max-connections = 4
max-incoming-connection-time = "10s"
incoming-connection-window = "30s"

Expected behavior

Once the network is provisioned, nodes should very slowly join the network, with performance of ~4 nodes per minute.
This should be monitored; it is expected that all nodes will join the network in approx. 15-20 minutes.

Once all nodes join the network, the seed should maintain 2 outgoing connections, and 0-2 incoming connections (this can be monitored with netstat inside the container). Incoming connections should be disconnected after 30 seconds.

Breaking Changes

None

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

Copy link
Collaborator

@shotonoff shotonoff left a comment

Choose a reason for hiding this comment

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

LGMT. please have a look at the comments

internal/p2p/peermanager.go Outdated Show resolved Hide resolved
internal/p2p/peermanager.go Outdated Show resolved Hide resolved
test/e2e/networks/dashcore.toml Outdated Show resolved Hide resolved
shotonoff
shotonoff previously approved these changes Dec 20, 2022
Copy link
Collaborator

@shotonoff shotonoff left a comment

Choose a reason for hiding this comment

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

PR looks good to me

internal/p2p/peermanager.go Outdated Show resolved Hide resolved
test/e2e/pkg/exec/exec.go Show resolved Hide resolved
Copy link
Collaborator

@shotonoff shotonoff left a comment

Choose a reason for hiding this comment

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

🚀

@lklimek lklimek changed the title feat(p2p): Add max-incoming-connection-time to close connections on seed feat(p2p): Seed connectivity tuning options: max-incoming-connection-time, incoming-connection-window Dec 20, 2022
@lklimek lklimek merged commit 4c7237a into v0.10-dev Dec 20, 2022
@lklimek lklimek deleted the fix-seed-dos branch December 20, 2022 15:54
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