From 071b03b29c607d5be51022a12436ad7009f5c9bd Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Fri, 31 Dec 2021 12:27:25 +0100 Subject: [PATCH] refactor: statsd supervisor API no need to return error Also to make dialyzer happy --- .../src/emqx_mgmt_api_trace.erl | 2 +- apps/emqx_statsd/src/emqx_statsd_api.erl | 6 ++--- apps/emqx_statsd/src/emqx_statsd_app.erl | 2 +- apps/emqx_statsd/src/emqx_statsd_sup.erl | 27 +++++++++++-------- 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/apps/emqx_management/src/emqx_mgmt_api_trace.erl b/apps/emqx_management/src/emqx_mgmt_api_trace.erl index 56f1aa3fa..d92fd6ad8 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_trace.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_trace.erl @@ -308,7 +308,7 @@ download_trace_log(get, #{bindings := #{name := Name}}) -> %% TODO use file replace file_binary.(delete file after send is not ready now). {ok, Binary} = file:read_file(ZipFile), ZipName = filename:basename(ZipFile), - file:delete(ZipFile), + _ = file:delete(ZipFile), Headers = #{ <<"content-type">> => <<"application/x-zip">>, <<"content-disposition">> => iolist_to_binary("attachment; filename=" ++ ZipName) diff --git a/apps/emqx_statsd/src/emqx_statsd_api.erl b/apps/emqx_statsd/src/emqx_statsd_api.erl index 3294152ac..389c382f0 100644 --- a/apps/emqx_statsd/src/emqx_statsd_api.erl +++ b/apps/emqx_statsd/src/emqx_statsd_api.erl @@ -61,10 +61,10 @@ statsd(put, #{body := Body}) -> {ok, #{raw_config := NewConfig, config := Config}} -> case maps:get(<<"enable">>, Body) of true -> - _ = emqx_statsd_sup:stop_child(?APP), - _ = emqx_statsd_sup:start_child(?APP, maps:get(config, Config)); + ok = emqx_statsd_sup:ensure_child_stopped(?APP), + ok = emqx_statsd_sup:ensure_child_started(?APP, maps:get(config, Config)); false -> - _ = emqx_statsd_sup:stop_child(?APP) + ok = emqx_statsd_sup:ensure_child_stopped(?APP) end, {200, NewConfig}; {error, Reason} -> diff --git a/apps/emqx_statsd/src/emqx_statsd_app.erl b/apps/emqx_statsd/src/emqx_statsd_app.erl index 820a86c04..c0a17fc5b 100644 --- a/apps/emqx_statsd/src/emqx_statsd_app.erl +++ b/apps/emqx_statsd/src/emqx_statsd_app.erl @@ -34,7 +34,7 @@ stop(_) -> maybe_enable_statsd() -> case emqx_conf:get([statsd, enable], false) of true -> - emqx_statsd_sup:start_child(?APP, emqx_conf:get([statsd], #{})); + emqx_statsd_sup:ensure_child_started(?APP, emqx_conf:get([statsd], #{})); false -> ok end. diff --git a/apps/emqx_statsd/src/emqx_statsd_sup.erl b/apps/emqx_statsd/src/emqx_statsd_sup.erl index d87b1c961..983cf97d4 100644 --- a/apps/emqx_statsd/src/emqx_statsd_sup.erl +++ b/apps/emqx_statsd/src/emqx_statsd_sup.erl @@ -8,9 +8,9 @@ -behaviour(supervisor). -export([ start_link/0 - , start_child/1 - , start_child/2 - , stop_child/1 + , ensure_child_started/1 + , ensure_child_started/2 + , ensure_child_stopped/1 ]). -export([init/1]). @@ -26,19 +26,24 @@ start_link() -> supervisor:start_link({local, ?MODULE}, ?MODULE, []). --spec start_child(supervisor:child_spec()) -> ok. -start_child(ChildSpec) when is_map(ChildSpec) -> +-spec ensure_child_started(supervisor:child_spec()) -> ok. +ensure_child_started(ChildSpec) when is_map(ChildSpec) -> assert_started(supervisor:start_child(?MODULE, ChildSpec)). --spec start_child(atom(), map()) -> ok. -start_child(Mod, Opts) when is_atom(Mod) andalso is_map(Opts) -> +-spec ensure_child_started(atom(), map()) -> ok. +ensure_child_started(Mod, Opts) when is_atom(Mod) andalso is_map(Opts) -> assert_started(supervisor:start_child(?MODULE, ?CHILD(Mod, Opts))). --spec(stop_child(any()) -> ok | {error, term()}). -stop_child(ChildId) -> +%% @doc Stop the child worker process. +-spec ensure_child_stopped(any()) -> ok. +ensure_child_stopped(ChildId) -> case supervisor:terminate_child(?MODULE, ChildId) of - ok -> supervisor:delete_child(?MODULE, ChildId); - Error -> Error + ok -> + %% with terminate_child/2 returned 'ok', it's not possible + %% for supervisor:delete_child/2 to return {error, Reason} + ok = supervisor:delete_child(?MODULE, ChildId); + {error, not_found} -> + ok end. init([]) ->