From 0c76b6cf4d5338e21414a833dd1b4f0386a95944 Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Fri, 4 Mar 2022 11:00:49 -0300 Subject: [PATCH 1/4] fix(congestion): fix congestion message formatting It seems that the `~ts` in the format string does not handle maps, which results in a crash when trying to format the congestion message. ```erlang 28> io_lib:format("~ts", [#{}]). ** exception error: bad argument in function io_lib:format/2 called as io_lib:format("~ts",[#{}]) ``` Example: ``` 2022-03-04T12:43:26.900609+00:00 [warning] '$kind': unclean_terminate, clientid: 192.168.2.116_bench_sub_11412_1782328074, context: badarg, exception: error, line: 582, mfa: emqx_connection:terminate/2, peername: 192.168.2.116:47358, stacktrace: [{io_lib,format,["connection congested: ~ts",[#{buffer => 4096,clientid => <<"192.168.2.116_bench_sub_11412_1782328074">>,conn_state => disconnected,connected_at => 1646397524470,high_msgq_watermark => 8192,high_watermark => 1048576,memory => 57064,message_queue_len => 20,peername => <<"192.168.2.116:47358">>,pid => <<"<0.259.28>">>,proto_name => <<"MQTT">>,proto_ver => 5,recbuf => 4096,recv_cnt => 2,recv_oct => 81,reductions => 267523,send_cnt => 333,send_oct => 149869,send_pend => 128,sndbuf => 332800,sockname => <<"10.10.1.13:1883">>,socktype => tcp,username => undefined}]],[{file,"io_lib.erl"},{line,187}]},{emqx_congestion,do_cancel_alarm_congestion,4,[{file,"/emqx/apps/emqx/src/emqx_congestion.erl"},{line,88}]},{lists,foreach,2,[{file,"lists.erl"},{line,1342}]},{emqx_connection,terminate,2,[{file,"/emqx/apps/emqx/src/emqx_connection.erl"},{line,576}]},{proc_lib,wake_up,3,[{file,"proc_lib.erl"},{line,236}]}] ``` --- apps/emqx/src/emqx_congestion.erl | 4 ++-- apps/emqx/test/emqx_connection_SUITE.erl | 26 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/apps/emqx/src/emqx_congestion.erl b/apps/emqx/src/emqx_congestion.erl index fa2788bc1..4f4423b12 100644 --- a/apps/emqx/src/emqx_congestion.erl +++ b/apps/emqx/src/emqx_congestion.erl @@ -78,14 +78,14 @@ cancel_alarm_congestion(Socket, Transport, Channel, Reason) -> do_alarm_congestion(Socket, Transport, Channel, Reason) -> ok = update_alarm_sent_at(Reason), AlarmDetails = tcp_congestion_alarm_details(Socket, Transport, Channel), - Message = io_lib:format("connection congested: ~ts", [AlarmDetails]), + Message = io_lib:format("connection congested: ~p", [AlarmDetails]), emqx_alarm:activate(?ALARM_CONN_CONGEST(Channel, Reason), AlarmDetails, Message), ok. do_cancel_alarm_congestion(Socket, Transport, Channel, Reason) -> ok = remove_alarm_sent_at(Reason), AlarmDetails = tcp_congestion_alarm_details(Socket, Transport, Channel), - Message = io_lib:format("connection congested: ~ts", [AlarmDetails]), + Message = io_lib:format("connection congested: ~p", [AlarmDetails]), emqx_alarm:deactivate(?ALARM_CONN_CONGEST(Channel, Reason), AlarmDetails, Message), ok. diff --git a/apps/emqx/test/emqx_connection_SUITE.erl b/apps/emqx/test/emqx_connection_SUITE.erl index 5d2c31441..57ae9a817 100644 --- a/apps/emqx/test/emqx_connection_SUITE.erl +++ b/apps/emqx/test/emqx_connection_SUITE.erl @@ -432,6 +432,32 @@ t_oom_shutdown(_) -> end, Opts), ok. +t_cancel_congestion_alarm(_) -> + Opts = #{trap_exit => false}, + ok = meck:expect(emqx_transport, getstat, + fun(_Sock, [send_pend]) -> + %% simulate congestion + {ok, [{send_pend, 999}]}; + (_Sock, Options) -> + {ok, [{K, 0} || K <- Options]} + end), + with_conn( + fun(Pid) -> + #{ channel := Channel + , transport := Transport + , socket := Socket + } = emqx_connection:get_state(Pid), + %% precondition + Zone = emqx_channel:info(zone, Channel), + true = emqx_config:get_zone_conf(Zone, [conn_congestion, enable_alarm]), + %% should not raise errors + ok = emqx_congestion:maybe_alarm_conn_congestion(Socket, Transport, Channel), + %% should not raise errors either + ok = emqx_congestion:cancel_alarms(Socket, Transport, Channel), + ok + end, Opts), + ok. + %%-------------------------------------------------------------------- %% Helper functions %%-------------------------------------------------------------------- From e76f67e9850b96d41236ee3d04461ccd857f1e73 Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Fri, 4 Mar 2022 11:14:13 -0300 Subject: [PATCH 2/4] style: please elvis checks --- apps/emqx/src/emqx_congestion.erl | 2 ++ apps/emqx/test/emqx_connection_SUITE.erl | 11 ++++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/apps/emqx/src/emqx_congestion.erl b/apps/emqx/src/emqx_congestion.erl index 4f4423b12..71ce35676 100644 --- a/apps/emqx/src/emqx_congestion.erl +++ b/apps/emqx/src/emqx_congestion.erl @@ -20,6 +20,8 @@ , cancel_alarms/3 ]). +-elvis([{elvis_style, invalid_dynamic_call, #{ignore => [emqx_congestion]}}]). + -define(ALARM_CONN_CONGEST(Channel, Reason), list_to_binary( io_lib:format("~ts/~ts/~ts", diff --git a/apps/emqx/test/emqx_connection_SUITE.erl b/apps/emqx/test/emqx_connection_SUITE.erl index 57ae9a817..f96ac6ca7 100644 --- a/apps/emqx/test/emqx_connection_SUITE.erl +++ b/apps/emqx/test/emqx_connection_SUITE.erl @@ -335,7 +335,8 @@ t_handle_info(_) -> t_ensure_rate_limit(_) -> WhenOk = fun emqx_connection:next_incoming_msgs/3, - {ok, [], State} = emqx_connection:check_limiter([], [], WhenOk, [], st(#{limiter => undefined})), + {ok, [], State} = emqx_connection:check_limiter([], [], WhenOk, [], + st(#{limiter => undefined})), ?assertEqual(undefined, emqx_connection:info(limiter, State)), Limiter = init_limiter(), @@ -344,7 +345,8 @@ t_ensure_rate_limit(_) -> ok = meck:expect(emqx_htb_limiter, check, fun(_, Client) -> {pause, 3000, undefined, Client} end), - {ok, State2} = emqx_connection:check_limiter([{1000, bytes_in}], [], WhenOk, [], st(#{limiter => Limiter})), + {ok, State2} = emqx_connection:check_limiter([{1000, bytes_in}], [], + WhenOk, [], st(#{limiter => Limiter})), meck:unload(emqx_htb_limiter), ok = meck:new(emqx_htb_limiter, [passthrough, no_history, no_link]), ?assertNotEqual(undefined, emqx_connection:info(limiter_timer, State2)). @@ -550,7 +552,10 @@ channel(InitFields) -> maps:fold(fun(Field, Value, Channel) -> emqx_channel:set_field(Field, Value, Channel) end, - emqx_channel:init(ConnInfo, #{zone => default, limiter => limiter_cfg(), listener => {tcp, default}}), + emqx_channel:init(ConnInfo, #{ zone => default + , limiter => limiter_cfg() + , listener => {tcp, default} + }), maps:merge(#{clientinfo => ClientInfo, session => Session, conn_state => connected From 6905147093071fe5232694d071aefec1154ceecf Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Fri, 4 Mar 2022 11:18:28 -0300 Subject: [PATCH 3/4] style: fix nls at eof --- .github/workflows/build_packages.yaml | 2 +- .github/workflows/release.yaml | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/build_packages.yaml b/.github/workflows/build_packages.yaml index 4796c2fb3..520868fa5 100644 --- a/.github/workflows/build_packages.yaml +++ b/.github/workflows/build_packages.yaml @@ -651,4 +651,4 @@ jobs: exit 1 fi aws s3 cp --recursive packages/$PROFILE s3://${{ secrets.AWS_S3_BUCKET }}/$s3dir/${{ github.ref_name }} - aws cloudfront create-invalidation --distribution-id ${{ secrets.AWS_CLOUDFRONT_ID }} --paths "/$s3dir/${{ github.ref_name }}/*" \ No newline at end of file + aws cloudfront create-invalidation --distribution-id ${{ secrets.AWS_CLOUDFRONT_ID }} --paths "/$s3dir/${{ github.ref_name }}/*" diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index c14d7abca..4c7445c1c 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -76,4 +76,3 @@ jobs: -d "{\"ref\":\"v1.0.4\",\"inputs\":{\"version\": \"${{ github.ref_name }}\"}}" \ "https://api.github.com/repos/emqx/emqx-ci-helper/actions/workflows/update_emqx_homebrew.yaml/dispatches" fi - \ No newline at end of file From 3e08282ffbc07faf24276069675917fa7fa22162 Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Fri, 4 Mar 2022 15:22:33 -0300 Subject: [PATCH 4/4] chore: improve formatting Without `~0p`, had some weird indentation while printing --- apps/emqx/src/emqx_congestion.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/emqx/src/emqx_congestion.erl b/apps/emqx/src/emqx_congestion.erl index 71ce35676..ca94ded90 100644 --- a/apps/emqx/src/emqx_congestion.erl +++ b/apps/emqx/src/emqx_congestion.erl @@ -80,14 +80,14 @@ cancel_alarm_congestion(Socket, Transport, Channel, Reason) -> do_alarm_congestion(Socket, Transport, Channel, Reason) -> ok = update_alarm_sent_at(Reason), AlarmDetails = tcp_congestion_alarm_details(Socket, Transport, Channel), - Message = io_lib:format("connection congested: ~p", [AlarmDetails]), + Message = io_lib:format("connection congested: ~0p", [AlarmDetails]), emqx_alarm:activate(?ALARM_CONN_CONGEST(Channel, Reason), AlarmDetails, Message), ok. do_cancel_alarm_congestion(Socket, Transport, Channel, Reason) -> ok = remove_alarm_sent_at(Reason), AlarmDetails = tcp_congestion_alarm_details(Socket, Transport, Channel), - Message = io_lib:format("connection congested: ~p", [AlarmDetails]), + Message = io_lib:format("connection congested: ~0p", [AlarmDetails]), emqx_alarm:deactivate(?ALARM_CONN_CONGEST(Channel, Reason), AlarmDetails, Message), ok.