From 1d1cdc10096b246ca78d774c515020e56d274ee4 Mon Sep 17 00:00:00 2001 From: Kjell Winblad Date: Mon, 11 Mar 2024 17:36:27 +0100 Subject: [PATCH 1/4] fix: return error reason from dynamo connector status check Fixes: https://emqx.atlassian.net/browse/EMQX-11934 --- .../src/emqx_bridge_dynamo_connector.erl | 42 +++++++++++++++---- .../emqx_bridge_dynamo_connector_client.erl | 19 ++++----- 2 files changed, 42 insertions(+), 19 deletions(-) diff --git a/apps/emqx_bridge_dynamo/src/emqx_bridge_dynamo_connector.erl b/apps/emqx_bridge_dynamo/src/emqx_bridge_dynamo_connector.erl index 4aad2bd97..6e2caf1ab 100644 --- a/apps/emqx_bridge_dynamo/src/emqx_bridge_dynamo_connector.erl +++ b/apps/emqx_bridge_dynamo/src/emqx_bridge_dynamo_connector.erl @@ -178,14 +178,42 @@ on_batch_query(InstanceId, [{_ChannelId, _} | _] = Query, State) -> on_batch_query(_InstanceId, Query, _State) -> {error, {unrecoverable_error, {invalid_request, Query}}}. -on_get_status(_InstanceId, #{pool_name := Pool}) -> - Health = emqx_resource_pool:health_check_workers( - Pool, {emqx_bridge_dynamo_connector_client, is_connected, []} - ), - status_result(Health). +health_check_timeout() -> + 15000. -status_result(_Status = true) -> ?status_connected; -status_result(_Status = false) -> ?status_connecting. +on_get_status(_InstanceId, #{pool_name := Pool} = State) -> + Health = emqx_resource_pool:health_check_workers( + Pool, + {emqx_bridge_dynamo_connector_client, is_connected, [ + emqx_resource_pool:health_check_timeout() + ]}, + health_check_timeout(), + #{return_values => true} + ), + case Health of + {error, timeout} -> + {?status_connecting, State, <<"timeout_while_checking_connection">>}; + {ok, [_ | _] = Results} -> + status_result(Results, State) + end. + +status_result(Results, State) -> + case lists:all(fun(Res) -> Res =:= true end, Results) of + true -> + ?status_connected; + false -> + {value, {false, Error}} = + lists:search( + fun + ({false, _Error}) -> + true; + (_) -> + false + end, + Results + ), + {?status_connecting, State, Error} + end. %%======================================================================================== %% Helper fns diff --git a/apps/emqx_bridge_dynamo/src/emqx_bridge_dynamo_connector_client.erl b/apps/emqx_bridge_dynamo/src/emqx_bridge_dynamo_connector_client.erl index 622eaa382..23caa0c30 100644 --- a/apps/emqx_bridge_dynamo/src/emqx_bridge_dynamo_connector_client.erl +++ b/apps/emqx_bridge_dynamo/src/emqx_bridge_dynamo_connector_client.erl @@ -9,7 +9,7 @@ %% API -export([ start_link/1, - is_connected/1, + is_connected/2, query/4 ]). @@ -27,20 +27,15 @@ -export([execute/2]). -endif. -%% The default timeout for DynamoDB REST API calls is 10 seconds, -%% but this value for `gen_server:call` is 5s, -%% so we should pass the timeout to `gen_server:call` --define(HEALTH_CHECK_TIMEOUT, 10000). - %%%=================================================================== %%% API %%%=================================================================== -is_connected(Pid) -> +is_connected(Pid, Timeout) -> try - gen_server:call(Pid, is_connected, ?HEALTH_CHECK_TIMEOUT) + gen_server:call(Pid, is_connected, Timeout) catch - _:_ -> - false + _:Error -> + {false, Error} end. query(Pid, Table, Query, Templates) -> @@ -76,8 +71,8 @@ handle_call(is_connected, _From, State) -> case erlcloud_ddb2:list_tables([{limit, 1}]) of {ok, _} -> true; - _ -> - false + Error -> + {false, Error} end, {reply, IsConnected, State}; handle_call({query, Table, Query, Templates}, _From, State) -> From 29e0ddefbc224977b785a495243c1df947e4aafe Mon Sep 17 00:00:00 2001 From: Kjell Winblad Date: Mon, 11 Mar 2024 17:46:04 +0100 Subject: [PATCH 2/4] docs: add change log entry --- changes/ee/fix-12678.en.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/ee/fix-12678.en.md diff --git a/changes/ee/fix-12678.en.md b/changes/ee/fix-12678.en.md new file mode 100644 index 000000000..a1a03962f --- /dev/null +++ b/changes/ee/fix-12678.en.md @@ -0,0 +1 @@ +The DynamoDB connector now explicitly reports the error reason upon connection failure. This update addresses the previous limitation where connection failures did not result in any explanation. From 91514f3ace4cb3a8f36f98f6bcc559bd10749a9e Mon Sep 17 00:00:00 2001 From: Kjell Winblad Date: Tue, 12 Mar 2024 09:12:39 +0100 Subject: [PATCH 3/4] refactor: clean up over complicated code This commit cleans up overly complicated code and handles the case when the worker pool is empty. Thank you @thalesmg for suggesting this change. Co-authored-by: Thales Macedo Garitezi --- .../src/emqx_bridge_dynamo_connector.erl | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/apps/emqx_bridge_dynamo/src/emqx_bridge_dynamo_connector.erl b/apps/emqx_bridge_dynamo/src/emqx_bridge_dynamo_connector.erl index 6e2caf1ab..1d29bd427 100644 --- a/apps/emqx_bridge_dynamo/src/emqx_bridge_dynamo_connector.erl +++ b/apps/emqx_bridge_dynamo/src/emqx_bridge_dynamo_connector.erl @@ -198,20 +198,12 @@ on_get_status(_InstanceId, #{pool_name := Pool} = State) -> end. status_result(Results, State) -> - case lists:all(fun(Res) -> Res =:= true end, Results) of - true -> + case lists:filter(fun(Res) -> Res =/= true end, Results) of + [] when Results =:= [] -> + ?status_connecting; + [] -> ?status_connected; - false -> - {value, {false, Error}} = - lists:search( - fun - ({false, _Error}) -> - true; - (_) -> - false - end, - Results - ), + [{false, Error} | _] -> {?status_connecting, State, Error} end. From c90f4f579410135fb3ee01acbf268f696be8906f Mon Sep 17 00:00:00 2001 From: Kjell Winblad Date: Tue, 12 Mar 2024 17:22:22 +0100 Subject: [PATCH 4/4] fix(DynamoDB connector): fix error in status check when no workers --- apps/emqx_bridge_dynamo/src/emqx_bridge_dynamo_connector.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/emqx_bridge_dynamo/src/emqx_bridge_dynamo_connector.erl b/apps/emqx_bridge_dynamo/src/emqx_bridge_dynamo_connector.erl index 1d29bd427..da71abea7 100644 --- a/apps/emqx_bridge_dynamo/src/emqx_bridge_dynamo_connector.erl +++ b/apps/emqx_bridge_dynamo/src/emqx_bridge_dynamo_connector.erl @@ -193,7 +193,7 @@ on_get_status(_InstanceId, #{pool_name := Pool} = State) -> case Health of {error, timeout} -> {?status_connecting, State, <<"timeout_while_checking_connection">>}; - {ok, [_ | _] = Results} -> + {ok, Results} -> status_result(Results, State) end.