From d48e84b1112a6c4396364ee1b1400b1fdf156532 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Fri, 4 Mar 2022 15:47:58 +0100 Subject: [PATCH] Migrate segmented cache to maps --- big_tests/tests/domain_removal_SUITE.erl | 2 +- big_tests/tests/muc_light_SUITE.erl | 8 ++--- doc/modules/mod_mam.md | 2 +- src/mod_cache_users.erl | 6 ++-- src/mongoose_user_cache.erl | 25 +++++++------ src/muc_light/mod_muc_light.erl | 6 +++- src/muc_light/mod_muc_light_cache.erl | 2 +- test/batches_SUITE.erl | 29 +++++++++------ test/common/config_parser_helper.erl | 10 ++++-- test/config_parser_SUITE.erl | 45 ++++++++++++++---------- test/mod_mam_meta_SUITE.erl | 9 +++-- 11 files changed, 90 insertions(+), 54 deletions(-) diff --git a/big_tests/tests/domain_removal_SUITE.erl b/big_tests/tests/domain_removal_SUITE.erl index 8605ce3f9e..984e45de41 100644 --- a/big_tests/tests/domain_removal_SUITE.erl +++ b/big_tests/tests/domain_removal_SUITE.erl @@ -82,7 +82,7 @@ end_per_group(_Groupname, Config) -> group_to_modules(auth_removal) -> []; group_to_modules(cache_removal) -> - [{mod_cache_users, []}, + [{mod_cache_users, config_parser_helper:default_mod_config(mod_cache_users)}, {mod_mam_meta, mam_helper:config_opts(#{pm => #{}})}]; group_to_modules(mam_removal) -> MucHost = subhost_pattern(muc_light_helper:muc_host_pattern()), diff --git a/big_tests/tests/muc_light_SUITE.erl b/big_tests/tests/muc_light_SUITE.erl index e45193d91d..da7c1a038a 100644 --- a/big_tests/tests/muc_light_SUITE.erl +++ b/big_tests/tests/muc_light_SUITE.erl @@ -300,10 +300,10 @@ schema_opts(CaseName) -> common_muc_light_opts() -> [{host, subhost_pattern(muc_light_helper:muc_host_pattern())}, {backend, mongoose_helper:mnesia_or_rdbms_backend()}, - {cache_affs, [{module, internal}, - {strategy, fifo}, - {time_to_live, 2}, - {number_of_segments, 3}]}, + {cache_affs, #{module => internal, + strategy => fifo, + time_to_live => 2, + number_of_segments => 3}}, {rooms_in_rosters, true}]. %%-------------------------------------------------------------------- diff --git a/doc/modules/mod_mam.md b/doc/modules/mod_mam.md index 39159c14bb..91d60d03d5 100644 --- a/doc/modules/mod_mam.md +++ b/doc/modules/mod_mam.md @@ -223,7 +223,7 @@ modules.mod_mam_meta.cache.number_of_segments * **Default:** `internal` * **Example:** `modules.mod_mam_meta.cache.module = "mod_cache_users"` -Configures which cache to use, either start an internal instance, or reuse the cache created by `mod_cache_users`, if such module was enabled. Note that if reuse is desired – that is, `cache.module = "mod_cache_users"`, other cache configuration parameters are not allowed. +Configures which cache to use, either start an internal instance, or reuse the cache created by `mod_cache_users`, if such module was enabled. Note that if reuse is desired – that is, `cache.module = "mod_cache_users"`, other cache configuration parameters will be ignored. #### `modules.mod_mam_meta.async_writer.enabled` * **Syntax:** boolean diff --git a/src/mod_cache_users.erl b/src/mod_cache_users.erl index c5c3880771..8db0100790 100644 --- a/src/mod_cache_users.erl +++ b/src/mod_cache_users.erl @@ -33,8 +33,10 @@ stop(HostType) -> -spec config_spec() -> mongoose_config_spec:config_section(). config_spec() -> - Sec = #section{items = Items} = mongoose_user_cache:config_spec(), - Sec#section{items = maps:remove(<<"module">>, Items)}. + #section{items = Items, + defaults = Defaults} = Sec = mongoose_user_cache:config_spec(), + Sec#section{items = maps:remove(<<"module">>, Items), + defaults = maps:remove(<<"module">>, Defaults)}. -spec supported_features() -> [atom()]. supported_features() -> diff --git a/src/mongoose_user_cache.erl b/src/mongoose_user_cache.erl index b0c4f36348..ab0badb695 100644 --- a/src/mongoose_user_cache.erl +++ b/src/mongoose_user_cache.erl @@ -41,16 +41,21 @@ config_spec() -> <<"time_to_live">> => #option{type = int_or_infinity, validate = positive}, <<"number_of_segments">> => #option{type = integer, validate = positive} }, - process = fun ?MODULE:process_cache_config/1 + process = fun ?MODULE:process_cache_config/1, + defaults = #{<<"module">> => internal, + <<"strategy">> => fifo, + <<"time_to_live">> => 480, + <<"number_of_segments">> => 3}, + format_items = map }. %% If an external module is provided, disallow any other configuration key -process_cache_config(KVs) -> - case lists:keyfind(module, 1, KVs) of - {module, Module} when Module =/= internal -> [{module, _}] = KVs; - _ -> ok - end, - KVs. +process_cache_config(#{module := internal} = Config) -> + Config; +process_cache_config(#{module := Module}) -> + #{module => Module}; +process_cache_config(Config) -> + Config. -spec start_new_cache(mongooseim:host_type(), module(), gen_mod:module_opts()) -> any(). start_new_cache(HostType, Module, Opts) -> @@ -62,9 +67,9 @@ start_new_cache(HostType, Module, Opts) -> do_start_new_cache(HostType, Module, Opts) -> CacheName = gen_mod:get_module_proc(HostType, Module), CacheOpts = #{merger_fun => fun maps:merge/2, - segment_num => gen_mod:get_opt(number_of_segments, Opts, 3), - strategy => gen_mod:get_opt(strategy, Opts, fifo), - ttl => gen_mod:get_opt(time_to_live, Opts, {hours, 8})}, + segment_num => gen_mod:get_opt(number_of_segments, Opts), + strategy => gen_mod:get_opt(strategy, Opts), + ttl => gen_mod:get_opt(time_to_live, Opts)}, Spec = #{id => CacheName, start => {segmented_cache, start_link, [CacheName, CacheOpts]}, restart => permanent, shutdown => 5000, type => worker, modules => [segmented_cache]}, diff --git a/src/muc_light/mod_muc_light.erl b/src/muc_light/mod_muc_light.erl index 107905f1cd..6a79491899 100644 --- a/src/muc_light/mod_muc_light.erl +++ b/src/muc_light/mod_muc_light.erl @@ -218,7 +218,7 @@ config_spec() -> #section{ items = #{<<"backend">> => #option{type = atom, validate = {module, mod_muc_light_db}}, - <<"cache_affs">> => mongoose_user_cache:config_spec(), + <<"cache_affs">> => cache_config_spec(), <<"host">> => #option{type = string, validate = subdomain_template, process = fun mongoose_subdomain_utils:make_subdomain_pattern/1}, @@ -239,6 +239,10 @@ config_spec() -> } }. +cache_config_spec() -> + Sec = mongoose_user_cache:config_spec(), + Sec#section{defaults = #{}}. + config_schema_spec() -> #section{ items = #{<<"field">> => #option{type = binary, diff --git a/src/muc_light/mod_muc_light_cache.erl b/src/muc_light/mod_muc_light_cache.erl index bc5dc5a93f..fe20f4206e 100644 --- a/src/muc_light/mod_muc_light_cache.erl +++ b/src/muc_light/mod_muc_light_cache.erl @@ -123,7 +123,7 @@ force_clear(HostType) -> %%==================================================================== -spec start_cache(mongooseim:host_type(), gen_mod:module_opts()) -> any(). start_cache(HostType, Opts) -> - FinalOpts = maps:to_list(maps:merge(defaults(), maps:from_list(Opts))), + FinalOpts = maps:merge(defaults(), Opts), mongoose_user_cache:start_new_cache(HostType, ?MODULE, FinalOpts). defaults() -> diff --git a/test/batches_SUITE.erl b/test/batches_SUITE.erl index c2a14cf1a9..cb5b499f56 100644 --- a/test/batches_SUITE.erl +++ b/test/batches_SUITE.erl @@ -58,23 +58,32 @@ end_per_testcase(_TestCase, _Config) -> meck:unload(gen_mod), ok. +cache_config() -> + config_parser_helper:default_mod_config(mod_cache_users). + +cache_config(internal) -> + Def = config_parser_helper:default_mod_config(mod_cache_users), + Def#{module => internal}; +cache_config(Module) -> + #{module => Module}. + %% Tests internal_starts_another_cache(_) -> - mongoose_user_cache:start_new_cache(host_type(), ?mod(1), []), - mongoose_user_cache:start_new_cache(host_type(), ?mod(2), [{module, internal}]), + mongoose_user_cache:start_new_cache(host_type(), ?mod(1), cache_config()), + mongoose_user_cache:start_new_cache(host_type(), ?mod(2), cache_config(internal)), L = [S || S = {_Name, _Pid, worker, [segmented_cache]} <- supervisor:which_children(ejabberd_sup)], ?assertEqual(2, length(L)). external_does_not_start_another_cache(_) -> - mongoose_user_cache:start_new_cache(host_type(), ?mod(1), []), - mongoose_user_cache:start_new_cache(host_type(), ?mod(2), [{module, ?mod(1)}]), + mongoose_user_cache:start_new_cache(host_type(), ?mod(1), cache_config()), + mongoose_user_cache:start_new_cache(host_type(), ?mod(2), cache_config(?mod(1))), L = [S || S = {_Name, _Pid, worker, [segmented_cache]} <- supervisor:which_children(ejabberd_sup)], ?assertEqual(1, length(L)). internal_stop_does_stop_the_cache(_) -> meck:expect(gen_mod, get_module_opt, fun(_, _, module, _) -> internal end), - mongoose_user_cache:start_new_cache(host_type(), ?mod(1), []), - mongoose_user_cache:start_new_cache(host_type(), ?mod(2), [{module, internal}]), + mongoose_user_cache:start_new_cache(host_type(), ?mod(1), cache_config()), + mongoose_user_cache:start_new_cache(host_type(), ?mod(2), cache_config(internal)), L1 = [S || S = {_Name, _Pid, worker, [segmented_cache]} <- supervisor:which_children(ejabberd_sup)], ct:pal("Value ~p~n", [L1]), mongoose_user_cache:stop_cache(host_type(), ?mod(2)), @@ -84,8 +93,8 @@ internal_stop_does_stop_the_cache(_) -> external_stop_does_nothing(_) -> meck:expect(gen_mod, get_module_opt, fun(_, _, module, _) -> ?mod(1) end), - mongoose_user_cache:start_new_cache(host_type(), ?mod(1), []), - mongoose_user_cache:start_new_cache(host_type(), ?mod(2), [{module, ?mod(1)}]), + mongoose_user_cache:start_new_cache(host_type(), ?mod(1), cache_config()), + mongoose_user_cache:start_new_cache(host_type(), ?mod(2), cache_config(?mod(1))), L1 = [S || S = {_Name, _Pid, worker, [segmented_cache]} <- supervisor:which_children(ejabberd_sup)], mongoose_user_cache:stop_cache(host_type(), ?mod(2)), L2 = [S || S = {_Name, _Pid, worker, [segmented_cache]} <- supervisor:which_children(ejabberd_sup)], @@ -93,8 +102,8 @@ external_stop_does_nothing(_) -> shared_cache_inserts_in_shared_table(_) -> meck:expect(gen_mod, get_module_opt, fun(_, _, module, _) -> ?mod(1) end), - mongoose_user_cache:start_new_cache(host_type(), ?mod(1), []), - mongoose_user_cache:start_new_cache(host_type(), ?mod(2), [{module, ?mod(1)}]), + mongoose_user_cache:start_new_cache(host_type(), ?mod(1), cache_config()), + mongoose_user_cache:start_new_cache(host_type(), ?mod(2), cache_config(?mod(1))), mongoose_user_cache:merge_entry(host_type(), ?mod(2), some_jid(), #{}), ?assert(mongoose_user_cache:is_member(host_type(), ?mod(1), some_jid())). diff --git a/test/common/config_parser_helper.erl b/test/common/config_parser_helper.erl index cc9470f54c..e7e94d0373 100644 --- a/test/common/config_parser_helper.erl +++ b/test/common/config_parser_helper.erl @@ -399,7 +399,7 @@ all_modules() -> host => {fqdn, <<"muc.example.com">>}, no_stanzaid_element => true}), mod_caps => [{cache_life_time, 86}, {cache_size, 1000}], - mod_mam_cache_user => #{cache => [], muc => true, pm => true}, + mod_mam_cache_user => #{cache => default_config([modules, mod_mam_meta, cache]), muc => true, pm => true}, mod_offline => [{access_max_user_messages, max_user_offline_messages}, {backend, riak}, @@ -822,6 +822,8 @@ default_pool_conn_opts(_Type) -> mod_config(Module, ExtraOpts) -> maps:merge(default_mod_config(Module), ExtraOpts). +default_mod_config(mod_cache_users) -> + #{strategy => fifo, time_to_live => 480, number_of_segments => 3}; default_mod_config(mod_adhoc) -> #{iqdisc => one_queue, report_commands_node => false}; default_mod_config(mod_auth_token) -> @@ -907,7 +909,8 @@ default_mod_config(mod_vcard) -> default_mod_config(mod_version) -> #{iqdisc => no_queue, os_info => false}; default_mod_config(mod_mam_meta) -> - (common_mam_config())#{backend => rdbms, cache_users => true, cache => []}; + (common_mam_config())#{backend => rdbms, cache_users => true, + cache => default_config([modules, mod_mam_meta, cache])}; default_mod_config(mod_mam) -> maps:merge(common_mam_config(), default_config([modules, mod_mam_meta, pm])); default_mod_config(mod_mam_muc) -> @@ -927,6 +930,9 @@ default_config([modules, mod_mam_meta, pm]) -> #{archive_groupchats => false, same_mam_id_for_peers => false}; default_config([modules, mod_mam_meta, muc]) -> #{host => {prefix, <<"conference.">>}}; +default_config([modules, mod_mam_meta, cache]) -> + #{module => internal, strategy => fifo, + time_to_live => 480, number_of_segments => 3}; default_config([modules, mod_mam_meta, async_writer]) -> #{batch_size => 30, enabled => true, flush_interval => 2000, pool_size => 4 * erlang:system_info(schedulers_online)}; diff --git a/test/config_parser_SUITE.erl b/test/config_parser_SUITE.erl index 72f3784b3d..cd0d3b660d 100644 --- a/test/config_parser_SUITE.erl +++ b/test/config_parser_SUITE.erl @@ -1634,12 +1634,13 @@ mod_caps(_Config) -> ?errh(T(<<"cache_life_time">>, <<"infinity">>)). mod_cache_users(_Config) -> + check_module_defaults(mod_cache_users), + P = [modules, mod_cache_users], T = fun(K, V) -> #{<<"modules">> => #{<<"mod_cache_users">> => #{K => V}}} end, - M = fun(K, V) -> modopts(mod_cache_users, [{K, V}]) end, - ?cfgh(M(time_to_live, 8600), T(<<"time_to_live">>, 8600)), - ?cfgh(M(time_to_live, infinity), T(<<"time_to_live">>, <<"infinity">>)), - ?cfgh(M(number_of_segments, 10), T(<<"number_of_segments">>, 10)), - ?cfgh(M(strategy, fifo), T(<<"strategy">>, <<"fifo">>)), + ?cfgh(P ++ [time_to_live], 8600, T(<<"time_to_live">>, 8600)), + ?cfgh(P ++ [time_to_live], infinity, T(<<"time_to_live">>, <<"infinity">>)), + ?cfgh(P ++ [number_of_segments], 10, T(<<"number_of_segments">>, 10)), + ?cfgh(P ++ [strategy], fifo, T(<<"strategy">>, <<"fifo">>)), ?errh(T(<<"time_to_live">>, 0)), ?errh(T(<<"strategy">>, <<"lifo">>)), ?errh(T(<<"number_of_segments">>, 0)), @@ -2135,8 +2136,7 @@ mod_mam_meta(_Config) -> check_module_defaults(mod_mam_meta), T = fun(Opts) -> #{<<"modules">> => #{<<"mod_mam_meta">> => Opts}} end, P = [modules, mod_mam_meta], - test_segmented_cache_config(<<"cache">>, cache, T, - fun([{cache, Opts}]) -> [{P ++ [cache], Opts}] end), + test_cache_config(T, P), test_mod_mam_meta(T, P). mod_mam_meta_riak(_Config) -> @@ -2221,26 +2221,33 @@ test_mod_mam_meta(T, P) -> ?errh(T(#{<<"extra_fin_element">> => <<"bad_module">>})), ?errh(T(#{<<"extra_lookup_params">> => <<"bad_module">>})). -test_cache_config(T, M) -> - ?cfgh(M([{cache_users, false}]), - T(#{<<"cache_users">> => false})), - ?errh(T(#{<<"cache_users">> => []})), - test_segmented_cache_config(<<"cache">>, cache, T, M). +test_cache_config(ParentT, ParentP) -> + P = ParentP ++ [cache], + T = fun(Opts) -> ParentT(#{<<"cache">> => Opts}) end, + ?cfgh(P ++ [module], internal, T(#{<<"module">> => <<"internal">>})), + ?cfgh(P ++ [time_to_live], 8600, T(#{<<"time_to_live">> => 8600})), + ?cfgh(P ++ [time_to_live], infinity, T(#{<<"time_to_live">> => <<"infinity">>})), + ?cfgh(P ++ [number_of_segments], 10, T(#{<<"number_of_segments">> => 10})), + ?cfgh(P ++ [strategy], fifo, T(#{<<"strategy">> => <<"fifo">>})), + ?errh(T(#{<<"module">> => <<"mod_wrong_cache">>})), + ?errh(T(#{<<"time_to_live">> => 0})), + ?errh(T(#{<<"strategy">> => <<"lifo">>})), + ?errh(T(#{<<"number_of_segments">> => 0})), + ?errh(T(#{<<"number_of_segments">> => <<"infinity">>})), + ?errh(T(#{<<"cache">> => []})). test_segmented_cache_config(NameK, NameV, T, M) -> - ?cfgh(M([{NameV, [{module, internal}]}]), + ?cfgh(M([{NameV, #{module => internal}}]), T(#{NameK => #{<<"module">> => <<"internal">>}})), - ?cfgh(M([{NameV, [{time_to_live, 8600}]}]), + ?cfgh(M([{NameV, #{time_to_live => 8600}}]), T(#{NameK => #{<<"time_to_live">> => 8600}})), - ?cfgh(M([{NameV, [{time_to_live, infinity}]}]), + ?cfgh(M([{NameV, #{time_to_live => infinity}}]), T(#{NameK => #{<<"time_to_live">> => <<"infinity">>}})), - ?cfgh(M([{NameV, [{number_of_segments, 10}]}]), + ?cfgh(M([{NameV, #{number_of_segments => 10}}]), T(#{NameK => #{<<"number_of_segments">> => 10}})), - ?cfgh(M([{NameV, [{strategy, fifo}]}]), + ?cfgh(M([{NameV, #{strategy => fifo}}]), T(#{NameK => #{<<"strategy">> => <<"fifo">>}})), ?errh(T(#{NameK => #{<<"module">> => <<"mod_wrong_cache">>}})), - ?errh(T(#{NameK => #{<<"module">> => <<"mod_cache_users">>, - <<"time_to_live">> => 8600}})), ?errh(T(#{NameK => #{<<"time_to_live">> => 0}})), ?errh(T(#{NameK => #{<<"strategy">> => <<"lifo">>}})), ?errh(T(#{NameK => #{<<"number_of_segments">> => 0}})), diff --git a/test/mod_mam_meta_SUITE.erl b/test/mod_mam_meta_SUITE.erl index 86a4b33a9f..8481d24430 100644 --- a/test/mod_mam_meta_SUITE.erl +++ b/test/mod_mam_meta_SUITE.erl @@ -68,6 +68,7 @@ produces_valid_configurations(_Config) -> MUC = config([modules, mod_mam_meta, muc], maps:merge(MUCCoreOpts, MUCArchOpts#{user_prefs_store => mnesia})), Deps = deps(#{pm => PM, muc => MUC}), + Cache = default_config([modules, mod_mam_meta, cache]), check_equal_opts(mod_mam, mod_config(mod_mam, PMCoreOpts), Deps), check_equal_opts(mod_mam_muc, mod_config(mod_mam_muc, MUCCoreOpts), Deps), @@ -75,7 +76,7 @@ produces_valid_configurations(_Config) -> check_equal_opts(mod_mam_muc_rdbms_arch, mod_config(mod_mam_muc_rdbms_arch, MUCArchOpts#{no_writer => true}), Deps), check_equal_opts(mod_mam_rdbms_user, #{pm => true, muc => true}, Deps), - check_equal_opts(mod_mam_cache_user, #{pm => true, muc => true, cache => []}, Deps), + check_equal_opts(mod_mam_cache_user, #{pm => true, muc => true, cache => Cache}, Deps), check_equal_opts(mod_mam_mnesia_prefs, #{muc => true}, Deps), check_equal_opts(mod_mam_rdbms_prefs, #{pm => true}, Deps), check_equal_opts(mod_mam_muc_rdbms_arch_async, AsyncOpts, Deps). @@ -111,10 +112,11 @@ example_muc_only_no_pref_good_performance(_Config) -> MUC = config([modules, mod_mam_meta, muc], MUCOpts), Deps = deps(#{muc => MUC}), AsyncOpts = default_config([modules, mod_mam_meta, async_writer]), + Cache = default_config([modules, mod_mam_meta, cache]), check_equal_deps( [{mod_mam_rdbms_user, #{muc => true, pm => true}}, - {mod_mam_cache_user, #{muc => true, cache => []}}, + {mod_mam_cache_user, #{muc => true, cache => Cache}}, {mod_mam_muc_rdbms_arch, mod_config(mod_mam_muc_rdbms_arch, #{no_writer => true})}, {mod_mam_muc_rdbms_arch_async, AsyncOpts}, {mod_mam_muc, mod_config(mod_mam_muc, mod_config(mod_mam_muc, MUCOpts))} @@ -124,10 +126,11 @@ example_pm_only_good_performance(_Config) -> PM = default_config([modules, mod_mam_meta, pm]), Deps = deps(#{pm => PM, user_prefs_store => mnesia}), AsyncOpts = default_config([modules, mod_mam_meta, async_writer]), + Cache = default_config([modules, mod_mam_meta, cache]), check_equal_deps( [{mod_mam_rdbms_user, #{pm => true}}, - {mod_mam_cache_user, #{pm => true, cache => []}}, + {mod_mam_cache_user, #{pm => true, cache => Cache}}, {mod_mam_mnesia_prefs, #{pm => true}}, {mod_mam_rdbms_arch, mod_config(mod_mam_rdbms_arch, #{no_writer => true})}, {mod_mam_rdbms_arch_async, AsyncOpts},