Add mongoose_backend:get_backend_name #3345
Conversation
It returns the atom describing the backend, like `mssql`, `pgsql`, etc. This is analogous to `backend_name` function that was exposed by the dynamically compiled modules in `backend_module.erl`. It is used by some tests and `mongoose_rdbms`.
Because backends can be running per host type. This also fixes an issue of suites logging that they finished dirty, after having dynamically compiled backends removed. The old interface is called for the time being, but will be removed when all modules will have dynamic backends converted.
Codecov Report
@@ Coverage Diff @@
## without-dynamic-backend-modules #3345 +/- ##
===================================================================
- Coverage 80.73% 80.73% -0.01%
===================================================================
Files 401 401
Lines 32491 32496 +5
===================================================================
+ Hits 26232 26235 +3
- Misses 6259 6261 +2
Continue to review full report at Codecov.
|
|
small_tests_24 / small_tests / bffd681 internal_mnesia_24 / internal_mnesia / bffd681 small_tests_23 / small_tests / bffd681 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / bffd681 ldap_mnesia_24 / ldap_mnesia / bffd681 dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / bffd681 inbox_SUITE:one_to_one:check_total_unread_count_and_active_conv_count{error,
{timeout_when_waiting_for_stanza,
[{escalus_client,wait_for_stanza,
[{client,
<<"mike_check_total_unread_count_and_active_conv_count_94.546700@domain.example.com/res1">>,
escalus_tcp,<0.16349.0>,
[{event_manager,<0.16260.0>},
{server,<<"domain.example.com">>},
{username,
<<"mike_check_total_unread_count_and_active_conv_count_94.546700">>},
{resource,<<"res1">>}],
[{event_client,
[{event_manager,<0.16260.0>},
{server,<<"domain.example.com">>},
{username,
<<"mike_check_total_unread_count_and_active_conv_count_94.546700">>},
{resource,<<"res1">>}]},
{resource,<<"res1">>},
{username,
<<"mike_check_total_unread_count_and_active_conv_count_94.546700">>},
{server,<<"domain.example.com">>},
{host,<<"localhost">>},
{port,5222},
{auth,{escalus_auth,auth_plain}},
{wspath,undefined},
{username,
<<"mike_check_total_unread_count_and_active_conv_count_94.546700">>},
{server,<<"domain.example.com">>},
{host,<<"localhost">>},
{password,<<"nicniema">>},
{stream_id,<<"5d9a9069338ee0a7">>}]},
5000],
[{file,
"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
{line,136}]},
{inbox_helper,send_msg,3,
[...inbox_SUITE:one_to_one:carbons_are_not_stored{error,
{timeout_when_waiting_for_stanza,
[{escalus_client,wait_for_stanza,
[{client,
<<"bOb_carbons_are_not_stored_94.523012@domain.example.com/res1">>,
escalus_tcp,<0.16365.0>,
[{event_manager,<0.16197.0>},
{server,<<"domain.example.com">>},
{username,<<"bOb_carbons_are_not_stored_94.523012">>},
{resource,<<"res1">>}],
[{event_client,
[{event_manager,<0.16197.0>},
{server,<<"domain.example.com">>},
{username,<<"bOb_carbons_are_not_stored_94.523012">>},
{resource,<<"res1">>}]},
{resource,<<"res1">>},
{username,<<"bOb_carbons_are_not_stored_94.523012">>},
{server,<<"domain.example.com">>},
{host,<<"localhost">>},
{port,5222},
{auth,{escalus_auth,auth_plain}},
{wspath,undefined},
{username,<<"bOb_carbons_are_not_stored_94.523012">>},
{server,<<"domain.example.com">>},
{host,<<"localhost">>},
{password,<<"makrolika">>},
{stream_id,<<"2311b35fa666ba46">>}]},
5000],
[{file,
"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
{line,136}]},
{inbox_helper,'-given_conversations_between/2-fun-1-',4,
[{file,"/home/circleci/app/big_tests/tests/inbox_helper.erl"},
{line,420}]},
{lists,foldl,3,[{file,"lists.erl"},{line,1267}]},
{inbox...inbox_SUITE:one_to_one:reset_unread_counter_and_show_only_unread{error,
{timeout_when_waiting_for_stanza,
[{escalus_client,wait_for_stanza,
[{client,
<<"mike_reset_unread_counter_and_show_only_unread_94.553609@domain.example.com/res1">>,
escalus_tcp,<0.16360.0>,
[{event_manager,<0.16275.0>},
{server,<<"domain.example.com">>},
{username,
<<"mike_reset_unread_counter_and_show_only_unread_94.553609">>},
{resource,<<"res1">>}],
[{event_client,
[{event_manager,<0.16275.0>},
{server,<<"domain.example.com">>},
{username,
<<"mike_reset_unread_counter_and_show_only_unread_94.553609">>},
{resource,<<"res1">>}]},
{resource,<<"res1">>},
{username,
<<"mike_reset_unread_counter_and_show_only_unread_94.553609">>},
{server,<<"domain.example.com">>},
{host,<<"localhost">>},
{port,5222},
{auth,{escalus_auth,auth_plain}},
{wspath,undefined},
{username,
<<"mike_reset_unread_counter_and_show_only_unread_94.553609">>},
{server,<<"domain.example.com">>},
{host,<<"localhost">>},
{password,<<"nicniema">>},
{stream_id,<<"75c192fe88c622b8">>}]},
5000],
[{file,
"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
{line,136}]},
{inbox_helper,send_msg,3,
[{file,"/home/circleci/app...inbox_SUITE:one_to_one:user_has_only_unread_messages_or_only_read{error,
{timeout_when_waiting_for_stanza,
[{escalus_client,wait_for_stanza,
[{client,
<<"kate_user_has_only_unread_messages_or_only_read_94.560106@domain.example.com/res1">>,
escalus_tcp,<0.16379.0>,
[{event_manager,<0.16285.0>},
{server,<<"domain.example.com">>},
{username,
<<"kate_user_has_only_unread_messages_or_only_read_94.560106">>},
{resource,<<"res1">>}],
[{event_client,
[{event_manager,<0.16285.0>},
{server,<<"domain.example.com">>},
{username,
<<"kate_user_has_only_unread_messages_or_only_read_94.560106">>},
{resource,<<"res1">>}]},
{resource,<<"res1">>},
{username,
<<"kate_user_has_only_unread_messages_or_only_read_94.560106">>},
{server,<<"domain.example.com">>},
{host,<<"localhost">>},
{port,5222},
{auth,{escalus_auth,auth_plain}},
{wspath,undefined},
{username,
<<"kate_user_has_only_unread_messages_or_only_read_94.560106">>},
{server,<<"domain.example.com">>},
{host,<<"localhost">>},
{password,<<"makrowe;p">>},
{stream_id,<<"069cedf4642321a6">>}]},
5000],
[{file,
"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
{line,136}]},
{inbox_helper,'-given_conversations_between/2-fun-1-'...inbox_SUITE:one_to_one:user_has_two_conversations{error,
{timeout_when_waiting_for_stanza,
[{escalus_client,wait_for_stanza,
[{client,
<<"kate_user_has_two_conversations_94.525228@domain.example.com/res1">>,
escalus_tcp,<0.16372.0>,
[{event_manager,<0.16213.0>},
{server,<<"domain.example.com">>},
{username,<<"kate_user_has_two_conversations_94.525228">>},
{resource,<<"res1">>}],
[{event_client,
[{event_manager,<0.16213.0>},
{server,<<"domain.example.com">>},
{username,
<<"kate_user_has_two_conversations_94.525228">>},
{resource,<<"res1">>}]},
{resource,<<"res1">>},
{username,<<"kate_user_has_two_conversations_94.525228">>},
{server,<<"domain.example.com">>},
{host,<<"localhost">>},
{port,5222},
{auth,{escalus_auth,auth_plain}},
{wspath,undefined},
{username,<<"kate_user_has_two_conversations_94.525228">>},
{server,<<"domain.example.com">>},
{host,<<"localhost">>},
{password,<<"makrowe;p">>},
{stream_id,<<"ce7e5ab9357e6396">>}]},
5000],
[{file,
"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
{line,136}]},
{inbox_helper,'-given_conversations_between/2-fun-1-',4,
[{file,"/home/circleci/app/big_tests/tests/inbox_helper.erl"},
{line,420}]},
{lists,foldl,3,[{file...dynamic_domains_mysql_redis_24 / mysql_redis / bffd681 mam_SUITE:rdbms_prefs_cases:prefs_set_cdata_request{error,{test_case_failed,"ASSERT EQUAL\n\tExpected {prefs_result_iq,<<\"roster\">>,\n [<<\"montague@montague.net\">>,\n <<\"romeo@montague.net\">>],\n []}\n\tValue {prefs_result_iq,<<\"always\">>,\n [<<\"montague@montague.net\">>,\n <<\"romeo@montague.net\">>],\n []}\n"}}ldap_mnesia_23 / ldap_mnesia / bffd681 elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / bffd681 dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / bffd681 pgsql_mnesia_24 / pgsql_mnesia / bffd681 mssql_mnesia_24 / odbc_mssql_mnesia / bffd681 mysql_redis_24 / mysql_redis / bffd681 pgsql_mnesia_23 / pgsql_mnesia / bffd681 riak_mnesia_24 / riak_mnesia / bffd681 mod_ping_SUITE:server_ping:server_ping_pong{error,{{badmatch,[{[<<"localhost">>,mod_ping,ping_response],
{expected_diff,5},
{before_story,0},
{after_story,4}}]},
[{escalus_mongooseim,post_story_check_metrics,1,
[{file,"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_mongooseim.erl"},
{line,74}]},
{escalus_mongooseim,maybe_check_metrics_post_story,1,
[{file,"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_mongooseim.erl"},
{line,51}]},
{escalus_story,story,4,
[{file,"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,75}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1783}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1292}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1224}]}]}} |
src/mongoose_backend.erl
Outdated
| @@ -50,18 +51,28 @@ ensure_backend_metrics(MainModule, FunNames) -> | |||
| end, | |||
| lists:foreach(EnsureFun, FunNames). | |||
|
|
|||
| persist_backend_name(HostType, MainModule, Backend) -> | |||
| persist_backend_name(HostType, MainModule, Backend, BackendModule) -> | |||
| Key = backend_key(HostType, MainModule), | |||
There was a problem hiding this comment.
It is more performant to use two different keys for backend_name and backend_module.
And put two persistent terms.
There was a problem hiding this comment.
also, if it is only used in tests, you can just introduce:
get_backend_name(HostType, MainModule) ->
gen_mod:get_module_opt(HostType, MainModule, backend, mnesia).
There was a problem hiding this comment.
backend_name is used in mongoose_rdbms and tests - I think it's better to keep this API somewhere, because many places rely on it being just the backend_name.
We will probably need API to get backend_module in the near future anyway, as some modules use it with dynamically compiled backends... and we just deleted something like this from I got confused, of course it's in gen_mod :/mongoose_backend already...
Do you think it's better to have get_backend_name in gen_mod or mongoose_backend using persistent term or the module opts? I thought since we already put it in persistent term in mongoose_backend we could just use it similarly to backend_module (with different keys if it's more performant though).
There was a problem hiding this comment.
I've pushed the version with backend_name and backend_module split. Please see if it's OK.
There was a problem hiding this comment.
It is more performant to use two different keys for backend_name and backend_module.
And put two persistent terms.
Well, technically speaking, that might be not true. Persistent terms are implemented as very compact hash tables with shared memory, which both things statistically improve cache locality by a lot. And it makes me suspicious to see that we're injecting so so many small values to the persistent term table 🤔
Nevertheless that's an implementation detail well-hidden behind this module, it's easy to change and load-test later in the future if necessary, so both options are absolutely fine for now, it will still be much faster than ets 😄
src/mongoose_backend.erl
Outdated
| MainModule :: main_module()) -> BackendName :: atom(). | ||
| get_backend_name(HostType, MainModule) -> | ||
| Key = backend_key(HostType, MainModule), | ||
| %% Would crash, if the key is missing |
There was a problem hiding this comment.
You can handle it use persistent_term:get/2. Plus maybe make sense to log it(if key is missing).
There was a problem hiding this comment.
I think we want to have it crashed here - I followed what's in get_backend_module.
There was a problem hiding this comment.
then no make sense to keep this comment here - %% Would crash, if the key is missing, because crash is expected behavior for persistent_term:get/1.
src/mongoose_backend.erl
Outdated
|
|
||
| %% Get a backend name, stored in init_per_host_type | ||
| %% @doc Get a backend module, stored in init_per_host_type. | ||
| -spec get_backend_module(HostType :: mongooseim:host_type(), | ||
| MainModule :: main_module()) -> | ||
| BackendModule :: backend_module(). | ||
| get_backend_module(HostType, MainModule) -> | ||
| Key = backend_key(HostType, MainModule), | ||
| %% Would crash, if the key is missing |
There was a problem hiding this comment.
Maybe make sense remove this strange comment from here because crash is expected behavior if key is missing and this behavior clearly described in official OTP documentation, not sure that it make a big sense to repeat it in source code too...
There was a problem hiding this comment.
This is the same if someone will write something like:
...
%% Would crash, if the key is missing
#{key := Key} = Data,
...😁
|
small_tests_24 / small_tests / 99731fb internal_mnesia_24 / internal_mnesia / 99731fb amp_big_SUITE:offline:offline_failure:notify_deliver_to_offline_user_recipient_privacy_test{error,
{{assertion_failed,assert,is_presence,
{xmlel,<<"stream:error">>,[],
[{xmlel,<<"conflict">>,
[{<<"xmlns">>,<<"urn:ietf:params:xml:ns:xmpp-streams">>}],
[]},
{xmlel,<<"text">>,
[{<<"xml:lang">>,<<"en">>},
{<<"xmlns">>,<<"urn:ietf:params:xml:ns:xmpp-streams">>}],
[{xmlcdata,<<"Replaced by new connection">>}]}]},
"<stream:error><conflict xmlns='urn:ietf:params:xml:ns:xmpp-streams'/><text xml:lang='en' xmlns='urn:ietf:params:xml:ns:xmpp-streams'>Replaced by new connection</text></stream:error>"},
[{escalus_new_assert,assert_true,2,
[{file,
"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_new_assert.erl"},
{line,84}]},
{escalus_story,'-drop_presences/2-lc$^0/1-0-',1,
[{file,
"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,190}]},
{escalus_story,drop_presences,2,
[{file,
"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,190}]},
{escalus_story,'-start_ready_clients/2-fun-0-',3,
[{file,
"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,135}]},
{lists,foldl,3,[{file,"lists.erl"},{line,1267}]},
{escalus_story,start_ready_clients,2,
[{file,
"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,128}]},
...small_tests_23 / small_tests / 99731fb dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 99731fb dynamic_domains_mysql_redis_24 / mysql_redis / 99731fb ldap_mnesia_24 / ldap_mnesia / 99731fb dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 99731fb dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 99731fb elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 99731fb ldap_mnesia_23 / ldap_mnesia / 99731fb mysql_redis_24 / mysql_redis / 99731fb mssql_mnesia_24 / odbc_mssql_mnesia / 99731fb pgsql_mnesia_24 / pgsql_mnesia / 99731fb pgsql_mnesia_23 / pgsql_mnesia / 99731fb riak_mnesia_24 / riak_mnesia / 99731fb |
gustawlippa
force-pushed
the
wdbm-tests-mongoose_helper
branch
from
99731fb
to
a4f31fc
Compare
gustawlippa
force-pushed
the
wdbm-tests-mongoose_helper
branch
from
a4f31fc
to
01fb11a
Compare
|
small_tests_24 / small_tests / a4f31fc internal_mnesia_24 / internal_mnesia / a4f31fc |
|
small_tests_24 / small_tests / 01fb11a internal_mnesia_24 / internal_mnesia / 01fb11a small_tests_23 / small_tests / 01fb11a ldap_mnesia_24 / ldap_mnesia / 01fb11a dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 01fb11a dynamic_domains_mysql_redis_24 / mysql_redis / 01fb11a dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 01fb11a rest_client_SUITE:muc:messages_can_be_paginated_in_room{error,
{{assertion_failed,assert,is_chat_message,
[<<"11884d29">>],
{xmlel,<<"message">>,
[{<<"to">>,<<"1634-721436-59482@muclight2.domain.example.com">>},
{<<"type">>,<<"chat">>}],
[{xmlel,<<"body">>,[],[{xmlcdata,<<"63c121a8">>}]}]},
"<message to='1634-721436-59482@muclight2.domain.example.com' type='chat'><body>63c121a8</body></message>"},
[{escalus_new_assert,assert_true,2,
[{file,
"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_new_assert.erl"},
{line,84}]},
{rest_client_SUITE,assert_room_messages,2,
[{file,"/home/circleci/app/big_tests/tests/rest_client_SUITE.erl"},
{line,772}]},
{escalus_story,story,4,
[{file,
"/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1784}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1293}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1225}]}]}}ldap_mnesia_23 / ldap_mnesia / 01fb11a dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 01fb11a elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 01fb11a pgsql_mnesia_24 / pgsql_mnesia / 01fb11a mysql_redis_24 / mysql_redis / 01fb11a mssql_mnesia_24 / odbc_mssql_mnesia / 01fb11a pgsql_mnesia_23 / pgsql_mnesia / 01fb11a riak_mnesia_24 / riak_mnesia / 01fb11a |
This is analogous to the function exposed in the dynamically compiled backends. It is used in some places in MIM, and while the "backend_name" (the last part of the backend module name, like
pgsql,mnesiaetc.) can be extracted via string manipulation, it seems cleaner to just reintroduce this function.This change goes together with an update in
mongoose_helperfor the big tests. It is used to check if the suites finished dirty or not. After all backends will have been refactored, we can get rid of the*_oldfunctions of course.