Don't crash the server when there's a listen error#587
Conversation
|
As far as I can see this will hang single CPU with 100% usage in case Sorry, for not having enough time for finishing #386 |
| let stream = match stream { | ||
| Err(ref e) if is_transient_error(e) => continue, | ||
| Err(error) => { | ||
| let delay = std::time::Duration::from_millis(500); |
There was a problem hiding this comment.
This seems kind of large? I guess it should be pretty unusual so maybe it's fine. Ideally we'd have a dynamically increasing backoff here.
There was a problem hiding this comment.
I didn't have a good sense of an appropriate number here so I used the same number in the book section cited above
There was a problem hiding this comment.
Well, it's 500ms in the book because it's reasonably fine to write a log message every 500ms. Skipping log message always will make the issue a nightmare to debug. Sleeping less but keeping message rarely will substantially complicate the code (which is fine for prod, but bad for a book).
But at the end of the day, you should not expect code to hit this timeout for a long time. I always consider this as an infrastructure issue: either bump file descriptor limit if you have enough memory, or lower listen queue, or put a load-balancer/queue in front of the process to get rid of error. So large timeout here is fine. Also, TCP delays can be larger than 500ms, so any client should be fine to wait that much.
|
Merging this based on @yoshuawuyts' comment above, especially since this is a short-term fix for multiple reasons:
|
4 participants
This change is an attempt to address #577, and also seems like correct behavior for a server. Regardless of what goes wrong with any individual request, we attempt to continue to respond to subsequent requests. Previously, the use of ? was returning from the
listenfunction scope, which shut down the server.Extracting the inner functions was mostly for clarity and will be further refactored/extracted in future PRs
closes #577