Skip to content
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

Config in one persistent term #4093

Merged
merged 2 commits into from
Aug 17, 2023
Merged

Config in one persistent term #4093

merged 2 commits into from
Aug 17, 2023

Conversation

chrzaszcz
Copy link
Member

Put the whole config in one persistent term.

Motivation:

  • No need for a separate persistent term with config state.
  • More consistent get/set operations.
  • No leftover terms in small tests.
  • Marginally increased performance.

@chrzaszcz chrzaszcz changed the title WIP Config in one persistent term Aug 10, 2023
@mongoose-im

This comment was marked as outdated.

@codecov
Copy link

codecov bot commented Aug 10, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.03% 🎉

Comparison is base (e6b9cc2) 83.88% compared to head (6b846c6) 83.92%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4093      +/-   ##
==========================================
+ Coverage   83.88%   83.92%   +0.03%     
==========================================
  Files         551      551              
  Lines       33594    33583      -11     
==========================================
+ Hits        28180    28184       +4     
+ Misses       5414     5399      -15     
Files Changed Coverage Δ
src/pubsub/mod_pubsub.erl 75.82% <ø> (ø)
src/config/mongoose_config.erl 100.00% <100.00%> (+2.22%) ⬆️
src/config/mongoose_config_parser.erl 91.83% <100.00%> (ø)

... and 16 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mongoose-im

This comment was marked as outdated.

@mongoose-im

This comment was marked as outdated.

@mongoose-im

This comment was marked as outdated.

Motivation:
- Performance: adding Nth term has O(N) complexity
- Simplicity: no need to store and expose config state
- Consistency: top-level options are accessed the same way as nested ones

Getters for config states from the cluster are removed, because:
- Config module is responsible only for the local config (SoC)
- There is no "config state" anymore - you can just get the persistent term
- The code was a leftover from the legacy config
- Set the whole config at once
- Erase the whole config

Also

- Add some options that were not needed only because of incomplete
  cleanup of previous tests
@mongoose-im
Copy link
Collaborator

mongoose-im commented Aug 17, 2023

elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 6b846c6
Reports root/ big
OK: 369 / Failed: 0 / User-skipped: 38 / Auto-skipped: 0


small_tests_24 / small_tests / 6b846c6
Reports root / small


small_tests_25 / small_tests / 6b846c6
Reports root / small


small_tests_25_arm64 / small_tests / 6b846c6
Reports root / small


ldap_mnesia_24 / ldap_mnesia / 6b846c6
Reports root/ big
OK: 2274 / Failed: 0 / User-skipped: 831 / Auto-skipped: 0


ldap_mnesia_25 / ldap_mnesia / 6b846c6
Reports root/ big
OK: 2274 / Failed: 0 / User-skipped: 831 / Auto-skipped: 0


dynamic_domains_mysql_redis_25 / mysql_redis / 6b846c6
Reports root/ big
OK: 4201 / Failed: 0 / User-skipped: 116 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 6b846c6
Reports root/ big
OK: 4233 / Failed: 0 / User-skipped: 84 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 6b846c6
Reports root/ big
OK: 4233 / Failed: 0 / User-skipped: 84 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 6b846c6
Reports root/ big
OK: 4230 / Failed: 0 / User-skipped: 87 / Auto-skipped: 0


internal_mnesia_25 / internal_mnesia / 6b846c6
Reports root/ big
OK: 2420 / Failed: 0 / User-skipped: 685 / Auto-skipped: 0


mysql_redis_25 / mysql_redis / 6b846c6
Reports root/ big
OK: 4602 / Failed: 0 / User-skipped: 111 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / 6b846c6
Reports root/ big
OK: 4622 / Failed: 0 / User-skipped: 91 / Auto-skipped: 0


pgsql_cets_25 / pgsql_cets / 6b846c6
Reports root/ big
OK: 4591 / Failed: 1 / User-skipped: 121 / Auto-skipped: 0

carboncopy_SUITE:one2one:dropped_client_doesnt_create_duplicate_carbons
{error,
  {{badmatch,
     [{xmlel,<<"message">>,
        [{<<"from">>,
        <<"alice_dropped_client_doesnt_create_duplicate_carbons_565@localhost">>},
         {<<"to">>,
        <<"alice_dropped_client_doesnt_create_duplicate_carbons_565@localhost/res2">>},
         {<<"xmlns">>,<<"jabber:client">>},
         {<<"type">>,<<"chat">>}],
        [{xmlel,<<"sent">>,
           [{<<"xmlns">>,<<"urn:xmpp:carbons:2">>}],
           [{xmlel,<<"forwarded">>,
            [{<<"xmlns">>,<<"urn:xmpp:forward:0">>}],
            [{xmlel,<<"message">>,
               [{<<"from">>,
                 <<"alice_dropped_client_doesnt_create_duplicate_carbons_565@localhost/res1">>},
                {<<"to">>,
                 <<"bob_dropped_client_doesnt_create_duplicate_carbons_565@localhost/res1">>},
                {<<"type">>,<<"chat">>},
                {<<"xmlns">>,<<"jabber:client">>}],
               [{xmlel,<<"body">>,[],
                  [{xmlcdata,
                     <<"And pious action">>}]}]}]}]}]}]},
   [{carboncopy_SUITE,
      '-dropped_client_doesnt_create_duplicate_carbons/1-fun-0-',4,
      [{file,
         "/home/circleci/project/big_tests/tests/carboncopy_SUITE.erl"},
       {line,189}]},
    {escalus_story,story,4,
      [{file,
         "/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
       {line,72}]},
    {test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
    {test_server,run_test_case_eval1,6,
      [{file,"test_serv...

Report log


mssql_mnesia_25 / odbc_mssql_mnesia / 6b846c6
Reports root/ big
OK: 4619 / Failed: 0 / User-skipped: 94 / Auto-skipped: 0


pgsql_mnesia_25 / pgsql_mnesia / 6b846c6
Reports root/ big
OK: 4622 / Failed: 0 / User-skipped: 91 / Auto-skipped: 0


pgsql_cets_25 / pgsql_cets / 6b846c6
Reports root/ big
OK: 4592 / Failed: 0 / User-skipped: 121 / Auto-skipped: 0

@chrzaszcz chrzaszcz marked this pull request as ready for review August 17, 2023 09:33
Copy link
Collaborator

@NelsonVides NelsonVides left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!! 🔥

test/mod_global_distrib_SUITE.erl Show resolved Hide resolved
@NelsonVides NelsonVides merged commit 8c4047d into master Aug 17, 2023
@NelsonVides NelsonVides deleted the config-in-one-term branch August 17, 2023 13:36
@chrzaszcz chrzaszcz added this to the 6.2.0 milestone Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants