-
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
Use wpool for notify_loop in mod_pubsub #3726
Conversation
small_tests_24 / small_tests / bfc2e1b small_tests_25 / small_tests / bfc2e1b dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / bfc2e1b ldap_mnesia_24 / ldap_mnesia / bfc2e1b dynamic_domains_mysql_redis_25 / mysql_redis / bfc2e1b ldap_mnesia_25 / ldap_mnesia / bfc2e1b dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / bfc2e1b dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / bfc2e1b disco_and_caps_SUITE:disco_with_caps:user_can_query_friend_resources{error,{{assertion_failed,assert_many,false,[is_roster_set],[],[]},
[{escalus_new_assert,assert_true,2,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_new_assert.erl"},
{line,84}]},
{escalus_story,'-make_all_clients_friends/1-fun-0-',2,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,108}]},
{escalus_utils,'-each_with_index/3-fun-0-',3,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_utils.erl"},
{line,87}]},
{lists,foldl_1,3,[{file,"lists.erl"},{line,1355}]},
{escalus_utils,'-each_with_index/3-fun-0-',3,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_utils.erl"},
{line,87}]},
{lists,foldl,3,[{file,"lists.erl"},{line,1350}]},
{escalus_utils,distinct_pairs,2,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_utils.erl"},
{line,60}]},
{escalus_story,make_all_clients_friends,1,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,106}]}]}} pgsql_mnesia_24 / pgsql_mnesia / bfc2e1b internal_mnesia_25 / internal_mnesia / bfc2e1b elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / bfc2e1b pgsql_mnesia_25 / pgsql_mnesia / bfc2e1b mysql_redis_25 / mysql_redis / bfc2e1b mssql_mnesia_25 / odbc_mssql_mnesia / bfc2e1b pep_SUITE:pep_tests:unsubscribe_after_presence_unsubscription{error,{{assertion_failed,assert_many,false,[is_presence],[],[]},
[{escalus_new_assert,assert_true,2,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_new_assert.erl"},
{line,84}]},
{escalus_story,'-make_all_clients_friends/1-fun-0-',2,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,109}]},
{escalus_utils,'-each_with_index/3-fun-0-',3,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_utils.erl"},
{line,87}]},
{lists,foldl_1,3,[{file,"lists.erl"},{line,1355}]},
{escalus_utils,'-each_with_index/3-fun-0-',3,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_utils.erl"},
{line,87}]},
{lists,foldl,3,[{file,"lists.erl"},{line,1350}]},
{escalus_utils,distinct_pairs,2,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_utils.erl"},
{line,60}]},
{escalus_story,make_all_clients_friends,1,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,106}]}]}} riak_mnesia_24 / riak_mnesia / bfc2e1b |
Codecov Report
@@ Coverage Diff @@
## master #3726 +/- ##
==========================================
- Coverage 82.24% 82.22% -0.02%
==========================================
Files 523 523
Lines 33801 33794 -7
==========================================
- Hits 27799 27787 -12
- Misses 6002 6007 +5
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
bfc2e1b
to
772f587
Compare
small_tests_24 / small_tests / 772f587 small_tests_25 / small_tests / 772f587 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 772f587 ldap_mnesia_24 / ldap_mnesia / 772f587 dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 772f587 dynamic_domains_mysql_redis_25 / mysql_redis / 772f587 mam_SUITE:rdbms_async_cache_prefs_cases:end_per_group{error,
{{badrpc,
{'EXIT',
{{start_child_failed,
{error,
{{shutdown,
{failed_to_start_child,'pm_mam_async_pool_test type',
{shutdown,
{failed_to_start_child,
'wpool_pool-pm_mam_async_pool_test type-process-sup',
{shutdown,
{failed_to_start_child,
'wpool_pool-pm_mam_async_pool_test type-6',
{already_started,<9142.9055.1>}}}}}}},
{child,undefined,'pm_mam_sup_async_pool_test type',
{mongoose_async_pools,start_link,
[<<"test type">>,pm_mam,
#{batch_name => insert_mam_messages30,batch_size => 30,
enabled => true,
flush_callback => fun mod_mam_rdbms_arch_async:flush/2,
flush_extra =>
#{batch_name => insert_mam_messages30,batch_size => 30,
enabled => true,flush_interval => 2000,pool_size => 16},
flush_interval => 2000,pool_size => 16,pool_type => batch}]},
transient,false,infinity,supervisor,
[mongoose_async_pools]}}},
#{id => 'pm_mam_sup_async_pool_test type',restart => transient,
start =>
{mongoose_async_pools,start_link,
[<<"test type">>,pm_mam,
#{batch_name => insert_mam_messages30,batch_size => 30,
enabled => true,
flush_callback => fun mod_mam_rdbms_arch_async:flush/2,
flush_extra =>
#{batch_name => insert_mam_messages30,batch_size => 30,
enabled => true,flush_interval => 2000,pool_size => 16},
flush_interval => 2000,pool_size => 16,pool_type => batch}]},
type =>... mam_SUITE:rdbms_async_cache_disabled_retraction:mam06:retract_message{error,{{archive_size,1,[{times,200,0}]},
[{mongoose_helper,do_wait_until,2,
[{file,"/home/circleci/project/big_tests/tests/mongoose_helper.erl"},
{line,371}]},
{mam_SUITE,'-test_retract_message/1-fun-0-',4,
[{file,"/home/circleci/project/big_tests/tests/mam_SUITE.erl"},
{line,1717}]},
{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}]}]}} ldap_mnesia_25 / ldap_mnesia / 772f587 internal_mnesia_25 / internal_mnesia / 772f587 dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 772f587 elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 772f587 pgsql_mnesia_24 / pgsql_mnesia / 772f587 mam_SUITE:rdbms_async_pool_disabled_text_search:end_per_group{error,
{{badrpc,
{'EXIT',
{{start_child_failed,
{error,
{{shutdown,
{failed_to_start_child,pm_mam_async_pool_localhost,
{shutdown,
{failed_to_start_child,
'wpool_pool-pm_mam_async_pool_localhost-process-sup',
{shutdown,
{failed_to_start_child,
'wpool_pool-pm_mam_async_pool_localhost-1',
{already_started,<8842.18910.1>}}}}}}},
{child,undefined,pm_mam_sup_async_pool_localhost,
{mongoose_async_pools,start_link,
[<<"localhost">>,pm_mam,
#{batch_name => insert_mam_messages30,batch_size => 30,
enabled => true,
flush_callback => fun mod_mam_rdbms_arch_async:flush/2,
flush_extra =>
#{batch_name => insert_mam_messages30,batch_size => 30,
enabled => true,flush_interval => 2000,pool_size => 16},
flush_interval => 2000,pool_size => 16,pool_type => batch}]},
transient,false,infinity,supervisor,
[mongoose_async_pools]}}},
#{id => pm_mam_sup_async_pool_localhost,restart => transient,
start =>
{mongoose_async_pools,start_link,
[<<"localhost">>,pm_mam,
#{batch_name => insert_mam_messages30,batch_size => 30,
enabled => true,
flush_callback => fun mod_mam_rdbms_arch_async:flush/2,
flush_extra =>
#{batch_name => insert_mam_messages30,batch_size => 30,
enabled => true,flush_interval => 2000,pool_size => 16},
flush_interval => 2000,pool_size => 16,pool_type => batch}]},
type => supe... mam_SUITE:rdbms_async_pool_disabled_retraction:mam06:retract_message{error,{{archive_size,1,[{times,200,0}]},
[{mongoose_helper,do_wait_until,2,
[{file,"/home/circleci/project/big_tests/tests/mongoose_helper.erl"},
{line,371}]},
{mam_SUITE,'-test_retract_message/1-fun-0-',4,
[{file,"/home/circleci/project/big_tests/tests/mam_SUITE.erl"},
{line,1717}]},
{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,1783}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1292}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1224}]}]}} pep_SUITE:pep_tests:unsubscribe_after_presence_unsubscription{error,
{{badmatch,
[{xmlel,<<"message">>,
[{<<"from">>,
<<"alice_unsubscribe_after_presence_unsubscription_2442@localhost">>},
{<<"to">>,
<<"bob_unsubscribe_after_presence_unsubscription_2442@localhost/res1">>},
{<<"type">>,<<"headline">>}],
[{xmlel,<<"event">>,
[{<<"xmlns">>,
<<"http://jabber.org/protocol/pubsub#event">>}],
[{xmlel,<<"items">>,
[{<<"node">>,<<"kMBSKiCP50M4Rp9UdpqP9w==">>}],
[{xmlel,<<"item">>,
[{<<"id">>,<<"salmon">>}],
[{xmlel,<<"entry">>,
[{<<"xmlns">>,
<<"http://www.w3.org/2005/Atom">>}],
[]}]}]}]},
{xmlel,<<"headers">>,
[{<<"xmlns">>,<<"http://jabber.org/protocol/shim">>}],
[]}]}]},
[{pep_SUITE,'-unsubscribe_after_presence_unsubscription/1-fun-0-',2,
[{file,"/home/circleci/project/big_tests/tests/pep_SUITE.erl"},
{line,384}]},
{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,1783}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1292}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1224}]}]}} mysql_redis_25 / mysql_redis / 772f587 pep_SUITE:pep_tests:unsubscribe_after_presence_unsubscription{error,
{{badmatch,
[{xmlel,<<"message">>,
[{<<"from">>,
<<"alice_unsubscribe_after_presence_unsubscription_2196@localhost">>},
{<<"to">>,
<<"bob_unsubscribe_after_presence_unsubscription_2196@localhost/res1">>},
{<<"type">>,<<"headline">>}],
[{xmlel,<<"event">>,
[{<<"xmlns">>,
<<"http://jabber.org/protocol/pubsub#event">>}],
[{xmlel,<<"items">>,
[{<<"node">>,<<"izUDxxwg5aNhbVIMBWQt9A==">>}],
[{xmlel,<<"item">>,
[{<<"id">>,<<"salmon">>}],
[{xmlel,<<"entry">>,
[{<<"xmlns">>,
<<"http://www.w3.org/2005/Atom">>}],
[]}]}]}]},
{xmlel,<<"headers">>,
[{<<"xmlns">>,<<"http://jabber.org/protocol/shim">>}],
[]}]}]},
[{pep_SUITE,'-unsubscribe_after_presence_unsubscription/1-fun-0-',2,
[{file,"/home/circleci/project/big_tests/tests/pep_SUITE.erl"},
{line,384}]},
{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_mnesia_25 / pgsql_mnesia / 772f587 mssql_mnesia_25 / odbc_mssql_mnesia / 772f587 riak_mnesia_24 / riak_mnesia / 772f587 dynamic_domains_mysql_redis_25 / mysql_redis / 772f587 pgsql_mnesia_24 / pgsql_mnesia / 772f587 dynamic_domains_mysql_redis_25 / mysql_redis / 772f587 pgsql_mnesia_24 / pgsql_mnesia / 772f587 pep_SUITE:pep_tests:unsubscribe_after_presence_unsubscription{error,
{{badmatch,
[{xmlel,<<"message">>,
[{<<"from">>,
<<"alice_unsubscribe_after_presence_unsubscription_2262@localhost">>},
{<<"to">>,
<<"bob_unsubscribe_after_presence_unsubscription_2262@localhost/res1">>},
{<<"type">>,<<"headline">>}],
[{xmlel,<<"event">>,
[{<<"xmlns">>,
<<"http://jabber.org/protocol/pubsub#event">>}],
[{xmlel,<<"items">>,
[{<<"node">>,<<"FK3dkVnhoWORNTA/ArEs5A==">>}],
[{xmlel,<<"item">>,
[{<<"id">>,<<"salmon">>}],
[{xmlel,<<"entry">>,
[{<<"xmlns">>,
<<"http://www.w3.org/2005/Atom">>}],
[]}]}]}]},
{xmlel,<<"headers">>,
[{<<"xmlns">>,<<"http://jabber.org/protocol/shim">>}],
[]}]}]},
[{pep_SUITE,'-unsubscribe_after_presence_unsubscription/1-fun-0-',2,
[{file,"/home/circleci/project/big_tests/tests/pep_SUITE.erl"},
{line,384}]},
{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,1783}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1292}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1224}]}]}} |
This comment was marked as outdated.
This comment was marked as outdated.
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 in general, I only have minor comments.
src/pubsub/mod_pubsub.erl
Outdated
Plugins) -> | ||
init_state(ServerHost, #{last_item_cache := LastItemCache, max_items_node := MaxItemsNode, | ||
pep_mapping := PepMapping, ignore_pep_from_offline := PepOffline, | ||
access_createnode := Access, plugins := Plugins}, Plugins) -> |
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.
The code before this change worked if a plugin failed to initialize, but now it would crash. Is it intentional?
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.
not really, I just does not understand the code 100% :) could change to the old behaviour though
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 maybe remove plugins := Plugins
to avoid this unexpected pattern-matching?
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.
tricky, there are these gen_server calls to get plugins (see plugins/1 function) instead of calling opts instead.
And gen_server is a pretty heavy operation.
Is there an use case for a plugin failing to start?
Maybe we could crash the module start function if the plugin crashes instead?
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.
For me this is out of scope of this PR - so why not keep the functionality as it was before?
src/pubsub/mod_pubsub.erl
Outdated
send_last_pubsub_items_for_plugin(Host, PluginType, Recipient) | ||
end, Plugins). |
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.
Minor: indentation is inconsistent here.
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.
good, I need to fix vim config on my new laptop finally :)
small_tests_24 / small_tests / 8a4f083 small_tests_25 / small_tests / 8a4f083 ldap_mnesia_24 / ldap_mnesia / 8a4f083 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 8a4f083 dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 8a4f083 dynamic_domains_mysql_redis_25 / mysql_redis / 8a4f083 ldap_mnesia_25 / ldap_mnesia / 8a4f083 dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 8a4f083 pgsql_mnesia_24 / pgsql_mnesia / 8a4f083 internal_mnesia_25 / internal_mnesia / 8a4f083 elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 8a4f083 pgsql_mnesia_25 / pgsql_mnesia / 8a4f083 mysql_redis_25 / mysql_redis / 8a4f083 mssql_mnesia_25 / odbc_mssql_mnesia / 8a4f083 disco_and_caps_SUITE:disco_with_caps:user_can_query_friend_resources{error,{{assertion_failed,assert_many,false,[is_roster_set],[],[]},
[{escalus_new_assert,assert_true,2,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_new_assert.erl"},
{line,84}]},
{escalus_story,'-make_all_clients_friends/1-fun-0-',2,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,108}]},
{escalus_utils,'-each_with_index/3-fun-0-',3,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_utils.erl"},
{line,87}]},
{lists,foldl_1,3,[{file,"lists.erl"},{line,1355}]},
{escalus_utils,'-each_with_index/3-fun-0-',3,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_utils.erl"},
{line,87}]},
{lists,foldl,3,[{file,"lists.erl"},{line,1350}]},
{escalus_utils,distinct_pairs,2,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_utils.erl"},
{line,60}]},
{escalus_story,make_all_clients_friends,1,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,106}]}]}} riak_mnesia_24 / riak_mnesia / 8a4f083 |
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 👍
This PR addresses MIM-1687.
Proposed changes include: