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

AutoDial retries creeps up CPU usage over time #2418

Closed
mkykadir opened this issue Feb 19, 2024 · 3 comments · Fixed by #2639
Closed

AutoDial retries creeps up CPU usage over time #2418

mkykadir opened this issue Feb 19, 2024 · 3 comments · Fixed by #2639
Labels
need/triage Needs initial labeling and prioritization

Comments

@mkykadir
Copy link

  • Version: 1.2.3

  • Platform: Linux 7e698928f50f 5.15.49-linuxkit-pr #1 SMP PREEMPT Thu May 25 07:27:39 UTC 2023 aarch64 Linux

  • Subsystem: Connection manager

Severity: Medium

Description: I was relying on ConnectionManager's minConnections configuration's default value to be 0 but after an update this default value increased to 50. My P2P network does not have that much nodes, since minConnections become 50; autoDial keeps trying to dialing new nodes and this creates increase in CPU usage over time; which eventually overloads node.js event loop.

Steps to reproduce the error: Set minConnections config a high number; let node to autoDial over time and monitor Node's CPU usage over time. This can take time.

@mkykadir mkykadir added the need/triage Needs initial labeling and prioritization label Feb 19, 2024
@abuvanth
Copy link

set in config

connectionManager: {
      minConnections: 5
    },

@mkykadir
Copy link
Author

yes I already did that as a temporary fix but I don't think this auto dialing feature should creep up memory and cpu usage over time; its really hard to mitigate and identify the exact root cause

@abuvanth
Copy link

yes I already did that as a temporary fix but I don't think this auto dialing feature should creep up memory and cpu usage over time; its really hard to mitigate and identify the exact root cause

yes

achingbrain added a commit that referenced this issue Aug 15, 2024
The autodialer is a feature from an older time when the DHT was less reliable and we didn't have things like the random walk component.

There's not a lot of benefit in opening connections to any old peer, instead protocols now have better ways of targetting the kind of peers they require.

Actively dialing peers harms platforms where connections are extremely expensive such as react-native so this feature has been removed.

Closes #2621
Fixes #2418

BREAKING CHANGE: the autodialer has been removed as well as the corresponding config keys

---------

Co-authored-by: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com>
achingbrain added a commit that referenced this issue Sep 6, 2024
The autodialer is a feature from an older time when the DHT was less reliable and we didn't have things like the random walk component.

There's not a lot of benefit in opening connections to any old peer, instead protocols now have better ways of targetting the kind of peers they require.

Actively dialing peers harms platforms where connections are extremely expensive such as react-native so this feature has been removed.

Closes #2621
Fixes #2418

BREAKING CHANGE: the autodialer has been removed as well as the corresponding config keys

---------

Co-authored-by: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com>
achingbrain added a commit that referenced this issue Sep 6, 2024
The autodialer is a feature from an older time when the DHT was less reliable and we didn't have things like the random walk component.

There's not a lot of benefit in opening connections to any old peer, instead protocols now have better ways of targetting the kind of peers they require.

Actively dialing peers harms platforms where connections are extremely expensive such as react-native so this feature has been removed.

Closes #2621
Fixes #2418

BREAKING CHANGE: the autodialer has been removed as well as the corresponding config keys

---------

Co-authored-by: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/triage Needs initial labeling and prioritization
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants