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

[Merged by Bors] - p2p: add connection gater to prevent dials over max limit #4775

Closed
wants to merge 4 commits into from

Conversation

dshulyak
Copy link
Contributor

@dshulyak dshulyak commented Aug 4, 2023

closes: #4771

this change will reject dials from outbound peers when inbound number is 0.8 of high peers number.
it will not allow dials to other peers when total number of peers is higher then 1.1 of high peers number.
and node is allowed to always dial direct peers

note that dials are concurrent, so if there is a burst it is still possible that they will go over the limit for short period of time

@dshulyak dshulyak marked this pull request as ready for review August 5, 2023 05:11
@dshulyak dshulyak requested a review from pigmej August 5, 2023 05:11
@codecov
Copy link

codecov bot commented Aug 5, 2023

Codecov Report

Merging #4775 (6717dae) into develop (6a4208b) will decrease coverage by 0.1%.
The diff coverage is 62.8%.

@@            Coverage Diff            @@
##           develop   #4775     +/-   ##
=========================================
- Coverage     76.8%   76.8%   -0.1%     
=========================================
  Files          259     260      +1     
  Lines        29491   29524     +33     
=========================================
+ Hits         22659   22678     +19     
- Misses        5392    5402     +10     
- Partials      1440    1444      +4     
Files Changed Coverage Δ
p2p/host.go 37.0% <50.0%> (+2.7%) ⬆️
p2p/gater.go 80.0% <80.0%> (ø)

... and 2 files with indirect coverage changes

@pigmej
Copy link
Member

pigmej commented Aug 5, 2023

  1. Connections just sporadically go out of the high. 🟢
  2. Direct peers are working even when high is exceeded 🟢

side note:
WITHOUT that patch my limits were ok when I did not have port forwarded (fewer peers connecting??), with port forwarded then limits were often exceeded but then it was going lower.

@dshulyak
Copy link
Contributor Author

dshulyak commented Aug 6, 2023

Connections just sporadically go out of the high

can you clarify this part? i expect them to go above high if node is making many concurrent dials, so for example you are at 80 peers, node make 50 dials at the same time, connections will to 130, but that should be rare. is it consistent with your observations or not at all?

@pigmej
Copy link
Member

pigmej commented Aug 6, 2023

Yeah, more or less what you described. Sporadically going out of bounds and then quickly down.

@dshulyak
Copy link
Contributor Author

dshulyak commented Aug 7, 2023

bors try

bors bot added a commit that referenced this pull request Aug 7, 2023
@bors
Copy link

bors bot commented Aug 7, 2023

try

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@dshulyak
Copy link
Contributor Author

dshulyak commented Aug 7, 2023

bors merge

bors bot pushed a commit that referenced this pull request Aug 7, 2023
closes: #4771

this change will reject dials from outbound peers when inbound number is 0.8 of high peers number.
it will not allow dials to other peers when total number of peers is higher then 1.1 of high peers number.
and node is allowed to always dial direct peers

note that dials are concurrent, so if there is a burst it is still possible that they will go over the limit for short period of time
@bors
Copy link

bors bot commented Aug 7, 2023

Pull request successfully merged into develop.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title p2p: add connection gater to prevent dials over max limit [Merged by Bors] - p2p: add connection gater to prevent dials over max limit Aug 7, 2023
@bors bors bot closed this Aug 7, 2023
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.

Node ignores high-peers value defined in the config map
3 participants