-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(connection): configurable TCP keepalive #10933
feat(connection): configurable TCP keepalive #10933
Conversation
7f73c08
to
e5c7669
Compare
apps/emqx/src/emqx_schema.erl
Outdated
sc( | ||
string(), | ||
#{ | ||
default => "none", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default => "none", | |
default => <<"none">>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack
apps/emqx/src/emqx_connection.erl
Outdated
@@ -353,6 +367,20 @@ init_state( | |||
false -> disabled | |||
end, | |||
IdleTimeout = emqx_channel:get_mqtt_conf(Zone, idle_timeout), | |||
|
|||
{ListenerType, ListenerId} = Listener, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe move this hunk to a small function (take get_active_n function as an example).
also, we should not try to set tcp-ka for QUIC connections.
ok = maybe_set_tcp_keepalive(Listener),
maybe_set_tcp_keepalive({quic, _}) -> ok;
maybe_set_tcp_keepalive({Type, Id}) ->
Conf = mqx_config:get_listener_conf(ListenerType, ListenerId, [tcp_options, keepalive], <<"none">>),
case iolist_to_binary(Conf) of
<<"none">> ->
ok;
Value ->
%% the value is already validated by schema, so we do not validate it again.
{Idle, Interval, Probes} = emqx_schema:parse_tcp_keepalive(Value),
async_set_keepalive(Idle, Interfval, Probe)
end.
-module(emqx_schema).
-export([parse_tcp_keepalive/1]).
%% @doc This function is used as value validator and also run-time parser.
parse_tcp_keepalive(Str) ->
try
[Idle, Interval, Probes] = binary:split(iolist_to_binary(Str), <<",">>, [global]),
%% use 10 times the Linux defaults as range limit
IdleInt = parse_ka_int(Idle, "Idle", 1, 7200_0),
IntervalInt = parse_ka_int(Interval, "Interva", 75_0),
ProbesInt = parse_ka_int(Probe, "Probes", 1, 9_0),
{IdleInt, IntervalInt, ProbesInt}
catch
error:_ ->
throw(#{reason => “Not comma separated positive integers of 'Idle,Interval,Probes' format", value => Str})
end.
parse_ka_int(Bin, Name, Min, Max) ->
I = binary_to_integer(Bin),
case I >= Min andalso I =< Max of
true ->
I;
false ->
Msg = io_lib:format("TCP-Keepalive '~s' value must be in the rage of [~p, ~p].", [Name, Min, Max]),
throw(#{reason => lists:flatten(Msg), value => I}
end.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack
apps/emqx/src/emqx_schema.erl
Outdated
case Value of | ||
"none" -> | ||
ok; | ||
_ -> | ||
try | ||
[IdleStr, IntervalStr, ProbesStr] = string:tokens(Value, ", "), | ||
_ = list_to_integer(IdleStr), | ||
_ = list_to_integer(IntervalStr), | ||
_ = list_to_integer(ProbesStr), | ||
ok | ||
catch | ||
_:_ -> | ||
{error, "value does not contain 3 comma-separate integer values"} | ||
end | ||
end. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
case Value of | |
"none" -> | |
ok; | |
_ -> | |
try | |
[IdleStr, IntervalStr, ProbesStr] = string:tokens(Value, ", "), | |
_ = list_to_integer(IdleStr), | |
_ = list_to_integer(IntervalStr), | |
_ = list_to_integer(ProbesStr), | |
ok | |
catch | |
_:_ -> | |
{error, "value does not contain 3 comma-separate integer values"} | |
end | |
end. | |
case iolist_to_binary(Value) of | |
<<"none">> -> | |
ok; | |
_ -> | |
_ = parse_tcp_keealive(Value), | |
ok | |
end. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack
e5c7669
to
18aefb5
Compare
Pull Request Test Coverage Report for Build 5194657467
💛 - Coveralls |
18aefb5
to
00ce9b2
Compare
985b602
to
fce4f1b
Compare
fce4f1b
to
a440776
Compare
Enhancements
|
增强
|
Fixes https://emqx.atlassian.net/browse/EMQX-9852
Summary
🤖 Generated by Copilot at 7f73c08
This pull request adds support for TCP keepalive on Mac OS and configurable TCP keepalive options for listeners. It modifies the
emqx_connection
,emqx_schema
,emqx_gateway_api_listeners
, andemqx_listeners
modules, as well as theemqx_mqtt_SUITE
test case and theemqx_schema.hocon
file. It also fixes some socket errors and refactors some code for readability and consistency.PR Checklist
Please convert it to a draft if any of the following conditions are not met. Reviewers may skip over until all the items are checked:
changes/{ce,ee}/(feat|perf|fix)-<PR-id>.en.md
filesIf there should be document changes, a PR to emqx-docs.git is sent, or a jira ticket is created to follow upChecklist for CI (.github/workflows) changes
If changed package build workflow, pass this action (manual trigger)Change log has been added tochanges/
dir for user-facing artifacts update