Repository navigation
feat(acceptor): count early-closed and forbidden sockets - #230
Merged
Merged
Conversation
`ets:update_counter/3` raises `badarg` when the row does not exist. The acceptor then crashes on its first update of a metric that the listener did not initialize, e.g. a metric added after the listener started. Pass a default row to `ets:update_counter/4` instead. Claude-Session: https://claude.ai/code/session_01NZuG3jQw5LtYcbUao51j5D
The acceptor counted most sockets that it closed before a connection
process started as `closed_other_reasons`. Split out two counters:
- `closed_early`: the socket failed with `econnaborted`, `econnreset`,
`enotconn`, `einval` or `closed` before the connection process was
started. This most likely means the peer closed or reset it.
- `closed_forbidden`: the access rules denied the peer address.
`closed_other_reasons` now only counts `ignore`, connection start
errors and crashes.
Also stop counting `{error, closed}` from an async accept. It means
the listen socket is closed, not a client socket.
Claude-Session: https://claude.ai/code/session_01NZuG3jQw5LtYcbUao51j5D
zmstone
requested review from
keynslug and
qzhuyan
and removed request for
keynslug
September 11, 2026 11:10
zmstone
marked this pull request as ready for review
September 11, 2026 11:10
keynslug
approved these changes
Sep 11, 2026
This was referenced Sep 11, 2026
3 tasks done
2 of 3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The acceptor closes some sockets before a connection process starts. It counted most of them as
closed_other_reasons. This PR splits out two new accept-result counters, whichesockd:get_stats/1returns:closed_early: the socket failed witheconnaborted,econnreset,enotconn,einvalorclosedbefore the connection process was started. This most likely means the peer closed or reset it. Scanners, health checks and load balancers usually cause these.closed_forbidden: the access rules denied the peer address.closed_other_reasonsnow only countsignore, connection start errors and crashes. A rise in this counter now points at a real problem.Related fixes
esockd_server:inc_stats/3anddec_stats/3no longer crash the caller when the metric row is missing. They pass a default row toets:update_counter/4. Without this, an acceptor on a listener started before the new counters existed would crash withbadargon its first early close.{error, closed}from an async accept. That error means the listen socket is closed, not a client socket. The sync accept path already skipped the count.Tests
esockd_acceptor_fsm_SUITE:t_closed_early,t_forbiddenandt_tune_fun_other_error.t_einvalnow expectsclosed_early. The listen-socket-closed case asserts that no close counter moves.esockd_SUITE:t_closed_forbiddenuses a real deny rule.t_closed_earlyresets the client connection while the acceptor waits intune_fun.esockd_server_SUITE:t_inc_uninitialized_stats.Follow-up: EMQX only reads
esockd:get_shutdown_count/1today. A separate EMQX PR will expose the accept-result counters in the CLI and in Prometheus.