Skip to content

fix: do not stop accepting connections on transient errors - #577

Open
AndreyOsipuk wants to merge 2 commits into
9seconds:masterfrom
AndreyOsipuk:fix/accept-loop-resilience-upstream
Open

AndreyOsipuk wants to merge 2 commits into
9seconds:masterfrom
AndreyOsipuk:fix/accept-loop-resilience-upstream

Conversation

@AndreyOsipuk

Copy link
Copy Markdown

mtg can stay alive but stop accepting connections until it is restarted:

  1. Proxy.Serve returns on any Accept error, and runProxy starts it as go proxy.Serve(listener) //nolint: errcheck. When a connection flood exhausts file descriptors (EMFILE), the accept loop ends while the process keeps running; process-level monitoring does not notice.
  2. utils.Listener.Accept returns an error when socket options cannot be set on one accepted connection (for example, a client that resets right after the TCP handshake), which ends the accept loop because of a single bad client.

Changes:

  • temporary accept errors (EMFILE, ENFILE, ENOBUFS, ENOMEM, ECONNABORTED, ECONNRESET, timeouts) are retried with a pause growing from 5ms to 1s, as net/http does, with a warning in the log; shutdown interrupts the pause;
  • a connection whose socket options cannot be set is closed and skipped;
  • if Serve still returns an error, runProxy shuts down and returns it, so systemd/docker restarts the process.

Tests: acceptWithRetry survives EMFILE, stops on a closed listener and on shutdown during the pause; IsTemporaryAcceptError and the delay cap; utils.Listener skips a connection with a broken socket (fails on current master). go vet, tests and GOOS=windows/darwin go build ./... pass.

The same fix for mtg-multi (which also covers its MultiListener): dolonet#46.

Proxy.Serve returned on any Accept error and run_proxy started it with
'go proxy.Serve(listener)' ignoring the result, so running out of file
descriptors under a connection flood (EMFILE) left the process alive but no
longer accepting connections. utils.Listener also returned an error when
socket options could not be set on one accepted connection (e.g. reset by the
client right away), which stopped the accept loop because of one bad client.

Temporary accept errors are now retried with a pause growing from 5ms to 1s,
like net/http; a connection with broken socket options is closed and skipped;
and if Serve still returns, runProxy shuts down and returns the error so a
supervisor restarts the process.
The listener is closed during shutdown, so Serve returns an accept error
and every stop or restart exited with code 1. Only a Serve that failed
before shutdown started is reported now.
@AndreyOsipuk

Copy link
Copy Markdown
Author

Found on our production nodes after deploying this change: an ordinary systemctl stop/restart exited with code 1, because shutdown closes the listener and the resulting accept error was reported as a failure. Fixed in the last commit: only a Serve that failed before shutdown started is reported (waitAndShutdown, with tests for both cases). Checked live: SIGTERM now exits with 0.

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.

1 participant