-
Notifications
You must be signed in to change notification settings - Fork 428
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Service management rework #3620
Conversation
small_tests_24 / small_tests / 7ebbfc3 small_tests_23 / small_tests / 7ebbfc3 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 7ebbfc3 dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 7ebbfc3 dynamic_domains_mysql_redis_24 / mysql_redis / 7ebbfc3 dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 7ebbfc3 ldap_mnesia_23 / ldap_mnesia / 7ebbfc3 ldap_mnesia_24 / ldap_mnesia / 7ebbfc3 internal_mnesia_24 / internal_mnesia / 7ebbfc3 pgsql_mnesia_24 / pgsql_mnesia / 7ebbfc3 elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 7ebbfc3 pgsql_mnesia_23 / pgsql_mnesia / 7ebbfc3 mysql_redis_24 / mysql_redis / 7ebbfc3 riak_mnesia_24 / riak_mnesia / 7ebbfc3 mssql_mnesia_24 / odbc_mssql_mnesia / 7ebbfc3 inbox_extensions_SUITE:async_pools:one_to_one:archive_full_archive_can_be_fetched{error,
{{inbox_size,ok,
[{times,1,
{error,
{inbox_size,3,
[{times,1,
#{respond_iq =>
{xmlel,<<"iq">>,
[{<<"from">>,
<<"alicE_archive_full_archive_can_be_fetched_912@localhost">>},
{<<"to">>,
<<"alicE_archive_full_archive_can_be_fetched_912@localhost/res1">>},
{<<"id">>,<<"d55356146a3b9c8d899ca19f88f5dcbd">>},
{<<"type">>,<<"result">>}],
[{xmlel,<<"fin">>,
[{<<"xmlns">>,<<"erlang-solutions.com:xmpp:inbox:0">>}],
[{xmlel,<<"active-conversations">>,[],[{xmlcdata,<<"0">>}]},
{xmlel,<<"count">>,[],[{xmlcdata,<<"2">>}]},
{xmlel,<<"unread-messages">>,[],[{xmlcdata,<<"0">>}]}]}]},
respond_messages =>
[{xmlel,<<"message">>,
[{<<"from">>,
<<"alicE_archive_full_archive_can_be_fetched_912@localhost">>},
{<<"to">>,
<<"alicE_archive_full_archive_can_be_fetched_912@localhost/res1">>},
{<<"id">>,<<"1648-800900-713129">>}],
[{xmlel,<<"result">>,
[{<<"xmlns">>,<<"erlang-solutions.com:xmpp:inbox:0">>},
{<<"unread">>,<<"0">>},
{<<"queryid">>,<<"d55356146a3b9c8d899ca19f88f5dcbd">>}],
[{xmlel,<<"forwarded">>,
[{<<"xmlns">>,<<"urn:xmpp:forward:0">>}],
[{xmlel,<<"delay">>,
[{<<"xmlns">>,<<"urn:xmpp:delay">>},
{<<"stamp">>,<<"2022-04-01T08:14:58.273585Z">>}],
[]},
{xmlel,<<"message">>,
[{<<"from">>,
<<"alic... |
Codecov Report
@@ Coverage Diff @@
## master #3620 +/- ##
==========================================
+ Coverage 80.96% 80.98% +0.01%
==========================================
Files 424 425 +1
Lines 32061 32069 +8
==========================================
+ Hits 25958 25970 +12
+ Misses 6103 6099 -4
Continue to review full report at Codecov.
|
small_tests_24 / small_tests / 4a3fec4 small_tests_23 / small_tests / 4a3fec4 dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 4a3fec4 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 4a3fec4 dynamic_domains_mysql_redis_24 / mysql_redis / 4a3fec4 ldap_mnesia_23 / ldap_mnesia / 4a3fec4 dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 4a3fec4 ldap_mnesia_24 / ldap_mnesia / 4a3fec4 internal_mnesia_24 / internal_mnesia / 4a3fec4 pgsql_mnesia_23 / pgsql_mnesia / 4a3fec4 pgsql_mnesia_24 / pgsql_mnesia / 4a3fec4 elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 4a3fec4 mysql_redis_24 / mysql_redis / 4a3fec4 mssql_mnesia_24 / odbc_mssql_mnesia / 4a3fec4 riak_mnesia_24 / riak_mnesia / 4a3fec4 |
Unify service and module processing: - Resolve deps - Unfold opts (this is temporary until only maps are used) Also: move 'build_state' to a more suitable place.
Unify the way services and modules are management Follow the solution for modules: - Store the config in a map and do not copy the config to ETS. This prevents inconsistency between the two and speeds up access. - Delegate dependency management to a separate module. - Start and stop all services in the dependency-related order. - Provide API for ensuring a service is stopped/started in tests, updating the configuration accordingly. - Drop the check if MongooseIM is running, because it does not seem to be a real risk. Anyway, service management code is not special and should not take the responsibility of stopping the whole node.
Follow the logic that is already present for modules. There are two main steps: 1. Dependency resolution. 2. Service sorting according to the dependency graph. Only maps are supported, but it is not a problem, because no services have deps anyway and all of them are going to use maps very soon.
- Services are always present - Service opts are unfolded Also: remove unused function
Follow the module tests - Simplify service start/stop tests - Check a subset of the deps cases for module corresponding to the features that are present for services
This check is in gen_mod, so the test should be here as well. Also: update module tests to use maps.
small_tests_24 / small_tests / 1a7a30d small_tests_23 / small_tests / 1a7a30d dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 1a7a30d dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 1a7a30d dynamic_domains_mysql_redis_24 / mysql_redis / 1a7a30d ldap_mnesia_24 / ldap_mnesia / 1a7a30d ldap_mnesia_23 / ldap_mnesia / 1a7a30d dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 1a7a30d internal_mnesia_24 / internal_mnesia / 1a7a30d pgsql_mnesia_24 / pgsql_mnesia / 1a7a30d pgsql_mnesia_23 / pgsql_mnesia / 1a7a30d elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 1a7a30d mysql_redis_24 / mysql_redis / 1a7a30d mssql_mnesia_24 / odbc_mssql_mnesia / 1a7a30d riak_mnesia_24 / riak_mnesia / 1a7a30d |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looks good to me :) I like the new service tests, look more clearly.
Rework service configuration and management, following the logic that is already present for modules, and thus making the two more unified.
Main changes:
mongoose_service
, like modules are managed inmongoose_modules
. There is some discrepancy in naming, but the name is not changed asmongoose_service
is itself a behaviour. There is noreplace_services
logic for tests, because it is would be just unused in the current tests.mongoose_service_deps
, similarly togen_mod_deps
. Service deps are always hard and never have opts, which simplifies the logic.See the commit messages for more details.