Introduce pool names to mongoose_rdbms #4231

NelsonVides · 2024-02-26T14:12:28Z

Solves MIM-2182

Currently the API for mongoose_rdbms allows only for providing the host-type, but not the pool tag. Change all APIs to look approximately like the following:

function(HostType, Name, Parameters) ->
    function(HostType, ?DEFAULT_POOL_NAME, Name, Parameters).

-spec function(HostType :: server(), PoolName :: atom(), Name :: atom(), Parameters :: term()) ->
    result().
function(HostType, PoolName, Name, Parameters) when is_atom(PoolName), is_atom(Name) ->
    sql_cast(HostType, PoolName, {sql_execute, Name, Parameters}).

codecov · 2024-02-26T14:22:18Z

Codecov Report

Attention: Patch coverage is 97.72727% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 84.35%. Comparing base (880776f) to head (9445cc3).
Report is 10 commits behind head on master.

❗ Current head 9445cc3 differs from pull request most recent head 03a4175. Consider uploading reports for the commit 03a4175 to get more accurate results

Files	Patch %	Lines
src/rdbms/mongoose_rdbms.erl	96.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4231      +/-   ##
==========================================
- Coverage   84.37%   84.35%   -0.03%     
==========================================
  Files         552      552              
  Lines       33484    33497      +13     
==========================================
+ Hits        28251    28255       +4     
- Misses       5233     5242       +9

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

arcusfelis · 2024-02-27T10:13:43Z

big_tests/tests/rdbms_SUITE.erl


 sql_execute_cast(_Config, Name, Parameters) ->
-    slow_rpc(mongoose_rdbms, execute_cast, [host_type(), Name, Parameters]).
+    slow_rpc(mongoose_rdbms, execute_cast, [host_type(), tag(), Name, Parameters]).


but it doesn't test version without a tag now

Every other test suite is still testing the old way indirectly so I thought no need to duplicate test time here 🤔

I also think it would be nice to check both, but it's not a must for me.

arcusfelis

looks ok

chrzaszcz

Looks good in general. I added some comments - it's still ok for me without addressing them, but I wanted to share these.

chrzaszcz · 2024-02-27T11:14:51Z

doc/configuration/outgoing-connections.md

@@ -30,6 +30,10 @@ This allows you to create multiple dedicated pools of the same type.
 * `host` - the pool will be started for each XMPP host or host type served by MongooseIM
 * `single_host` - the pool will be started for the selected host or host type only (you must provide the name).

+    !!! Note
+        A global pool with name `default` is used by services that are not configured by host_type, like `service_domain_db` or `service_mongoose_system_metrics`, or by modules that don't support dynamic domains, like `mod_pubsub`.


We don't define "name" here, so maybe instead of "name default" whe should say "tag default"?

chrzaszcz · 2024-02-27T11:16:35Z

src/rdbms/mongoose_rdbms.erl

-ignore_xref([sql_query_cast/2, sql_query_request/2,
-              execute_cast/3, execute_request/3,
+-ignore_xref([
+              sql_query_cast/2, sql_query_request/2,


Would it be possible to put them in the same order as in exports?

chrzaszcz · 2024-02-27T11:21:59Z

src/rdbms/mongoose_rdbms.erl

+execute(HostType, Name, Parameters) ->
+    execute(HostType, ?DEFAULT_POOL_NAME, Name, Parameters).
+
+-spec execute(HostType :: server(), PoolName :: atom(), Name :: atom(), Parameters :: [term()]) ->


What we call Name in this module, is called Tag in mongoose_wpool, and there is a type mongoose_wpool:tag() as well. So for me it should be:

Suggested change

-spec execute(HostType :: server(), PoolName :: atom(), Name :: atom(), Parameters :: [term()]) ->

-spec execute(HostType :: server(), PoolName :: atom(), Tag :: mongoose_wpool:tag(), Parameters :: [term()]) ->

This way it would be more consistent. AFAIR, I think we chose Tag instead of Name to avoid confusion with PoolName.

chrzaszcz · 2024-02-27T11:22:50Z

big_tests/tests/rdbms_SUITE.erl


 sql_execute_cast(_Config, Name, Parameters) ->
-    slow_rpc(mongoose_rdbms, execute_cast, [host_type(), Name, Parameters]).
+    slow_rpc(mongoose_rdbms, execute_cast, [host_type(), tag(), Name, Parameters]).


I also think it would be nice to check both, but it's not a must for me.

mongoose-im · 2024-02-27T13:27:13Z

elasticsearch_and_cassandra_26 / elasticsearch_and_cassandra_mnesia / 070ad29
Reports root/ big
OK: 417 / Failed: 0 / User-skipped: 43 / Auto-skipped: 0

small_tests_25 / small_tests / 070ad29
Reports root / small

small_tests_26 / small_tests / 070ad29
Reports root / small

small_tests_26_arm64 / small_tests / 070ad29
Reports root / small

ldap_mnesia_25 / ldap_mnesia / 070ad29
Reports root/ big
OK: 2275 / Failed: 0 / User-skipped: 895 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_26 / pgsql_mnesia / 070ad29
Reports root/ big
OK: 4460 / Failed: 0 / User-skipped: 111 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 070ad29
Reports root/ big
OK: 4460 / Failed: 0 / User-skipped: 111 / Auto-skipped: 0

ldap_mnesia_26 / ldap_mnesia / 070ad29
Reports root/ big
OK: 2275 / Failed: 0 / User-skipped: 895 / Auto-skipped: 0

internal_mnesia_26 / internal_mnesia / 070ad29
Reports root/ big
OK: 2415 / Failed: 0 / User-skipped: 755 / Auto-skipped: 0

pgsql_cets_26 / pgsql_cets / 070ad29
Reports root/ big
OK: 4395 / Failed: 0 / User-skipped: 178 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_26 / odbc_mssql_mnesia / 070ad29
Reports root/ big
OK: 4457 / Failed: 0 / User-skipped: 114 / Auto-skipped: 0

pgsql_mnesia_25 / pgsql_mnesia / 070ad29
Reports root/ big
OK: 4849 / Failed: 0 / User-skipped: 118 / Auto-skipped: 0

pgsql_mnesia_26 / pgsql_mnesia / 070ad29
Reports root/ big
OK: 4849 / Failed: 0 / User-skipped: 118 / Auto-skipped: 0

mysql_redis_26 / mysql_redis / 070ad29
Reports root/ big
OK: 4828 / Failed: 0 / User-skipped: 139 / Auto-skipped: 0

mssql_mnesia_26 / odbc_mssql_mnesia / 070ad29
Reports root/ big
OK: 4846 / Failed: 0 / User-skipped: 121 / Auto-skipped: 0

chrzaszcz

It looks correct, so I approved, but personally I would change the naming as in the comments. If you don't have time, I could fix it as well.

src/rdbms/mongoose_rdbms.erl

src/rdbms/rdbms_queries.erl

mongoose-im · 2024-02-28T10:23:20Z

elasticsearch_and_cassandra_26 / elasticsearch_and_cassandra_mnesia / 03a4175
Reports root/ big
OK: 417 / Failed: 0 / User-skipped: 43 / Auto-skipped: 0

small_tests_25 / small_tests / 03a4175
Reports root / small

small_tests_26 / small_tests / 03a4175
Reports root / small

small_tests_26_arm64 / small_tests / 03a4175
Reports root / small

ldap_mnesia_25 / ldap_mnesia / 03a4175
Reports root/ big
OK: 2275 / Failed: 0 / User-skipped: 895 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 03a4175
Reports root/ big
OK: 4460 / Failed: 0 / User-skipped: 111 / Auto-skipped: 0

ldap_mnesia_26 / ldap_mnesia / 03a4175
Reports root/ big
OK: 2275 / Failed: 0 / User-skipped: 895 / Auto-skipped: 0

dynamic_domains_mysql_redis_26 / mysql_redis / 03a4175
Reports root/ big
OK: 4426 / Failed: 1 / User-skipped: 144 / Auto-skipped: 0

carboncopy_SUITE:one2one:dropped_client_doesnt_create_duplicate_carbons

{error,
  {{badmatch,
     [{xmlel,<<"message">>,
        [{<<"from">>,
        <<"alice_dropped_client_doesnt_create_duplicate_carbons_615@domain.example.com">>},
         {<<"to">>,
        <<"alice_dropped_client_doesnt_create_duplicate_carbons_615@domain.example.com/res2">>},
         {<<"xmlns">>,<<"jabber:client">>},
         {<<"type">>,<<"chat">>}],
        [{xmlel,<<"sent">>,
           [{<<"xmlns">>,<<"urn:xmpp:carbons:2">>}],
           [{xmlel,<<"forwarded">>,
            [{<<"xmlns">>,<<"urn:xmpp:forward:0">>}],
            [{xmlel,<<"message">>,
               [{<<"from">>,
                 <<"alice_dropped_client_doesnt_create_duplicate_carbons_615@domain.example.com/res1">>},
                {<<"type">>,<<"chat">>},
                {<<"to">>,
                 <<"bob_dropped_client_doesnt_create_duplicate_carbons_615@domain.example.com/res1">>},
                {<<"xmlns">>,<<"jabber:client">>}],
               [{xmlel,<<"body">>,[],
                  [{xmlcdata,
                     <<"And pious action">>}]}]}]}]}]}]},
   [{carboncopy_SUITE,
      '-dropped_client_doesnt_create_duplicate_carbons/1-fun-0-',4,
      [{file,
         "/home/circleci/project/big_tests/tests/carboncopy_SUITE.erl"},
       {line,189}]},
    {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,1793}]},
    {test_server,run_test_c...

Report log

internal_mnesia_26 / internal_mnesia / 03a4175
Reports root/ big
OK: 2415 / Failed: 0 / User-skipped: 755 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_26 / pgsql_mnesia / 03a4175
Reports root/ big
OK: 4460 / Failed: 0 / User-skipped: 111 / Auto-skipped: 0

pgsql_cets_26 / pgsql_cets / 03a4175
Reports root/ big
OK: 4395 / Failed: 0 / User-skipped: 178 / Auto-skipped: 0

pgsql_mnesia_25 / pgsql_mnesia / 03a4175
Reports root/ big
OK: 4849 / Failed: 0 / User-skipped: 118 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_26 / odbc_mssql_mnesia / 03a4175
Reports root/ big
OK: 4453 / Failed: 1 / User-skipped: 114 / Auto-skipped: 3

metrics_c2s_SUITE:single:stanza_one

{error,
  {{xmppStanzaReceived,
     {value,20194},
     [{times,25,
        {error,
          {badmatch,{value,20195}},
          [{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/../test/common/mongoose_helper.erl"},
            {line,362}]},
           {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,1793}]},
           {test_server,run_test_case_eval1,6,
             [{file,"test_server.erl"},{line,1302}]},
           {test_server,run_test_case_eval,9,
             [{file,"test_server.erl"},{line,1234}]}]}}],
     ok},
   [{mongoose_helper,do_wait_until,2,
      [{file,
         "/home/circleci/project/big_tests/../test/common/mongoose_helper.erl"},
       {line,359}]},
    {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,1793}]},
    {test_server,run_test_case_eval1,6,
      [{file,"test_server.erl"},{line,1302}]},
    {test_server,run_test_case_eval,9,
      [{file,"test_server.erl"},{line,1234}]}]}}

Report log

mysql_redis_26 / mysql_redis / 03a4175
Reports root/ big
OK: 4828 / Failed: 0 / User-skipped: 139 / Auto-skipped: 0

pgsql_mnesia_26 / pgsql_mnesia / 03a4175
Reports root/ big
OK: 4849 / Failed: 0 / User-skipped: 118 / Auto-skipped: 0

mssql_mnesia_26 / odbc_mssql_mnesia / 03a4175
Reports root/ big
OK: 4846 / Failed: 0 / User-skipped: 121 / Auto-skipped: 0

dynamic_domains_mysql_redis_26 / mysql_redis / 03a4175
Reports root/ big
OK: 4427 / Failed: 0 / User-skipped: 144 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_26 / odbc_mssql_mnesia / 03a4175
Reports root/ big
OK: 4457 / Failed: 0 / User-skipped: 114 / Auto-skipped: 0

chrzaszcz

Looks great now 👍

This comment was marked as outdated.

Sign in to view

Introduce pool names to mongoose_rdbms

4ff34bc

NelsonVides force-pushed the pools/rdbms_pool_tags branch from 486e295 to 4ff34bc Compare February 26, 2024 14:26

This comment was marked as outdated.

Sign in to view

NelsonVides force-pushed the pools/rdbms_pool_tags branch 3 times, most recently from 00d883c to e1f6ee8 Compare February 26, 2024 15:08

This comment was marked as outdated.

Sign in to view

Test rdbms_SUITE entirely on host_type pool

3d20474

NelsonVides force-pushed the pools/rdbms_pool_tags branch from e1f6ee8 to 3d20474 Compare February 26, 2024 15:36

This comment was marked as outdated.

Sign in to view

NelsonVides marked this pull request as ready for review February 26, 2024 22:26

arcusfelis reviewed Feb 27, 2024

View reviewed changes

arcusfelis approved these changes Feb 27, 2024

View reviewed changes

chrzaszcz reviewed Feb 27, 2024

View reviewed changes

This comment was marked as outdated.

Sign in to view

Test both default and tagged pools

600c0d7

NelsonVides force-pushed the pools/rdbms_pool_tags branch from 907a32a to 070ad29 Compare February 27, 2024 13:19

NelsonVides requested review from arcusfelis and chrzaszcz February 27, 2024 13:51

chrzaszcz reviewed Feb 27, 2024

View reviewed changes

src/rdbms/mongoose_rdbms.erl Outdated Show resolved Hide resolved

src/rdbms/mongoose_rdbms.erl Outdated Show resolved Hide resolved

NelsonVides force-pushed the pools/rdbms_pool_tags branch from 070ad29 to 7cdbf55 Compare February 28, 2024 08:15

This comment was marked as outdated.

Sign in to view

chrzaszcz reviewed Feb 28, 2024

View reviewed changes

src/rdbms/rdbms_queries.erl Outdated Show resolved Hide resolved

NelsonVides force-pushed the pools/rdbms_pool_tags branch from 9445cc3 to 6597730 Compare February 28, 2024 10:14

NelsonVides added 2 commits February 28, 2024 11:16

Make tag/name and type declarations consistent

b208c19

Fix flaky test in rdbms_SUITE

03a4175

NelsonVides force-pushed the pools/rdbms_pool_tags branch from 6597730 to 03a4175 Compare February 28, 2024 10:16

chrzaszcz approved these changes Feb 28, 2024

View reviewed changes

chrzaszcz merged commit 786f3a2 into master Feb 28, 2024
4 checks passed

chrzaszcz deleted the pools/rdbms_pool_tags branch February 28, 2024 15:17

jacekwegr added this to the 6.2.1 milestone Apr 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce pool names to mongoose_rdbms #4231

Introduce pool names to mongoose_rdbms #4231

NelsonVides commented Feb 26, 2024 •

edited

Loading

This comment was marked as outdated.

codecov bot commented Feb 26, 2024 •

edited

Loading

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

arcusfelis Feb 27, 2024

NelsonVides Feb 27, 2024

chrzaszcz Feb 27, 2024

arcusfelis left a comment

chrzaszcz left a comment

chrzaszcz Feb 27, 2024

chrzaszcz Feb 27, 2024

chrzaszcz Feb 27, 2024 •

edited

Loading

chrzaszcz Feb 27, 2024

This comment was marked as outdated.

mongoose-im commented Feb 27, 2024 •

edited

Loading

chrzaszcz left a comment

This comment was marked as outdated.

This comment was marked as outdated.

mongoose-im commented Feb 28, 2024 •

edited

Loading

chrzaszcz left a comment

	-spec execute(HostType :: server(), PoolName :: atom(), Name :: atom(), Parameters :: [term()]) ->
	-spec execute(HostType :: server(), PoolName :: atom(), Tag :: mongoose_wpool:tag(), Parameters :: [term()]) ->

Introduce pool names to mongoose_rdbms #4231

Introduce pool names to mongoose_rdbms #4231

Conversation

NelsonVides commented Feb 26, 2024 • edited Loading

This comment was marked as outdated.

codecov bot commented Feb 26, 2024 • edited Loading

Codecov Report

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

arcusfelis Feb 27, 2024

Choose a reason for hiding this comment

NelsonVides Feb 27, 2024

Choose a reason for hiding this comment

chrzaszcz Feb 27, 2024

Choose a reason for hiding this comment

arcusfelis left a comment

Choose a reason for hiding this comment

chrzaszcz left a comment

Choose a reason for hiding this comment

chrzaszcz Feb 27, 2024

Choose a reason for hiding this comment

chrzaszcz Feb 27, 2024

Choose a reason for hiding this comment

chrzaszcz Feb 27, 2024 • edited Loading

Choose a reason for hiding this comment

chrzaszcz Feb 27, 2024

Choose a reason for hiding this comment

This comment was marked as outdated.

mongoose-im commented Feb 27, 2024 • edited Loading

chrzaszcz left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

This comment was marked as outdated.

mongoose-im commented Feb 28, 2024 • edited Loading

chrzaszcz left a comment

Choose a reason for hiding this comment

NelsonVides commented Feb 26, 2024 •

edited

Loading

codecov bot commented Feb 26, 2024 •

edited

Loading

chrzaszcz Feb 27, 2024 •

edited

Loading

mongoose-im commented Feb 27, 2024 •

edited

Loading

mongoose-im commented Feb 28, 2024 •

edited

Loading