Allowing EMFILE in acceptNewConnection

[kazu-yamamoto]

As @domenkozar described in haskell/network#493, the following error can happen:

Network.Socket.accept: resource exhausted (Too many open files)

This is EMFILE.

@snoyberg I think eMFILE should be also allowed in Run.acceptNewConnection. What do you think?

[snoyberg]

This sounds like a bad idea to me, I disagree with @domenkozar here. Looking at the proposed patch, let me compare current vs proposed behavior:

Current: if the FD limit is set too low, the main warp thread will exit with an exception, which under normal circumstances will cause the entire process to exit, causing a discernible error. (If the process isn't exiting, I'd like to know why). This means that there will be evidence that something is wrong, and an operator can increase the FD limit, which is the right solution.

Proposed: instead of exiting, the Warp thread and process will run in a busy-loop hoping that an FD clears up so that it can accept a new connection. CPU usage will be high, the throughput of the application low (due to limited FDs), and there will be no evidence in the logs or alerts from automated systems to indicate something is wrong.

Basically, I don't see a problem that should be fixed in Warp. The program crashing because of insufficient FDs is a Good Thing IMO.

[domenkozar]

Current: if the FD limit is set too low, the main warp thread will exit with an exception, which under normal circumstances will cause the entire process to exit, causing a discernible error. (If the process isn't exiting, I'd like to know why). This means that there will be evidence that something is wrong, and an operator can increase the FD limit, which is the right solution.

That's not the current behavior: the process will print the error and not accept new connections according to my observation.

If it did crash that would be fine.

Proposed: instead of exiting, the Warp thread and process will run in a busy-loop hoping that an FD clears up so that it can accept a new connection. CPU usage will be high, the throughput of the application low (due to limited FDs), and there will be no evidence in the logs or alerts from automated systems to indicate something is wrong.

Why would it busy-loop, it should try to handle new connection, opening a socket fails and that should be handled but not retried?

[snoyberg]

That's not the current behavior: the process will print the error and not accept new connections according to my observation.

You're right. I think that's the bug, not the fact that it doesn't retry.

Why would it busy-loop, it should try to handle new connection, opening a socket fails and that should be handled but not retried?

I'm commenting on the implementation in @kazu-yamamoto's PR. I'm not sure what behavior you'd expect to see besides busy-looping. The server will repeatedly try to accept new connections regardless of how many open FDs are available until an existing connection closes and an FD is freed up.

[domenkozar]

The server will repeatedly try to accept new connections regardless of how many open FDs are available until an existing connection closes and an FD is freed up.

I'm not familiar with the code so I can't comment about what's in the PR but: If a new client connection comes in, then FD error pops up, which is logged and that connection is closed. I don't see how it could busy-loop?

[kazu-yamamoto]

Relating to #553.

[kazu-yamamoto]

@snoyberg Is the following the intended behavior?

• Stop the accept loop when a listening socket is closed (for some reasons).
• Stop the accept loop in the case of unrecoverable errors (e.g. most ResourceEhausted)
• Continue the accept loop in the case of recoverable error (eCONNABORTED according to accept(2) error handling #553)

And do you categorize eMFILE as unrecoverable?

[snoyberg]

Yes, that sounds right @kazu-yamamoto. I think perhaps we should go to a whitelist of "recoverable errors" and default to treating errors as unrecoverable.

[domenkozar]

I gave this a bit more thought and I don't think stopping the accept loop is the best way to handle this. Consider the scenario of hitting the limit for just a fraction of connections being handled, then you're giving up 99% of connections because the server couldn't handle 1%.

I see two solutions here:

• sleep in case of EMFILE and retry
• reserve one fd at the beginning and free it when hitting EMFILE (not sure how much cost would locking bring here)

[ProofOfKeags]

Currently running into this now. If warp ever gets an exception for too many open files, it just refuses all future connections, irrespective of whether load has decreased. I have tried increasing the ulimit but to no avail.

[ProofOfKeags]

I rebased #831 and put in a PR: #870 . I have verified that this actually works in the production case where we had this problem. Feedback appreciated.

[kazu-yamamoto]

Thank you for the verification.

[kazu-yamamoto]

#831 has been merged and a new version is released.
Thank you, all, for your contribution!

[nh2]

This change created busy-looping problems, see #928.