mongoose_c2s prototype #3729
mongoose_c2s prototype #3729
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Codecov ReportBase: 82.73% // Head: 13.58% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## feature/mongoose_c2s #3729 +/- ##
=========================================================
- Coverage 82.73% 13.58% -69.16%
=========================================================
Files 529 535 +6
Lines 33957 34535 +578
=========================================================
- Hits 28094 4690 -23404
- Misses 5863 29845 +23982
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
NelsonVides
force-pushed
the
c2s_prototype
branch
from
c46aeb4
to
db412cc
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
NelsonVides
force-pushed
the
c2s_prototype
branch
from
f7992c4
to
6360feb
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This time the listener is supervised using ranch, which already does everything our mongoose_tcp_listener does, plus a few more details like using pools of supervisors instead of all the many c2s processes having to be started and handled their termination through a single supervisor that can become a bottleneck. Using ranch of course also means one piece of our architecture that we don't need to maintain, but that the industry already maintains and optimise for us. Open source! The new mongoose_c2s_listener module implements our mongoose_listener behaviour, that gives mongoose_listener_sup a child_spec with the ranch definitions.
NelsonVides
force-pushed
the
c2s_prototype
branch
from
dc640ac
to
fc5fb5c
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
NelsonVides
force-pushed
the
c2s_prototype
branch
from
2bcb140
to
48c4da8
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
The flow is as follows:
init casts a `connect` message, to indicate the process that it needs to
establish the tcp connection, outside of the init function that blocks
the supervisor.
Then all states are handled in a single handle_event. The event connect
in state connecting indicates to end negotiating the connection. The
event info with payload from tcp or ssl, regardless of the state, simply
parses the payload using exml_stream, and adds all parsed packets as
internal action events of the OTP's gen_statem behaviour, that is, they
will be processed immediately as the next event in the state machine,
without involved message passing.
Then, as internal events, all packets are processed, according to their
running state machine state. One specific event is expected in that
state, and an appropriate handle_* helper does the work. States can be
compound, so now instead of sticking data of a specific state into the
state-machine memory, we can make it part of the state itself, and it
will be used in the next event in that state, see `{wait_for_feature,
after auth, _}` and `{wait_for_feature, before_auth, _}`.
Code that specifies stanzas is moved to a helper module that construct
all the verbose #xmlel{} packets, to keep the c2s module smaller and
separate protocol from state machine.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
NelsonVides
force-pushed
the
c2s_prototype
branch
from
50d60b4
to
a21bb34
Compare
NelsonVides
force-pushed
the
c2s_prototype
branch
from
a21bb34
to
225849a
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
As of now, in small-tests only websockets are failing, because they look at internals of ranch and ejabberd_sm, which are now different. So we could either ignore that error or manually comment out those test, and fix it in a subsequent PR. |
| FinalEl = jlib:replace_from_to(From, To, El), | ||
| ParamsAcc = #{from_jid => From, to_jid => To, element => FinalEl}, | ||
| Acc1 = mongoose_acc:update_stanza(ParamsAcc, Acc), | ||
| case mongoose_c2s_hooks:user_receive_packet(HostType, Acc1, hook_arg(StateData)) of |
There was a problem hiding this comment.
I agree that it might be cleaner not to have to pattern match for the specific stanza types in handlers. I'm just not convinced that it's faster, because for each message we need to run two hooks instead of one, and calling a hook takes some time, including reading the ETS table. Anyway, we've already discussed this, and the reviewed version is certainly good enough for me.
| <<"proxy_protocol">> => false, | ||
| <<"hibernate_after">> => 0, | ||
| <<"max_stanza_size">> => infinity, |
There was a problem hiding this comment.
Why change it? I think it makes more sense than after the change, and we use the atom infinity quite consistently throughout the code. OTP also uses it.
There was a problem hiding this comment.
This one because the process function is ran after the defaults, not before, so if I leave infinity as default, then c2s will get an infinity and will have to do the conversion for exml.
There was a problem hiding this comment.
I meant "why not keep infinity". I see that exml uses zero. Let's keep it.
NelsonVides
force-pushed
the
c2s_prototype
branch
from
6e1dac5
to
3d7a35a
Compare
|
dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 3d7a35a ldap_mnesia_24 / ldap_mnesia / 3d7a35a pgsql_mnesia_24 / pgsql_mnesia / 3d7a35a riak_mnesia_24 / riak_mnesia / 3d7a35a ldap_mnesia_25 / ldap_mnesia / 3d7a35a dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 3d7a35a internal_mnesia_25 / internal_mnesia / 3d7a35a mysql_redis_25 / mysql_redis / 3d7a35a dynamic_domains_mysql_redis_25 / mysql_redis / 3d7a35a pgsql_mnesia_25 / pgsql_mnesia / 3d7a35a elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 3d7a35a dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 3d7a35a mssql_mnesia_25 / odbc_mssql_mnesia / 3d7a35a small_tests_24 / small_tests / 3d7a35a small_tests_25 / small_tests / 3d7a35a |
This PR prepares the backbone of this effort. I define here the interface for the new
mongoose_c2sin a way that I'm confident allows implementing everything else (presences, roster, stream management, ping, privacy lists, as plug&play modules that hook in the eventing interface.What has been done in this PR:
mongoose_tcp_listener: take advantage of the optimised supervision trees and linux port capabilities.ejabberd_c2sandejabberd_receiver: decrypt and parse on the spot, avoiding more message passing.gen_statem: specially take advantage of thegen_statem:actions()capabilities.session, make presences independent of messaging.For implementation detail, see the explanations in the internal documentation.