From 08121e7df6cb98b1fa60c40f35f86a56d6b3b929 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Thu, 10 Nov 2022 13:45:47 +0800 Subject: [PATCH 01/18] fix(mgmt): optimize the speed of query tail pages In the previous, when you query the tail pages, all the front of rows will be queried out and formatted. It greatly hurts the speed of query. Currently, we only format the final result rows. i.e, the query for the last page of data will be 10x faster. --- apps/emqx/src/emqx_alarm.erl | 14 +- .../emqx_enhanced_authn_scram_mnesia.erl | 15 +- .../src/simple_authn/emqx_authn_mnesia.erl | 15 +- apps/emqx_authz/src/emqx_authz_api_mnesia.erl | 18 +- .../src/emqx_gateway_api_clients.erl | 22 +- apps/emqx_management/src/emqx_mgmt_api.erl | 239 ++++++++++-------- .../src/emqx_mgmt_api_alarms.erl | 20 +- .../src/emqx_mgmt_api_clients.erl | 26 +- .../src/emqx_mgmt_api_subscriptions.erl | 22 +- .../src/emqx_mgmt_api_topics.erl | 7 +- apps/emqx_modules/src/emqx_delayed.erl | 7 +- .../src/emqx_rule_engine_api.erl | 5 +- 12 files changed, 227 insertions(+), 183 deletions(-) diff --git a/apps/emqx/src/emqx_alarm.erl b/apps/emqx/src/emqx_alarm.erl index a89063870..7368b442d 100644 --- a/apps/emqx/src/emqx_alarm.erl +++ b/apps/emqx/src/emqx_alarm.erl @@ -41,7 +41,8 @@ delete_all_deactivated_alarms/0, get_alarms/0, get_alarms/1, - format/1 + format/1, + format/2 ]). %% gen_server callbacks @@ -169,12 +170,15 @@ get_alarms(activated) -> get_alarms(deactivated) -> gen_server:call(?MODULE, {get_alarms, deactivated}). -format(#activated_alarm{name = Name, message = Message, activate_at = At, details = Details}) -> +format(Alarm) -> + format(node(), Alarm). + +format(Node, #activated_alarm{name = Name, message = Message, activate_at = At, details = Details}) -> Now = erlang:system_time(microsecond), %% mnesia db stored microsecond for high frequency alarm %% format for dashboard using millisecond #{ - node => node(), + node => Node, name => Name, message => Message, %% to millisecond @@ -182,7 +186,7 @@ format(#activated_alarm{name = Name, message = Message, activate_at = At, detail activate_at => to_rfc3339(At), details => Details }; -format(#deactivated_alarm{ +format(Node, #deactivated_alarm{ name = Name, message = Message, activate_at = At, @@ -190,7 +194,7 @@ format(#deactivated_alarm{ deactivate_at = DAt }) -> #{ - node => node(), + node => Node, name => Name, message => Message, %% to millisecond diff --git a/apps/emqx_authn/src/enhanced_authn/emqx_enhanced_authn_scram_mnesia.erl b/apps/emqx_authn/src/enhanced_authn/emqx_enhanced_authn_scram_mnesia.erl index cb3fad7dc..1fcc00dc9 100644 --- a/apps/emqx_authn/src/enhanced_authn/emqx_enhanced_authn_scram_mnesia.erl +++ b/apps/emqx_authn/src/enhanced_authn/emqx_enhanced_authn_scram_mnesia.erl @@ -262,7 +262,14 @@ lookup_user(UserID, #{user_group := UserGroup}) -> list_users(QueryString, #{user_group := UserGroup}) -> NQueryString = QueryString#{<<"user_group">> => UserGroup}, - emqx_mgmt_api:node_query(node(), NQueryString, ?TAB, ?AUTHN_QSCHEMA, ?QUERY_FUN). + emqx_mgmt_api:node_query( + node(), + NQueryString, + ?TAB, + ?AUTHN_QSCHEMA, + ?QUERY_FUN, + fun ?MODULE:format_user_info/1 + ). %%-------------------------------------------------------------------- %% Query Functions @@ -273,8 +280,7 @@ query(Tab, {QString, []}, Continuation, Limit) -> Tab, Ms, Continuation, - Limit, - fun format_user_info/1 + Limit ); query(Tab, {QString, FuzzyQString}, Continuation, Limit) -> Ms = ms_from_qstring(QString), @@ -283,8 +289,7 @@ query(Tab, {QString, FuzzyQString}, Continuation, Limit) -> Tab, {Ms, FuzzyFilterFun}, Continuation, - Limit, - fun format_user_info/1 + Limit ). %%-------------------------------------------------------------------- diff --git a/apps/emqx_authn/src/simple_authn/emqx_authn_mnesia.erl b/apps/emqx_authn/src/simple_authn/emqx_authn_mnesia.erl index 7276ad428..f84bfc195 100644 --- a/apps/emqx_authn/src/simple_authn/emqx_authn_mnesia.erl +++ b/apps/emqx_authn/src/simple_authn/emqx_authn_mnesia.erl @@ -288,7 +288,14 @@ lookup_user(UserID, #{user_group := UserGroup}) -> list_users(QueryString, #{user_group := UserGroup}) -> NQueryString = QueryString#{<<"user_group">> => UserGroup}, - emqx_mgmt_api:node_query(node(), NQueryString, ?TAB, ?AUTHN_QSCHEMA, ?QUERY_FUN). + emqx_mgmt_api:node_query( + node(), + NQueryString, + ?TAB, + ?AUTHN_QSCHEMA, + ?QUERY_FUN, + fun ?MODULE:format_user_info/1 + ). %%-------------------------------------------------------------------- %% Query Functions @@ -299,8 +306,7 @@ query(Tab, {QString, []}, Continuation, Limit) -> Tab, Ms, Continuation, - Limit, - fun format_user_info/1 + Limit ); query(Tab, {QString, FuzzyQString}, Continuation, Limit) -> Ms = ms_from_qstring(QString), @@ -309,8 +315,7 @@ query(Tab, {QString, FuzzyQString}, Continuation, Limit) -> Tab, {Ms, FuzzyFilterFun}, Continuation, - Limit, - fun format_user_info/1 + Limit ). %%-------------------------------------------------------------------- diff --git a/apps/emqx_authz/src/emqx_authz_api_mnesia.erl b/apps/emqx_authz/src/emqx_authz_api_mnesia.erl index 609974e98..8edd62e21 100644 --- a/apps/emqx_authz/src/emqx_authz_api_mnesia.erl +++ b/apps/emqx_authz/src/emqx_authz_api_mnesia.erl @@ -408,7 +408,8 @@ users(get, #{query_string := QueryString}) -> QueryString, ?ACL_TABLE, ?ACL_USERNAME_QSCHEMA, - ?QUERY_USERNAME_FUN + ?QUERY_USERNAME_FUN, + fun ?MODULE:format_result/1 ) of {error, page_limit_invalid} -> @@ -443,7 +444,8 @@ clients(get, #{query_string := QueryString}) -> QueryString, ?ACL_TABLE, ?ACL_CLIENTID_QSCHEMA, - ?QUERY_CLIENTID_FUN + ?QUERY_CLIENTID_FUN, + fun ?MODULE:format_result/1 ) of {error, page_limit_invalid} -> @@ -582,8 +584,7 @@ query_username(Tab, {_QString, []}, Continuation, Limit) -> Tab, Ms, Continuation, - Limit, - fun format_result/1 + Limit ); query_username(Tab, {_QString, FuzzyQString}, Continuation, Limit) -> Ms = emqx_authz_mnesia:list_username_rules(), @@ -592,8 +593,7 @@ query_username(Tab, {_QString, FuzzyQString}, Continuation, Limit) -> Tab, {Ms, FuzzyFilterFun}, Continuation, - Limit, - fun format_result/1 + Limit ). query_clientid(Tab, {_QString, []}, Continuation, Limit) -> @@ -602,8 +602,7 @@ query_clientid(Tab, {_QString, []}, Continuation, Limit) -> Tab, Ms, Continuation, - Limit, - fun format_result/1 + Limit ); query_clientid(Tab, {_QString, FuzzyQString}, Continuation, Limit) -> Ms = emqx_authz_mnesia:list_clientid_rules(), @@ -612,8 +611,7 @@ query_clientid(Tab, {_QString, FuzzyQString}, Continuation, Limit) -> Tab, {Ms, FuzzyFilterFun}, Continuation, - Limit, - fun format_result/1 + Limit ). %%-------------------------------------------------------------------- diff --git a/apps/emqx_gateway/src/emqx_gateway_api_clients.erl b/apps/emqx_gateway/src/emqx_gateway_api_clients.erl index 5f6cc25b6..f0fbcf968 100644 --- a/apps/emqx_gateway/src/emqx_gateway_api_clients.erl +++ b/apps/emqx_gateway/src/emqx_gateway_api_clients.erl @@ -56,7 +56,8 @@ %% internal exports (for client query) -export([ query/4, - format_channel_info/1 + format_channel_info/1, + format_channel_info/2 ]). -define(TAGS, [<<"Gateway Clients">>]). @@ -112,7 +113,8 @@ clients(get, #{ QString, TabName, ?CLIENT_QSCHEMA, - ?QUERY_FUN + ?QUERY_FUN, + fun ?MODULE:format_channel_info/2 ); Node0 -> case emqx_misc:safe_to_existing_atom(Node0) of @@ -123,7 +125,8 @@ clients(get, #{ QStringWithoutNode, TabName, ?CLIENT_QSCHEMA, - ?QUERY_FUN + ?QUERY_FUN, + fun ?MODULE:format_channel_info/2 ); {error, _} -> {error, Node0, {badrpc, <<"invalid node">>}} @@ -272,8 +275,7 @@ query(Tab, {Qs, []}, Continuation, Limit) -> Tab, Ms, Continuation, - Limit, - fun format_channel_info/1 + Limit ); query(Tab, {Qs, Fuzzy}, Continuation, Limit) -> Ms = qs2ms(Qs), @@ -282,8 +284,7 @@ query(Tab, {Qs, Fuzzy}, Continuation, Limit) -> Tab, {Ms, FuzzyFilterFun}, Continuation, - Limit, - fun format_channel_info/1 + Limit ). qs2ms(Qs) -> @@ -363,8 +364,11 @@ run_fuzzy_filter( %%-------------------------------------------------------------------- %% format funcs -format_channel_info({_, Infos, Stats} = R) -> - Node = maps:get(node, Infos, node()), +format_channel_info(ChannInfo) -> + format_channel_info(node(), ChannInfo). + +format_channel_info(WhichNode, {_, Infos, Stats} = R) -> + Node = maps:get(node, Infos, WhichNode), ClientInfo = maps:get(clientinfo, Infos, #{}), ConnInfo = maps:get(conninfo, Infos, #{}), SessInfo = maps:get(session, Infos, #{}), diff --git a/apps/emqx_management/src/emqx_mgmt_api.erl b/apps/emqx_management/src/emqx_mgmt_api.erl index 342228b19..cc0a81864 100644 --- a/apps/emqx_management/src/emqx_mgmt_api.erl +++ b/apps/emqx_management/src/emqx_mgmt_api.erl @@ -29,9 +29,9 @@ %% first_next query APIs -export([ - node_query/5, - cluster_query/4, - select_table_with_count/5, + node_query/6, + cluster_query/5, + select_table_with_count/4, b2i/1 ]). @@ -117,30 +117,24 @@ limit(Params) when is_map(Params) -> limit(Params) -> proplists:get_value(<<"limit">>, Params, emqx_mgmt:max_row_limit()). -init_meta(Params) -> - Limit = b2i(limit(Params)), - Page = b2i(page(Params)), - #{ - page => Page, - limit => Limit, - count => 0 - }. - %%-------------------------------------------------------------------- %% Node Query %%-------------------------------------------------------------------- -node_query(Node, QString, Tab, QSchema, QueryFun) -> - {_CodCnt, NQString} = parse_qstring(QString, QSchema), - page_limit_check_query( - init_meta(QString), - {fun do_node_query/5, [Node, Tab, NQString, QueryFun, init_meta(QString)]} - ). +node_query(Node, QString, Tab, QSchema, QueryFun, FmtFun) -> + case parse_pager_params(QString) of + false -> + {error, page_limit_invalid}; + Meta -> + {_CodCnt, NQString} = parse_qstring(QString, QSchema), + ResultAcc = #{cursor => 0, count => 0, rows => []}, + NResultAcc = do_node_query( + Node, Tab, NQString, QueryFun, ?FRESH_SELECT, Meta, ResultAcc + ), + format_query_result(FmtFun, Meta, NResultAcc) + end. %% @private -do_node_query(Node, Tab, QString, QueryFun, Meta) -> - do_node_query(Node, Tab, QString, QueryFun, _Continuation = ?FRESH_SELECT, Meta, _Results = []). - do_node_query( Node, Tab, @@ -148,45 +142,44 @@ do_node_query( QueryFun, Continuation, Meta = #{limit := Limit}, - Results + ResultAcc ) -> case do_query(Node, Tab, QString, QueryFun, Continuation, Limit) of {error, {badrpc, R}} -> {error, Node, {badrpc, R}}; - {Len, Rows, ?FRESH_SELECT} -> - {NMeta, NResults} = sub_query_result(Len, Rows, Limit, Results, Meta), - #{meta => NMeta, data => NResults}; - {Len, Rows, NContinuation} -> - {NMeta, NResults} = sub_query_result(Len, Rows, Limit, Results, Meta), - do_node_query(Node, Tab, QString, QueryFun, NContinuation, NMeta, NResults) + {Rows, ?FRESH_SELECT} -> + {_, NResultAcc} = accumulate_query_rows(Node, Rows, ResultAcc, Meta), + NResultAcc; + {Rows, NContinuation} -> + case accumulate_query_rows(Node, Rows, ResultAcc, Meta) of + {enough, NResultAcc} -> + NResultAcc; + {more, NResultAcc} -> + do_node_query(Node, Tab, QString, QueryFun, NContinuation, Meta, NResultAcc) + end end. %%-------------------------------------------------------------------- %% Cluster Query %%-------------------------------------------------------------------- -cluster_query(QString, Tab, QSchema, QueryFun) -> - {_CodCnt, NQString} = parse_qstring(QString, QSchema), - Nodes = mria_mnesia:running_nodes(), - page_limit_check_query( - init_meta(QString), - {fun do_cluster_query/5, [Nodes, Tab, NQString, QueryFun, init_meta(QString)]} - ). +cluster_query(QString, Tab, QSchema, QueryFun, FmtFun) -> + case parse_pager_params(QString) of + false -> + {error, page_limit_invalid}; + Meta -> + {_CodCnt, NQString} = parse_qstring(QString, QSchema), + Nodes = mria_mnesia:running_nodes(), + ResultAcc = #{cursor => 0, count => 0, rows => []}, + NResultAcc = do_cluster_query( + Nodes, Tab, NQString, QueryFun, ?FRESH_SELECT, Meta, ResultAcc + ), + format_query_result(FmtFun, Meta, NResultAcc) + end. %% @private -do_cluster_query(Nodes, Tab, QString, QueryFun, Meta) -> - do_cluster_query( - Nodes, - Tab, - QString, - QueryFun, - _Continuation = ?FRESH_SELECT, - Meta, - _Results = [] - ). - -do_cluster_query([], _Tab, _QString, _QueryFun, _Continuation, Meta, Results) -> - #{meta => Meta, data => Results}; +do_cluster_query([], _Tab, _QString, _QueryFun, _Continuation, _Meta, ResultAcc) -> + ResultAcc; do_cluster_query( [Node | Tail] = Nodes, Tab, @@ -194,17 +187,27 @@ do_cluster_query( QueryFun, Continuation, Meta = #{limit := Limit}, - Results + ResultAcc ) -> case do_query(Node, Tab, QString, QueryFun, Continuation, Limit) of {error, {badrpc, R}} -> - {error, Node, {bar_rpc, R}}; - {Len, Rows, ?FRESH_SELECT} -> - {NMeta, NResults} = sub_query_result(Len, Rows, Limit, Results, Meta), - do_cluster_query(Tail, Tab, QString, QueryFun, ?FRESH_SELECT, NMeta, NResults); - {Len, Rows, NContinuation} -> - {NMeta, NResults} = sub_query_result(Len, Rows, Limit, Results, Meta), - do_cluster_query(Nodes, Tab, QString, QueryFun, NContinuation, NMeta, NResults) + {error, Node, {badrpc, R}}; + {Rows, NContinuation} -> + case accumulate_query_rows(Node, Rows, ResultAcc, Meta) of + {enough, NResultAcc} -> + NResultAcc; + {more, NResultAcc} -> + case NContinuation of + ?FRESH_SELECT -> + do_cluster_query( + Tail, Tab, QString, QueryFun, ?FRESH_SELECT, Meta, NResultAcc + ); + _ -> + do_cluster_query( + Nodes, Tab, QString, QueryFun, NContinuation, Meta, NResultAcc + ) + end + end end. %%-------------------------------------------------------------------- @@ -228,60 +231,76 @@ do_query(Node, Tab, QString, QueryFun, Continuation, Limit) -> Ret -> Ret end. -sub_query_result(Len, Rows, Limit, Results, Meta) -> - {Flag, NMeta} = judge_page_with_counting(Len, Meta), - NResults = - case Flag of - more -> - []; - cutrows -> - {SubStart, NeedNowNum} = rows_sub_params(Len, NMeta), - ThisRows = lists:sublist(Rows, SubStart, NeedNowNum), - lists:sublist(lists:append(Results, ThisRows), SubStart, Limit); - enough -> - lists:sublist(lists:append(Results, Rows), 1, Limit) - end, - {NMeta, NResults}. +%% ResultAcc :: #{count := integer(), +%% cursor := integer(), +%% rows := [{node(), Rows :: list()}] +%% } +accumulate_query_rows( + Node, + Rows, + ResultAcc = #{cursor := Cursor, count := Count, rows := RowsAcc}, + _Meta = #{page := Page, limit := Limit} +) -> + PageStart = (Page - 1) * Limit + 1, + PageEnd = Page * Limit, + Len = length(Rows), + case Cursor + Len of + NCursor when NCursor < PageStart -> + {more, ResultAcc#{cursor => NCursor}}; + NCursor when NCursor < PageEnd -> + {more, ResultAcc#{ + cursor => NCursor, + count => Count + length(Rows), + rows => [{Node, Rows} | RowsAcc] + }}; + NCursor when NCursor >= PageEnd -> + SubRows = lists:sublist(Rows, Limit - Count), + {enough, ResultAcc#{ + cursor => NCursor, + count => Count + length(SubRows), + rows => [{Node, SubRows} | RowsAcc] + }} + end. %%-------------------------------------------------------------------- %% Table Select %%-------------------------------------------------------------------- -select_table_with_count(Tab, {Ms, FuzzyFilterFun}, ?FRESH_SELECT, Limit, FmtFun) when +select_table_with_count(Tab, {Ms, FuzzyFilterFun}, ?FRESH_SELECT, Limit) when is_function(FuzzyFilterFun) andalso Limit > 0 -> case ets:select(Tab, Ms, Limit) of '$end_of_table' -> - {0, [], ?FRESH_SELECT}; + {[], ?FRESH_SELECT}; {RawResult, NContinuation} -> Rows = FuzzyFilterFun(RawResult), - {length(Rows), lists:map(FmtFun, Rows), NContinuation} + {Rows, NContinuation} end; -select_table_with_count(_Tab, {Ms, FuzzyFilterFun}, Continuation, _Limit, FmtFun) when +select_table_with_count(_Tab, {Ms, FuzzyFilterFun}, Continuation, _Limit) when is_function(FuzzyFilterFun) -> case ets:select(ets:repair_continuation(Continuation, Ms)) of '$end_of_table' -> - {0, [], ?FRESH_SELECT}; + {[], ?FRESH_SELECT}; {RawResult, NContinuation} -> Rows = FuzzyFilterFun(RawResult), - {length(Rows), lists:map(FmtFun, Rows), NContinuation} + {Rows, NContinuation} end; -select_table_with_count(Tab, Ms, ?FRESH_SELECT, Limit, FmtFun) when +select_table_with_count(Tab, Ms, ?FRESH_SELECT, Limit) when Limit > 0 -> case ets:select(Tab, Ms, Limit) of '$end_of_table' -> - {0, [], ?FRESH_SELECT}; + {[], ?FRESH_SELECT}; {RawResult, NContinuation} -> - {length(RawResult), lists:map(FmtFun, RawResult), NContinuation} + {RawResult, NContinuation} end; -select_table_with_count(_Tab, Ms, Continuation, _Limit, FmtFun) -> +select_table_with_count(_Tab, Ms, Continuation, _Limit) -> case ets:select(ets:repair_continuation(Continuation, Ms)) of '$end_of_table' -> - {0, [], ?FRESH_SELECT}; + {[], ?FRESH_SELECT}; {RawResult, NContinuation} -> - {length(RawResult), lists:map(FmtFun, RawResult), NContinuation} + {RawResult, NContinuation} end. %%-------------------------------------------------------------------- @@ -379,40 +398,38 @@ is_fuzzy_key(<<"match_", _/binary>>) -> is_fuzzy_key(_) -> false. -page_start(1, _) -> 1; -page_start(Page, Limit) -> (Page - 1) * Limit + 1. +format_query_result(_FmtFun, _Meta, Error = {error, _Node, _Reason}) -> + Error; +format_query_result( + FmtFun, Meta, _ResultAcc = #{count := _Count, cursor := Cursor, rows := RowsAcc} +) -> + #{ + meta => Meta#{count => Cursor}, + data => lists:flatten( + lists:foldr( + fun({Node, Rows}, Acc) -> + [lists:map(fun(Row) -> exec_format_fun(FmtFun, Node, Row) end, Rows) | Acc] + end, + [], + RowsAcc + ) + ) + }. -judge_page_with_counting(Len, Meta = #{page := Page, limit := Limit, count := Count}) -> - PageStart = page_start(Page, Limit), - PageEnd = Page * Limit, - case Count + Len of - NCount when NCount < PageStart -> - {more, Meta#{count => NCount}}; - NCount when NCount < PageEnd -> - {cutrows, Meta#{count => NCount}}; - NCount when NCount >= PageEnd -> - {enough, Meta#{count => NCount}} +exec_format_fun(FmtFun, Node, Row) -> + case erlang:fun_info(FmtFun, arity) of + {arity, 1} -> FmtFun(Row); + {arity, 2} -> FmtFun(Node, Row) end. -rows_sub_params(Len, _Meta = #{page := Page, limit := Limit, count := Count}) -> - PageStart = page_start(Page, Limit), - case (Count - Len) < PageStart of +parse_pager_params(Params) -> + Page = b2i(page(Params)), + Limit = b2i(limit(Params)), + case Page > 0 andalso Limit > 0 of true -> - NeedNowNum = Count - PageStart + 1, - SubStart = Len - NeedNowNum + 1, - {SubStart, NeedNowNum}; + #{page => Page, limit => Limit, count => 0}; false -> - {_SubStart = 1, _NeedNowNum = Len} - end. - -page_limit_check_query(Meta, {F, A}) -> - case Meta of - #{page := Page, limit := Limit} when - Page < 1; Limit < 1 - -> - {error, page_limit_invalid}; - _ -> - erlang:apply(F, A) + false end. %%-------------------------------------------------------------------- diff --git a/apps/emqx_management/src/emqx_mgmt_api_alarms.erl b/apps/emqx_management/src/emqx_mgmt_api_alarms.erl index 36845e4e7..d574ad4ee 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_alarms.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_alarms.erl @@ -24,7 +24,7 @@ -export([api_spec/0, paths/0, schema/1, fields/1]). --export([alarms/2]). +-export([alarms/2, format_alarm/2]). -define(TAGS, [<<"Alarms">>]). @@ -112,7 +112,15 @@ alarms(get, #{query_string := QString}) -> true -> ?ACTIVATED_ALARM; false -> ?DEACTIVATED_ALARM end, - case emqx_mgmt_api:cluster_query(QString, Table, [], {?MODULE, query}) of + case + emqx_mgmt_api:cluster_query( + QString, + Table, + [], + {?MODULE, query}, + fun ?MODULE:format_alarm/2 + ) + of {error, page_limit_invalid} -> {400, #{code => <<"INVALID_PARAMETER">>, message => <<"page_limit_invalid">>}}; {error, Node, {badrpc, R}} -> @@ -130,9 +138,7 @@ alarms(delete, _Params) -> query(Table, _QsSpec, Continuation, Limit) -> Ms = [{'$1', [], ['$1']}], - emqx_mgmt_api:select_table_with_count(Table, Ms, Continuation, Limit, fun format_alarm/1). + emqx_mgmt_api:select_table_with_count(Table, Ms, Continuation, Limit). -format_alarm(Alarms) when is_list(Alarms) -> - [emqx_alarm:format(Alarm) || Alarm <- Alarms]; -format_alarm(Alarm) -> - emqx_alarm:format(Alarm). +format_alarm(WhichNode, Alarm) -> + emqx_alarm:format(WhichNode, Alarm). diff --git a/apps/emqx_management/src/emqx_mgmt_api_clients.erl b/apps/emqx_management/src/emqx_mgmt_api_clients.erl index f4fe0387f..56730fe3e 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_clients.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_clients.erl @@ -47,7 +47,8 @@ -export([ query/4, - format_channel_info/1 + format_channel_info/1, + format_channel_info/2 ]). %% for batch operation @@ -645,7 +646,8 @@ list_clients(QString) -> QString, ?CLIENT_QTAB, ?CLIENT_QSCHEMA, - ?QUERY_FUN + ?QUERY_FUN, + fun ?MODULE:format_channel_info/2 ); Node0 -> case emqx_misc:safe_to_existing_atom(Node0) of @@ -656,7 +658,8 @@ list_clients(QString) -> QStringWithoutNode, ?CLIENT_QTAB, ?CLIENT_QSCHEMA, - ?QUERY_FUN + ?QUERY_FUN, + fun ?MODULE:format_channel_info/2 ); {error, _} -> {error, Node0, {badrpc, <<"invalid node">>}} @@ -789,8 +792,7 @@ query(Tab, {QString, []}, Continuation, Limit) -> Tab, Ms, Continuation, - Limit, - fun format_channel_info/1 + Limit ); query(Tab, {QString, FuzzyQString}, Continuation, Limit) -> Ms = qs2ms(QString), @@ -799,8 +801,7 @@ query(Tab, {QString, FuzzyQString}, Continuation, Limit) -> Tab, {Ms, FuzzyFilterFun}, Continuation, - Limit, - fun format_channel_info/1 + Limit ). %%-------------------------------------------------------------------- @@ -876,12 +877,11 @@ run_fuzzy_filter(E = {_, #{clientinfo := ClientInfo}, _}, [{Key, like, SubStr} | %%-------------------------------------------------------------------- %% format funcs -format_channel_info({_, ClientInfo0, ClientStats}) -> - Node = - case ClientInfo0 of - #{node := N} -> N; - _ -> node() - end, +format_channel_info(ChannInfo = {_, _ClientInfo, _ClientStats}) -> + format_channel_info(node(), ChannInfo). + +format_channel_info(WhichNode, {_, ClientInfo0, ClientStats}) -> + Node = maps:get(node, ClientInfo0, WhichNode), ClientInfo1 = emqx_map_lib:deep_remove([conninfo, clientid], ClientInfo0), ClientInfo2 = emqx_map_lib:deep_remove([conninfo, username], ClientInfo1), StatsMap = maps:without( diff --git a/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl b/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl index 03b833e84..eb0f83cc0 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl @@ -33,7 +33,7 @@ -export([ query/4, - format/1 + format/2 ]). -define(SUBS_QTABLE, emqx_suboption). @@ -142,7 +142,8 @@ subscriptions(get, #{query_string := QString}) -> QString, ?SUBS_QTABLE, ?SUBS_QSCHEMA, - ?QUERY_FUN + ?QUERY_FUN, + fun ?MODULE:format/2 ); Node0 -> case emqx_misc:safe_to_existing_atom(Node0) of @@ -152,7 +153,8 @@ subscriptions(get, #{query_string := QString}) -> QString, ?SUBS_QTABLE, ?SUBS_QSCHEMA, - ?QUERY_FUN + ?QUERY_FUN, + fun ?MODULE:format/2 ); {error, _} -> {error, Node0, {badrpc, <<"invalid node">>}} @@ -168,16 +170,12 @@ subscriptions(get, #{query_string := QString}) -> {200, Result} end. -format(Items) when is_list(Items) -> - [format(Item) || Item <- Items]; -format({{Subscriber, Topic}, Options}) -> - format({Subscriber, Topic, Options}); -format({_Subscriber, Topic, Options}) -> +format(WhichNode, {{_Subscriber, Topic}, Options}) -> maps:merge( #{ topic => get_topic(Topic, Options), clientid => maps:get(subid, Options), - node => node() + node => WhichNode }, maps:with([qos, nl, rap, rh], Options) ). @@ -199,8 +197,7 @@ query(Tab, {Qs, []}, Continuation, Limit) -> Tab, Ms, Continuation, - Limit, - fun format/1 + Limit ); query(Tab, {Qs, Fuzzy}, Continuation, Limit) -> Ms = qs2ms(Qs), @@ -209,8 +206,7 @@ query(Tab, {Qs, Fuzzy}, Continuation, Limit) -> Tab, {Ms, FuzzyFilterFun}, Continuation, - Limit, - fun format/1 + Limit ). fuzzy_filter_fun(Fuzzy) -> diff --git a/apps/emqx_management/src/emqx_mgmt_api_topics.erl b/apps/emqx_management/src/emqx_mgmt_api_topics.erl index c357855b2..3cc2168e7 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_topics.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_topics.erl @@ -109,7 +109,12 @@ topic(get, #{bindings := Bindings}) -> do_list(Params) -> case emqx_mgmt_api:node_query( - node(), Params, emqx_route, ?TOPICS_QUERY_SCHEMA, {?MODULE, query} + node(), + Params, + emqx_route, + ?TOPICS_QUERY_SCHEMA, + {?MODULE, query}, + fun ?MODULE:format/1 ) of {error, page_limit_invalid} -> diff --git a/apps/emqx_modules/src/emqx_delayed.erl b/apps/emqx_modules/src/emqx_delayed.erl index f511d74d9..44e7a85e3 100644 --- a/apps/emqx_modules/src/emqx_delayed.erl +++ b/apps/emqx_modules/src/emqx_delayed.erl @@ -166,11 +166,14 @@ list(Params) -> emqx_mgmt_api:paginate(?TAB, Params, ?FORMAT_FUN). cluster_list(Params) -> - emqx_mgmt_api:cluster_query(Params, ?TAB, [], {?MODULE, cluster_query}). + %% FIXME: why cluster_query??? + emqx_mgmt_api:cluster_query( + Params, ?TAB, [], {?MODULE, cluster_query}, fun ?MODULE:format_delayed/1 + ). cluster_query(Table, _QsSpec, Continuation, Limit) -> Ms = [{'$1', [], ['$1']}], - emqx_mgmt_api:select_table_with_count(Table, Ms, Continuation, Limit, fun format_delayed/1). + emqx_mgmt_api:select_table_with_count(Table, Ms, Continuation, Limit). format_delayed(Delayed) -> format_delayed(Delayed, false). diff --git a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl index 597ee838f..2157b108c 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl +++ b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl @@ -277,7 +277,8 @@ param_path_id() -> QueryString, ?RULE_TAB, ?RULE_QS_SCHEMA, - {?MODULE, query} + {?MODULE, query}, + fun ?MODULE:format_rule_resp/1 ) of {error, page_limit_invalid} -> @@ -556,7 +557,7 @@ query(Tab, {Qs, Fuzzy}, Start, Limit) -> Ms = qs2ms(), FuzzyFun = fuzzy_match_fun(Qs, Ms, Fuzzy), emqx_mgmt_api:select_table_with_count( - Tab, {Ms, FuzzyFun}, Start, Limit, fun format_rule_resp/1 + Tab, {Ms, FuzzyFun}, Start, Limit ). %% rule is not a record, so everything is fuzzy filter. From 1fe9c105aa6a801346a0ec0e03b710a7c5a57d56 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Wed, 16 Nov 2022 15:49:06 +0800 Subject: [PATCH 02/18] refactor(mgmt): smplify the node_query/cluster_query implementation --- .../emqx_enhanced_authn_scram_mnesia.erl | 34 +--- .../src/simple_authn/emqx_authn_mnesia.erl | 34 +--- apps/emqx_authz/src/emqx_authz_api_mnesia.erl | 62 ++---- .../src/emqx_gateway_api_clients.erl | 36 ++-- apps/emqx_management/src/emqx_mgmt_api.erl | 177 ++++++++++-------- .../src/emqx_mgmt_api_alarms.erl | 12 +- .../src/emqx_mgmt_api_clients.erl | 38 ++-- .../src/emqx_mgmt_api_subscriptions.erl | 73 +++----- .../src/emqx_mgmt_api_topics.erl | 21 +-- apps/emqx_modules/src/emqx_delayed.erl | 18 +- .../src/emqx_rule_engine_api.erl | 13 +- 11 files changed, 217 insertions(+), 301 deletions(-) diff --git a/apps/emqx_authn/src/enhanced_authn/emqx_enhanced_authn_scram_mnesia.erl b/apps/emqx_authn/src/enhanced_authn/emqx_enhanced_authn_scram_mnesia.erl index 1fcc00dc9..dfa2f32ef 100644 --- a/apps/emqx_authn/src/enhanced_authn/emqx_enhanced_authn_scram_mnesia.erl +++ b/apps/emqx_authn/src/enhanced_authn/emqx_enhanced_authn_scram_mnesia.erl @@ -47,7 +47,7 @@ ]). -export([ - query/4, + qs2ms/2, format_user_info/1, group_match_spec/1 ]). @@ -66,7 +66,6 @@ {<<"user_group">>, binary}, {<<"is_superuser">>, atom} ]). --define(QUERY_FUN, {?MODULE, query}). -type user_group() :: binary(). @@ -264,38 +263,23 @@ list_users(QueryString, #{user_group := UserGroup}) -> NQueryString = QueryString#{<<"user_group">> => UserGroup}, emqx_mgmt_api:node_query( node(), - NQueryString, ?TAB, + NQueryString, ?AUTHN_QSCHEMA, - ?QUERY_FUN, + fun ?MODULE:qs2ms/2, fun ?MODULE:format_user_info/1 ). %%-------------------------------------------------------------------- -%% Query Functions +%% QueryString to MatchSpec -query(Tab, {QString, []}, Continuation, Limit) -> - Ms = ms_from_qstring(QString), - emqx_mgmt_api:select_table_with_count( - Tab, - Ms, - Continuation, - Limit - ); -query(Tab, {QString, FuzzyQString}, Continuation, Limit) -> - Ms = ms_from_qstring(QString), - FuzzyFilterFun = fuzzy_filter_fun(FuzzyQString), - emqx_mgmt_api:select_table_with_count( - Tab, - {Ms, FuzzyFilterFun}, - Continuation, - Limit - ). - -%%-------------------------------------------------------------------- -%% Match funcs +-spec qs2ms(atom(), {list(), list()}) -> {ets:match_spec(), fun() | undefined}. +qs2ms(_Tab, {QString, Fuzzy}) -> + {ms_from_qstring(QString), fuzzy_filter_fun(Fuzzy)}. %% Fuzzy username funcs +fuzzy_filter_fun([]) -> + undefined; fuzzy_filter_fun(Fuzzy) -> fun(MsRaws) when is_list(MsRaws) -> lists:filter( diff --git a/apps/emqx_authn/src/simple_authn/emqx_authn_mnesia.erl b/apps/emqx_authn/src/simple_authn/emqx_authn_mnesia.erl index f84bfc195..50edb6612 100644 --- a/apps/emqx_authn/src/simple_authn/emqx_authn_mnesia.erl +++ b/apps/emqx_authn/src/simple_authn/emqx_authn_mnesia.erl @@ -49,7 +49,7 @@ ]). -export([ - query/4, + qs2ms/2, format_user_info/1, group_match_spec/1 ]). @@ -84,7 +84,6 @@ {<<"user_group">>, binary}, {<<"is_superuser">>, atom} ]). --define(QUERY_FUN, {?MODULE, query}). %%------------------------------------------------------------------------------ %% Mnesia bootstrap @@ -290,38 +289,23 @@ list_users(QueryString, #{user_group := UserGroup}) -> NQueryString = QueryString#{<<"user_group">> => UserGroup}, emqx_mgmt_api:node_query( node(), - NQueryString, ?TAB, + NQueryString, ?AUTHN_QSCHEMA, - ?QUERY_FUN, + fun ?MODULE:qs2ms/2, fun ?MODULE:format_user_info/1 ). %%-------------------------------------------------------------------- -%% Query Functions +%% QueryString to MatchSpec -query(Tab, {QString, []}, Continuation, Limit) -> - Ms = ms_from_qstring(QString), - emqx_mgmt_api:select_table_with_count( - Tab, - Ms, - Continuation, - Limit - ); -query(Tab, {QString, FuzzyQString}, Continuation, Limit) -> - Ms = ms_from_qstring(QString), - FuzzyFilterFun = fuzzy_filter_fun(FuzzyQString), - emqx_mgmt_api:select_table_with_count( - Tab, - {Ms, FuzzyFilterFun}, - Continuation, - Limit - ). - -%%-------------------------------------------------------------------- -%% Match funcs +-spec qs2ms(atom(), {list(), list()}) -> {ets:match_spec(), fun() | undefined}. +qs2ms(_Tab, {QString, FuzzyQString}) -> + {ms_from_qstring(QString), fuzzy_filter_fun(FuzzyQString)}. %% Fuzzy username funcs +fuzzy_filter_fun([]) -> + undefined; fuzzy_filter_fun(Fuzzy) -> fun(MsRaws) when is_list(MsRaws) -> lists:filter( diff --git a/apps/emqx_authz/src/emqx_authz_api_mnesia.erl b/apps/emqx_authz/src/emqx_authz_api_mnesia.erl index 8edd62e21..1ab7feea8 100644 --- a/apps/emqx_authz/src/emqx_authz_api_mnesia.erl +++ b/apps/emqx_authz/src/emqx_authz_api_mnesia.erl @@ -24,8 +24,8 @@ -import(hoconsc, [mk/1, mk/2, ref/1, ref/2, array/1, enum/1]). --define(QUERY_USERNAME_FUN, {?MODULE, query_username}). --define(QUERY_CLIENTID_FUN, {?MODULE, query_clientid}). +-define(QUERY_USERNAME_FUN, fun ?MODULE:query_username/2). +-define(QUERY_CLIENTID_FUN, fun ?MODULE:query_clientid/2). -define(ACL_USERNAME_QSCHEMA, [{<<"like_username">>, binary}]). -define(ACL_CLIENTID_QSCHEMA, [{<<"like_clientid">>, binary}]). @@ -49,12 +49,11 @@ %% query funs -export([ - query_username/4, - query_clientid/4 + query_username/2, + query_clientid/2, + format_result/1 ]). --export([format_result/1]). - -define(BAD_REQUEST, 'BAD_REQUEST'). -define(NOT_FOUND, 'NOT_FOUND'). -define(ALREADY_EXISTS, 'ALREADY_EXISTS'). @@ -405,8 +404,8 @@ users(get, #{query_string := QueryString}) -> case emqx_mgmt_api:node_query( node(), - QueryString, ?ACL_TABLE, + QueryString, ?ACL_USERNAME_QSCHEMA, ?QUERY_USERNAME_FUN, fun ?MODULE:format_result/1 @@ -441,8 +440,8 @@ clients(get, #{query_string := QueryString}) -> case emqx_mgmt_api:node_query( node(), - QueryString, ?ACL_TABLE, + QueryString, ?ACL_CLIENTID_QSCHEMA, ?QUERY_CLIENTID_FUN, fun ?MODULE:format_result/1 @@ -576,48 +575,19 @@ purge(delete, _) -> end. %%-------------------------------------------------------------------- -%% Query Functions +%% QueryString to MatchSpec -query_username(Tab, {_QString, []}, Continuation, Limit) -> - Ms = emqx_authz_mnesia:list_username_rules(), - emqx_mgmt_api:select_table_with_count( - Tab, - Ms, - Continuation, - Limit - ); -query_username(Tab, {_QString, FuzzyQString}, Continuation, Limit) -> - Ms = emqx_authz_mnesia:list_username_rules(), - FuzzyFilterFun = fuzzy_filter_fun(FuzzyQString), - emqx_mgmt_api:select_table_with_count( - Tab, - {Ms, FuzzyFilterFun}, - Continuation, - Limit - ). +-spec query_username(atom(), {list(), list()}) -> {ets:match_spec(), fun() | undefined}. +query_username(_Tab, {_QString, FuzzyQString}) -> + {emqx_authz_mnesia:list_username_rules(), fuzzy_filter_fun(FuzzyQString)}. -query_clientid(Tab, {_QString, []}, Continuation, Limit) -> - Ms = emqx_authz_mnesia:list_clientid_rules(), - emqx_mgmt_api:select_table_with_count( - Tab, - Ms, - Continuation, - Limit - ); -query_clientid(Tab, {_QString, FuzzyQString}, Continuation, Limit) -> - Ms = emqx_authz_mnesia:list_clientid_rules(), - FuzzyFilterFun = fuzzy_filter_fun(FuzzyQString), - emqx_mgmt_api:select_table_with_count( - Tab, - {Ms, FuzzyFilterFun}, - Continuation, - Limit - ). - -%%-------------------------------------------------------------------- -%% Match funcs +-spec query_clientid(atom(), {list(), list()}) -> {ets:match_spec(), fun() | undefined}. +query_clientid(_Tab, {_QString, FuzzyQString}) -> + {emqx_authz_mnesia:list_clientid_rules(), fuzzy_filter_fun(FuzzyQString)}. %% Fuzzy username funcs +fuzzy_filter_fun([]) -> + undefined; fuzzy_filter_fun(Fuzzy) -> fun(MsRaws) when is_list(MsRaws) -> lists:filter( diff --git a/apps/emqx_gateway/src/emqx_gateway_api_clients.erl b/apps/emqx_gateway/src/emqx_gateway_api_clients.erl index f0fbcf968..ba0f34206 100644 --- a/apps/emqx_gateway/src/emqx_gateway_api_clients.erl +++ b/apps/emqx_gateway/src/emqx_gateway_api_clients.erl @@ -55,7 +55,7 @@ %% internal exports (for client query) -export([ - query/4, + qs2ms/2, format_channel_info/1, format_channel_info/2 ]). @@ -98,8 +98,6 @@ paths() -> {<<"lte_lifetime">>, integer} ]). --define(QUERY_FUN, {?MODULE, query}). - clients(get, #{ bindings := #{name := Name0}, query_string := QString @@ -110,10 +108,10 @@ clients(get, #{ case maps:get(<<"node">>, QString, undefined) of undefined -> emqx_mgmt_api:cluster_query( - QString, TabName, + QString, ?CLIENT_QSCHEMA, - ?QUERY_FUN, + fun ?MODULE:qs2ms/2, fun ?MODULE:format_channel_info/2 ); Node0 -> @@ -122,10 +120,10 @@ clients(get, #{ QStringWithoutNode = maps:without([<<"node">>], QString), emqx_mgmt_api:node_query( Node1, - QStringWithoutNode, TabName, + QStringWithoutNode, ?CLIENT_QSCHEMA, - ?QUERY_FUN, + fun ?MODULE:qs2ms/2, fun ?MODULE:format_channel_info/2 ); {error, _} -> @@ -267,25 +265,11 @@ extra_sub_props(Props) -> ). %%-------------------------------------------------------------------- -%% query funcs +%% QueryString to MatchSpec -query(Tab, {Qs, []}, Continuation, Limit) -> - Ms = qs2ms(Qs), - emqx_mgmt_api:select_table_with_count( - Tab, - Ms, - Continuation, - Limit - ); -query(Tab, {Qs, Fuzzy}, Continuation, Limit) -> - Ms = qs2ms(Qs), - FuzzyFilterFun = fuzzy_filter_fun(Fuzzy), - emqx_mgmt_api:select_table_with_count( - Tab, - {Ms, FuzzyFilterFun}, - Continuation, - Limit - ). +-spec qs2ms(atom(), {list(), list()}) -> {ets:match_spec(), fun() | undefined}. +qs2ms(_Tab, {Qs, Fuzzy}) -> + {qs2ms(Qs), fuzzy_filter_fun(Fuzzy)}. qs2ms(Qs) -> {MtchHead, Conds} = qs2ms(Qs, 2, {#{}, []}), @@ -340,6 +324,8 @@ ms(lifetime, X) -> %%-------------------------------------------------------------------- %% Fuzzy filter funcs +fuzzy_filter_fun([]) -> + undefined; fuzzy_filter_fun(Fuzzy) -> fun(MsRaws) when is_list(MsRaws) -> lists:filter( diff --git a/apps/emqx_management/src/emqx_mgmt_api.erl b/apps/emqx_management/src/emqx_mgmt_api.erl index cc0a81864..5fd16e2ff 100644 --- a/apps/emqx_management/src/emqx_mgmt_api.erl +++ b/apps/emqx_management/src/emqx_mgmt_api.erl @@ -31,11 +31,10 @@ -export([ node_query/6, cluster_query/5, - select_table_with_count/4, b2i/1 ]). --export([do_query/6]). +-export([do_query/5]). paginate(Tables, Params, {Module, FormatFun}) -> Qh = query_handle(Tables), @@ -121,15 +120,37 @@ limit(Params) -> %% Node Query %%-------------------------------------------------------------------- -node_query(Node, QString, Tab, QSchema, QueryFun, FmtFun) -> +-type query_params() :: list() | map(). + +-type query_schema() :: [{Key :: binary(), Type :: atom | integer | timestamp | ip | ip_port}]. + +-type query_to_match_spec_fun() :: + fun((list(), list()) -> {ets:match_spec(), fun()}). + +-type format_result_fun() :: + fun((node(), term()) -> term()) + | fun((term()) -> term()). + +-type query_return() :: #{meta := map(), data := [term()]}. + +-spec node_query( + node(), + atom(), + query_params(), + query_schema(), + query_to_match_spec_fun(), + format_result_fun() +) -> {error, page_limit_invalid} | {error, atom(), term()} | query_return(). +node_query(Node, Tab, QString, QSchema, MsFun, FmtFun) -> case parse_pager_params(QString) of false -> {error, page_limit_invalid}; Meta -> {_CodCnt, NQString} = parse_qstring(QString, QSchema), ResultAcc = #{cursor => 0, count => 0, rows => []}, + QueryState = init_query_state(Meta), NResultAcc = do_node_query( - Node, Tab, NQString, QueryFun, ?FRESH_SELECT, Meta, ResultAcc + Node, Tab, NQString, MsFun, QueryState, ResultAcc ), format_query_result(FmtFun, Meta, NResultAcc) end. @@ -139,31 +160,36 @@ do_node_query( Node, Tab, QString, - QueryFun, - Continuation, - Meta = #{limit := Limit}, + MsFun, + QueryState, ResultAcc ) -> - case do_query(Node, Tab, QString, QueryFun, Continuation, Limit) of + case do_query(Node, Tab, QString, MsFun, QueryState) of {error, {badrpc, R}} -> {error, Node, {badrpc, R}}; - {Rows, ?FRESH_SELECT} -> - {_, NResultAcc} = accumulate_query_rows(Node, Rows, ResultAcc, Meta), + {Rows, NQueryState = #{continuation := ?FRESH_SELECT}} -> + {_, NResultAcc} = accumulate_query_rows(Node, Rows, NQueryState, ResultAcc), NResultAcc; - {Rows, NContinuation} -> - case accumulate_query_rows(Node, Rows, ResultAcc, Meta) of + {Rows, NQueryState} -> + case accumulate_query_rows(Node, Rows, NQueryState, ResultAcc) of {enough, NResultAcc} -> NResultAcc; {more, NResultAcc} -> - do_node_query(Node, Tab, QString, QueryFun, NContinuation, Meta, NResultAcc) + do_node_query(Node, Tab, QString, MsFun, NQueryState, NResultAcc) end end. %%-------------------------------------------------------------------- %% Cluster Query %%-------------------------------------------------------------------- - -cluster_query(QString, Tab, QSchema, QueryFun, FmtFun) -> +-spec cluster_query( + atom(), + query_params(), + query_schema(), + query_to_match_spec_fun(), + format_result_fun() +) -> {error, page_limit_invalid} | {error, atom(), term()} | query_return(). +cluster_query(Tab, QString, QSchema, MsFun, FmtFun) -> case parse_pager_params(QString) of false -> {error, page_limit_invalid}; @@ -171,42 +197,38 @@ cluster_query(QString, Tab, QSchema, QueryFun, FmtFun) -> {_CodCnt, NQString} = parse_qstring(QString, QSchema), Nodes = mria_mnesia:running_nodes(), ResultAcc = #{cursor => 0, count => 0, rows => []}, + QueryState = init_query_state(Meta), NResultAcc = do_cluster_query( - Nodes, Tab, NQString, QueryFun, ?FRESH_SELECT, Meta, ResultAcc + Nodes, Tab, NQString, MsFun, QueryState, ResultAcc ), format_query_result(FmtFun, Meta, NResultAcc) end. %% @private -do_cluster_query([], _Tab, _QString, _QueryFun, _Continuation, _Meta, ResultAcc) -> +do_cluster_query([], _Tab, _QString, _QueryFun, _QueryState, ResultAcc) -> ResultAcc; do_cluster_query( [Node | Tail] = Nodes, Tab, QString, - QueryFun, - Continuation, - Meta = #{limit := Limit}, + MsFun, + QueryState, ResultAcc ) -> - case do_query(Node, Tab, QString, QueryFun, Continuation, Limit) of + case do_query(Node, Tab, QString, MsFun, QueryState) of {error, {badrpc, R}} -> {error, Node, {badrpc, R}}; - {Rows, NContinuation} -> - case accumulate_query_rows(Node, Rows, ResultAcc, Meta) of + {Rows, NQueryState} -> + case accumulate_query_rows(Node, Rows, NQueryState, ResultAcc) of {enough, NResultAcc} -> NResultAcc; {more, NResultAcc} -> - case NContinuation of - ?FRESH_SELECT -> - do_cluster_query( - Tail, Tab, QString, QueryFun, ?FRESH_SELECT, Meta, NResultAcc - ); - _ -> - do_cluster_query( - Nodes, Tab, QString, QueryFun, NContinuation, Meta, NResultAcc - ) - end + NextNodes = + case NQueryState of + #{continuation := ?FRESH_SELECT} -> Tail; + _ -> Nodes + end, + do_cluster_query(NextNodes, Tab, QString, MsFun, NQueryState, NResultAcc) end end. @@ -214,16 +236,31 @@ do_cluster_query( %% Do Query (or rpc query) %%-------------------------------------------------------------------- +%% QueryState :: +%% #{continuation := ets:continuation(), +%% page := pos_integer(), +%% limit := pos_integer(), +%% total := #{node() := non_neg_integer()} +%% } +init_query_state(_Meta = #{page := Page, limit := Limit}) -> + #{ + continuation => ?FRESH_SELECT, + page => Page, + limit => Limit, + total => [] + }. + %% @private This function is exempt from BPAPI -do_query(Node, Tab, QString, {M, F}, Continuation, Limit) when Node =:= node() -> - erlang:apply(M, F, [Tab, QString, Continuation, Limit]); -do_query(Node, Tab, QString, QueryFun, Continuation, Limit) -> +do_query(Node, Tab, QString, MsFun, QueryState) when Node =:= node(), is_function(MsFun) -> + {Ms, FuzzyFun} = erlang:apply(MsFun, [Tab, QString]), + do_select(Tab, Ms, FuzzyFun, QueryState); +do_query(Node, Tab, QString, MsFun, QueryState) when is_function(MsFun) -> case rpc:call( Node, ?MODULE, do_query, - [Node, Tab, QString, QueryFun, Continuation, Limit], + [Node, Tab, QString, MsFun, QueryState], 50000 ) of @@ -231,6 +268,31 @@ do_query(Node, Tab, QString, QueryFun, Continuation, Limit) -> Ret -> Ret end. +do_select( + Tab, + Ms, + FuzzyFun, + QueryState = #{continuation := Continuation, limit := Limit} +) -> + Result = + case Continuation of + ?FRESH_SELECT -> + ets:select(Tab, Ms, Limit); + _ -> + ets:select(ets:repair_continuation(Continuation, Ms)) + end, + case Result of + '$end_of_table' -> + {[], QueryState#{continuation => ?FRESH_SELECT}}; + {Rows, NContinuation} -> + NRows = + case is_function(FuzzyFun) of + true -> FuzzyFun(Rows); + false -> Rows + end, + {NRows, QueryState#{continuation => NContinuation}} + end. + %% ResultAcc :: #{count := integer(), %% cursor := integer(), %% rows := [{node(), Rows :: list()}] @@ -238,8 +300,8 @@ do_query(Node, Tab, QString, QueryFun, Continuation, Limit) -> accumulate_query_rows( Node, Rows, - ResultAcc = #{cursor := Cursor, count := Count, rows := RowsAcc}, - _Meta = #{page := Page, limit := Limit} + _QueryState = #{page := Page, limit := Limit}, + ResultAcc = #{cursor := Cursor, count := Count, rows := RowsAcc} ) -> PageStart = (Page - 1) * Limit + 1, PageEnd = Page * Limit, @@ -266,43 +328,6 @@ accumulate_query_rows( %% Table Select %%-------------------------------------------------------------------- -select_table_with_count(Tab, {Ms, FuzzyFilterFun}, ?FRESH_SELECT, Limit) when - is_function(FuzzyFilterFun) andalso Limit > 0 --> - case ets:select(Tab, Ms, Limit) of - '$end_of_table' -> - {[], ?FRESH_SELECT}; - {RawResult, NContinuation} -> - Rows = FuzzyFilterFun(RawResult), - {Rows, NContinuation} - end; -select_table_with_count(_Tab, {Ms, FuzzyFilterFun}, Continuation, _Limit) when - is_function(FuzzyFilterFun) --> - case ets:select(ets:repair_continuation(Continuation, Ms)) of - '$end_of_table' -> - {[], ?FRESH_SELECT}; - {RawResult, NContinuation} -> - Rows = FuzzyFilterFun(RawResult), - {Rows, NContinuation} - end; -select_table_with_count(Tab, Ms, ?FRESH_SELECT, Limit) when - Limit > 0 --> - case ets:select(Tab, Ms, Limit) of - '$end_of_table' -> - {[], ?FRESH_SELECT}; - {RawResult, NContinuation} -> - {RawResult, NContinuation} - end; -select_table_with_count(_Tab, Ms, Continuation, _Limit) -> - case ets:select(ets:repair_continuation(Continuation, Ms)) of - '$end_of_table' -> - {[], ?FRESH_SELECT}; - {RawResult, NContinuation} -> - {RawResult, NContinuation} - end. - %%-------------------------------------------------------------------- %% Internal Functions %%-------------------------------------------------------------------- diff --git a/apps/emqx_management/src/emqx_mgmt_api_alarms.erl b/apps/emqx_management/src/emqx_mgmt_api_alarms.erl index d574ad4ee..895a5bcf8 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_alarms.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_alarms.erl @@ -29,7 +29,7 @@ -define(TAGS, [<<"Alarms">>]). %% internal export (for query) --export([query/4]). +-export([qs2ms/2]). api_spec() -> emqx_dashboard_swagger:spec(?MODULE, #{check_schema => true}). @@ -114,10 +114,10 @@ alarms(get, #{query_string := QString}) -> end, case emqx_mgmt_api:cluster_query( - QString, Table, + QString, [], - {?MODULE, query}, + fun ?MODULE:qs2ms/2, fun ?MODULE:format_alarm/2 ) of @@ -136,9 +136,9 @@ alarms(delete, _Params) -> %%%============================================================================================== %% internal -query(Table, _QsSpec, Continuation, Limit) -> - Ms = [{'$1', [], ['$1']}], - emqx_mgmt_api:select_table_with_count(Table, Ms, Continuation, Limit). +-spec qs2ms(atom(), {list(), list()}) -> {ets:match_spec(), fun() | undefined}. +qs2ms(_Tab, {_Qs, _Fuzzy}) -> + {[{'$1', [], ['$1']}], undefined}. format_alarm(WhichNode, Alarm) -> emqx_alarm:format(WhichNode, Alarm). diff --git a/apps/emqx_management/src/emqx_mgmt_api_clients.erl b/apps/emqx_management/src/emqx_mgmt_api_clients.erl index 56730fe3e..cec9b9889 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_clients.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_clients.erl @@ -46,7 +46,7 @@ ]). -export([ - query/4, + qs2ms/2, format_channel_info/1, format_channel_info/2 ]). @@ -74,7 +74,6 @@ {<<"lte_connected_at">>, timestamp} ]). --define(QUERY_FUN, {?MODULE, query}). -define(FORMAT_FUN, {?MODULE, format_channel_info}). -define(CLIENT_ID_NOT_FOUND, @@ -643,10 +642,10 @@ list_clients(QString) -> case maps:get(<<"node">>, QString, undefined) of undefined -> emqx_mgmt_api:cluster_query( - QString, ?CLIENT_QTAB, + QString, ?CLIENT_QSCHEMA, - ?QUERY_FUN, + fun ?MODULE:qs2ms/2, fun ?MODULE:format_channel_info/2 ); Node0 -> @@ -655,10 +654,10 @@ list_clients(QString) -> QStringWithoutNode = maps:without([<<"node">>], QString), emqx_mgmt_api:node_query( Node1, - QStringWithoutNode, ?CLIENT_QTAB, + QStringWithoutNode, ?CLIENT_QSCHEMA, - ?QUERY_FUN, + fun ?MODULE:qs2ms/2, fun ?MODULE:format_channel_info/2 ); {error, _} -> @@ -783,30 +782,13 @@ do_unsubscribe(ClientID, Topic) -> Res end. -%%-------------------------------------------------------------------- -%% Query Functions - -query(Tab, {QString, []}, Continuation, Limit) -> - Ms = qs2ms(QString), - emqx_mgmt_api:select_table_with_count( - Tab, - Ms, - Continuation, - Limit - ); -query(Tab, {QString, FuzzyQString}, Continuation, Limit) -> - Ms = qs2ms(QString), - FuzzyFilterFun = fuzzy_filter_fun(FuzzyQString), - emqx_mgmt_api:select_table_with_count( - Tab, - {Ms, FuzzyFilterFun}, - Continuation, - Limit - ). - %%-------------------------------------------------------------------- %% QueryString to Match Spec +-spec qs2ms(atom(), {list(), list()}) -> {ets:match_spec(), fun() | undefined}. +qs2ms(_Tab, {QString, FuzzyQString}) -> + {qs2ms(QString), fuzzy_filter_fun(FuzzyQString)}. + -spec qs2ms(list()) -> ets:match_spec(). qs2ms(Qs) -> {MtchHead, Conds} = qs2ms(Qs, 2, {#{}, []}), @@ -856,6 +838,8 @@ ms(created_at, X) -> %%-------------------------------------------------------------------- %% Match funcs +fuzzy_filter_fun([]) -> + undefined; fuzzy_filter_fun(Fuzzy) -> fun(MsRaws) when is_list(MsRaws) -> lists:filter( diff --git a/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl b/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl index eb0f83cc0..e4678d641 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl @@ -32,7 +32,7 @@ -export([subscriptions/2]). -export([ - query/4, + qs2ms/2, format/2 ]). @@ -47,8 +47,6 @@ {<<"match_topic">>, binary} ]). --define(QUERY_FUN, {?MODULE, query}). - api_spec() -> emqx_dashboard_swagger:spec(?MODULE, #{check_schema => true}). @@ -139,10 +137,10 @@ subscriptions(get, #{query_string := QString}) -> case maps:get(<<"node">>, QString, undefined) of undefined -> emqx_mgmt_api:cluster_query( - QString, ?SUBS_QTABLE, + QString, ?SUBS_QSCHEMA, - ?QUERY_FUN, + fun ?MODULE:qs2ms/2, fun ?MODULE:format/2 ); Node0 -> @@ -150,10 +148,10 @@ subscriptions(get, #{query_string := QString}) -> {ok, Node1} -> emqx_mgmt_api:node_query( Node1, - QString, ?SUBS_QTABLE, + QString, ?SUBS_QSCHEMA, - ?QUERY_FUN, + fun ?MODULE:qs2ms/2, fun ?MODULE:format/2 ); {error, _} -> @@ -188,26 +186,30 @@ get_topic(Topic, _) -> Topic. %%-------------------------------------------------------------------- -%% Query Function +%% QueryString to MatchSpec %%-------------------------------------------------------------------- -query(Tab, {Qs, []}, Continuation, Limit) -> - Ms = qs2ms(Qs), - emqx_mgmt_api:select_table_with_count( - Tab, - Ms, - Continuation, - Limit - ); -query(Tab, {Qs, Fuzzy}, Continuation, Limit) -> - Ms = qs2ms(Qs), - FuzzyFilterFun = fuzzy_filter_fun(Fuzzy), - emqx_mgmt_api:select_table_with_count( - Tab, - {Ms, FuzzyFilterFun}, - Continuation, - Limit - ). +-spec qs2ms(atom(), {list(), list()}) -> {ets:match_spec(), fun() | undefined}. +qs2ms(_Tab, {Qs, Fuzzy}) -> + {gen_match_spec(Qs), fuzzy_filter_fun(Fuzzy)}. + +gen_match_spec(Qs) -> + MtchHead = gen_match_spec(Qs, {{'_', '_'}, #{}}), + [{MtchHead, [], ['$_']}]. + +gen_match_spec([], MtchHead) -> + MtchHead; +gen_match_spec([{Key, '=:=', Value} | More], MtchHead) -> + gen_match_spec(More, update_ms(Key, Value, MtchHead)). + +update_ms(clientid, X, {{Pid, Topic}, Opts}) -> + {{Pid, Topic}, Opts#{subid => X}}; +update_ms(topic, X, {{Pid, _Topic}, Opts}) -> + {{Pid, X}, Opts}; +update_ms(share_group, X, {{Pid, Topic}, Opts}) -> + {{Pid, Topic}, Opts#{share => X}}; +update_ms(qos, X, {{Pid, Topic}, Opts}) -> + {{Pid, Topic}, Opts#{qos => X}}. fuzzy_filter_fun(Fuzzy) -> fun(MsRaws) when is_list(MsRaws) -> @@ -221,24 +223,3 @@ run_fuzzy_filter(_, []) -> true; run_fuzzy_filter(E = {{_, Topic}, _}, [{topic, match, TopicFilter} | Fuzzy]) -> emqx_topic:match(Topic, TopicFilter) andalso run_fuzzy_filter(E, Fuzzy). - -%%-------------------------------------------------------------------- -%% Query String to Match Spec - -qs2ms(Qs) -> - MtchHead = qs2ms(Qs, {{'_', '_'}, #{}}), - [{MtchHead, [], ['$_']}]. - -qs2ms([], MtchHead) -> - MtchHead; -qs2ms([{Key, '=:=', Value} | More], MtchHead) -> - qs2ms(More, update_ms(Key, Value, MtchHead)). - -update_ms(clientid, X, {{Pid, Topic}, Opts}) -> - {{Pid, Topic}, Opts#{subid => X}}; -update_ms(topic, X, {{Pid, _Topic}, Opts}) -> - {{Pid, X}, Opts}; -update_ms(share_group, X, {{Pid, Topic}, Opts}) -> - {{Pid, Topic}, Opts#{share => X}}; -update_ms(qos, X, {{Pid, Topic}, Opts}) -> - {{Pid, Topic}, Opts#{qos => X}}. diff --git a/apps/emqx_management/src/emqx_mgmt_api_topics.erl b/apps/emqx_management/src/emqx_mgmt_api_topics.erl index 3cc2168e7..ca707ba20 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_topics.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_topics.erl @@ -34,7 +34,7 @@ topic/2 ]). --export([query/4]). +-export([qs2ms/2, format/1]). -define(TOPIC_NOT_FOUND, 'TOPIC_NOT_FOUND'). @@ -110,10 +110,10 @@ do_list(Params) -> case emqx_mgmt_api:node_query( node(), - Params, emqx_route, + Params, ?TOPICS_QUERY_SCHEMA, - {?MODULE, query}, + fun ?MODULE:qs2ms/2, fun ?MODULE:format/1 ) of @@ -143,16 +143,15 @@ generate_topic(Params = #{topic := Topic}) -> generate_topic(Params) -> Params. -query(Tab, {Qs, _}, Continuation, Limit) -> - Ms = qs2ms(Qs, [{{route, '_', '_'}, [], ['$_']}]), - emqx_mgmt_api:select_table_with_count(Tab, Ms, Continuation, Limit, fun format/1). +qs2ms(_Tab, {Qs, _}) -> + {gen_match_spec(Qs, [{{route, '_', '_'}, [], ['$_']}]), undefined}. -qs2ms([], Res) -> +gen_match_spec([], Res) -> Res; -qs2ms([{topic, '=:=', T} | Qs], [{{route, _, N}, [], ['$_']}]) -> - qs2ms(Qs, [{{route, T, N}, [], ['$_']}]); -qs2ms([{node, '=:=', N} | Qs], [{{route, T, _}, [], ['$_']}]) -> - qs2ms(Qs, [{{route, T, N}, [], ['$_']}]). +gen_match_spec([{topic, '=:=', T} | Qs], [{{route, _, N}, [], ['$_']}]) -> + gen_match_spec(Qs, [{{route, T, N}, [], ['$_']}]); +gen_match_spec([{node, '=:=', N} | Qs], [{{route, T, _}, [], ['$_']}]) -> + gen_match_spec(Qs, [{{route, T, N}, [], ['$_']}]). format(#route{topic = Topic, dest = {_, Node}}) -> #{topic => Topic, node => Node}; diff --git a/apps/emqx_modules/src/emqx_delayed.erl b/apps/emqx_modules/src/emqx_delayed.erl index 44e7a85e3..0d83e65f1 100644 --- a/apps/emqx_modules/src/emqx_delayed.erl +++ b/apps/emqx_modules/src/emqx_delayed.erl @@ -56,10 +56,12 @@ get_delayed_message/2, delete_delayed_message/1, delete_delayed_message/2, - cluster_list/1, - cluster_query/4 + cluster_list/1 ]). +%% internal exports +-export([qs2ms/2]). + -export([ post_config_update/5 ]). @@ -168,12 +170,16 @@ list(Params) -> cluster_list(Params) -> %% FIXME: why cluster_query??? emqx_mgmt_api:cluster_query( - Params, ?TAB, [], {?MODULE, cluster_query}, fun ?MODULE:format_delayed/1 + ?TAB, + Params, + [], + fun ?MODULE:qs2ms/2, + fun ?MODULE:format_delayed/1 ). -cluster_query(Table, _QsSpec, Continuation, Limit) -> - Ms = [{'$1', [], ['$1']}], - emqx_mgmt_api:select_table_with_count(Table, Ms, Continuation, Limit). +-spec qs2ms(atom(), {list(), list()}) -> {ets:match_spec(), fun() | undefined}. +qs2ms(_Table, {_Qs, _Fuzzy}) -> + {[{'$1', [], ['$1']}], undefined}. format_delayed(Delayed) -> format_delayed(Delayed, false). diff --git a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl index 2157b108c..f2d5914d4 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl +++ b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl @@ -34,7 +34,7 @@ -export(['/rule_events'/2, '/rule_test'/2, '/rules'/2, '/rules/:id'/2, '/rules/:id/reset_metrics'/2]). %% query callback --export([query/4]). +-export([qs2ms/2, format_rule_resp/1]). -define(ERR_NO_RULE(ID), list_to_binary(io_lib:format("Rule ~ts Not Found", [(ID)]))). -define(ERR_BADARGS(REASON), begin @@ -274,10 +274,10 @@ param_path_id() -> case emqx_mgmt_api:node_query( node(), - QueryString, ?RULE_TAB, + QueryString, ?RULE_QS_SCHEMA, - {?MODULE, query}, + fun ?MODULE:qs2ms/2, fun ?MODULE:format_rule_resp/1 ) of @@ -553,12 +553,9 @@ filter_out_request_body(Conf) -> ], maps:without(ExtraConfs, Conf). -query(Tab, {Qs, Fuzzy}, Start, Limit) -> +qs2ms(_Tab, {Qs, Fuzzy}) -> Ms = qs2ms(), - FuzzyFun = fuzzy_match_fun(Qs, Ms, Fuzzy), - emqx_mgmt_api:select_table_with_count( - Tab, {Ms, FuzzyFun}, Start, Limit - ). + {Ms, fuzzy_match_fun(Qs, Ms, Fuzzy)}. %% rule is not a record, so everything is fuzzy filter. qs2ms() -> From 28d391f26c893f97742fc1140cab6291b70abd3d Mon Sep 17 00:00:00 2001 From: JianBo He Date: Wed, 16 Nov 2022 17:16:17 +0800 Subject: [PATCH 03/18] fix(mgmt): collect total number in node_query/cluster_query --- apps/emqx_management/src/emqx_mgmt_api.erl | 98 ++++++++++++++++++---- 1 file changed, 81 insertions(+), 17 deletions(-) diff --git a/apps/emqx_management/src/emqx_mgmt_api.erl b/apps/emqx_management/src/emqx_mgmt_api.erl index 5fd16e2ff..da5a75c62 100644 --- a/apps/emqx_management/src/emqx_mgmt_api.erl +++ b/apps/emqx_management/src/emqx_mgmt_api.erl @@ -147,7 +147,7 @@ node_query(Node, Tab, QString, QSchema, MsFun, FmtFun) -> {error, page_limit_invalid}; Meta -> {_CodCnt, NQString} = parse_qstring(QString, QSchema), - ResultAcc = #{cursor => 0, count => 0, rows => []}, + ResultAcc = init_query_result(), QueryState = init_query_state(Meta), NResultAcc = do_node_query( Node, Tab, NQString, MsFun, QueryState, ResultAcc @@ -196,7 +196,7 @@ cluster_query(Tab, QString, QSchema, MsFun, FmtFun) -> Meta -> {_CodCnt, NQString} = parse_qstring(QString, QSchema), Nodes = mria_mnesia:running_nodes(), - ResultAcc = #{cursor => 0, count => 0, rows => []}, + ResultAcc = init_query_result(), QueryState = init_query_state(Meta), NResultAcc = do_cluster_query( Nodes, Tab, NQString, MsFun, QueryState, ResultAcc @@ -205,7 +205,7 @@ cluster_query(Tab, QString, QSchema, MsFun, FmtFun) -> end. %% @private -do_cluster_query([], _Tab, _QString, _QueryFun, _QueryState, ResultAcc) -> +do_cluster_query([], _Tab, _QString, _MsFun, _QueryState, ResultAcc) -> ResultAcc; do_cluster_query( [Node | Tail] = Nodes, @@ -221,7 +221,7 @@ do_cluster_query( {Rows, NQueryState} -> case accumulate_query_rows(Node, Rows, NQueryState, ResultAcc) of {enough, NResultAcc} -> - NResultAcc; + maybe_collect_total_from_tail_nodes(Tail, Tab, QString, MsFun, NResultAcc); {more, NResultAcc} -> NextNodes = case NQueryState of @@ -232,6 +232,29 @@ do_cluster_query( end end. +maybe_collect_total_from_tail_nodes([], _Tab, _QString, _MsFun, ResultAcc) -> + ResultAcc; +maybe_collect_total_from_tail_nodes(Nodes, Tab, QString, MsFun, ResultAcc = #{total := TotalAcc}) -> + {Ms, FuzzyFun} = erlang:apply(MsFun, [Tab, QString]), + case is_countable_total(Ms, FuzzyFun) of + true -> + %% XXX: badfun risk? if the FuzzyFun is an anonumous func in local node + case rpc:multicall(Nodes, ?MODULE, apply_total_query, [Tab, Ms, FuzzyFun]) of + {_, [Node | _]} -> + {error, Node, {badrpc, badnode}}; + {ResL0, []} -> + ResL = lists:zip(Nodes, ResL0), + case lists:filter(fun({_, I}) -> not is_integer(I) end, ResL) of + [{Node, {badrpc, Reason}} | _] -> + {error, Node, {badrpc, Reason}}; + [] -> + ResultAcc#{total => ResL ++ TotalAcc} + end + end; + false -> + ResultAcc + end. + %%-------------------------------------------------------------------- %% Do Query (or rpc query) %%-------------------------------------------------------------------- @@ -240,7 +263,7 @@ do_cluster_query( %% #{continuation := ets:continuation(), %% page := pos_integer(), %% limit := pos_integer(), -%% total := #{node() := non_neg_integer()} +%% total := [{node(), non_neg_integer()}] %% } init_query_state(_Meta = #{page := Page, limit := Limit}) -> #{ @@ -253,7 +276,7 @@ init_query_state(_Meta = #{page := Page, limit := Limit}) -> %% @private This function is exempt from BPAPI do_query(Node, Tab, QString, MsFun, QueryState) when Node =:= node(), is_function(MsFun) -> {Ms, FuzzyFun} = erlang:apply(MsFun, [Tab, QString]), - do_select(Tab, Ms, FuzzyFun, QueryState); + do_select(Node, Tab, Ms, FuzzyFun, QueryState); do_query(Node, Tab, QString, MsFun, QueryState) when is_function(MsFun) -> case rpc:call( @@ -269,11 +292,13 @@ do_query(Node, Tab, QString, MsFun, QueryState) when is_function(MsFun) -> end. do_select( + Node, Tab, Ms, FuzzyFun, - QueryState = #{continuation := Continuation, limit := Limit} + QueryState0 = #{continuation := Continuation, limit := Limit} ) -> + QueryState = maybe_apply_total_query(Node, Tab, Ms, FuzzyFun, QueryState0), Result = case Continuation of ?FRESH_SELECT -> @@ -293,14 +318,48 @@ do_select( {NRows, QueryState#{continuation => NContinuation}} end. +maybe_apply_total_query(Node, Tab, Ms, FuzzyFun, QueryState = #{total := TotalAcc}) -> + case proplists:get_value(Node, TotalAcc, undefined) of + undefined -> + Total = apply_total_query(Tab, Ms, FuzzyFun), + QueryState#{total := [{Node, Total} | TotalAcc]}; + _ -> + QueryState + end. + +%% XXX: Calculating the total number of data that match a certain condition under a large table +%% is very expensive because the entire ETS table needs to be scanned. +apply_total_query(Tab, Ms, FuzzyFun) -> + case is_countable_total(Ms, FuzzyFun) of + true -> + ets:info(Tab, size); + false -> + %% return a fake total number if the query have any conditions + 0 + end. + +is_countable_total(Ms, FuzzyFun) -> + FuzzyFun =:= undefined andalso is_non_conditions_match_spec(Ms). + +is_non_conditions_match_spec([{_MatchHead, _Conds = [], _Return} | More]) -> + is_non_conditions_match_spec(More); +is_non_conditions_match_spec([{_MatchHead, Conds, _Return} | _More]) when length(Conds) =/= 0 -> + false; +is_non_conditions_match_spec([]) -> + true. + %% ResultAcc :: #{count := integer(), %% cursor := integer(), -%% rows := [{node(), Rows :: list()}] +%% rows := [{node(), Rows :: list()}], +%% total := [{node() => integer()}] %% } +init_query_result() -> + #{cursor => 0, count => 0, rows => [], total => []}. + accumulate_query_rows( Node, Rows, - _QueryState = #{page := Page, limit := Limit}, + _QueryState = #{page := Page, limit := Limit, total := TotalAcc}, ResultAcc = #{cursor := Cursor, count := Count, rows := RowsAcc} ) -> PageStart = (Page - 1) * Limit + 1, @@ -308,11 +367,12 @@ accumulate_query_rows( Len = length(Rows), case Cursor + Len of NCursor when NCursor < PageStart -> - {more, ResultAcc#{cursor => NCursor}}; + {more, ResultAcc#{cursor => NCursor, total => TotalAcc}}; NCursor when NCursor < PageEnd -> {more, ResultAcc#{ cursor => NCursor, count => Count + length(Rows), + total => TotalAcc, rows => [{Node, Rows} | RowsAcc] }}; NCursor when NCursor >= PageEnd -> @@ -320,6 +380,7 @@ accumulate_query_rows( {enough, ResultAcc#{ cursor => NCursor, count => Count + length(SubRows), + total => TotalAcc, rows => [{Node, SubRows} | RowsAcc] }} end. @@ -426,10 +487,13 @@ is_fuzzy_key(_) -> format_query_result(_FmtFun, _Meta, Error = {error, _Node, _Reason}) -> Error; format_query_result( - FmtFun, Meta, _ResultAcc = #{count := _Count, cursor := Cursor, rows := RowsAcc} + FmtFun, Meta, _ResultAcc = #{total := TotalAcc, rows := RowsAcc} ) -> + Total = lists:foldr(fun({_Node, T}, N) -> N + T end, 0, TotalAcc), #{ - meta => Meta#{count => Cursor}, + %% The `count` is used in HTTP API to indicate the total number of + %% queries that can be read + meta => Meta#{count => Total}, data => lists:flatten( lists:foldr( fun({Node, Rows}, Acc) -> @@ -500,6 +564,11 @@ to_ip_port(IPAddress) -> Port = list_to_integer(Port0), {IP, Port}. +b2i(Bin) when is_binary(Bin) -> + binary_to_integer(Bin); +b2i(Any) -> + Any. + %%-------------------------------------------------------------------- %% EUnits %%-------------------------------------------------------------------- @@ -544,8 +613,3 @@ params2qs_test() -> {0, {[], []}} = parse_qstring([{not_a_predefined_params, val}], QSchema). -endif. - -b2i(Bin) when is_binary(Bin) -> - binary_to_integer(Bin); -b2i(Any) -> - Any. From 8f7337c9d26098ba735803375cff53359cab73df Mon Sep 17 00:00:00 2001 From: JianBo He Date: Wed, 16 Nov 2022 17:46:30 +0800 Subject: [PATCH 04/18] chore: return undefined fuzzy searching func --- apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl b/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl index e4678d641..cb88742c0 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl @@ -211,6 +211,8 @@ update_ms(share_group, X, {{Pid, Topic}, Opts}) -> update_ms(qos, X, {{Pid, Topic}, Opts}) -> {{Pid, Topic}, Opts#{qos => X}}. +fuzzy_filter_fun([]) -> + undefined; fuzzy_filter_fun(Fuzzy) -> fun(MsRaws) when is_list(MsRaws) -> lists:filter( From 9c7bf9d601bd54edb7f284d110d91442cf066211 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Wed, 16 Nov 2022 18:08:05 +0800 Subject: [PATCH 05/18] chore: update app.src --- apps/emqx_authz/src/emqx_authz.app.src | 2 +- apps/emqx_modules/src/emqx_modules.app.src | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/emqx_authz/src/emqx_authz.app.src b/apps/emqx_authz/src/emqx_authz.app.src index 6ec14ac3b..e98e8c6b3 100644 --- a/apps/emqx_authz/src/emqx_authz.app.src +++ b/apps/emqx_authz/src/emqx_authz.app.src @@ -1,7 +1,7 @@ %% -*- mode: erlang -*- {application, emqx_authz, [ {description, "An OTP application"}, - {vsn, "0.1.7"}, + {vsn, "0.1.8"}, {registered, []}, {mod, {emqx_authz_app, []}}, {applications, [ diff --git a/apps/emqx_modules/src/emqx_modules.app.src b/apps/emqx_modules/src/emqx_modules.app.src index e2a142a99..29a7d0362 100644 --- a/apps/emqx_modules/src/emqx_modules.app.src +++ b/apps/emqx_modules/src/emqx_modules.app.src @@ -1,7 +1,7 @@ %% -*- mode: erlang -*- {application, emqx_modules, [ {description, "EMQX Modules"}, - {vsn, "5.0.6"}, + {vsn, "5.0.7"}, {modules, []}, {applications, [kernel, stdlib, emqx]}, {mod, {emqx_modules_app, []}}, From 09958d9a33ce2ad8a320a45ea0ad6fe53acf32e8 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Wed, 16 Nov 2022 20:46:41 +0800 Subject: [PATCH 06/18] chore: fix diaylzer warnings --- apps/emqx_management/src/emqx_mgmt_api.erl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/emqx_management/src/emqx_mgmt_api.erl b/apps/emqx_management/src/emqx_mgmt_api.erl index da5a75c62..bbac80c1d 100644 --- a/apps/emqx_management/src/emqx_mgmt_api.erl +++ b/apps/emqx_management/src/emqx_mgmt_api.erl @@ -122,7 +122,9 @@ limit(Params) -> -type query_params() :: list() | map(). --type query_schema() :: [{Key :: binary(), Type :: atom | integer | timestamp | ip | ip_port}]. +-type query_schema() :: [ + {Key :: binary(), Type :: atom | binary | integer | timestamp | ip | ip_port} +]. -type query_to_match_spec_fun() :: fun((list(), list()) -> {ets:match_spec(), fun()}). From 8a0c468b0183f1b8ee446eea2183f94121c1c3e7 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Thu, 17 Nov 2022 12:43:55 +0800 Subject: [PATCH 07/18] test: refine tests for lots of List HTTP API --- .../test/emqx_authn_mnesia_SUITE.erl | 2 +- apps/emqx_management/src/emqx_mgmt_api.erl | 83 ++++++++++--------- .../test/emqx_mgmt_api_subscription_SUITE.erl | 4 +- .../src/emqx_rule_engine_api.erl | 66 +++++++++------ .../test/emqx_rule_engine_api_SUITE.erl | 12 +-- 5 files changed, 96 insertions(+), 71 deletions(-) diff --git a/apps/emqx_authn/test/emqx_authn_mnesia_SUITE.erl b/apps/emqx_authn/test/emqx_authn_mnesia_SUITE.erl index 2ab9efb1d..d9fa225bb 100644 --- a/apps/emqx_authn/test/emqx_authn_mnesia_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_mnesia_SUITE.erl @@ -213,7 +213,7 @@ t_list_users(_) -> #{ data := [#{is_superuser := false, user_id := <<"u3">>}], - meta := #{page := 1, limit := 20, count := 1} + meta := #{page := 1, limit := 20, count := 0} } = emqx_authn_mnesia:list_users( #{ <<"page">> => 1, diff --git a/apps/emqx_management/src/emqx_mgmt_api.erl b/apps/emqx_management/src/emqx_mgmt_api.erl index bbac80c1d..ee733e2dc 100644 --- a/apps/emqx_management/src/emqx_mgmt_api.erl +++ b/apps/emqx_management/src/emqx_mgmt_api.erl @@ -21,6 +21,7 @@ -elvis([{elvis_style, dont_repeat_yourself, #{min_complexity => 100}}]). -define(FRESH_SELECT, fresh_select). +-define(LONG_QUERY_TIMEOUT, 50000). -export([ paginate/3, @@ -35,6 +36,7 @@ ]). -export([do_query/5]). +-export([parse_qstring/2]). paginate(Tables, Params, {Module, FormatFun}) -> Qh = query_handle(Tables), @@ -236,25 +238,30 @@ do_cluster_query( maybe_collect_total_from_tail_nodes([], _Tab, _QString, _MsFun, ResultAcc) -> ResultAcc; -maybe_collect_total_from_tail_nodes(Nodes, Tab, QString, MsFun, ResultAcc = #{total := TotalAcc}) -> +maybe_collect_total_from_tail_nodes(Nodes, Tab, QString, MsFun, ResultAcc) -> {Ms, FuzzyFun} = erlang:apply(MsFun, [Tab, QString]), - case is_countable_total(Ms, FuzzyFun) of - true -> - %% XXX: badfun risk? if the FuzzyFun is an anonumous func in local node - case rpc:multicall(Nodes, ?MODULE, apply_total_query, [Tab, Ms, FuzzyFun]) of - {_, [Node | _]} -> - {error, Node, {badrpc, badnode}}; - {ResL0, []} -> - ResL = lists:zip(Nodes, ResL0), - case lists:filter(fun({_, I}) -> not is_integer(I) end, ResL) of - [{Node, {badrpc, Reason}} | _] -> - {error, Node, {badrpc, Reason}}; - [] -> - ResultAcc#{total => ResL ++ TotalAcc} - end - end; + case counting_total_fun(Ms, FuzzyFun) of false -> - ResultAcc + ResultAcc; + _Fun -> + collect_total_from_tail_nodes(Nodes, Tab, Ms, FuzzyFun, ResultAcc) + end. + +collect_total_from_tail_nodes(Nodes, Tab, Ms, FuzzyFun, ResultAcc = #{total := TotalAcc}) -> + %% XXX: badfun risk? if the FuzzyFun is an anonumous func in local node + case + rpc:multicall(Nodes, ?MODULE, apply_total_query, [Tab, Ms, FuzzyFun], ?LONG_QUERY_TIMEOUT) + of + {_, [Node | _]} -> + {error, Node, {badrpc, badnode}}; + {ResL0, []} -> + ResL = lists:zip(Nodes, ResL0), + case lists:filter(fun({_, I}) -> not is_integer(I) end, ResL) of + [{Node, {badrpc, Reason}} | _] -> + {error, Node, {badrpc, Reason}}; + [] -> + ResultAcc#{total => ResL ++ TotalAcc} + end end. %%-------------------------------------------------------------------- @@ -286,7 +293,7 @@ do_query(Node, Tab, QString, MsFun, QueryState) when is_function(MsFun) -> ?MODULE, do_query, [Node, Tab, QString, MsFun, QueryState], - 50000 + ?LONG_QUERY_TIMEOUT ) of {badrpc, _} = R -> {error, R}; @@ -329,26 +336,31 @@ maybe_apply_total_query(Node, Tab, Ms, FuzzyFun, QueryState = #{total := TotalAc QueryState end. -%% XXX: Calculating the total number of data that match a certain condition under a large table -%% is very expensive because the entire ETS table needs to be scanned. apply_total_query(Tab, Ms, FuzzyFun) -> - case is_countable_total(Ms, FuzzyFun) of - true -> - ets:info(Tab, size); + case counting_total_fun(Ms, FuzzyFun) of false -> %% return a fake total number if the query have any conditions - 0 + 0; + Fun -> + Fun(Tab) end. -is_countable_total(Ms, FuzzyFun) -> - FuzzyFun =:= undefined andalso is_non_conditions_match_spec(Ms). - -is_non_conditions_match_spec([{_MatchHead, _Conds = [], _Return} | More]) -> - is_non_conditions_match_spec(More); -is_non_conditions_match_spec([{_MatchHead, Conds, _Return} | _More]) when length(Conds) =/= 0 -> - false; -is_non_conditions_match_spec([]) -> - true. +counting_total_fun(Ms, undefined) -> + %% XXX: Calculating the total number of data that match a certain + %% condition under a large table is very expensive because the + %% entire ETS table needs to be scanned. + %% + %% XXX: How to optimize it? i.e, using: + %% `fun(Tab) -> ets:info(Tab, size) end` + [{MatchHead, Conditions, _Return}] = Ms, + CountingMs = [{MatchHead, Conditions, [true]}], + fun(Tab) -> + ets:select_count(Tab, CountingMs) + end; +counting_total_fun(_Ms, FuzzyFun) when is_function(FuzzyFun) -> + %% XXX: Calculating the total number for a fuzzy searching is very very expensive + %% so it is not supported now + false. %% ResultAcc :: #{count := integer(), %% cursor := integer(), @@ -387,10 +399,6 @@ accumulate_query_rows( }} end. -%%-------------------------------------------------------------------- -%% Table Select -%%-------------------------------------------------------------------- - %%-------------------------------------------------------------------- %% Internal Functions %%-------------------------------------------------------------------- @@ -402,6 +410,7 @@ parse_qstring(QString, QSchema) -> {length(NQString) + length(FuzzyQString), {NQString, FuzzyQString}}. do_parse_qstring([], _, Acc1, Acc2) -> + %% remove fuzzy keys if present in accurate query NAcc2 = [E || E <- Acc2, not lists:keymember(element(1, E), 1, Acc1)], {lists:reverse(Acc1), lists:reverse(NAcc2)}; do_parse_qstring([{Key, Value} | RestQString], QSchema, Acc1, Acc2) -> diff --git a/apps/emqx_management/test/emqx_mgmt_api_subscription_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_subscription_SUITE.erl index d1cf4e418..9a6de9938 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_subscription_SUITE.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_subscription_SUITE.erl @@ -93,6 +93,7 @@ t_subscription_api(_) -> {"match_topic", "t/#"} ]), Headers = emqx_mgmt_api_test_util:auth_header_(), + {ok, ResponseTopic2} = emqx_mgmt_api_test_util:request_api(get, Path, QS, Headers), DataTopic2 = emqx_json:decode(ResponseTopic2, [return_maps]), Meta2 = maps:get(<<"meta">>, DataTopic2), @@ -114,7 +115,8 @@ t_subscription_api(_) -> MatchMeta = maps:get(<<"meta">>, MatchData), ?assertEqual(1, maps:get(<<"page">>, MatchMeta)), ?assertEqual(emqx_mgmt:max_row_limit(), maps:get(<<"limit">>, MatchMeta)), - ?assertEqual(1, maps:get(<<"count">>, MatchMeta)), + %% count equals 0 in fuzzy searching + ?assertEqual(0, maps:get(<<"count">>, MatchMeta)), MatchSubs = maps:get(<<"data">>, MatchData), ?assertEqual(1, length(MatchSubs)), diff --git a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl index f2d5914d4..ad0018c0c 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl +++ b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl @@ -554,34 +554,46 @@ filter_out_request_body(Conf) -> maps:without(ExtraConfs, Conf). qs2ms(_Tab, {Qs, Fuzzy}) -> - Ms = qs2ms(), - {Ms, fuzzy_match_fun(Qs, Ms, Fuzzy)}. - -%% rule is not a record, so everything is fuzzy filter. -qs2ms() -> - [{'_', [], ['$_']}]. - -fuzzy_match_fun(Qs, Ms, Fuzzy) -> - MsC = ets:match_spec_compile(Ms), - fun(Rows) -> - Ls = ets:match_spec_run(Rows, MsC), - lists:filter( - fun(E) -> - run_qs_match(E, Qs) andalso - run_fuzzy_match(E, Fuzzy) - end, - Ls - ) + case lists:keytake(from, 1, Qs) of + false -> + {generate_match_spec(Qs), fuzzy_match_fun(Fuzzy)}; + {value, {from, '=:=', From}, Ls} -> + {generate_match_spec(Ls), fuzzy_match_fun([{from, '=:=', From} | Fuzzy])} end. -run_qs_match(_, []) -> - true; -run_qs_match(E = {_Id, #{enable := Enable}}, [{enable, '=:=', Pattern} | Qs]) -> - Enable =:= Pattern andalso run_qs_match(E, Qs); -run_qs_match(E = {_Id, #{from := From}}, [{from, '=:=', Pattern} | Qs]) -> - lists:member(Pattern, From) andalso run_qs_match(E, Qs); -run_qs_match(E, [_ | Qs]) -> - run_qs_match(E, Qs). +generate_match_spec(Qs) -> + {MtchHead, Conds} = generate_match_spec(Qs, 2, {#{}, []}), + [{{'_', MtchHead}, Conds, ['$_']}]. + +generate_match_spec([], _, {MtchHead, Conds}) -> + {MtchHead, lists:reverse(Conds)}; +generate_match_spec([Qs | Rest], N, {MtchHead, Conds}) -> + Holder = binary_to_atom(iolist_to_binary(["$", integer_to_list(N)]), utf8), + NMtchHead = emqx_mgmt_util:merge_maps(MtchHead, ms(element(1, Qs), Holder)), + NConds = put_conds(Qs, Holder, Conds), + generate_match_spec(Rest, N + 1, {NMtchHead, NConds}). + +put_conds({_, Op, V}, Holder, Conds) -> + [{Op, Holder, V} | Conds]; +put_conds({_, Op1, V1, Op2, V2}, Holder, Conds) -> + [ + {Op2, Holder, V2}, + {Op1, Holder, V1} + | Conds + ]. + +ms(enable, X) -> + #{enable => X}. + +fuzzy_match_fun([]) -> + undefined; +fuzzy_match_fun(Fuzzy) -> + fun(MsRaws) when is_list(MsRaws) -> + lists:filter( + fun(E) -> run_fuzzy_match(E, Fuzzy) end, + MsRaws + ) + end. run_fuzzy_match(_, []) -> true; @@ -589,6 +601,8 @@ run_fuzzy_match(E = {Id, _}, [{id, like, Pattern} | Fuzzy]) -> binary:match(Id, Pattern) /= nomatch andalso run_fuzzy_match(E, Fuzzy); run_fuzzy_match(E = {_Id, #{description := Desc}}, [{description, like, Pattern} | Fuzzy]) -> binary:match(Desc, Pattern) /= nomatch andalso run_fuzzy_match(E, Fuzzy); +run_fuzzy_match(E = {_, #{from := Topics}}, [{from, '=:=', Pattern} | Fuzzy]) -> + lists:member(Pattern, Topics) /= false andalso run_fuzzy_match(E, Fuzzy); run_fuzzy_match(E = {_Id, #{from := Topics}}, [{from, match, Pattern} | Fuzzy]) -> lists:any(fun(For) -> emqx_topic:match(For, Pattern) end, Topics) andalso run_fuzzy_match(E, Fuzzy); diff --git a/apps/emqx_rule_engine/test/emqx_rule_engine_api_SUITE.erl b/apps/emqx_rule_engine/test/emqx_rule_engine_api_SUITE.erl index da4e299f9..93912dd6c 100644 --- a/apps/emqx_rule_engine/test/emqx_rule_engine_api_SUITE.erl +++ b/apps/emqx_rule_engine/test/emqx_rule_engine_api_SUITE.erl @@ -133,23 +133,23 @@ t_list_rule_api(_Config) -> QueryStr2 = #{query_string => #{<<"like_description">> => <<"也能"/utf8>>}}, {200, Result2} = emqx_rule_engine_api:'/rules'(get, QueryStr2), - ?assertEqual(Result1, Result2), + ?assertEqual(maps:get(data, Result1), maps:get(data, Result2)), QueryStr3 = #{query_string => #{<<"from">> => <<"t/1">>}}, - {200, #{meta := #{count := Count3}}} = emqx_rule_engine_api:'/rules'(get, QueryStr3), - ?assertEqual(19, Count3), + {200, #{data := Data3}} = emqx_rule_engine_api:'/rules'(get, QueryStr3), + ?assertEqual(19, length(Data3)), QueryStr4 = #{query_string => #{<<"like_from">> => <<"t/1/+">>}}, {200, Result4} = emqx_rule_engine_api:'/rules'(get, QueryStr4), - ?assertEqual(Result1, Result4), + ?assertEqual(maps:get(data, Result1), maps:get(data, Result4)), QueryStr5 = #{query_string => #{<<"match_from">> => <<"t/+/+">>}}, {200, Result5} = emqx_rule_engine_api:'/rules'(get, QueryStr5), - ?assertEqual(Result1, Result5), + ?assertEqual(maps:get(data, Result1), maps:get(data, Result5)), QueryStr6 = #{query_string => #{<<"like_id">> => RuleID}}, {200, Result6} = emqx_rule_engine_api:'/rules'(get, QueryStr6), - ?assertEqual(Result1, Result6), + ?assertEqual(maps:get(data, Result1), maps:get(data, Result6)), %% clean up lists:foreach( From 79a2682fd3d9d87615b8ffe829635d96170f8a5a Mon Sep 17 00:00:00 2001 From: JianBo He Date: Thu, 17 Nov 2022 18:36:01 +0800 Subject: [PATCH 08/18] chore: improve the no-conditions query --- apps/emqx/test/emqx_bpapi_static_checks.erl | 2 +- apps/emqx_management/src/emqx_mgmt_api.erl | 100 ++++++++++---------- 2 files changed, 52 insertions(+), 50 deletions(-) diff --git a/apps/emqx/test/emqx_bpapi_static_checks.erl b/apps/emqx/test/emqx_bpapi_static_checks.erl index d87258201..7849cd617 100644 --- a/apps/emqx/test/emqx_bpapi_static_checks.erl +++ b/apps/emqx/test/emqx_bpapi_static_checks.erl @@ -62,7 +62,7 @@ %% List of business-layer functions that are exempt from the checks: %% erlfmt-ignore -define(EXEMPTIONS, - "emqx_mgmt_api:do_query/6" % Reason: legacy code. A fun and a QC query are + "emqx_mgmt_api:do_query/2" % Reason: legacy code. A fun and a QC query are % passed in the args, it's futile to try to statically % check it ). diff --git a/apps/emqx_management/src/emqx_mgmt_api.erl b/apps/emqx_management/src/emqx_mgmt_api.erl index ee733e2dc..9f757b0cc 100644 --- a/apps/emqx_management/src/emqx_mgmt_api.erl +++ b/apps/emqx_management/src/emqx_mgmt_api.erl @@ -35,8 +35,7 @@ b2i/1 ]). --export([do_query/5]). --export([parse_qstring/2]). +-export([do_query/2, apply_total_query/1]). paginate(Tables, Params, {Module, FormatFun}) -> Qh = query_handle(Tables), @@ -152,9 +151,9 @@ node_query(Node, Tab, QString, QSchema, MsFun, FmtFun) -> Meta -> {_CodCnt, NQString} = parse_qstring(QString, QSchema), ResultAcc = init_query_result(), - QueryState = init_query_state(Meta), + QueryState = init_query_state(Tab, NQString, MsFun, Meta), NResultAcc = do_node_query( - Node, Tab, NQString, MsFun, QueryState, ResultAcc + Node, QueryState, ResultAcc ), format_query_result(FmtFun, Meta, NResultAcc) end. @@ -162,13 +161,10 @@ node_query(Node, Tab, QString, QSchema, MsFun, FmtFun) -> %% @private do_node_query( Node, - Tab, - QString, - MsFun, QueryState, ResultAcc ) -> - case do_query(Node, Tab, QString, MsFun, QueryState) of + case do_query(Node, QueryState) of {error, {badrpc, R}} -> {error, Node, {badrpc, R}}; {Rows, NQueryState = #{continuation := ?FRESH_SELECT}} -> @@ -179,7 +175,7 @@ do_node_query( {enough, NResultAcc} -> NResultAcc; {more, NResultAcc} -> - do_node_query(Node, Tab, QString, MsFun, NQueryState, NResultAcc) + do_node_query(Node, NQueryState, NResultAcc) end end. @@ -201,57 +197,51 @@ cluster_query(Tab, QString, QSchema, MsFun, FmtFun) -> {_CodCnt, NQString} = parse_qstring(QString, QSchema), Nodes = mria_mnesia:running_nodes(), ResultAcc = init_query_result(), - QueryState = init_query_state(Meta), + QueryState = init_query_state(Tab, NQString, MsFun, Meta), NResultAcc = do_cluster_query( - Nodes, Tab, NQString, MsFun, QueryState, ResultAcc + Nodes, QueryState, ResultAcc ), format_query_result(FmtFun, Meta, NResultAcc) end. %% @private -do_cluster_query([], _Tab, _QString, _MsFun, _QueryState, ResultAcc) -> +do_cluster_query([], _QueryState, ResultAcc) -> ResultAcc; do_cluster_query( [Node | Tail] = Nodes, - Tab, - QString, - MsFun, QueryState, ResultAcc ) -> - case do_query(Node, Tab, QString, MsFun, QueryState) of + case do_query(Node, QueryState) of {error, {badrpc, R}} -> {error, Node, {badrpc, R}}; {Rows, NQueryState} -> case accumulate_query_rows(Node, Rows, NQueryState, ResultAcc) of {enough, NResultAcc} -> - maybe_collect_total_from_tail_nodes(Tail, Tab, QString, MsFun, NResultAcc); + maybe_collect_total_from_tail_nodes(Tail, NQueryState, NResultAcc); {more, NResultAcc} -> NextNodes = case NQueryState of #{continuation := ?FRESH_SELECT} -> Tail; _ -> Nodes end, - do_cluster_query(NextNodes, Tab, QString, MsFun, NQueryState, NResultAcc) + do_cluster_query(NextNodes, NQueryState, NResultAcc) end end. -maybe_collect_total_from_tail_nodes([], _Tab, _QString, _MsFun, ResultAcc) -> +maybe_collect_total_from_tail_nodes([], _QueryState, ResultAcc) -> ResultAcc; -maybe_collect_total_from_tail_nodes(Nodes, Tab, QString, MsFun, ResultAcc) -> - {Ms, FuzzyFun} = erlang:apply(MsFun, [Tab, QString]), - case counting_total_fun(Ms, FuzzyFun) of +maybe_collect_total_from_tail_nodes(Nodes, QueryState, ResultAcc) -> + case counting_total_fun(QueryState) of false -> ResultAcc; _Fun -> - collect_total_from_tail_nodes(Nodes, Tab, Ms, FuzzyFun, ResultAcc) + collect_total_from_tail_nodes(Nodes, QueryState, ResultAcc) end. -collect_total_from_tail_nodes(Nodes, Tab, Ms, FuzzyFun, ResultAcc = #{total := TotalAcc}) -> +collect_total_from_tail_nodes(Nodes, QueryState, ResultAcc = #{total := TotalAcc}) -> %% XXX: badfun risk? if the FuzzyFun is an anonumous func in local node - case - rpc:multicall(Nodes, ?MODULE, apply_total_query, [Tab, Ms, FuzzyFun], ?LONG_QUERY_TIMEOUT) - of + case rpc:multicall(Nodes, ?MODULE, apply_total_query, [QueryState], ?LONG_QUERY_TIMEOUT) of {_, [Node | _]} -> {error, Node, {badrpc, badnode}}; {ResL0, []} -> @@ -272,27 +262,35 @@ collect_total_from_tail_nodes(Nodes, Tab, Ms, FuzzyFun, ResultAcc = #{total := T %% #{continuation := ets:continuation(), %% page := pos_integer(), %% limit := pos_integer(), -%% total := [{node(), non_neg_integer()}] +%% total := [{node(), non_neg_integer()}], +%% table := atom(), +%% qs := {Qs, Fuzzy} %% parsed query params +%% msfun := query_to_match_spec_fun() %% } -init_query_state(_Meta = #{page := Page, limit := Limit}) -> +init_query_state(Tab, QString, MsFun, _Meta = #{page := Page, limit := Limit}) -> + {Ms, FuzzyFun} = erlang:apply(MsFun, [Tab, QString]), #{ - continuation => ?FRESH_SELECT, page => Page, limit => Limit, - total => [] + table => Tab, + qs => QString, + msfun => MsFun, + mactch_spec => Ms, + fuzzy_fun => FuzzyFun, + total => [], + continuation => ?FRESH_SELECT }. %% @private This function is exempt from BPAPI -do_query(Node, Tab, QString, MsFun, QueryState) when Node =:= node(), is_function(MsFun) -> - {Ms, FuzzyFun} = erlang:apply(MsFun, [Tab, QString]), - do_select(Node, Tab, Ms, FuzzyFun, QueryState); -do_query(Node, Tab, QString, MsFun, QueryState) when is_function(MsFun) -> +do_query(Node, QueryState) when Node =:= node() -> + do_select(Node, QueryState); +do_query(Node, QueryState) -> case rpc:call( Node, ?MODULE, do_query, - [Node, Tab, QString, MsFun, QueryState], + [Node, QueryState], ?LONG_QUERY_TIMEOUT ) of @@ -302,18 +300,21 @@ do_query(Node, Tab, QString, MsFun, QueryState) when is_function(MsFun) -> do_select( Node, - Tab, - Ms, - FuzzyFun, - QueryState0 = #{continuation := Continuation, limit := Limit} + QueryState0 = #{ + table := Tab, + mactch_spec := Ms, + fuzzy_fun := FuzzyFun, + continuation := Continuation, + limit := Limit + } ) -> - QueryState = maybe_apply_total_query(Node, Tab, Ms, FuzzyFun, QueryState0), + QueryState = maybe_apply_total_query(Node, QueryState0), Result = case Continuation of ?FRESH_SELECT -> ets:select(Tab, Ms, Limit); _ -> - ets:select(ets:repair_continuation(Continuation, Ms)) + ets:select(Continuation) end, case Result of '$end_of_table' -> @@ -327,17 +328,17 @@ do_select( {NRows, QueryState#{continuation => NContinuation}} end. -maybe_apply_total_query(Node, Tab, Ms, FuzzyFun, QueryState = #{total := TotalAcc}) -> +maybe_apply_total_query(Node, QueryState = #{total := TotalAcc}) -> case proplists:get_value(Node, TotalAcc, undefined) of undefined -> - Total = apply_total_query(Tab, Ms, FuzzyFun), + Total = apply_total_query(QueryState), QueryState#{total := [{Node, Total} | TotalAcc]}; _ -> QueryState end. -apply_total_query(Tab, Ms, FuzzyFun) -> - case counting_total_fun(Ms, FuzzyFun) of +apply_total_query(QueryState = #{table := Tab}) -> + case counting_total_fun(QueryState) of false -> %% return a fake total number if the query have any conditions 0; @@ -345,19 +346,20 @@ apply_total_query(Tab, Ms, FuzzyFun) -> Fun(Tab) end. -counting_total_fun(Ms, undefined) -> +counting_total_fun(_QueryState = #{qs := {[], []}}) -> + fun(Tab) -> ets:info(Tab, size) end; +counting_total_fun(_QueryState = #{mactch_spec := Ms, fuzzy_fun := undefined}) -> %% XXX: Calculating the total number of data that match a certain %% condition under a large table is very expensive because the %% entire ETS table needs to be scanned. %% %% XXX: How to optimize it? i.e, using: - %% `fun(Tab) -> ets:info(Tab, size) end` [{MatchHead, Conditions, _Return}] = Ms, CountingMs = [{MatchHead, Conditions, [true]}], fun(Tab) -> ets:select_count(Tab, CountingMs) end; -counting_total_fun(_Ms, FuzzyFun) when is_function(FuzzyFun) -> +counting_total_fun(_QueryState = #{fuzzy_fun := FuzzyFun}) when is_function(FuzzyFun) -> %% XXX: Calculating the total number for a fuzzy searching is very very expensive %% so it is not supported now false. From 0c1412315cb24d6042073b9163c9ca4b0abd4389 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Thu, 17 Nov 2022 21:43:30 +0800 Subject: [PATCH 09/18] chore(bpapi): ignore emqx_mgmt_api:collect_total_from_tail_nodes/3 --- apps/emqx/test/emqx_bpapi_static_checks.erl | 7 ++++--- .../test/emqx_enhanced_authn_scram_mnesia_SUITE.erl | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/apps/emqx/test/emqx_bpapi_static_checks.erl b/apps/emqx/test/emqx_bpapi_static_checks.erl index 7849cd617..69cede2bc 100644 --- a/apps/emqx/test/emqx_bpapi_static_checks.erl +++ b/apps/emqx/test/emqx_bpapi_static_checks.erl @@ -62,9 +62,10 @@ %% List of business-layer functions that are exempt from the checks: %% erlfmt-ignore -define(EXEMPTIONS, - "emqx_mgmt_api:do_query/2" % Reason: legacy code. A fun and a QC query are - % passed in the args, it's futile to try to statically - % check it + % Reason: legacy code. A fun and a QC query are + % passed in the args, it's futile to try to statically + % check it + "emqx_mgmt_api:do_query/2, emqx_mgmt_api:collect_total_from_tail_nodes/3" ). -define(XREF, myxref). diff --git a/apps/emqx_authn/test/emqx_enhanced_authn_scram_mnesia_SUITE.erl b/apps/emqx_authn/test/emqx_enhanced_authn_scram_mnesia_SUITE.erl index 41fa5a38c..a0b5ce980 100644 --- a/apps/emqx_authn/test/emqx_enhanced_authn_scram_mnesia_SUITE.erl +++ b/apps/emqx_authn/test/emqx_enhanced_authn_scram_mnesia_SUITE.erl @@ -319,7 +319,7 @@ t_list_users(_) -> is_superuser := _ } ], - meta := #{page := 1, limit := 3, count := 1} + meta := #{page := 1, limit := 3, count := 0} } = emqx_enhanced_authn_scram_mnesia:list_users( #{ <<"page">> => 1, From b9c5a5f82265660c356b27a22aa64116e5fe7a56 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Fri, 18 Nov 2022 13:28:59 +0800 Subject: [PATCH 10/18] fix(delayed): return correct node name --- .../test/emqx_mgmt_api_SUITE.erl | 42 +++++++++++++++++++ .../test/emqx_mgmt_api_topics_SUITE.erl | 24 ++++++++++- apps/emqx_modules/src/emqx_delayed.erl | 23 ++++++---- .../src/emqx_rule_engine_api.erl | 8 +--- 4 files changed, 79 insertions(+), 18 deletions(-) create mode 100644 apps/emqx_management/test/emqx_mgmt_api_SUITE.erl diff --git a/apps/emqx_management/test/emqx_mgmt_api_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_SUITE.erl new file mode 100644 index 000000000..c9abef375 --- /dev/null +++ b/apps/emqx_management/test/emqx_mgmt_api_SUITE.erl @@ -0,0 +1,42 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2022 EMQ Technologies Co., Ltd. All Rights Reserved. +%% +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%%-------------------------------------------------------------------- +-module(emqx_mgmt_api_SUITE). + +-compile(export_all). +-compile(nowarn_export_all). + +-include_lib("eunit/include/eunit.hrl"). + +%%-------------------------------------------------------------------- +%% setup +%%-------------------------------------------------------------------- + +all() -> + emqx_common_test_helpers:all(?MODULE). + +init_per_suite(Config) -> + emqx_mgmt_api_test_util:init_suite(), + Config. + +end_per_suite(_) -> + emqx_mgmt_api_test_util:end_suite(). + +%%-------------------------------------------------------------------- +%% cases +%%-------------------------------------------------------------------- + +t_cluster_query(_Config) -> + ok. diff --git a/apps/emqx_management/test/emqx_mgmt_api_topics_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_topics_SUITE.erl index a06154400..419c17adf 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_topics_SUITE.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_topics_SUITE.erl @@ -31,6 +31,7 @@ end_per_suite(_) -> emqx_mgmt_api_test_util:end_suite(). t_nodes_api(_) -> + Node = atom_to_binary(node(), utf8), Topic = <<"test_topic">>, {ok, Client} = emqtt:start_link(#{ username => <<"routes_username">>, clientid => <<"routes_cid">> @@ -49,11 +50,30 @@ t_nodes_api(_) -> Data = maps:get(<<"data">>, RoutesData), Route = erlang:hd(Data), ?assertEqual(Topic, maps:get(<<"topic">>, Route)), - ?assertEqual(atom_to_binary(node(), utf8), maps:get(<<"node">>, Route)), + ?assertEqual(Node, maps:get(<<"node">>, Route)), + + %% exact match + Topic2 = <<"test_topic_2">>, + {ok, _, _} = emqtt:subscribe(Client, Topic2), + QS = uri_string:compose_query([ + {"topic", Topic2}, + {"node", atom_to_list(node())} + ]), + Headers = emqx_mgmt_api_test_util:auth_header_(), + {ok, MatchResponse} = emqx_mgmt_api_test_util:request_api(get, Path, QS, Headers), + MatchData = emqx_json:decode(MatchResponse, [return_maps]), + ?assertMatch( + #{<<"count">> := 1, <<"page">> := 1, <<"limit">> := 100}, + maps:get(<<"meta">>, MatchData) + ), + ?assertMatch( + [#{<<"topic">> := Topic2, <<"node">> := Node}], + maps:get(<<"data">>, MatchData) + ), %% get topics/:topic RoutePath = emqx_mgmt_api_test_util:api_path(["topics", Topic]), {ok, RouteResponse} = emqx_mgmt_api_test_util:request_api(get, RoutePath), RouteData = emqx_json:decode(RouteResponse, [return_maps]), ?assertEqual(Topic, maps:get(<<"topic">>, RouteData)), - ?assertEqual(atom_to_binary(node(), utf8), maps:get(<<"node">>, RouteData)). + ?assertEqual(Node, maps:get(<<"node">>, RouteData)). diff --git a/apps/emqx_modules/src/emqx_delayed.erl b/apps/emqx_modules/src/emqx_delayed.erl index 0d83e65f1..c96e5a452 100644 --- a/apps/emqx_modules/src/emqx_delayed.erl +++ b/apps/emqx_modules/src/emqx_delayed.erl @@ -59,15 +59,17 @@ cluster_list/1 ]). -%% internal exports --export([qs2ms/2]). +%% exports for query +-export([ + qs2ms/2, + format_delayed/1, + format_delayed/2 +]). -export([ post_config_update/5 ]). --export([format_delayed/1]). - %% exported for `emqx_telemetry' -export([get_basic_usage_info/0]). @@ -168,13 +170,12 @@ list(Params) -> emqx_mgmt_api:paginate(?TAB, Params, ?FORMAT_FUN). cluster_list(Params) -> - %% FIXME: why cluster_query??? emqx_mgmt_api:cluster_query( ?TAB, Params, [], fun ?MODULE:qs2ms/2, - fun ?MODULE:format_delayed/1 + fun ?MODULE:format_delayed/2 ). -spec qs2ms(atom(), {list(), list()}) -> {ets:match_spec(), fun() | undefined}. @@ -182,9 +183,13 @@ qs2ms(_Table, {_Qs, _Fuzzy}) -> {[{'$1', [], ['$1']}], undefined}. format_delayed(Delayed) -> - format_delayed(Delayed, false). + format_delayed(node(), Delayed). + +format_delayed(WhichNode, Delayed) -> + format_delayed(WhichNode, Delayed, false). format_delayed( + WhichNode, #delayed_message{ key = {ExpectTimeStamp, Id}, delayed = Delayed, @@ -204,7 +209,7 @@ format_delayed( RemainingTime = ExpectTimeStamp - ?NOW, Result = #{ msgid => emqx_guid:to_hexstr(Id), - node => node(), + node => WhichNode, publish_at => PublishTime, delayed_interval => Delayed, delayed_remaining => RemainingTime div 1000, @@ -231,7 +236,7 @@ get_delayed_message(Id) -> {error, not_found}; Rows -> Message = hd(Rows), - {ok, format_delayed(Message, true)} + {ok, format_delayed(node(), Message, true)} end. get_delayed_message(Node, Id) when Node =:= node() -> diff --git a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl index ad0018c0c..b34d0ceae 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl +++ b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl @@ -574,13 +574,7 @@ generate_match_spec([Qs | Rest], N, {MtchHead, Conds}) -> generate_match_spec(Rest, N + 1, {NMtchHead, NConds}). put_conds({_, Op, V}, Holder, Conds) -> - [{Op, Holder, V} | Conds]; -put_conds({_, Op1, V1, Op2, V2}, Holder, Conds) -> - [ - {Op2, Holder, V2}, - {Op1, Holder, V1} - | Conds - ]. + [{Op, Holder, V} | Conds]. ms(enable, X) -> #{enable => X}. From 36de83a50a8489ecc12e782da16f1dff96a5c3b9 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Fri, 18 Nov 2022 18:20:34 +0800 Subject: [PATCH 11/18] feat(cm): change emqx_channel_info to ordered_set table --- apps/emqx/src/emqx_cm.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/emqx/src/emqx_cm.erl b/apps/emqx/src/emqx_cm.erl index d39f43686..483c51f26 100644 --- a/apps/emqx/src/emqx_cm.erl +++ b/apps/emqx/src/emqx_cm.erl @@ -650,7 +650,7 @@ init([]) -> TabOpts = [public, {write_concurrency, true}], ok = emqx_tables:new(?CHAN_TAB, [bag, {read_concurrency, true} | TabOpts]), ok = emqx_tables:new(?CHAN_CONN_TAB, [bag | TabOpts]), - ok = emqx_tables:new(?CHAN_INFO_TAB, [set, compressed | TabOpts]), + ok = emqx_tables:new(?CHAN_INFO_TAB, [ordered_set, compressed | TabOpts]), ok = emqx_tables:new(?CHAN_LIVE_TAB, [set, {write_concurrency, true} | TabOpts]), ok = emqx_stats:update_interval(chan_stats, fun ?MODULE:stats_fun/0), State = #{chan_pmon => emqx_pmon:new()}, From 6d9e1e0d7ac4025ba8c562dedd5024b0298b8a77 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Fri, 18 Nov 2022 18:21:03 +0800 Subject: [PATCH 12/18] test(mgmt): cover emqx_mgmt_api:cluster_query --- apps/emqx/test/emqx_common_test_helpers.erl | 37 +++++- apps/emqx_management/src/emqx_mgmt_api.erl | 6 +- .../test/emqx_mgmt_api_SUITE.erl | 105 +++++++++++++++++- 3 files changed, 142 insertions(+), 6 deletions(-) diff --git a/apps/emqx/test/emqx_common_test_helpers.erl b/apps/emqx/test/emqx_common_test_helpers.erl index f195e083c..2d51f6f14 100644 --- a/apps/emqx/test/emqx_common_test_helpers.erl +++ b/apps/emqx/test/emqx_common_test_helpers.erl @@ -519,21 +519,51 @@ ensure_quic_listener(Name, UdpPort) -> %% Clusterisation and multi-node testing %% +-type cluster_spec() :: [node_spec()]. +-type node_spec() :: role() | {role(), shortname()} | {role(), shortname(), node_opts()}. +-type role() :: core | replicant. +-type shortname() :: atom(). +-type nodename() :: atom(). +-type node_opts() :: #{ + %% Need to loaded apps. These apps will be loaded once the node started + load_apps => list(), + %% Need to started apps. It is the first arg passed to emqx_common_test_helpers:start_apps/2 + apps => list(), + %% Extras app starting handler. It is the second arg passed to emqx_common_test_helpers:start_apps/2 + env_handler => fun((AppName :: atom()) -> term()), + %% Application env preset before calling `emqx_common_test_helpers:start_apps/2` + env => {AppName :: atom(), Key :: atom(), Val :: term()}, + %% Whether to execute `emqx_config:init_load(SchemaMod)` + %% default: true + load_schema => boolean(), + %% Eval by emqx_config:put/2 + conf => [{KeyPath :: list(), Val :: term()}], + %% Fast option to config listener port + %% default rule: + %% - tcp: base_port + %% - ssl: base_port + 1 + %% - ws : base_port + 3 + %% - wss: base_port + 4 + listener_ports => [{Type :: tcp | ssl | ws | wss, inet:port_number()}] +}. + +-spec emqx_cluster(cluster_spec()) -> [{shortname(), node_opts()}]. emqx_cluster(Specs) -> emqx_cluster(Specs, #{}). +-spec emqx_cluster(cluster_spec(), node_opts()) -> [{shortname(), node_opts()}]. emqx_cluster(Specs, CommonOpts) when is_list(CommonOpts) -> emqx_cluster(Specs, maps:from_list(CommonOpts)); emqx_cluster(Specs0, CommonOpts) -> Specs1 = lists:zip(Specs0, lists:seq(1, length(Specs0))), Specs = expand_node_specs(Specs1, CommonOpts), - CoreNodes = [node_name(Name) || {{core, Name, _}, _} <- Specs], - %% Assign grpc ports: + %% Assign grpc ports GenRpcPorts = maps:from_list([ {node_name(Name), {tcp, gen_rpc_port(base_port(Num))}} || {{_, Name, _}, Num} <- Specs ]), %% Set the default node of the cluster: + CoreNodes = [node_name(Name) || {{core, Name, _}, _} <- Specs], JoinTo = case CoreNodes of [First | _] -> First; @@ -554,6 +584,8 @@ emqx_cluster(Specs0, CommonOpts) -> ]. %% Lower level starting API + +-spec start_slave(shortname(), node_opts()) -> nodename(). start_slave(Name, Opts) -> {ok, Node} = ct_slave:start( list_to_atom(atom_to_list(Name) ++ "@" ++ host()), @@ -590,6 +622,7 @@ epmd_path() -> %% Node initialization +-spec setup_node(nodename(), node_opts()) -> ok. setup_node(Node, Opts) when is_list(Opts) -> setup_node(Node, maps:from_list(Opts)); setup_node(Node, Opts) when is_map(Opts) -> diff --git a/apps/emqx_management/src/emqx_mgmt_api.erl b/apps/emqx_management/src/emqx_mgmt_api.erl index 9f757b0cc..9646eb747 100644 --- a/apps/emqx_management/src/emqx_mgmt_api.erl +++ b/apps/emqx_management/src/emqx_mgmt_api.erl @@ -314,7 +314,9 @@ do_select( ?FRESH_SELECT -> ets:select(Tab, Ms, Limit); _ -> - ets:select(Continuation) + %% XXX: Repair is necessary because we pass Continuation back + %% and forth through the nodes in the `do_cluster_query` + ets:select(ets:repair_continuation(Continuation, Ms)) end, case Result of '$end_of_table' -> @@ -508,7 +510,7 @@ format_query_result( %% queries that can be read meta => Meta#{count => Total}, data => lists:flatten( - lists:foldr( + lists:foldl( fun({Node, Rows}, Acc) -> [lists:map(fun(Row) -> exec_format_fun(FmtFun, Node, Row) end, Rows) | Acc] end, diff --git a/apps/emqx_management/test/emqx_mgmt_api_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_SUITE.erl index c9abef375..a065b9c83 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_SUITE.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_SUITE.erl @@ -28,15 +28,116 @@ all() -> emqx_common_test_helpers:all(?MODULE). init_per_suite(Config) -> - emqx_mgmt_api_test_util:init_suite(), Config. end_per_suite(_) -> - emqx_mgmt_api_test_util:end_suite(). + ok. %%-------------------------------------------------------------------- %% cases %%-------------------------------------------------------------------- t_cluster_query(_Config) -> + net_kernel:start(['master@127.0.0.1', longnames]), + ct:timetrap({seconds, 120}), + snabbkaffe:fix_ct_logging(), + [{Name, Opts}, {Name1, Opts1}] = cluster_specs(), + Node1 = emqx_common_test_helpers:start_slave(Name, Opts), + Node2 = emqx_common_test_helpers:start_slave(Name1, Opts1), + try + process_flag(trap_exit, true), + ClientLs1 = [start_emqtt_client(Node1, I, 2883) || I <- lists:seq(1, 10)], + ClientLs2 = [start_emqtt_client(Node2, I, 3883) || I <- lists:seq(1, 10)], + + %% returned list should be the same regardless of which node is requested + {200, ClientsAll} = query_clients(Node1, #{}), + ?assertEqual({200, ClientsAll}, query_clients(Node2, #{})), + ?assertMatch( + #{page := 1, limit := 100, count := 20}, + maps:get(meta, ClientsAll) + ), + ?assertMatch(20, length(maps:get(data, ClientsAll))), + %% query the first page, counting in entire cluster + {200, ClientsPage1} = query_clients(Node1, #{<<"limit">> => 5}), + ?assertMatch( + #{page := 1, limit := 5, count := 20}, + maps:get(meta, ClientsPage1) + ), + ?assertMatch(5, length(maps:get(data, ClientsPage1))), + + %% assert: AllPage = Page1 + Page2 + Page3 + Page4 + %% !!!Note: this equation requires that the queried tables must be ordered_set + {200, ClientsPage2} = query_clients(Node1, #{<<"page">> => 2, <<"limit">> => 5}), + {200, ClientsPage3} = query_clients(Node2, #{<<"page">> => 3, <<"limit">> => 5}), + {200, ClientsPage4} = query_clients(Node1, #{<<"page">> => 4, <<"limit">> => 5}), + GetClientIds = fun(L) -> lists:map(fun(#{clientid := Id}) -> Id end, L) end, + ?assertEqual( + GetClientIds(maps:get(data, ClientsAll)), + GetClientIds( + maps:get(data, ClientsPage1) ++ maps:get(data, ClientsPage2) ++ + maps:get(data, ClientsPage3) ++ maps:get(data, ClientsPage4) + ) + ), + + %% exact match can return non-zero total + {200, ClientsNode1} = query_clients(Node2, #{<<"username">> => <<"corenode1@127.0.0.1">>}), + ?assertMatch( + #{count := 10}, + maps:get(meta, ClientsNode1) + ), + + %% fuzzy searching can't return total + {200, ClientsNode2} = query_clients(Node2, #{<<"like_username">> => <<"corenode2">>}), + ?assertMatch( + #{count := 0}, + maps:get(meta, ClientsNode2) + ), + ?assertMatch(10, length(maps:get(data, ClientsNode2))), + + _ = lists:foreach(fun(C) -> emqtt:disconnect(C) end, ClientLs1), + _ = lists:foreach(fun(C) -> emqtt:disconnect(C) end, ClientLs2) + after + emqx_common_test_helpers:stop_slave(Node1), + emqx_common_test_helpers:stop_slave(Node2) + end, ok. + +%%-------------------------------------------------------------------- +%% helpers +%%-------------------------------------------------------------------- + +cluster_specs() -> + Specs = + %% default listeners port + [ + {core, corenode1, #{listener_ports => [{tcp, 2883}]}}, + {core, corenode2, #{listener_ports => [{tcp, 3883}]}} + ], + CommOpts = + [ + {env, [{emqx, boot_modules, all}]}, + {apps, []}, + {conf, [ + {[listeners, ssl, default, enabled], false}, + {[listeners, ws, default, enabled], false}, + {[listeners, wss, default, enabled], false} + ]} + ], + emqx_common_test_helpers:emqx_cluster( + Specs, + CommOpts + ). + +start_emqtt_client(Node0, N, Port) -> + Node = atom_to_binary(Node0), + ClientId = iolist_to_binary([Node, "-", integer_to_binary(N)]), + {ok, C} = emqtt:start_link([{clientid, ClientId}, {username, Node}, {port, Port}]), + {ok, _} = emqtt:connect(C), + C. + +query_clients(Node, Qs0) -> + Qs = maps:merge( + #{<<"page">> => 1, <<"limit">> => 100}, + Qs0 + ), + rpc:call(Node, emqx_mgmt_api_clients, clients, [get, #{query_string => Qs}]). From ffb3f2d2d2c420becf5e25da8996410d77af6a33 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Mon, 21 Nov 2022 17:09:27 +0800 Subject: [PATCH 13/18] chore: change emqx_live_connection tab type to ordered_set Co-authored-by: Zaiming (Stone) Shi --- apps/emqx/src/emqx_cm.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/emqx/src/emqx_cm.erl b/apps/emqx/src/emqx_cm.erl index 483c51f26..5adc7a811 100644 --- a/apps/emqx/src/emqx_cm.erl +++ b/apps/emqx/src/emqx_cm.erl @@ -651,7 +651,7 @@ init([]) -> ok = emqx_tables:new(?CHAN_TAB, [bag, {read_concurrency, true} | TabOpts]), ok = emqx_tables:new(?CHAN_CONN_TAB, [bag | TabOpts]), ok = emqx_tables:new(?CHAN_INFO_TAB, [ordered_set, compressed | TabOpts]), - ok = emqx_tables:new(?CHAN_LIVE_TAB, [set, {write_concurrency, true} | TabOpts]), + ok = emqx_tables:new(?CHAN_LIVE_TAB, [ordered_set, {write_concurrency, true} | TabOpts]), ok = emqx_stats:update_interval(chan_stats, fun ?MODULE:stats_fun/0), State = #{chan_pmon => emqx_pmon:new()}, {ok, State}. From 0122d3900f66b4d920644c90ce98c1a74c200a7b Mon Sep 17 00:00:00 2001 From: JianBo He Date: Tue, 22 Nov 2022 14:59:48 +0800 Subject: [PATCH 14/18] chore: update changes --- changes/v5.0.11-en.md | 2 ++ changes/v5.0.11-zh.md | 2 ++ 2 files changed, 4 insertions(+) diff --git a/changes/v5.0.11-en.md b/changes/v5.0.11-en.md index 5329be024..4b35aa5f4 100644 --- a/changes/v5.0.11-en.md +++ b/changes/v5.0.11-en.md @@ -27,6 +27,8 @@ - Support message properties in `/publish` API [#9401](https://github.com/emqx/emqx/pull/9401). +- Optimize client query performance for HTTP APIs [#9374](https://github.com/emqx/emqx/pull/9374). + ## Bug fixes - Fix `ssl.existingName` option of helm chart not working [#9307](https://github.com/emqx/emqx/issues/9307). diff --git a/changes/v5.0.11-zh.md b/changes/v5.0.11-zh.md index e0c82b022..ab9f5a0d4 100644 --- a/changes/v5.0.11-zh.md +++ b/changes/v5.0.11-zh.md @@ -25,6 +25,8 @@ - 支持在 /publish API 中添加消息属性 [#9401](https://github.com/emqx/emqx/pull/9401)。 +- 优化查询客户端列表的 HTTP API 性能 [#9374](https://github.com/emqx/emqx/pull/9374)。 + ## 修复 - 修复 helm chart 的 `ssl.existingName` 选项不起作用 [#9307](https://github.com/emqx/emqx/issues/9307)。 From 9786a6c26713ea38612306fc0c590a0a7e9af46a Mon Sep 17 00:00:00 2001 From: JianBo He Date: Tue, 22 Nov 2022 15:33:42 +0800 Subject: [PATCH 15/18] refactor(mgmt): convert fuzzy filter func to named func --- .../emqx_enhanced_authn_scram_mnesia.erl | 8 ++---- .../src/simple_authn/emqx_authn_mnesia.erl | 8 ++---- apps/emqx_authz/src/emqx_authz_api_mnesia.erl | 8 ++---- .../src/emqx_gateway_api_clients.erl | 8 ++---- apps/emqx_management/src/emqx_mgmt_api.erl | 26 +++++++++++++++---- .../src/emqx_mgmt_api_clients.erl | 8 ++---- .../src/emqx_mgmt_api_subscriptions.erl | 8 ++---- .../src/emqx_rule_engine_api.erl | 9 ++----- 8 files changed, 35 insertions(+), 48 deletions(-) diff --git a/apps/emqx_authn/src/enhanced_authn/emqx_enhanced_authn_scram_mnesia.erl b/apps/emqx_authn/src/enhanced_authn/emqx_enhanced_authn_scram_mnesia.erl index dfa2f32ef..66e19efd2 100644 --- a/apps/emqx_authn/src/enhanced_authn/emqx_enhanced_authn_scram_mnesia.erl +++ b/apps/emqx_authn/src/enhanced_authn/emqx_enhanced_authn_scram_mnesia.erl @@ -48,6 +48,7 @@ -export([ qs2ms/2, + run_fuzzy_filter/2, format_user_info/1, group_match_spec/1 ]). @@ -281,12 +282,7 @@ qs2ms(_Tab, {QString, Fuzzy}) -> fuzzy_filter_fun([]) -> undefined; fuzzy_filter_fun(Fuzzy) -> - fun(MsRaws) when is_list(MsRaws) -> - lists:filter( - fun(E) -> run_fuzzy_filter(E, Fuzzy) end, - MsRaws - ) - end. + {fun ?MODULE:run_fuzzy_filter/2, [Fuzzy]}. run_fuzzy_filter(_, []) -> true; diff --git a/apps/emqx_authn/src/simple_authn/emqx_authn_mnesia.erl b/apps/emqx_authn/src/simple_authn/emqx_authn_mnesia.erl index 50edb6612..930a03b6a 100644 --- a/apps/emqx_authn/src/simple_authn/emqx_authn_mnesia.erl +++ b/apps/emqx_authn/src/simple_authn/emqx_authn_mnesia.erl @@ -50,6 +50,7 @@ -export([ qs2ms/2, + run_fuzzy_filter/2, format_user_info/1, group_match_spec/1 ]). @@ -307,12 +308,7 @@ qs2ms(_Tab, {QString, FuzzyQString}) -> fuzzy_filter_fun([]) -> undefined; fuzzy_filter_fun(Fuzzy) -> - fun(MsRaws) when is_list(MsRaws) -> - lists:filter( - fun(E) -> run_fuzzy_filter(E, Fuzzy) end, - MsRaws - ) - end. + {fun ?MODULE:run_fuzzy_filter/2, [Fuzzy]}. run_fuzzy_filter(_, []) -> true; diff --git a/apps/emqx_authz/src/emqx_authz_api_mnesia.erl b/apps/emqx_authz/src/emqx_authz_api_mnesia.erl index 1ab7feea8..f480934a0 100644 --- a/apps/emqx_authz/src/emqx_authz_api_mnesia.erl +++ b/apps/emqx_authz/src/emqx_authz_api_mnesia.erl @@ -51,6 +51,7 @@ -export([ query_username/2, query_clientid/2, + run_fuzzy_filter/2, format_result/1 ]). @@ -589,12 +590,7 @@ query_clientid(_Tab, {_QString, FuzzyQString}) -> fuzzy_filter_fun([]) -> undefined; fuzzy_filter_fun(Fuzzy) -> - fun(MsRaws) when is_list(MsRaws) -> - lists:filter( - fun(E) -> run_fuzzy_filter(E, Fuzzy) end, - MsRaws - ) - end. + {fun ?MODULE:run_fuzzy_filter/2, [Fuzzy]}. run_fuzzy_filter(_, []) -> true; diff --git a/apps/emqx_gateway/src/emqx_gateway_api_clients.erl b/apps/emqx_gateway/src/emqx_gateway_api_clients.erl index ba0f34206..646e6b6fd 100644 --- a/apps/emqx_gateway/src/emqx_gateway_api_clients.erl +++ b/apps/emqx_gateway/src/emqx_gateway_api_clients.erl @@ -56,6 +56,7 @@ %% internal exports (for client query) -export([ qs2ms/2, + run_fuzzy_filter/2, format_channel_info/1, format_channel_info/2 ]). @@ -327,12 +328,7 @@ ms(lifetime, X) -> fuzzy_filter_fun([]) -> undefined; fuzzy_filter_fun(Fuzzy) -> - fun(MsRaws) when is_list(MsRaws) -> - lists:filter( - fun(E) -> run_fuzzy_filter(E, Fuzzy) end, - MsRaws - ) - end. + {fun ?MODULE:run_fuzzy_filter/2, [Fuzzy]}. run_fuzzy_filter(_, []) -> true; diff --git a/apps/emqx_management/src/emqx_mgmt_api.erl b/apps/emqx_management/src/emqx_mgmt_api.erl index 9646eb747..ff738a15a 100644 --- a/apps/emqx_management/src/emqx_mgmt_api.erl +++ b/apps/emqx_management/src/emqx_mgmt_api.erl @@ -128,7 +128,9 @@ limit(Params) -> ]. -type query_to_match_spec_fun() :: - fun((list(), list()) -> {ets:match_spec(), fun()}). + fun((list(), list()) -> {ets:match_spec(), fuzzy_filter_fun()}). + +-type fuzzy_filter_fun() :: undefined | {fun(), list()}. -type format_result_fun() :: fun((node(), term()) -> term()) @@ -269,6 +271,15 @@ collect_total_from_tail_nodes(Nodes, QueryState, ResultAcc = #{total := TotalAcc %% } init_query_state(Tab, QString, MsFun, _Meta = #{page := Page, limit := Limit}) -> {Ms, FuzzyFun} = erlang:apply(MsFun, [Tab, QString]), + %% assert FuzzyFun type + _ = + case FuzzyFun of + undefined -> + ok; + {NamedFun, Args} -> + true = is_list(Args), + {type, external} = erlang:fun_info(NamedFun, type) + end, #{ page => Page, limit => Limit, @@ -323,9 +334,14 @@ do_select( {[], QueryState#{continuation => ?FRESH_SELECT}}; {Rows, NContinuation} -> NRows = - case is_function(FuzzyFun) of - true -> FuzzyFun(Rows); - false -> Rows + case FuzzyFun of + undefined -> + Rows; + {FilterFun, Args0} when is_function(FilterFun), is_list(Args0) -> + lists:filter( + fun(E) -> erlang:apply(FilterFun, [E | Args0]) end, + Rows + ) end, {NRows, QueryState#{continuation => NContinuation}} end. @@ -361,7 +377,7 @@ counting_total_fun(_QueryState = #{mactch_spec := Ms, fuzzy_fun := undefined}) - fun(Tab) -> ets:select_count(Tab, CountingMs) end; -counting_total_fun(_QueryState = #{fuzzy_fun := FuzzyFun}) when is_function(FuzzyFun) -> +counting_total_fun(_QueryState = #{fuzzy_fun := FuzzyFun}) when FuzzyFun =/= undefined -> %% XXX: Calculating the total number for a fuzzy searching is very very expensive %% so it is not supported now false. diff --git a/apps/emqx_management/src/emqx_mgmt_api_clients.erl b/apps/emqx_management/src/emqx_mgmt_api_clients.erl index cec9b9889..1f6928f67 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_clients.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_clients.erl @@ -47,6 +47,7 @@ -export([ qs2ms/2, + run_fuzzy_filter/2, format_channel_info/1, format_channel_info/2 ]). @@ -841,12 +842,7 @@ ms(created_at, X) -> fuzzy_filter_fun([]) -> undefined; fuzzy_filter_fun(Fuzzy) -> - fun(MsRaws) when is_list(MsRaws) -> - lists:filter( - fun(E) -> run_fuzzy_filter(E, Fuzzy) end, - MsRaws - ) - end. + {fun ?MODULE:run_fuzzy_filter/2, [Fuzzy]}. run_fuzzy_filter(_, []) -> true; diff --git a/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl b/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl index cb88742c0..d39f34207 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl @@ -33,6 +33,7 @@ -export([ qs2ms/2, + run_fuzzy_filter/2, format/2 ]). @@ -214,12 +215,7 @@ update_ms(qos, X, {{Pid, Topic}, Opts}) -> fuzzy_filter_fun([]) -> undefined; fuzzy_filter_fun(Fuzzy) -> - fun(MsRaws) when is_list(MsRaws) -> - lists:filter( - fun(E) -> run_fuzzy_filter(E, Fuzzy) end, - MsRaws - ) - end. + {fun ?MODULE:run_fuzzy_filter/2, [Fuzzy]}. run_fuzzy_filter(_, []) -> true; diff --git a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl index b34d0ceae..b4901c1a0 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl +++ b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl @@ -34,7 +34,7 @@ -export(['/rule_events'/2, '/rule_test'/2, '/rules'/2, '/rules/:id'/2, '/rules/:id/reset_metrics'/2]). %% query callback --export([qs2ms/2, format_rule_resp/1]). +-export([qs2ms/2, run_fuzzy_match/2, format_rule_resp/1]). -define(ERR_NO_RULE(ID), list_to_binary(io_lib:format("Rule ~ts Not Found", [(ID)]))). -define(ERR_BADARGS(REASON), begin @@ -582,12 +582,7 @@ ms(enable, X) -> fuzzy_match_fun([]) -> undefined; fuzzy_match_fun(Fuzzy) -> - fun(MsRaws) when is_list(MsRaws) -> - lists:filter( - fun(E) -> run_fuzzy_match(E, Fuzzy) end, - MsRaws - ) - end. + {fun ?MODULE:run_fuzzy_match/2, [Fuzzy]}. run_fuzzy_match(_, []) -> true; From edb35c08a8732442742377c7491a25112ee7a2aa Mon Sep 17 00:00:00 2001 From: JianBo He Date: Thu, 24 Nov 2022 15:02:16 +0800 Subject: [PATCH 16/18] chore: refactor ms2qs function type --- .../enhanced_authn/emqx_enhanced_authn_scram_mnesia.erl | 7 +++++-- apps/emqx_authn/src/simple_authn/emqx_authn_mnesia.erl | 7 +++++-- apps/emqx_gateway/src/emqx_gateway_api_clients.erl | 4 ++-- apps/emqx_management/src/emqx_mgmt_api.erl | 7 ++++--- apps/emqx_management/src/emqx_mgmt_api_alarms.erl | 4 ++-- apps/emqx_management/src/emqx_mgmt_api_clients.erl | 7 +++++-- apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl | 4 ++-- apps/emqx_management/src/emqx_mgmt_api_topics.erl | 6 +++++- apps/emqx_modules/src/emqx_delayed.erl | 7 +++++-- apps/emqx_rule_engine/src/emqx_rule_engine_api.erl | 8 ++++++-- 10 files changed, 41 insertions(+), 20 deletions(-) diff --git a/apps/emqx_authn/src/enhanced_authn/emqx_enhanced_authn_scram_mnesia.erl b/apps/emqx_authn/src/enhanced_authn/emqx_enhanced_authn_scram_mnesia.erl index 66e19efd2..ddc1bb464 100644 --- a/apps/emqx_authn/src/enhanced_authn/emqx_enhanced_authn_scram_mnesia.erl +++ b/apps/emqx_authn/src/enhanced_authn/emqx_enhanced_authn_scram_mnesia.erl @@ -274,9 +274,12 @@ list_users(QueryString, #{user_group := UserGroup}) -> %%-------------------------------------------------------------------- %% QueryString to MatchSpec --spec qs2ms(atom(), {list(), list()}) -> {ets:match_spec(), fun() | undefined}. +-spec qs2ms(atom(), {list(), list()}) -> emqx_mgmt_api:match_spec_and_filter(). qs2ms(_Tab, {QString, Fuzzy}) -> - {ms_from_qstring(QString), fuzzy_filter_fun(Fuzzy)}. + #{ + match_spec => ms_from_qstring(QString), + fuzzy_fun => fuzzy_filter_fun(Fuzzy) + }. %% Fuzzy username funcs fuzzy_filter_fun([]) -> diff --git a/apps/emqx_authn/src/simple_authn/emqx_authn_mnesia.erl b/apps/emqx_authn/src/simple_authn/emqx_authn_mnesia.erl index 930a03b6a..415d23c25 100644 --- a/apps/emqx_authn/src/simple_authn/emqx_authn_mnesia.erl +++ b/apps/emqx_authn/src/simple_authn/emqx_authn_mnesia.erl @@ -300,9 +300,12 @@ list_users(QueryString, #{user_group := UserGroup}) -> %%-------------------------------------------------------------------- %% QueryString to MatchSpec --spec qs2ms(atom(), {list(), list()}) -> {ets:match_spec(), fun() | undefined}. +-spec qs2ms(atom(), {list(), list()}) -> emqx_mgmt_api:match_spec_and_filter(). qs2ms(_Tab, {QString, FuzzyQString}) -> - {ms_from_qstring(QString), fuzzy_filter_fun(FuzzyQString)}. + #{ + match_spec => ms_from_qstring(QString), + fuzzy_fun => fuzzy_filter_fun(FuzzyQString) + }. %% Fuzzy username funcs fuzzy_filter_fun([]) -> diff --git a/apps/emqx_gateway/src/emqx_gateway_api_clients.erl b/apps/emqx_gateway/src/emqx_gateway_api_clients.erl index 646e6b6fd..d219d07cd 100644 --- a/apps/emqx_gateway/src/emqx_gateway_api_clients.erl +++ b/apps/emqx_gateway/src/emqx_gateway_api_clients.erl @@ -268,9 +268,9 @@ extra_sub_props(Props) -> %%-------------------------------------------------------------------- %% QueryString to MatchSpec --spec qs2ms(atom(), {list(), list()}) -> {ets:match_spec(), fun() | undefined}. +-spec qs2ms(atom(), {list(), list()}) -> emqx_mgmt_api:match_spec_and_filter(). qs2ms(_Tab, {Qs, Fuzzy}) -> - {qs2ms(Qs), fuzzy_filter_fun(Fuzzy)}. + #{match_spec => qs2ms(Qs), fuzzy_fun => fuzzy_filter_fun(Fuzzy)}. qs2ms(Qs) -> {MtchHead, Conds} = qs2ms(Qs, 2, {#{}, []}), diff --git a/apps/emqx_management/src/emqx_mgmt_api.erl b/apps/emqx_management/src/emqx_mgmt_api.erl index ff738a15a..5f8e39f13 100644 --- a/apps/emqx_management/src/emqx_mgmt_api.erl +++ b/apps/emqx_management/src/emqx_mgmt_api.erl @@ -127,8 +127,9 @@ limit(Params) -> {Key :: binary(), Type :: atom | binary | integer | timestamp | ip | ip_port} ]. --type query_to_match_spec_fun() :: - fun((list(), list()) -> {ets:match_spec(), fuzzy_filter_fun()}). +-type query_to_match_spec_fun() :: fun((list(), list()) -> match_spec_and_filter()). + +-type match_spec_and_filter() :: #{match_spec := ets:match_spec(), fuzzy_fun := fuzzy_filter_fun()}. -type fuzzy_filter_fun() :: undefined | {fun(), list()}. @@ -270,7 +271,7 @@ collect_total_from_tail_nodes(Nodes, QueryState, ResultAcc = #{total := TotalAcc %% msfun := query_to_match_spec_fun() %% } init_query_state(Tab, QString, MsFun, _Meta = #{page := Page, limit := Limit}) -> - {Ms, FuzzyFun} = erlang:apply(MsFun, [Tab, QString]), + #{match_spec := Ms, fuzzy_fun := FuzzyFun} = erlang:apply(MsFun, [Tab, QString]), %% assert FuzzyFun type _ = case FuzzyFun of diff --git a/apps/emqx_management/src/emqx_mgmt_api_alarms.erl b/apps/emqx_management/src/emqx_mgmt_api_alarms.erl index 895a5bcf8..80f93ffe4 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_alarms.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_alarms.erl @@ -136,9 +136,9 @@ alarms(delete, _Params) -> %%%============================================================================================== %% internal --spec qs2ms(atom(), {list(), list()}) -> {ets:match_spec(), fun() | undefined}. +-spec qs2ms(atom(), {list(), list()}) -> emqx_mgmt_api:match_spec_and_filter(). qs2ms(_Tab, {_Qs, _Fuzzy}) -> - {[{'$1', [], ['$1']}], undefined}. + #{match_spec => [{'$1', [], ['$1']}], fuzzy_fun => undefined}. format_alarm(WhichNode, Alarm) -> emqx_alarm:format(WhichNode, Alarm). diff --git a/apps/emqx_management/src/emqx_mgmt_api_clients.erl b/apps/emqx_management/src/emqx_mgmt_api_clients.erl index 1f6928f67..588031fc1 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_clients.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_clients.erl @@ -786,9 +786,12 @@ do_unsubscribe(ClientID, Topic) -> %%-------------------------------------------------------------------- %% QueryString to Match Spec --spec qs2ms(atom(), {list(), list()}) -> {ets:match_spec(), fun() | undefined}. +-spec qs2ms(atom(), {list(), list()}) -> emqx_mgmt_api:match_spec_and_filter(). qs2ms(_Tab, {QString, FuzzyQString}) -> - {qs2ms(QString), fuzzy_filter_fun(FuzzyQString)}. + #{ + match_spec => qs2ms(QString), + fuzzy_fun => fuzzy_filter_fun(FuzzyQString) + }. -spec qs2ms(list()) -> ets:match_spec(). qs2ms(Qs) -> diff --git a/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl b/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl index d39f34207..c05878c6d 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_subscriptions.erl @@ -190,9 +190,9 @@ get_topic(Topic, _) -> %% QueryString to MatchSpec %%-------------------------------------------------------------------- --spec qs2ms(atom(), {list(), list()}) -> {ets:match_spec(), fun() | undefined}. +-spec qs2ms(atom(), {list(), list()}) -> emqx_mgmt_api:match_spec_and_filter(). qs2ms(_Tab, {Qs, Fuzzy}) -> - {gen_match_spec(Qs), fuzzy_filter_fun(Fuzzy)}. + #{match_spec => gen_match_spec(Qs), fuzzy_fun => fuzzy_filter_fun(Fuzzy)}. gen_match_spec(Qs) -> MtchHead = gen_match_spec(Qs, {{'_', '_'}, #{}}), diff --git a/apps/emqx_management/src/emqx_mgmt_api_topics.erl b/apps/emqx_management/src/emqx_mgmt_api_topics.erl index ca707ba20..5e2bd1ea0 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_topics.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_topics.erl @@ -143,8 +143,12 @@ generate_topic(Params = #{topic := Topic}) -> generate_topic(Params) -> Params. +-spec qs2ms(atom(), {list(), list()}) -> emqx_mgmt_api:match_spec_and_filter(). qs2ms(_Tab, {Qs, _}) -> - {gen_match_spec(Qs, [{{route, '_', '_'}, [], ['$_']}]), undefined}. + #{ + match_spec => gen_match_spec(Qs, [{{route, '_', '_'}, [], ['$_']}]), + fuzzy_fun => undefined + }. gen_match_spec([], Res) -> Res; diff --git a/apps/emqx_modules/src/emqx_delayed.erl b/apps/emqx_modules/src/emqx_delayed.erl index c96e5a452..241ac5a8a 100644 --- a/apps/emqx_modules/src/emqx_delayed.erl +++ b/apps/emqx_modules/src/emqx_delayed.erl @@ -178,9 +178,12 @@ cluster_list(Params) -> fun ?MODULE:format_delayed/2 ). --spec qs2ms(atom(), {list(), list()}) -> {ets:match_spec(), fun() | undefined}. +-spec qs2ms(atom(), {list(), list()}) -> emqx_mgmt_api:match_spec_and_filter(). qs2ms(_Table, {_Qs, _Fuzzy}) -> - {[{'$1', [], ['$1']}], undefined}. + #{ + match_spec => [{'$1', [], ['$1']}], + fuzzy_fun => undefined + }. format_delayed(Delayed) -> format_delayed(node(), Delayed). diff --git a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl index b4901c1a0..01d819a69 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl +++ b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl @@ -553,12 +553,16 @@ filter_out_request_body(Conf) -> ], maps:without(ExtraConfs, Conf). +-spec qs2ms(atom(), {list(), list()}) -> emqx_mgmt_api:match_spec_and_filter(). qs2ms(_Tab, {Qs, Fuzzy}) -> case lists:keytake(from, 1, Qs) of false -> - {generate_match_spec(Qs), fuzzy_match_fun(Fuzzy)}; + #{match_spec => generate_match_spec(Qs), fuzzy_fun => fuzzy_match_fun(Fuzzy)}; {value, {from, '=:=', From}, Ls} -> - {generate_match_spec(Ls), fuzzy_match_fun([{from, '=:=', From} | Fuzzy])} + #{ + match_spec => generate_match_spec(Ls), + fuzzy_fun => fuzzy_match_fun([{from, '=:=', From} | Fuzzy]) + } end. generate_match_spec(Qs) -> From dbdb78d38abf81da00aea801680da680a768f622 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Thu, 24 Nov 2022 15:24:55 +0800 Subject: [PATCH 17/18] chore: clarify the case when count returns zero --- apps/emqx_dashboard/src/emqx_dashboard_swagger.erl | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl b/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl index 5ea2e0773..5af1aee89 100644 --- a/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl +++ b/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl @@ -138,7 +138,12 @@ fields(limit) -> Meta = #{in => query, desc => Desc, default => ?DEFAULT_ROW, example => 50}, [{limit, hoconsc:mk(range(1, ?MAX_ROW_LIMIT), Meta)}]; fields(count) -> - Meta = #{desc => <<"Results count.">>, required => true}, + Desc = << + "Total number of records counted.
" + "Note: this field is 0 when the queryed table is empty, " + "or if the query can not be optimized and requires a full table scan." + >>, + Meta = #{desc => Desc, required => true}, [{count, hoconsc:mk(non_neg_integer(), Meta)}]; fields(meta) -> fields(page) ++ fields(limit) ++ fields(count). From 6ee475d9b12a311da2ff693e4e80532a77355e6f Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Thu, 24 Nov 2022 17:56:14 +0100 Subject: [PATCH 18/18] fix(emqx_authz_api_mnesia): return the right matchers --- apps/emqx_authz/src/emqx_authz_api_mnesia.erl | 14 +++++-- apps/emqx_management/src/emqx_mgmt_api.erl | 40 ++++++++++--------- 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/apps/emqx_authz/src/emqx_authz_api_mnesia.erl b/apps/emqx_authz/src/emqx_authz_api_mnesia.erl index f480934a0..090f9d1e2 100644 --- a/apps/emqx_authz/src/emqx_authz_api_mnesia.erl +++ b/apps/emqx_authz/src/emqx_authz_api_mnesia.erl @@ -578,13 +578,19 @@ purge(delete, _) -> %%-------------------------------------------------------------------- %% QueryString to MatchSpec --spec query_username(atom(), {list(), list()}) -> {ets:match_spec(), fun() | undefined}. +-spec query_username(atom(), {list(), list()}) -> emqx_mgmt_api:match_spec_and_filter(). query_username(_Tab, {_QString, FuzzyQString}) -> - {emqx_authz_mnesia:list_username_rules(), fuzzy_filter_fun(FuzzyQString)}. + #{ + match_spec => emqx_authz_mnesia:list_username_rules(), + fuzzy_fun => fuzzy_filter_fun(FuzzyQString) + }. --spec query_clientid(atom(), {list(), list()}) -> {ets:match_spec(), fun() | undefined}. +-spec query_clientid(atom(), {list(), list()}) -> emqx_mgmt_api:match_spec_and_filter(). query_clientid(_Tab, {_QString, FuzzyQString}) -> - {emqx_authz_mnesia:list_clientid_rules(), fuzzy_filter_fun(FuzzyQString)}. + #{ + match_spec => emqx_authz_mnesia:list_clientid_rules(), + fuzzy_fun => fuzzy_filter_fun(FuzzyQString) + }. %% Fuzzy username funcs fuzzy_filter_fun([]) -> diff --git a/apps/emqx_management/src/emqx_mgmt_api.erl b/apps/emqx_management/src/emqx_mgmt_api.erl index 5f8e39f13..9f69ed3f0 100644 --- a/apps/emqx_management/src/emqx_mgmt_api.erl +++ b/apps/emqx_management/src/emqx_mgmt_api.erl @@ -35,6 +35,28 @@ b2i/1 ]). +-export_type([ + match_spec_and_filter/0 +]). + +-type query_params() :: list() | map(). + +-type query_schema() :: [ + {Key :: binary(), Type :: atom | binary | integer | timestamp | ip | ip_port} +]. + +-type query_to_match_spec_fun() :: fun((list(), list()) -> match_spec_and_filter()). + +-type match_spec_and_filter() :: #{match_spec := ets:match_spec(), fuzzy_fun := fuzzy_filter_fun()}. + +-type fuzzy_filter_fun() :: undefined | {fun(), list()}. + +-type format_result_fun() :: + fun((node(), term()) -> term()) + | fun((term()) -> term()). + +-type query_return() :: #{meta := map(), data := [term()]}. + -export([do_query/2, apply_total_query/1]). paginate(Tables, Params, {Module, FormatFun}) -> @@ -121,24 +143,6 @@ limit(Params) -> %% Node Query %%-------------------------------------------------------------------- --type query_params() :: list() | map(). - --type query_schema() :: [ - {Key :: binary(), Type :: atom | binary | integer | timestamp | ip | ip_port} -]. - --type query_to_match_spec_fun() :: fun((list(), list()) -> match_spec_and_filter()). - --type match_spec_and_filter() :: #{match_spec := ets:match_spec(), fuzzy_fun := fuzzy_filter_fun()}. - --type fuzzy_filter_fun() :: undefined | {fun(), list()}. - --type format_result_fun() :: - fun((node(), term()) -> term()) - | fun((term()) -> term()). - --type query_return() :: #{meta := map(), data := [term()]}. - -spec node_query( node(), atom(),