Skip to content

Commit

Permalink
Avoid resetting HTTP/2 idle_timeout timer too often
Browse files Browse the repository at this point in the history
Following the same strategy as Websocket described in
commit f4e23b4

Gains are comparable as far as Websocket over HTTP/2
is concerned.
  • Loading branch information
essen committed Jan 8, 2025
1 parent 593a6fb commit c280269
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 10 deletions.
33 changes: 24 additions & 9 deletions src/cowboy_http2.erl
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,14 @@
state :: {module, any()}
}).

%% We don't want to reset the idle timeout too often,
%% so we don't reset it on data. Instead we reset the
%% number of ticks we have observed. We divide the
%% timeout value by a value and that value becomes
%% the number of ticks at which point we can drop
%% the connection. This value is the number of ticks.
-define(IDLE_TIMEOUT_TICKS, 10).

-record(state, {
parent = undefined :: pid(),
ref :: ranch:ref(),
Expand All @@ -95,6 +103,7 @@

%% Timer for idle_timeout; also used for goaway timers.
timer = undefined :: undefined | reference(),
idle_timeout_num = 0 :: 0..?IDLE_TIMEOUT_TICKS,

%% Remote address and port for the connection.
peer = undefined :: {inet:ip_address(), inet:port_number()},
Expand Down Expand Up @@ -166,7 +175,7 @@ init(Parent, Ref, Socket, Transport, ProxyHeader, Opts, Peer, Sock, Cert, Buffer
State = set_idle_timeout(init_rate_limiting(#state{parent=Parent, ref=Ref, socket=Socket,
transport=Transport, proxy_header=ProxyHeader,
opts=Opts, peer=Peer, sock=Sock, cert=Cert,
http2_status=sequence, http2_machine=HTTP2Machine})),
http2_status=sequence, http2_machine=HTTP2Machine}), 0),
safe_setopts_active(State),
case Buffer of
<<>> -> loop(State, Buffer);
Expand Down Expand Up @@ -222,7 +231,7 @@ init(Parent, Ref, Socket, Transport, ProxyHeader, Opts, Peer, Sock, Cert, Buffer
<<"connection">> => <<"Upgrade">>,
<<"upgrade">> => <<"h2c">>
}, ?MODULE, undefined}), %% @todo undefined or #{}?
State = set_idle_timeout(init_rate_limiting(State2#state{http2_status=sequence})),
State = set_idle_timeout(init_rate_limiting(State2#state{http2_status=sequence}), 0),
%% In the case of HTTP/1.1 Upgrade we cannot send the Preface
%% until we send the 101 response.
ok = maybe_socket_error(State, Transport:send(Socket, Preface)),
Expand All @@ -249,7 +258,7 @@ loop(State=#state{parent=Parent, socket=Socket, transport=Transport,
receive
%% Socket messages.
{OK, Socket, Data} when OK =:= element(1, Messages) ->
parse(set_idle_timeout(State), << Buffer/binary, Data/binary >>);
parse(State#state{idle_timeout_num=0}, << Buffer/binary, Data/binary >>);
{Closed, Socket} when Closed =:= element(2, Messages) ->
Reason = case State#state.http2_status of
closing -> {stop, closed, 'The client is going away.'};
Expand All @@ -273,8 +282,7 @@ loop(State=#state{parent=Parent, socket=Socket, transport=Transport,
sys:handle_system_msg(Request, From, Parent, ?MODULE, [], {State, Buffer});
%% Timeouts.
{timeout, TimerRef, idle_timeout} ->
terminate(State, {stop, timeout,
'Connection idle longer than configuration allows.'});
tick_idle_timeout(State, Buffer);
{timeout, Ref, {shutdown, Pid}} ->
cowboy_children:shutdown_timeout(Children, Ref, Pid),
loop(State, Buffer);
Expand Down Expand Up @@ -302,12 +310,19 @@ loop(State=#state{parent=Parent, socket=Socket, transport=Transport,
terminate(State, {internal_error, timeout, 'No message or data received before timeout.'})
end.

set_idle_timeout(State=#state{http2_status=Status, timer=TimerRef})
tick_idle_timeout(State=#state{idle_timeout_num=?IDLE_TIMEOUT_TICKS}, _) ->
terminate(State, {stop, timeout,
'Connection idle longer than configuration allows.'});
tick_idle_timeout(State=#state{idle_timeout_num=TimeoutNum}, Buffer) ->
loop(set_idle_timeout(State, TimeoutNum + 1), Buffer).

set_idle_timeout(State=#state{http2_status=Status, timer=TimerRef}, _)
when Status =:= closing_initiated orelse Status =:= closing,
TimerRef =/= undefined ->
State;
set_idle_timeout(State=#state{opts=Opts}) ->
set_timeout(State, maps:get(idle_timeout, Opts, 60000), idle_timeout).
set_idle_timeout(State=#state{opts=Opts}, TimeoutNum) ->
set_timeout(State#state{idle_timeout_num=TimeoutNum},
maps:get(idle_timeout, Opts, 60000), idle_timeout).

set_timeout(State=#state{timer=TimerRef0}, Timeout, Message) ->
ok = case TimerRef0 of
Expand All @@ -323,7 +338,7 @@ set_timeout(State=#state{timer=TimerRef0}, Timeout, Message) ->
maybe_reset_idle_timeout(State=#state{opts=Opts}) ->
case maps:get(reset_idle_timeout_on_send, Opts, false) of
true ->
set_idle_timeout(State);
State#state{idle_timeout_num=0};
false ->
State
end.
Expand Down
2 changes: 1 addition & 1 deletion test/ws_perf_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ groups() ->

init_per_suite(Config) ->
%% Optionally enable `perf` for the current node.
% spawn(fun() -> ct:pal(os:cmd("perf record -g -F 9999 -o /tmp/ws_perf.data -p " ++ os:getpid() ++ " -- sleep 11")) end),
% spawn(fun() -> ct:pal(os:cmd("perf record -g -F 9999 -o /tmp/ws_perf.data -p " ++ os:getpid() ++ " -- sleep 60")) end),
Config.

end_per_suite(_Config) ->
Expand Down

0 comments on commit c280269

Please sign in to comment.