Worker pool cleanups #3419
Worker pool cleanups #3419
Conversation
I suspect this might also have somehow better concurrency characteristics, as generating the name generates atoms, and therefore needs to grab locks on the atom table, which is not optimised for writes, even if the write is idempotent as the atom already exists, but this one is mostly guessing. Sequential performance is surely much better.
NelsonVides
requested review from
arcusfelis and
chrzaszcz
and removed request for
arcusfelis
|
small_tests_24 / small_tests / 928eb74 small_tests_23 / small_tests / 928eb74 dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 928eb74 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 928eb74 dynamic_domains_mysql_redis_24 / mysql_redis / 928eb74 dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 928eb74 ldap_mnesia_23 / ldap_mnesia / 928eb74 internal_mnesia_24 / internal_mnesia / 928eb74 amp_big_SUITE:offline:offline_success:notify_deliver_to_offline_user_recipient_privacy_test{error,
{{assertion_failed,assert,is_presence,
{xmlel,<<"stream:error">>,[],
[{xmlel,<<"conflict">>,
[{<<"xmlns">>,<<"urn:ietf:params:xml:ns:xmpp-streams">>}],
[]},
{xmlel,<<"text">>,
[{<<"xml:lang">>,<<"en">>},
{<<"xmlns">>,<<"urn:ietf:params:xml:ns:xmpp-streams">>}],
[{xmlcdata,<<"Replaced by new connection">>}]}]},
"<stream:error><conflict xmlns='urn:ietf:params:xml:ns:xmpp-streams'/><text xml:lang='en' xmlns='urn:ietf:params:xml:ns:xmpp-streams'>Replaced by new connection</text></stream:error>"},
[{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,'-drop_presences/2-lc$^0/1-0-',1,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,190}]},
{escalus_story,drop_presences,2,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,190}]},
{escalus_story,'-start_ready_clients/2-fun-0-',3,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,135}]},
{lists,foldl,3,[{file,"lists.erl"},{line,1267}]},
{escalus_story,start_ready_clients,2,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
...pgsql_mnesia_23 / pgsql_mnesia / 928eb74 ldap_mnesia_24 / ldap_mnesia / 928eb74 pgsql_mnesia_24 / pgsql_mnesia / 928eb74 mysql_redis_24 / mysql_redis / 928eb74 elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 928eb74 mssql_mnesia_24 / odbc_mssql_mnesia / 928eb74 riak_mnesia_24 / riak_mnesia / 928eb74 |
Codecov Report
@@ Coverage Diff @@
## master #3419 +/- ##
==========================================
+ Coverage 80.73% 80.78% +0.05%
==========================================
Files 414 414
Lines 32298 32302 +4
==========================================
+ Hits 26075 26095 +20
+ Misses 6223 6207 -16
Continue to review full report at Codecov.
|
NelsonVides
force-pushed
the
worker_pool_cleanups
branch
from
928eb74
to
80b1197
Compare
|
small_tests_23 / small_tests / 80b1197 small_tests_24 / small_tests / 80b1197 dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 80b1197 dynamic_domains_mysql_redis_24 / mysql_redis / 80b1197 mam_SUITE:rdbms_prefs_cases:prefs_set_request{error,{test_case_failed,"ASSERT EQUAL\n\tExpected {prefs_result_iq,<<\"roster\">>,\n [<<\"[email protected]\">>],\n [<<\"[email protected]\">>]}\n\tValue {prefs_result_iq,<<\"always\">>,\n [<<\"[email protected]\">>],\n [<<\"[email protected]\">>]}\n"}}dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 80b1197 dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 80b1197 ldap_mnesia_23 / ldap_mnesia / 80b1197 internal_mnesia_24 / internal_mnesia / 80b1197 ldap_mnesia_24 / ldap_mnesia / 80b1197 sm_SUITE:parallel:messages_are_properly_flushed_during_resumption{error,
{{badmatch,
{error,
{connection_step_failed,
{#Fun<sm_SUITE.11.68776247>,
{client,
<<"alicE_messages_are_properly_flushed_during_resumption_9.850405@localhost">>,
escalus_tcp,<0.17941.1>,undefined,
[{username,
<<"alicE_messages_are_properly_flushed_during_resumption_9.850405">>},
{server,<<"localhost">>},
{password,<<"matygrysa">>},
{stream_management,true},
{host,<<"localhost">>},
{stream_id,<<"9321f8e2d31d952d">>}]},
[{compression,[<<"zlib">>]},
{starttls,true},
{stream_management,true},
{advanced_message_processing,true},
{client_state_indication,false},
{sasl_mechanisms,[<<"PLAIN">>]},
{caps,undefined}]},
{timeout,get_resumed}}}},
[{sm_SUITE,'-messages_are_properly_flushed_during_resumption/1-fun-1-',
3,
[{file,"/home/circleci/project/big_tests/tests/sm_SUITE.erl"},
{line,1226}]},
{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}]}]}}pgsql_mnesia_23 / pgsql_mnesia / 80b1197 mysql_redis_24 / mysql_redis / 80b1197 mam_SUITE:rdbms_async_cache_prefs_cases:messages_filtered_when_prefs_default_policy_is_roster{error,{test_case_failed,"ASSERT EQUAL\n\tExpected []\n\tValue [ok]\n"}}elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 80b1197 mssql_mnesia_24 / odbc_mssql_mnesia / 80b1197 pgsql_mnesia_24 / pgsql_mnesia / 80b1197 riak_mnesia_24 / riak_mnesia / 80b1197 |
NelsonVides
force-pushed
the
worker_pool_cleanups
branch
from
80b1197
to
301a30d
Compare
|
small_tests_24 / small_tests / 301a30d small_tests_23 / small_tests / 301a30d dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 301a30d dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 301a30d dynamic_domains_mysql_redis_24 / mysql_redis / 301a30d dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 301a30d ldap_mnesia_24 / ldap_mnesia / 301a30d ldap_mnesia_23 / ldap_mnesia / 301a30d internal_mnesia_24 / internal_mnesia / 301a30d pgsql_mnesia_23 / pgsql_mnesia / 301a30d mysql_redis_24 / mysql_redis / 301a30d mam_SUITE:rdbms_cache_prefs_cases:messages_filtered_when_prefs_default_policy_is_never{error,{test_case_failed,"ASSERT EQUAL\n\tExpected []\n\tValue [ok,ok]\n"}}mssql_mnesia_24 / odbc_mssql_mnesia / 301a30d elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 301a30d pgsql_mnesia_24 / pgsql_mnesia / 301a30d riak_mnesia_24 / riak_mnesia / 301a30d amp_big_SUITE:offline:offline_success:notify_deliver_to_online_user_broken_connection_test{error,
{test_case_failed,
{has_stanzas_but_shouldnt,
{client,
<<"alicE_notify_deliver_to_online_user_broken_connection_test_74.140634@localhost/res1">>,
escalus_tcp,<0.4154.0>,
[{event_manager,<0.3872.0>},
{server,<<"localhost">>},
{username,
<<"alicE_notify_deliver_to_online_user_broken_connection_test_74.140634">>},
{resource,<<"res1">>}],
[{event_client,
[{event_manager,<0.3872.0>},
{server,<<"localhost">>},
{username,
<<"alicE_notify_deliver_to_online_user_broken_connection_test_74.140634">>},
{resource,<<"res1">>}]},
{resource,<<"res1">>},
{username,
<<"alicE_notify_deliver_to_online_user_broken_connection_test_74.140634">>},
{server,<<"localhost">>},
{host,<<"localhost">>},
{port,5222},
{auth,{escalus_auth,auth_plain}},
{wspath,undefined},
{username,
<<"alicE_notify_deliver_to_online_user_broken_connection_test_74.140634">>},
{server,<<"localhost">>},
{password,<<"matygrysa">>},
{stream_id,<<"257f06ac6d4e136b">>}]},
[{xmlel,<<"stream:error">>,[],
[{xmlel,<<"conflict">>,
[{<<"xmlns">>,
<<"urn:ietf:params:xml:ns:xmpp-streams">>}],
[]},
{xmlel,<<"text">>,
[{<<"xml:lang">>,<<"en">>},
{<<"xmlns">>,
<<"urn:ietf:params:x...riak_mnesia_24 / riak_mnesia / 301a30d |
Sounds good, but, how to do so exactly? I thought we aren't doing defaults for modules yet, so far only global options were done, right? 🤔 |
NelsonVides
force-pushed
the
worker_pool_cleanups
branch
from
301a30d
to
eee8837
Compare
|
small_tests_23 / small_tests / eee8837 small_tests_24 / small_tests / eee8837 dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / eee8837 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / eee8837 dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / eee8837 rest_client_SUITE:roster:add_contact_and_invite{error,
{timeout_when_waiting_for_stanza,
[{escalus_client,wait_for_stanza,
[{client,
<<"[email protected]/res1">>,
escalus_tcp,<0.25128.1>,
[{event_manager,<0.25097.1>},
{server,<<"domain.example.com">>},
{username,<<"alicE_add_contact_and_invite_44.662483">>},
{resource,<<"res1">>}],
[{event_client,
[{event_manager,<0.25097.1>},
{server,<<"domain.example.com">>},
{username,
<<"alicE_add_contact_and_invite_44.662483">>},
{resource,<<"res1">>}]},
{resource,<<"res1">>},
{username,<<"alicE_add_contact_and_invite_44.662483">>},
{server,<<"domain.example.com">>},
{host,<<"localhost">>},
{port,5222},
{auth,{escalus_auth,auth_plain}},
{wspath,undefined},
{username,<<"alicE_add_contact_and_invite_44.662483">>},
{server,<<"domain.example.com">>},
{host,<<"localhost">>},
{password,<<"matygrysa">>},
{stream_id,<<"b058b0504f0b2abd">>}]},
5000],
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
{line,136}]},
{rest_client_SUITE,'-add_contact_and_invite/1-fun-0-',2,
[{file,
"/home/circleci/project/big_tests/tests/rest_client_SUITE.erl"},
{line,1106}]},
{escalus_st...dynamic_domains_mysql_redis_24 / mysql_redis / eee8837 vcard_SUITE:ro_full:retrieve_own_card{error,{test_case_failed,"Expected <<\"alice\">> got undefined\n"}}ldap_mnesia_23 / ldap_mnesia / eee8837 ldap_mnesia_24 / ldap_mnesia / eee8837 sm_SUITE:parallel_manual_ack_freq_1:resume_session_state_send_message{error,
{{assertion_failed,assert_many,true,
[#Fun<sm_SUITE.22.68776247>,#Fun<sm_SUITE.22.68776247>,
#Fun<sm_SUITE.22.68776247>],
[{xmlel,<<"presence">>,
[{<<"from">>,
<<"alicE_resume_session_state_send_message_15.192148@localhost/escalus-default-resource">>},
{<<"to">>,
<<"alice_resume_session_state_send_message_15.192148@localhost/escalus-default-resource">>},
{<<"xml:lang">>,<<"en">>}],
[]},
{xmlel,<<"message">>,
[{<<"from">>,
<<"bOb_resume_session_state_send_message_15.167356@localhost/escalus-default-resource">>},
{<<"to">>,
<<"alicE_resume_session_state_send_message_15.192148@localhost">>},
{<<"xml:lang">>,<<"en">>},
{<<"type">>,<<"chat">>}],
[{xmlel,<<"body">>,[],[{xmlcdata,<<"msg-1">>}]},
{xmlel,<<"delay">>,
[{<<"xmlns">>,<<"urn:xmpp:delay">>},
{<<"stamp">>,<<"2021-11-29T13:31:55.203006Z">>},
{<<"from">>,<<"localhost">>}],
[{xmlcdata,<<"SM Storage">>}]}]},
{xmlel,<<"message">>,
[{<<"from">>,
<<"bOb_resume_session_state_send_message_15.167356@localhost/escalus-default-resource">>},
{<<"to">>,
<<"alicE_resume_session_state_send_message_15.192148@localhost">>},
{<<"xml:lang">>,<<"en">>},
{<<"type">>,<<"chat">>}],
[{xmlel,<<"body">>,[],[{xmlcdata,<<"msg-2">>}]},
{xmlel,<<"delay">>,
[{<<"xmlns">>,<<"urn:xmpp:...internal_mnesia_24 / internal_mnesia / eee8837 amp_big_SUITE:basic:notify_deliver_to_offline_user_recipient_privacy_test{error,
{test_case_failed,
{has_stanzas_but_shouldnt,
{client,
<<"alicE_notify_deliver_to_offline_user_recipient_privacy_test_61.237092@localhost/res1">>,
escalus_tcp,<0.3018.0>,
[{event_manager,<0.2716.0>},
{server,<<"localhost">>},
{username,
<<"alicE_notify_deliver_to_offline_user_recipient_privacy_test_61.237092">>},
{resource,<<"res1">>}],
[{event_client,
[{event_manager,<0.2716.0>},
{server,<<"localhost">>},
{username,
<<"alicE_notify_deliver_to_offline_user_recipient_privacy_test_61.237092">>},
{resource,<<"res1">>}]},
{resource,<<"res1">>},
{username,
<<"alicE_notify_deliver_to_offline_user_recipient_privacy_test_61.237092">>},
{server,<<"localhost">>},
{host,<<"localhost">>},
{port,5222},
{auth,{escalus_auth,auth_plain}},
{wspath,undefined},
{username,
<<"alicE_notify_deliver_to_offline_user_recipient_privacy_test_61.237092">>},
{server,<<"localhost">>},
{password,<<"matygrysa">>},
{stream_id,<<"877e6efbb655ace0">>}]},
[{xmlel,<<"stream:error">>,[],
[{xmlel,<<"conflict">>,
[{<<"xmlns">>,
<<"urn:ietf:params:xml:ns:xmpp-streams">>}],
[]},
{xmlel,<<"text">>,
[{<<"xml:lang">>,<<"en">>},
{<<"xmlns">>,
<<"urn:ietf:par...elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / eee8837 pgsql_mnesia_23 / pgsql_mnesia / eee8837 mssql_mnesia_24 / odbc_mssql_mnesia / eee8837 mysql_redis_24 / mysql_redis / eee8837 pgsql_mnesia_24 / pgsql_mnesia / eee8837 riak_mnesia_24 / riak_mnesia / eee8837 internal_mnesia_24 / internal_mnesia / eee8837 amp_big_SUITE:offline:offline_success:notify_deliver_to_offline_user_recipient_privacy_test{error,
{{assertion_failed,assert,is_presence,
{xmlel,<<"stream:error">>,[],
[{xmlel,<<"conflict">>,
[{<<"xmlns">>,<<"urn:ietf:params:xml:ns:xmpp-streams">>}],
[]},
{xmlel,<<"text">>,
[{<<"xml:lang">>,<<"en">>},
{<<"xmlns">>,<<"urn:ietf:params:xml:ns:xmpp-streams">>}],
[{xmlcdata,<<"Replaced by new connection">>}]}]},
"<stream:error><conflict xmlns='urn:ietf:params:xml:ns:xmpp-streams'/><text xml:lang='en' xmlns='urn:ietf:params:xml:ns:xmpp-streams'>Replaced by new connection</text></stream:error>"},
[{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,'-drop_presences/2-lc$^0/1-0-',1,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,190}]},
{escalus_story,drop_presences,2,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,190}]},
{escalus_story,'-start_ready_clients/2-fun-0-',3,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,135}]},
{lists,foldl,3,[{file,"lists.erl"},{line,1267}]},
{escalus_story,start_ready_clients,2,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
...sm_SUITE:parallel_manual_ack_freq_1:resume_session_state_send_message{error,
{{assertion_failed,assert_many,true,
[#Fun<sm_SUITE.22.68776247>,#Fun<sm_SUITE.22.68776247>,
#Fun<sm_SUITE.22.68776247>],
[{xmlel,<<"presence">>,
[{<<"from">>,
<<"alicE_resume_session_state_send_message_22.576770@localhost/escalus-default-resource">>},
{<<"to">>,
<<"alice_resume_session_state_send_message_22.576770@localhost/escalus-default-resource">>},
{<<"xml:lang">>,<<"en">>}],
[]},
{xmlel,<<"message">>,
[{<<"from">>,
<<"bOb_resume_session_state_send_message_22.568339@localhost/escalus-default-resource">>},
{<<"to">>,
<<"alicE_resume_session_state_send_message_22.576770@localhost">>},
{<<"xml:lang">>,<<"en">>},
{<<"type">>,<<"chat">>}],
[{xmlel,<<"body">>,[],[{xmlcdata,<<"msg-1">>}]},
{xmlel,<<"delay">>,
[{<<"xmlns">>,<<"urn:xmpp:delay">>},
{<<"stamp">>,<<"2021-11-29T14:17:02.588668Z">>},
{<<"from">>,<<"localhost">>}],
[{xmlcdata,<<"SM Storage">>}]}]},
{xmlel,<<"message">>,
[{<<"from">>,
<<"bOb_resume_session_state_send_message_22.568339@localhost/escalus-default-resource">>},
{<<"to">>,
<<"alicE_resume_session_state_send_message_22.576770@localhost">>},
{<<"xml:lang">>,<<"en">>},
{<<"type">>,<<"chat">>}],
[{xmlel,<<"body">>,[],[{xmlcdata,<<"msg-2">>}]},
{xmlel,<<"delay">>,
[{<<"xmlns">>,<<"urn:xmpp:...sm_SUITE:parallel_manual_ack_freq_1:resume_session_state_stop_c2s{error,{thrown,{timeout,msg}}}internal_mnesia_24 / internal_mnesia / eee8837 |
Let's leave it for later as I've looked at the |
|
Actually I think (correct me if I'm wrong) that this pool size option does nothing, the pool size is always 32 regardless of what I set there (in pm, muc or globally), because this option is never propagated to the async writer module (as some other options that are already there). |
|
As discussed with @chrzaszcz, turns out many options are not passed to the async workers, but this was broken before this PR, so here no bugs are introduced, and correctly passing the params to the workers can be done in a separate PR as a bug-fix. |
This PR is a general cleanup. It removes an unused header file, removes long-begone comments, recovers a configuration key for the pool sizes, increases symmetry between both async workers, and perhaps the only functional change, it stores the pool name on the same record that stores the pool details, instead of continuously generating the name again and again – this is a change in line with inaka/worker_pool#181.