-
Notifications
You must be signed in to change notification settings - Fork 429
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
CETS backend for mod_muc #4066
CETS backend for mod_muc #4066
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Just one function in mongoose_muc_online_mnesia so far
Because we don't really need to
b1bc341
to
e1d2037
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## feature/cets #4066 +/- ##
=============================================
Coverage 83.95% 83.95%
=============================================
Files 543 546 +3
Lines 33454 33519 +65
=============================================
+ Hits 28087 28142 +55
- Misses 5367 5377 +10
☔ View full report in Codecov by Sentry. |
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.
ac50890
to
660d2b0
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.
65dcec3
to
09a83fe
Compare
small_tests_24 / small_tests / 09a83fe elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 09a83fe small_tests_25_arm64 / small_tests / 09a83fe small_tests_25 / small_tests / 09a83fe dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 09a83fe ldap_mnesia_24 / ldap_mnesia / 09a83fe ldap_mnesia_25 / ldap_mnesia / 09a83fe dynamic_domains_mysql_redis_25 / mysql_redis / 09a83fe pgsql_mnesia_24 / pgsql_mnesia / 09a83fe dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 09a83fe dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 09a83fe pgsql_cets_25 / pgsql_cets / 09a83fe mysql_redis_25 / mysql_redis / 09a83fe mam_SUITE:rdbms_async_cache_muc_all:muc06:muc_message_with_stanzaid{failed,
{mam_SUITE,end_per_testcase,
{'EXIT',
{{room_archive_size,0,[{times,200,1}],ok},
[{mongoose_helper,do_wait_until,2,
[{file,
"/home/circleci/project/big_tests/tests/mongoose_helper.erl"},
{line,358}]},
{mam_helper,wait_for_room_archive_size,3,
[{file,
"/home/circleci/project/big_tests/tests/mam_helper.erl"},
{line,789}]},
{mam_helper,clean_room_archive,1,
[{file,
"/home/circleci/project/big_tests/tests/mam_helper.erl"},
{line,733}]},
{mam_helper,destroy_room,1,
[{file,
"/home/circleci/project/big_tests/tests/mam_helper.erl"},
{line,726}]},
{mam_SUITE,end_per_testcase,2,
[{file,
"/home/circleci/project/big_tests/tests/mam_SUITE.erl"},
{line,954}]},
{test_server,do_end_per_testcase,4,
[{file,"test_server.erl"},{line,1626}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1334}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}}}} pgsql_mnesia_25 / pgsql_mnesia / 09a83fe mod_global_distrib_SUITE:hosts_refresher:test_host_refreshing{error,
{{trees_for_connections_present,true,[{times,50,false}],ok},
[{mongoose_helper,do_wait_until,2,
[{file,"/home/circleci/project/big_tests/tests/mongoose_helper.erl"},
{line,358}]},
{mod_global_distrib_SUITE,test_host_refreshing,1,
[{file,
"/home/circleci/project/big_tests/tests/mod_global_distrib_SUITE.erl"},
{line,384}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}} internal_mnesia_25 / internal_mnesia / 09a83fe metrics_c2s_SUITE:single:presence_one{error,
{{xmppPresenceReceived,
{value,141},
[{times,25,
{error,
{badmatch,{value,142}},
[{metrics_helper,assert_counter,3,
[{file,
"/home/circleci/project/big_tests/tests/metrics_helper.erl"},
{line,36}]},
{mongoose_helper,do_wait_until,2,
[{file,
"/home/circleci/project/big_tests/tests/mongoose_helper.erl"},
{line,360}]},
{metrics_c2s_SUITE,'-presence_one/1-fun-0-',1,
[{file,
"/home/circleci/project/big_tests/tests/metrics_c2s_SUITE.erl"},
{line,125}]},
{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_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}}],
ok},
[{mongoose_helper,do_wait_until,2,
[{file,"/home/circleci/project/big_tests/tests/mongoose_helper.erl"},
{line,358}]},
{metrics_c2s_SUITE,'-presence_one/1-fun-0-',1,
[{file,
"/home/circleci/project/big_tests/tests/metrics_c2s_SUITE.erl"},
{line,125}]},
{escalus_story,story,4,
[{file,
"/home/circleci/project/big_test... mssql_mnesia_25 / odbc_mssql_mnesia / 09a83fe muc_SUITE:register:user_unregisters_nick{error,
{{assertion_failed,assert,is_iq_result,
[{xmlel,<<"iq">>,
[{<<"type">>,<<"set">>},
{<<"id">>,<<"d2f82dd8b8b6c039b0308dfbaf7eab08">>},
{<<"to">>,<<"muc.localhost">>}],
[{xmlel,<<"query">>,
[{<<"xmlns">>,<<"jabber:iq:register">>}],
[{xmlel,<<"x">>,
[{<<"xmlns">>,<<"jabber:x:data">>},
{<<"type">>,<<"submit">>}],
[{xmlel,<<"field">>,
[{<<"type">>,<<"hidden">>},
{<<"var">>,<<"FORM_TYPE">>}],
[{xmlel,<<"value">>,[],
[{xmlcdata,<<"jabber:iq:register">>}]}]},
{xmlel,<<"field">>,
[{<<"type">>,<<"text-single">>},
{<<"var">>,<<"nick">>}],
[{xmlel,<<"value">>,[],
[{xmlcdata,
<<"thirdwitch1room-ece75af22c">>}]}]}]}]}]}],
{xmlel,<<"iq">>,
[{<<"from">>,<<"muc.localhost">>},
{<<"to">>,<<"alice_user_unregisters_nick_2689@localhost/res1">>},
{<<"type">>,<<"error">>},
{<<"id">>,<<"d2f82dd8b8b6c039b0308dfbaf7eab08">>}],
[{xmlel,<<"query">>,
[{<<"xmlns">>,<<"jabber:iq:register">>}],
[{xmlel,<<"x">>,
[{<<"xmlns">>,<<"jabber:x:data">>},
{<<"type">>,<<"submit">>}],
[{xmlel,<<"field">>,
[{<<"type">>,<<"hidden">>},
{<<"var">>,<<"FORM_TYPE">>}],
[{xmlel,<<"value">>,[],
[{xmlcdata... internal_mnesia_25 / internal_mnesia / 09a83fe pgsql_mnesia_25 / pgsql_mnesia / 09a83fe mssql_mnesia_25 / odbc_mssql_mnesia / 09a83fe |
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.
Great work 👍 I left a question so I can understand one thing better. Also, I will have another look on this PR once you push new tests to satisfy CI
Tab = table_name(HostType), | ||
Pattern = {'_', '$1'}, | ||
Guard = {'==', {node, '$1'}, Node}, | ||
ets:select_delete(Tab, [{Pattern, [Guard], [true]}]), |
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.
It's strange that I didn't come up with this question in one of the earlier reviews 😅 I understand that for lookups and reads we can use straight ets
API but why is it safe to use it for deleting items? On the other hand in room_destroyed
function there is a call for cets
API - cets:delete_object
. What's the difference?
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.
node_cleanup is called on all nodes in the cluster, so it is just data removal from the local table - no need for it to be replicated. And it is actually a lot of data removal to replicate, so we skip it.
Also, there is a chance we could move node_cleanup into CETS's handle_down callback function - to avoid race conditions in node down detection. But it is another topic :)
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.
so, we skip replication step here, like it is done with mnesia backends
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.
Thanks
elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / db8b3e0 small_tests_24 / small_tests / db8b3e0 small_tests_25 / small_tests / db8b3e0 small_tests_25_arm64 / small_tests / db8b3e0 ldap_mnesia_24 / ldap_mnesia / db8b3e0 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / db8b3e0 mam_SUITE:rdbms_async_cache_muc_all:muc_configurable_archiveid:muc_only_stanzaid{failed,
{mam_SUITE,end_per_testcase,
{'EXIT',
{{room_archive_size,0,[{times,200,1}],ok},
[{mongoose_helper,do_wait_until,2,
[{file,
"/home/circleci/project/big_tests/tests/mongoose_helper.erl"},
{line,358}]},
{mam_helper,wait_for_room_archive_size,3,
[{file,
"/home/circleci/project/big_tests/tests/mam_helper.erl"},
{line,789}]},
{mam_helper,clean_room_archive,1,
[{file,
"/home/circleci/project/big_tests/tests/mam_helper.erl"},
{line,733}]},
{mam_helper,destroy_room,1,
[{file,
"/home/circleci/project/big_tests/tests/mam_helper.erl"},
{line,726}]},
{mam_SUITE,end_per_testcase,2,
[{file,
"/home/circleci/project/big_tests/tests/mam_SUITE.erl"},
{line,965}]},
{test_server,do_end_per_testcase,4,
[{file,"test_server.erl"},{line,1627}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1335}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1224}]}]}}}} ldap_mnesia_25 / ldap_mnesia / db8b3e0 dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / db8b3e0 pgsql_mnesia_24 / pgsql_mnesia / db8b3e0 dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / db8b3e0 internal_mnesia_25 / internal_mnesia / db8b3e0 dynamic_domains_mysql_redis_25 / mysql_redis / db8b3e0 graphql_session_SUITE:admin_session:admin_session_cli:admin_kick_user_session{error,
{function_clause,
[{graphql_helper,get_error,
[1,
{{<<"200">>,<<"OK">>},
#{<<"data">> =>
#{<<"session">> =>
#{<<"kickUserSession">> =>
#{<<"code">> => null,
<<"jid">> =>
<<"alice_admin_kick_user_session_1167@domain.example.com/res1">>,
<<"kicked">> => true,
<<"message">> => <<"Session kicked">>}}}}}],
[]},
{graphql_helper,get_err_msg,2,
[{file,
"/home/circleci/project/big_tests/tests/graphql_helper.erl"},
{line,227}]},
{graphql_session_SUITE,admin_kick_user_session_story,3,
[{file,
"/home/circleci/project/big_tests/tests/graphql_session_SUITE.erl"},
{line,441}]},
{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_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}} pgsql_cets_25 / pgsql_cets / db8b3e0 mysql_redis_25 / mysql_redis / db8b3e0 pgsql_mnesia_25 / pgsql_mnesia / db8b3e0 mssql_mnesia_25 / odbc_mssql_mnesia / db8b3e0 mod_ping_SUITE:server_ping_kill:server_ping_pong{error,{{badmatch,[{[<<"localhost">>,mod_ping,ping_response],
{expected_diff,5},
{before_story,5},
{after_story,9}}]},
[{escalus_mongooseim,post_story_check_metrics,1,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_mongooseim.erl"},
{line,74}]},
{escalus_mongooseim,maybe_check_metrics_post_story,1,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_mongooseim.erl"},
{line,51}]},
{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,"test_server.erl"},{line,1782}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}} dynamic_domains_mysql_redis_25 / mysql_redis / db8b3e0 mssql_mnesia_25 / odbc_mssql_mnesia / db8b3e0 |
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.
Looks good, I only added minor comments
small_tests_24 / small_tests / b3bede2 elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / b3bede2 small_tests_25 / small_tests / b3bede2 small_tests_25_arm64 / small_tests / b3bede2 ldap_mnesia_24 / ldap_mnesia / b3bede2 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / b3bede2 ldap_mnesia_25 / ldap_mnesia / b3bede2 dynamic_domains_mysql_redis_25 / mysql_redis / b3bede2 dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / b3bede2 muc_SUITE:register:user_submits_registration_form_twice{error,
{{assertion_failed,assert,is_iq_result,
[{xmlel,<<"iq">>,
[{<<"type">>,<<"set">>},
{<<"id">>,<<"c5a146637cc0360b70c15f16d7bc3d83">>},
{<<"to">>,<<"groupchats.domain.example.com">>}],
[{xmlel,<<"query">>,
[{<<"xmlns">>,<<"jabber:iq:register">>}],
[{xmlel,<<"x">>,
[{<<"xmlns">>,<<"jabber:x:data">>},
{<<"type">>,<<"submit">>}],
[{xmlel,<<"field">>,
[{<<"type">>,<<"hidden">>},
{<<"var">>,<<"FORM_TYPE">>}],
[{xmlel,<<"value">>,[],
[{xmlcdata,<<"jabber:iq:register">>}]}]},
{xmlel,<<"field">>,
[{<<"type">>,<<"text-single">>},
{<<"var">>,<<"nick">>}],
[{xmlel,<<"value">>,[],
[{xmlcdata,
<<"thirdwitchroom-ac87205789">>}]}]}]}]}]}],
{xmlel,<<"iq">>,
[{<<"from">>,<<"groupchats.domain.example.com">>},
{<<"to">>,
<<"alice_user_submits_registration_form_twice_2511@domain.example.com/res1">>},
{<<"type">>,<<"error">>},
{<<"id">>,<<"c5a146637cc0360b70c15f16d7bc3d83">>}],
[{xmlel,<<"query">>,
[{<<"xmlns">>,<<"jabber:iq:register">>}],
[{xmlel,<<"x">>,
[{<<"xmlns">>,<<"jabber:x:data">>},
{<<"type">>,<<"submit">>}],
[{xmlel,<<"field">>,
[{<<"type">>,<<"hidden">>},
{<<"var">>,<<"FORM_TYPE">>}],
... pgsql_mnesia_24 / pgsql_mnesia / b3bede2 dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / b3bede2 internal_mnesia_25 / internal_mnesia / b3bede2 mysql_redis_25 / mysql_redis / b3bede2 pgsql_cets_25 / pgsql_cets / b3bede2 pgsql_mnesia_25 / pgsql_mnesia / b3bede2 mssql_mnesia_25 / odbc_mssql_mnesia / b3bede2 dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / b3bede2 |
This PR addresses MIM-1933
Proposed changes include:
Kicking logic for one of the rooms in handle_conflict could be done in a separate PR (easier to review, slightly different concerns when implementing and reviewing - i.e. would need new tests, because of new functionality).