Skip to content

Conversation

fjahr
Copy link
Contributor

@fjahr fjahr commented May 23, 2023

I was playing with file descriptor limits to test another issue saw the following warning printed when I had configured a file descriptor limit of 150:

Warning: Reducing -maxconnections from 125 to -9, because of system limitations.

The warning could have been resolved with a one-line change but I found the surrounding code hard to reason about at first, so I added a few comments and reorganized the code a bit for better clarity.

Aside from the warning there is one further behavior change: If the limitation of file descriptors prevents the node from making any connections the node doesn't start unless the user has explicitly set -maxconnections to 0. This is not just about the node not being able to make connections: When I started the node right at the limit I wasn't able to shut it down cleanly any more several times and instead had to kill the process manually. So this shouldn't be anything normal users should be able to run into.

@DrahtBot
Copy link
Contributor

DrahtBot commented May 23, 2023

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #27539 (init: Fixes for file descriptor accounting by Empact)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@fanquake
Copy link
Member

Duplicate of #27539?

@fjahr
Copy link
Contributor Author

fjahr commented May 24, 2023

Duplicate of #27539?

Ugh, mostly yeah.

@fjahr fjahr closed this May 24, 2023
@bitcoin bitcoin locked and limited conversation to collaborators May 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants