Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use maps in outgoing pools config #3541

Merged
merged 16 commits into from
Feb 17, 2022
Merged

Use maps in outgoing pools config #3541

merged 16 commits into from
Feb 17, 2022

Conversation

gustawlippa
Copy link
Contributor

@gustawlippa gustawlippa commented Feb 11, 2022 •
edited
Loading

Parsed pools config is now a list of maps. Internal opts and conn_opts are maps also.
In some places it is not really leveraged, and they are quickly changed to KV lists. One such place that is a bit awkward are the wpool opts, and start_configured_pools changes the options from a map. There are probably some improvements left on the table with the maps, but I didn't want to make this PR too big.

Now with default values!

Also, some code simplifications and improvements, some better specs and so on.

@mongoose-im

This comment was marked as outdated.

Sign in to view
@gustawlippa gustawlippa changed the title Remove unnecessary dialyzer preprocessor directive Use maps for outgoing pools Feb 11, 2022
@mongoose-im mongoose-im mentioned this pull request Feb 11, 2022
Closed
@mongoose-im

This comment was marked as outdated.

Sign in to view
@mongoose-im

This comment was marked as outdated.

Sign in to view
@mongoose-im

This comment was marked as outdated.

Sign in to view
@codecov
Copy link

codecov bot commented Feb 11, 2022 •
edited
Loading

Codecov Report

Merging #3541 (65e89be) into master (182e047) will decrease coverage by 1.87%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3541      +/-   ##
==========================================
- Coverage   81.11%   79.23%   -1.88%     
==========================================
  Files         420      420              
  Lines       32288    32266      -22     
==========================================
- Hits        26189    25566     -623     
- Misses       6099     6700     +601
Impacted Files Coverage Δ
src/rdbms/mongoose_rdbms.erl 63.29% <ø> (ø)
src/rdbms/mongoose_rdbms_backend.erl 100.00% <ø> (ø)
src/wpool/mongoose_wpool_redis.erl 100.00% <ø> (ø)
src/config/mongoose_config_spec.erl 99.21% <100.00%> (+<0.01%) ⬆️
...stem_metrics/mongoose_system_metrics_collector.erl 93.81% <100.00%> (ø)
src/wpool/mongoose_wpool.erl 81.63% <100.00%> (-4.91%) ⬇️
src/wpool/mongoose_wpool_cassandra.erl 95.45% <100.00%> (-0.20%) ⬇️
src/wpool/mongoose_wpool_elastic.erl 100.00% <100.00%> (+9.09%) ⬆️
src/wpool/mongoose_wpool_http.erl 92.00% <100.00%> (ø)
src/wpool/mongoose_wpool_ldap.erl 100.00% <100.00%> (ø)
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 182e047...65e89be. Read the comment docs.

@gustawlippa gustawlippa changed the title Use maps for outgoing pools Use maps in outgoing pools config Feb 14, 2022
@mongoose-im

This comment was marked as outdated.

Sign in to view
@mongoose-im

This comment was marked as outdated.

Sign in to view
@mongoose-im

This comment was marked as outdated.

Sign in to view
@gustawlippa gustawlippa force-pushed the outgoing-pools-map branch 2 times, most recently from 38e18a7 to 0d8863f Compare February 15, 2022 11:55
@mongoose-im

This comment was marked as outdated.

Sign in to view
@mongoose-im

This comment was marked as outdated.

Sign in to view
@gustawlippa gustawlippa force-pushed the outgoing-pools-map branch 3 times, most recently from d652ad4 to c2a0b11 Compare February 15, 2022 13:07
@mongoose-im

This comment was marked as outdated.

Sign in to view
@mongoose-im

This comment was marked as outdated.

Sign in to view
@mongoose-im

This comment was marked as outdated.

Sign in to view
@mongoose-im

This comment was marked as outdated.

Sign in to view
@mongoose-im
Copy link
Collaborator

mongoose-im commented Feb 15, 2022 •
edited
Loading

small_tests_24 / small_tests / 1ce81c1
Reports root / small

small_tests_23 / small_tests / 1ce81c1
Reports root / small

dynamic_domains_mysql_redis_24 / mysql_redis / 1ce81c1
Reports root/ big
OK: 2703 / Failed: 0 / User-skipped: 256 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 1ce81c1
Reports root/ big
OK: 2720 / Failed: 0 / User-skipped: 239 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 1ce81c1
Reports root/ big
OK: 2720 / Failed: 0 / User-skipped: 239 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 1ce81c1
Reports root/ big
OK: 2720 / Failed: 0 / User-skipped: 239 / Auto-skipped: 0

ldap_mnesia_23 / ldap_mnesia / 1ce81c1
Reports root/ big
OK: 1509 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0

ldap_mnesia_24 / ldap_mnesia / 1ce81c1
Reports root/ big
OK: 1509 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0

internal_mnesia_24 / internal_mnesia / 1ce81c1
Reports root/ big
OK: 1550 / Failed: 0 / User-skipped: 349 / Auto-skipped: 0

pgsql_mnesia_24 / pgsql_mnesia / 1ce81c1
Reports root/ big
OK: 3094 / Failed: 0 / User-skipped: 248 / Auto-skipped: 0

elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 1ce81c1
Reports root/ big
OK: 1684 / Failed: 0 / User-skipped: 369 / Auto-skipped: 0

pgsql_mnesia_23 / pgsql_mnesia / 1ce81c1
Reports root/ big
OK: 3094 / Failed: 0 / User-skipped: 248 / Auto-skipped: 0

mysql_redis_24 / mysql_redis / 1ce81c1
Reports root/ big
OK: 3089 / Failed: 0 / User-skipped: 253 / Auto-skipped: 0

mssql_mnesia_24 / odbc_mssql_mnesia / 1ce81c1
Reports root/ big
OK: 3094 / Failed: 0 / User-skipped: 248 / Auto-skipped: 0

riak_mnesia_24 / riak_mnesia / 1ce81c1
Reports root/ big
OK: 1689 / Failed: 0 / User-skipped: 364 / Auto-skipped: 0

@gustawlippa gustawlippa force-pushed the outgoing-pools-map branch 2 times, most recently from 7e9821d to 7b62bfd Compare February 17, 2022 10:00
@mongoose-im

This comment was marked as outdated.

Sign in to view
@mongoose-im
Copy link
Collaborator

mongoose-im commented Feb 17, 2022 •
edited
Loading

small_tests_24 / small_tests / 7b62bfd
Reports root / small

small_tests_23 / small_tests / 7b62bfd
Reports root / small

dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 7b62bfd
Reports root/ big
OK: 2720 / Failed: 0 / User-skipped: 239 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 7b62bfd
Reports root/ big
OK: 2720 / Failed: 0 / User-skipped: 239 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 7b62bfd
Reports root/ big
OK: 2720 / Failed: 0 / User-skipped: 239 / Auto-skipped: 0

dynamic_domains_mysql_redis_24 / mysql_redis / 7b62bfd
Reports root/ big
OK: 2703 / Failed: 0 / User-skipped: 256 / Auto-skipped: 0

ldap_mnesia_23 / ldap_mnesia / 7b62bfd
Reports root/ big
OK: 1509 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0

ldap_mnesia_24 / ldap_mnesia / 7b62bfd
Reports root/ big
OK: 1509 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0

internal_mnesia_24 / internal_mnesia / 7b62bfd
Reports root/ big
OK: 1550 / Failed: 0 / User-skipped: 349 / Auto-skipped: 0

mysql_redis_24 / mysql_redis / 7b62bfd
Reports root/ big
OK: 3089 / Failed: 0 / User-skipped: 253 / Auto-skipped: 0

pgsql_mnesia_24 / pgsql_mnesia / 7b62bfd
Reports root/ big
OK: 3094 / Failed: 0 / User-skipped: 248 / Auto-skipped: 0

elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 7b62bfd
Reports root/ big
OK: 1684 / Failed: 0 / User-skipped: 369 / Auto-skipped: 0

pgsql_mnesia_23 / pgsql_mnesia / 7b62bfd
Reports root/ big
OK: 3094 / Failed: 0 / User-skipped: 248 / Auto-skipped: 0

mssql_mnesia_24 / odbc_mssql_mnesia / 7b62bfd
Reports root/ big
OK: 3094 / Failed: 0 / User-skipped: 248 / Auto-skipped: 0

riak_mnesia_24 / riak_mnesia / 7b62bfd
Reports root/ big
OK: 1689 / Failed: 0 / User-skipped: 364 / Auto-skipped: 0

@mongoose-im

This comment was marked as outdated.

Sign in to view
@mongoose-im
Copy link
Collaborator

mongoose-im commented Feb 17, 2022 •
edited
Loading

small_tests_23 / small_tests / 82a0a5e
Reports root / small

small_tests_24 / small_tests / 82a0a5e
Reports root / small

dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 82a0a5e
Reports root/ big
OK: 2720 / Failed: 0 / User-skipped: 239 / Auto-skipped: 0

ldap_mnesia_23 / ldap_mnesia / 82a0a5e
Reports root/ big
OK: 1509 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0

ldap_mnesia_24 / ldap_mnesia / 82a0a5e
Reports root/ big
OK: 1509 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 82a0a5e
Reports root/ big
OK: 2720 / Failed: 0 / User-skipped: 239 / Auto-skipped: 0

dynamic_domains_mysql_redis_24 / mysql_redis / 82a0a5e
Reports root/ big
OK: 2703 / Failed: 0 / User-skipped: 256 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 82a0a5e
Reports root/ big
OK: 2720 / Failed: 0 / User-skipped: 239 / Auto-skipped: 0

pgsql_mnesia_24 / pgsql_mnesia / 82a0a5e
Reports root/ big
OK: 3094 / Failed: 0 / User-skipped: 248 / Auto-skipped: 0

mysql_redis_24 / mysql_redis / 82a0a5e
Reports root/ big
OK: 3089 / Failed: 0 / User-skipped: 253 / Auto-skipped: 0

internal_mnesia_24 / internal_mnesia / 82a0a5e
Reports root/ big
OK: 1550 / Failed: 0 / User-skipped: 349 / Auto-skipped: 0

mssql_mnesia_24 / odbc_mssql_mnesia / 82a0a5e
Reports root/ big
OK: 3094 / Failed: 0 / User-skipped: 248 / Auto-skipped: 0

elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 82a0a5e
Reports root/ big
OK: 1684 / Failed: 0 / User-skipped: 369 / Auto-skipped: 0

pgsql_mnesia_23 / pgsql_mnesia / 82a0a5e
Reports root/ big
OK: 3094 / Failed: 0 / User-skipped: 248 / Auto-skipped: 0

riak_mnesia_24 / riak_mnesia / 82a0a5e
Reports root/ big
OK: 1689 / Failed: 0 / User-skipped: 364 / Auto-skipped: 0

@mongoose-im

This comment was marked as outdated.

Sign in to view
gustawlippa and others added 16 commits February 17, 2022 13:13
@gustawlippa
Remove unnecessary dialyzer preprocessor directive
25e232f
@gustawlippa
Store outgoing_pools config in a list of maps
61072bd
@gustawlippa
Make specs more strict
234f68a
@gustawlippa
Use maps for wpool opts and conn_opts
aa5a4c6
@gustawlippa
Remove unused LDAP pool connection.host option
270f5e2
@gustawlippa
Add defaults for wpool opts in config
ecdecb3
@gustawlippa
Add defaults for LDAP connection opts in config
dba7e41
@gustawlippa
Add defaults for redis pool conn_opts in config
695ef9a
@gustawlippa
Add defaults for elastic pool conn_opts in config
295a4fb
@gustawlippa
Add defaults for rabbit pool in config
e000b2c
@gustawlippa
Add defaults for cassandra pool in config
abb138d
@gustawlippa
Simplify wpool, add rdbms config defaults
0edfd11
@gustawlippa @chrzaszcz
Improve formatting
e366e5d 
Co-authored-by: Paweł Chrząszcz <[email protected]>
@gustawlippa
Simplify tests
aa6ddae 
No need to provide defaults, as pool_config/1 will add them. This includes
a `default` tag.

Options are now checked against values other than defaults.
@gustawlippa
Update documentation
0a05f15 
The call_timeout option was not part of the connection section.
@gustawlippa
Add a default for pool scope
65e89be
@mongoose-im
Copy link
Collaborator

mongoose-im commented Feb 17, 2022 •
edited
Loading

small_tests_24 / small_tests / 65e89be
Reports root / small

small_tests_23 / small_tests / 65e89be
Reports root / small

dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 65e89be
Reports root/ big
OK: 2721 / Failed: 0 / User-skipped: 239 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 65e89be
Reports root/ big
OK: 2721 / Failed: 0 / User-skipped: 239 / Auto-skipped: 0

ldap_mnesia_23 / ldap_mnesia / 65e89be
Reports root/ big
OK: 1510 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 65e89be
Reports root/ big
OK: 2721 / Failed: 0 / User-skipped: 239 / Auto-skipped: 0

dynamic_domains_mysql_redis_24 / mysql_redis / 65e89be
Reports root/ big
OK: 2704 / Failed: 0 / User-skipped: 256 / Auto-skipped: 0

internal_mnesia_24 / internal_mnesia / 65e89be
Reports root/ big
OK: 1551 / Failed: 0 / User-skipped: 349 / Auto-skipped: 0

elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 65e89be
Reports root/ big
OK: 1685 / Failed: 0 / User-skipped: 369 / Auto-skipped: 0

pgsql_mnesia_23 / pgsql_mnesia / 65e89be
Reports root/ big
OK: 3095 / Failed: 0 / User-skipped: 248 / Auto-skipped: 0

pgsql_mnesia_24 / pgsql_mnesia / 65e89be
Reports root/ big
OK: 3095 / Failed: 0 / User-skipped: 248 / Auto-skipped: 0

ldap_mnesia_24 / ldap_mnesia / 65e89be
Reports root/ big
OK: 1510 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0

mysql_redis_24 / mysql_redis / 65e89be
Reports root/ big
OK: 3090 / Failed: 0 / User-skipped: 253 / Auto-skipped: 0

mssql_mnesia_24 / odbc_mssql_mnesia / 65e89be
Reports root/ big
OK: 3095 / Failed: 0 / User-skipped: 248 / Auto-skipped: 0

riak_mnesia_24 / riak_mnesia / 65e89be
Reports root/ big
OK: 1690 / Failed: 0 / User-skipped: 364 / Auto-skipped: 0

chrzaszcz
chrzaszcz approved these changes Feb 17, 2022
View reviewed changes
Copy link
Member

@chrzaszcz chrzaszcz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@chrzaszcz chrzaszcz merged commit a89e2ec into master Feb 17, 2022
@chrzaszcz chrzaszcz deleted the outgoing-pools-map branch February 17, 2022 14:05
@Premwoik Premwoik added this to the 5.1.0 milestone May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NelsonVides NelsonVides NelsonVides left review comments

@chrzaszcz chrzaszcz chrzaszcz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
5.1.0
Development

Successfully merging this pull request may close these issues.
5 participants
@gustawlippa @mongoose-im @chrzaszcz @NelsonVides @Premwoik