diff --git a/src/roadrunner_acceptor.erl b/src/roadrunner_acceptor.erl index c5af7ad..00c2d6b 100644 --- a/src/roadrunner_acceptor.erl +++ b/src/roadrunner_acceptor.erl @@ -5,12 +5,21 @@ %% and hands each accepted connection off to a `roadrunner_conn` worker. %% %% Spawn-linked to the owning `roadrunner_listener`: a listener stop closes -%% the listen socket, the acceptor's `accept/1` returns `{error, _}`, -%% and the acceptor exits cleanly. Unrelated acceptor crashes propagate +%% the listen socket, the acceptor's `accept/1` returns `{error, closed}`, +%% and the acceptor exits cleanly. A transient accept error instead +%% (`emfile`/`enfile`/`system_limit` descriptor exhaustion when `max_clients` +%% sits above the OS `ulimit -n`, or a connection aborted before accept +%% completed) is reported via telemetry and retried after a short back-off, +%% so the pool never silently drains. Unrelated acceptor crashes propagate %% back via the link, taking the listener down for supervisor restart. %% Connection workers are spawned **without** a link so that a crash %% in one connection does not bring down the acceptor. +%% Back-off between retries after a transient accept error. Bounds the +%% retry/telemetry rate and gives the box a moment to reclaim descriptors +%% before the next `accept/1`. +-define(ACCEPT_ERROR_BACKOFF_MS, 100). + -export([start_link/3]). -doc """ @@ -37,10 +46,25 @@ loop(LSocket, ProtoOpts) -> {ok, Socket} -> handle_accepted(Socket, ProtoOpts), loop(LSocket, ProtoOpts); - {error, _} -> - %% Listen socket was closed (or another transport error) — - %% terminate cleanly; the linked listener will tear us down. - ok + {error, closed} -> + %% Listen socket was closed — the listener is stopping. Exit + %% cleanly; the linked listener tears the rest of the pool down. + ok; + {error, Reason} -> + %% Any other accept error is transient: descriptor exhaustion + %% (`emfile`/`enfile`/`system_limit`) when `max_clients` sits + %% above the OS `ulimit -n`, or a connection aborted before + %% accept completed. Surface it and back off, then keep + %% accepting — exiting here would silently drain the pool and + %% leave the listener permanently deaf with no diagnostic. + ok = roadrunner_telemetry:listener_accept_error(#{ + listener_name => maps:get(listener_name, ProtoOpts, undefined), + reason => Reason + }), + receive + after ?ACCEPT_ERROR_BACKOFF_MS -> ok + end, + loop(LSocket, ProtoOpts) end. -spec handle_accepted(roadrunner_transport:socket(), roadrunner_conn:proto_opts()) -> ok. diff --git a/src/roadrunner_telemetry.erl b/src/roadrunner_telemetry.erl index b54a771..b71b490 100644 --- a/src/roadrunner_telemetry.erl +++ b/src/roadrunner_telemetry.erl @@ -76,6 +76,16 @@ %% - **Measurements:** `system_time`. %% - **Metadata:** `listener_name`, `reason` (`max_clients`). %% +%% - `[roadrunner, listener, accept_error]` — fired when an acceptor's +%% `accept/1` returns a transient error: file-descriptor exhaustion +%% (`emfile`/`enfile`/`system_limit`, usually `max_clients` above the OS +%% `ulimit -n`) or a connection aborted before accept completed. The +%% acceptor backs off and keeps accepting instead of exiting, so this is +%% the signal that the box is out of descriptors — raise `ulimit -n`. +%% +%% - **Measurements:** `system_time`. +%% - **Metadata:** `listener_name`, `reason` (the accept error). +%% %% - `[roadrunner, request, throttled]` — fired before any handler runs when a %% request is refused at a listener limit, for one of two reasons: %% - `max_concurrent_requests`: an HTTP/2 or HTTP/3 stream over the @@ -169,6 +179,7 @@ listener_accept/1, listener_conn_close/2, listener_conn_rejected/1, + listener_accept_error/1, request_rejected/1, request_throttled/1, slots_reconciled/1, @@ -310,6 +321,25 @@ listener_conn_rejected(Metadata) -> ), ok. +-doc """ +Emit `[roadrunner, listener, accept_error]` when an acceptor's `accept/1` +fails with a transient error — file-descriptor exhaustion +(`emfile`/`enfile`/`system_limit`, typically `max_clients` sitting above +the OS `ulimit -n`) or a connection aborted before accept completed. The +acceptor reports it here, backs off, and keeps accepting rather than +exiting, so this event is the signal that the box is out of descriptors. +`Metadata` should include `listener_name` and `reason` (the accept +error). Carries no `peer`: there is no connection to name. +""". +-spec listener_accept_error(map()) -> ok. +listener_accept_error(Metadata) -> + telemetry:execute( + [roadrunner, listener, accept_error], + #{system_time => erlang:system_time()}, + Metadata + ), + ok. + -doc """ Emit `[roadrunner, request, rejected]` when a request is dropped at the parser/limit layer before any handler runs (malformed request line, diff --git a/test/roadrunner_acceptor_tests.erl b/test/roadrunner_acceptor_tests.erl index 35e88b6..eb33fc8 100644 --- a/test/roadrunner_acceptor_tests.erl +++ b/test/roadrunner_acceptor_tests.erl @@ -105,6 +105,47 @@ conn_process_carries_listener_name_and_peer_label_test_() -> end} end}. +acceptor_retries_on_transient_accept_error_test() -> + %% Accepting on a connected (non-listen) socket fails with a transient + %% error (einval here, standing in for emfile descriptor exhaustion). The + %% acceptor must report it via telemetry and keep looping, NOT exit and + %% leave the listener silently deaf. Closing the socket then yields + %% {error, closed}, which ends the loop cleanly. + {ok, LSock} = gen_tcp:listen(0, [binary, {active, false}]), + {ok, Port} = inet:port(LSock), + {ok, CSock} = gen_tcp:connect({127, 0, 0, 1}, Port, [binary, {active, false}], 1000), + {ok, _} = application:ensure_all_started(telemetry), + Self = self(), + HandlerId = {?MODULE, make_ref()}, + ok = telemetry:attach( + HandlerId, + [roadrunner, listener, accept_error], + fun(_Event, _Measure, Meta, _Cfg) -> Self ! {accept_error, Meta} end, + undefined + ), + {ok, Pid} = roadrunner_acceptor:start_link( + {gen_tcp, CSock}, #{listener_name => acceptor_test_accept_error}, 1 + ), + receive + {accept_error, Meta} -> + ?assertEqual(acceptor_test_accept_error, maps:get(listener_name, Meta)), + ?assertNotEqual(closed, maps:get(reason, Meta)) + after 2000 -> + error(no_accept_error_telemetry) + end, + %% Survived the transient error. + ?assert(is_process_alive(Pid)), + %% Closing the socket turns the next accept into {error, closed} → exit. + MRef = erlang:monitor(process, Pid), + ok = gen_tcp:close(CSock), + receive + {'DOWN', MRef, process, Pid, normal} -> ok + after 2000 -> + error(acceptor_did_not_stop) + end, + ok = telemetry:detach(HandlerId), + ok = gen_tcp:close(LSock). + %% --- helpers --- %% Poll until we see a refined `{roadrunner_conn, Name, Peer}` label or run