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

S2S/out rework #4479

Merged
merged 15 commits into from
Feb 11, 2025
Merged

S2S/out rework #4479

merged 15 commits into from
Feb 11, 2025

Conversation

NelsonVides
Copy link
Collaborator

@NelsonVides NelsonVides commented Feb 4, 2025

The first important changes to notice are those in mongoose_xmpp_socket; it adds:

  • API for the client side for connecting a new socket (whether TLS or not),
  • API for the client side upgrading a tcp socket to a TLS one.

The next big thing is the mongoose_addr_list module. It uses inet_res:lookup/5 alone to implement DNS discovery as required in the documents below, and also encapsulates preparing all data necessary to know where to connect to. It is currently used by s2s alone, but so much of the logic could be reused by a client by minimal extensions to this module. I'm not implementing the client side of it in this rework though.


Next, the actual big thing: replace s2s_out modules with a new gen_statem based mongoose_s2s_out module.

  • Dialback is still inside, but better separated which process will be an outgoing connection and which one will be just a dialback, and dialback processes are now actually terminated.
  • There's so much similar code now in s2s_out, s2s_in, component_connection, and c2s, that it is left to wonder to what extend it can be further unified and to what extend it is just a consistent pattern :)

At last, many related minor changes are scattered around code, like:

  • Optimisations on mongoose_s2s_dialback (exml attrs matching),
  • Optimisations on mongoose_s2s_lib (avoid binary<->string back-nd-forth conversions if not needed),
  • Typing information in ejabberd_s2s (more explicit binary shapes than just binary).

@mongoose-im

This comment was marked as outdated.

Copy link

codecov bot commented Feb 4, 2025

Codecov Report

Attention: Patch coverage is 82.09366% with 65 lines in your changes missing coverage. Please review.

Project coverage is 85.64%. Comparing base (a610a89) to head (e70c80a).
Report is 20 commits behind head on feature/listeners.

Files with missing lines Patch % Lines
src/s2s/mongoose_s2s_out.erl 76.81% 48 Missing ⚠️
src/mongoose_addr_list.erl 91.13% 7 Missing ⚠️
src/listeners/mongoose_xmpp_socket.erl 85.71% 3 Missing ⚠️
src/mod_bosh_socket.erl 0.00% 2 Missing ⚠️
src/mod_websockets.erl 0.00% 2 Missing ⚠️
src/cert_utils.erl 0.00% 1 Missing ⚠️
src/config/mongoose_config_spec.erl 80.00% 1 Missing ⚠️
src/s2s/ejabberd_s2s.erl 91.66% 1 Missing ⚠️
Additional details and impacted files
@@                  Coverage Diff                  @@
##           feature/listeners    #4479      +/-   ##
=====================================================
+ Coverage              85.48%   85.64%   +0.15%     
=====================================================
  Files                    558      558              
  Lines                  33971    33754     -217     
=====================================================
- Hits                   29041    28907     -134     
+ Misses                  4930     4847      -83     

☔ 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.

@NelsonVides NelsonVides changed the title S2s/out rework S2S/out rework Feb 5, 2025
@mongoose-im

This comment was marked as outdated.

@mongoose-im

This comment was marked as outdated.

@mongoose-im

This comment was marked as outdated.

@mongoose-im

This comment was marked as outdated.

@NelsonVides NelsonVides force-pushed the s2s/out_rework branch 3 times, most recently from 3d394ec to 26fe76e Compare February 5, 2025 11:25
@mongoose-im

This comment was marked as outdated.

@NelsonVides NelsonVides marked this pull request as ready for review February 5, 2025 15:09
Copy link
Collaborator

@DenysGonchar DenysGonchar left a comment

Choose a reason for hiding this comment

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

looks very good but some minor comments must be addressed.

@mongoose-im
Copy link
Collaborator

mongoose-im commented Feb 11, 2025

elasticsearch_and_cassandra_27 / elasticsearch_and_cassandra_mnesia / e70c80a
Reports root/ big
OK: 683 / Failed: 0 / User-skipped: 72 / Auto-skipped: 0


small_tests_26 / small_tests / e70c80a
Reports root / small


small_tests_27 / small_tests / e70c80a
Reports root / small


small_tests_27_arm64 / small_tests / e70c80a
Reports root / small


dynamic_domains_mysql_redis_27 / mysql_redis / e70c80a
Reports root/ big
OK: 4809 / Failed: 0 / User-skipped: 154 / Auto-skipped: 0


ldap_mnesia_27 / ldap_mnesia / e70c80a
Reports root/ big
OK: 2297 / Failed: 0 / User-skipped: 1022 / Auto-skipped: 0


ldap_mnesia_26 / ldap_mnesia / e70c80a
Reports root/ big
OK: 2297 / Failed: 0 / User-skipped: 1007 / Auto-skipped: 0


internal_mnesia_27 / internal_mnesia / e70c80a
Reports root/ big
OK: 2431 / Failed: 0 / User-skipped: 888 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_27 / pgsql_mnesia / e70c80a
Reports root/ big
OK: 4844 / Failed: 0 / User-skipped: 119 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_27 / odbc_mssql_mnesia / e70c80a
Reports root/ big
OK: 4839 / Failed: 0 / User-skipped: 124 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_26 / pgsql_mnesia / e70c80a
Reports root/ big
OK: 4829 / Failed: 0 / User-skipped: 119 / Auto-skipped: 0


pgsql_cets_27 / pgsql_cets / e70c80a
Reports root/ big
OK: 4916 / Failed: 0 / User-skipped: 188 / Auto-skipped: 0


pgsql_mnesia_26 / pgsql_mnesia / e70c80a
Reports root/ big
OK: 5204 / Failed: 0 / User-skipped: 128 / Auto-skipped: 0


pgsql_mnesia_27 / pgsql_mnesia / e70c80a
Reports root/ big
OK: 5219 / Failed: 0 / User-skipped: 128 / Auto-skipped: 0


cockroachdb_cets_27 / cockroachdb_cets / e70c80a
Reports root/ big
OK: 4916 / Failed: 0 / User-skipped: 188 / Auto-skipped: 0


mysql_redis_27 / mysql_redis / e70c80a
Reports root/ big
OK: 5197 / Failed: 1 / User-skipped: 149 / Auto-skipped: 0

accounts_SUITE:register:already_registered
{error,
  {timeout_when_waiting_for_stanza,
    [{escalus_client,wait_for_stanza,
       [{client,<<"alice_already_registered_25@localhost/res1">>,
          escalus_tcp,<0.1376.0>,
          [{event_manager,<0.1342.0>},
           {server,<<"localhost">>},
           {username,<<"alicE_already_registered_25">>},
           {resource,<<"res1">>}],
          [{event_client,
             [{event_manager,<0.1342.0>},
            {server,<<"localhost">>},
            {username,<<"alicE_already_registered_25">>},
            {resource,<<"res1">>}]},
           {resource,<<"res1">>},
           {username,<<"alice_already_registered_25">>},
           {server,<<"localhost">>},
           {host,<<"localhost">>},
           {port,5222},
           {auth,fun escalus_auth:auth_plain/2},
           {wspath,undefined},
           {username,<<"alicE_already_registered_25">>},
           {server,<<"localhost">>},
           {password,<<"matygrysa">>},
           {stream_id,<<"9282954657ca95a3">>}]},
        5000],
       [{file,
          "/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
        {line,136}]},
     {accounts_SUITE,already_registered_story,1,
       [{file,
          "/home/circleci/project/big_tests/tests/accounts_SUITE.erl"},
        {line,198}]},
     {escalus_story,story,4,
       [{file,
          "/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
        {line,75}]},
     {test_server,ts_tc,3,[{file...

Report log


mssql_mnesia_27 / odbc_mssql_mnesia / e70c80a
Reports root/ big
OK: 5214 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


mysql_redis_27 / mysql_redis / e70c80a
Reports root/ big
OK: 20 / Failed: 0 / User-skipped: 0 / Auto-skipped: 0

Copy link
Collaborator

@DenysGonchar DenysGonchar left a comment

Choose a reason for hiding this comment

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

Thanks

@DenysGonchar DenysGonchar merged commit ed4c58c into feature/listeners Feb 11, 2025
3 of 4 checks passed
@DenysGonchar DenysGonchar deleted the s2s/out_rework branch February 11, 2025 14:01
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