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

Rework S2S incoming connections and further listener unification #4470

Open
wants to merge 11 commits into
base: feature/listeners
Choose a base branch
from

Conversation

NelsonVides
Copy link
Collaborator

@NelsonVides NelsonVides commented Jan 23, 2025

With this transformation, now all three pure XMPP listeners, that is, c2s, s2s, and components, are very very similar. So given their only difference is now the state machine transformations, all other things like sockets or instrumentation can be unified.

So this, instead of creating mongoose_s2s_in+mongoose_s2s_in_socket+mongoose_s2s_in_ranch, like it was analogous for c2s or components, actually removes mongoose_(c2s|s2s|components)_(socket|ranch) and unifies all those six modules into a single mongoose_xmpp_socket, that is actually little over 250LOC of very predictable code, that can be used by all three c2s|s2s|component gen_statems.

This also unifies some instrumentation, as we had (c2s|s2s|components)_(tcp|tls)_data_(in|out), now we'll have just (tcp|tls)_data_(in|out), where the connection type will actually be a label (multi-dimensional metrics 😉).

Note: documentation needs a lot of verification later on, for example for the instrumentation renamings. This will be done in an upcoming PR once the whole feature branch is more or less ready.

@mongoose-im

This comment was marked as outdated.

Copy link

codecov bot commented Jan 23, 2025

Codecov Report

Attention: Patch coverage is 78.17048% with 105 lines in your changes missing coverage. Please review.

Project coverage is 85.48%. Comparing base (1d45385) to head (06a7d3a).

Files with missing lines Patch % Lines
src/s2s/mongoose_s2s_in.erl 69.56% 84 Missing ⚠️
src/listeners/mongoose_xmpp_socket.erl 85.00% 12 Missing ⚠️
src/c2s/mongoose_c2s.erl 88.88% 5 Missing ⚠️
src/component/mongoose_component_connection.erl 91.66% 2 Missing ⚠️
src/logger/mongoose_log_filter.erl 0.00% 1 Missing ⚠️
src/s2s/ejabberd_s2s_out.erl 75.00% 1 Missing ⚠️
Additional details and impacted files
@@                  Coverage Diff                  @@
##           feature/listeners    #4470      +/-   ##
=====================================================
- Coverage              85.55%   85.48%   -0.07%     
=====================================================
  Files                    562      558       -4     
  Lines                  34083    33967     -116     
=====================================================
- Hits                   29159    29038     -121     
- Misses                  4924     4929       +5     

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

This comes from the early 2000s when ejabberd's federation forgot to
enforce the version attribute to the stream opening header and other
servers had to workout a hack around ejabberd's bug.
@NelsonVides NelsonVides force-pushed the listeners/socket_unification branch 2 times, most recently from 2de5b95 to 6392f8e Compare January 23, 2025 15:50
@NelsonVides NelsonVides changed the title wip Rework S2S incoming connections and further listener unification Jan 23, 2025
@mongoose-im

This comment was marked as outdated.

@NelsonVides NelsonVides force-pushed the listeners/socket_unification branch from 6392f8e to 1e1eaf3 Compare January 23, 2025 20:40
@mongoose-im

This comment was marked as outdated.

@mongoose-im

This comment was marked as outdated.

@NelsonVides NelsonVides force-pushed the listeners/socket_unification branch from 0db6508 to e3dcc31 Compare January 23, 2025 21:24
@mongoose-im

This comment was marked as outdated.

@NelsonVides NelsonVides force-pushed the listeners/socket_unification branch from e3dcc31 to 06a7d3a Compare January 24, 2025 06:52
@mongoose-im
Copy link
Collaborator

mongoose-im commented Jan 24, 2025

elasticsearch_and_cassandra_27 / elasticsearch_and_cassandra_mnesia / 06a7d3a
Reports root/ big
OK: 472 / Failed: 0 / User-skipped: 49 / Auto-skipped: 0


small_tests_26 / small_tests / 06a7d3a
Reports root / small


small_tests_27 / small_tests / 06a7d3a
Reports root / small


small_tests_27_arm64 / small_tests / 06a7d3a
Reports root / small


ldap_mnesia_26 / ldap_mnesia / 06a7d3a
Reports root/ big
OK: 2295 / Failed: 0 / User-skipped: 1007 / Auto-skipped: 0


ldap_mnesia_27 / ldap_mnesia / 06a7d3a
Reports root/ big
OK: 2295 / Failed: 0 / User-skipped: 1022 / Auto-skipped: 0


dynamic_domains_mysql_redis_27 / mysql_redis / 06a7d3a
Reports root/ big
OK: 4797 / Failed: 0 / User-skipped: 152 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_27 / pgsql_mnesia / 06a7d3a
Reports root/ big
OK: 4832 / Failed: 0 / User-skipped: 117 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_26 / pgsql_mnesia / 06a7d3a
Reports root/ big
OK: 4817 / Failed: 0 / User-skipped: 117 / Auto-skipped: 0


internal_mnesia_27 / internal_mnesia / 06a7d3a
Reports root/ big
OK: 2429 / Failed: 0 / User-skipped: 888 / Auto-skipped: 0


pgsql_cets_27 / pgsql_cets / 06a7d3a
Reports root/ big
OK: 4904 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0


pgsql_mnesia_26 / pgsql_mnesia / 06a7d3a
Reports root/ big
OK: 5190 / Failed: 0 / User-skipped: 126 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_27 / odbc_mssql_mnesia / 06a7d3a
Reports root/ big
OK: 4827 / Failed: 0 / User-skipped: 122 / Auto-skipped: 0


mysql_redis_27 / mysql_redis / 06a7d3a
Reports root/ big
OK: 5184 / Failed: 0 / User-skipped: 147 / Auto-skipped: 0


pgsql_mnesia_27 / pgsql_mnesia / 06a7d3a
Reports root/ big
OK: 5205 / Failed: 0 / User-skipped: 126 / Auto-skipped: 0


cockroachdb_cets_27 / cockroachdb_cets / 06a7d3a
Reports root/ big
OK: 4904 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0


mssql_mnesia_27 / odbc_mssql_mnesia / 06a7d3a
Reports root/ big
OK: 5200 / Failed: 0 / User-skipped: 131 / Auto-skipped: 0

@NelsonVides NelsonVides marked this pull request as ready for review January 24, 2025 09:47
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.

2 participants