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

Fix busy loop when FDs are exhausted #91

Merged
merged 2 commits into from
Jul 25, 2024
Merged

Conversation

nojima
Copy link
Collaborator

@nojima nojima commented Jul 24, 2024

We found that once yrmcds exhausts FDs, tcp_server_socket::on_readable() causes a busy loop with logging accept: Too many open files. This PR is to avoid that busy loop.

@nojima
Copy link
Collaborator Author

nojima commented Jul 24, 2024

I found another issue when FDs are exhausted.
yrmcds crashes when it calls cybozu::has_ip_address when no FDs available.

2024-07-24 09:36:27 ERROR [std::system_error] (system:24) getifaddrs: Too many open files

@ymmt2005
Copy link
Member

@nojima
Thank you! Can we add a test?

@nojima
Copy link
Collaborator Author

nojima commented Jul 25, 2024

I added the test for FD flood.

@ymmt2005
Copy link
Member

@nojima
Thank you, LGTM.

BTW, what about this?

yrmcds crashes when it calls cybozu::has_ip_address when no FDs available.

@nojima
Copy link
Collaborator Author

nojima commented Jul 25, 2024

@ymmt2005

BTW, what about this?

yrmcds crashes when it calls cybozu::has_ip_address when no FDs available.

I am not sure what to do.
Perhaps we should first define how yrmcds should behave when FDs are exhausted.

In any case, I think it is outside the scope of this PR.

@ymmt2005
Copy link
Member

OK. Please create an issue for the problem so that we can revisit it later.

@ymmt2005 ymmt2005 merged commit a4e7453 into master Jul 25, 2024
4 checks passed
@ymmt2005 ymmt2005 deleted the too-many-open-files branch July 25, 2024 06:08
@nojima
Copy link
Collaborator Author

nojima commented Jul 25, 2024

I added issue #92.

@ymmt2005
Copy link
Member

Thanks.

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