Unify pool config - part 1 #2077
Conversation
michalwski
force-pushed
the
unify-pool-config
branch
from
da43a5a
to
6c4a526
Compare
|
5581.1 / Erlang 19.3 / small_tests / 0c56fcf 5581.3 / Erlang 19.3 / mysql_redis / 0c56fcf 5581.6 / Erlang 19.3 / elasticsearch_and_cassandra_mnesia / 0c56fcf 5581.5 / Erlang 19.3 / ldap_mnesia / 0c56fcf 5581.2 / Erlang 19.3 / internal_mnesia / 0c56fcf 5581.4 / Erlang 19.3 / odbc_mssql_mnesia / 0c56fcf 5581.9 / Erlang 21.0 / riak_mnesia / 0c56fcf 5581.8 / Erlang 20.0 / pgsql_mnesia / 0c56fcf |
|
5590.1 / Erlang 19.3 / small_tests / a1132a5 5590.2 / Erlang 19.3 / internal_mnesia / a1132a5 5590.4 / Erlang 19.3 / odbc_mssql_mnesia / a1132a5 5590.6 / Erlang 19.3 / elasticsearch_and_cassandra_mnesia / a1132a5 5590.5 / Erlang 19.3 / ldap_mnesia / a1132a5 5590.3 / Erlang 19.3 / mysql_redis / a1132a5 5590.8 / Erlang 20.0 / pgsql_mnesia / a1132a5 5590.9 / Erlang 21.0 / riak_mnesia / a1132a5 |
example of more complex pool is cassandra where cqerl pool needs to be started before more highlevel pool (managed by MongooseIM) is started
the doc shows: * how to use them in general * how to configure riak pool * how to configure cassandra pool
michalwski
force-pushed
the
unify-pool-config
branch
from
9d4e3e8
to
99e50e4
Compare
| @@ -0,0 +1,107 @@ | |||
| # Outgoing connections | |||
|
|
|||
| MongooseIM can be cofigured to talk to external service like databases or HTTP servers in order get or set required data. | |||
There was a problem hiding this comment.
configured
I'm gonna leave the actual language check to @goddammit ;)
| * `strategy` - specifies worker selection strategy for the given pool, default is `best_worker`, | ||
| more details on this can be found in [Choosing strategy in worker_pool doc](https://github.com/inaka/worker_pool#choosing-a-strategy) | ||
| * `call_timeout` - specifies the timeout for a call operation to the pool | ||
| * `ConnectionOptions` - list of `{key, value}` pairs passed to the `start` function of the pool type |
There was a problem hiding this comment.
Unfortunately not always, e.g. a tuple configuring Riak connection's SSL has 3 elements.
| @@ -51,8 +51,9 @@ start(Host, Opts) -> | |||
|
|
|||
| MaxHTTPConnections = gen_mod:get_opt(max_http_connections, Opts, 100), | |||
| mongoose_wpool:ensure_started(), | |||
| {ok, _} = mongoose_wpool:start(?MODULE, Host, [{strategy, available_worker}, | |||
| {workers, MaxHTTPConnections}]), | |||
| {ok, _} = mongoose_wpool:start(generic, Host, mod_push, | |||
There was a problem hiding this comment.
I kind of dislike replacing ?MODULE with tags that are meant to reference the module but in a subtler way. If we see a pool process with e.g. pusher_push in its name, we'll grep around the code to find where it comes from. If the tag was simply ?MODULE, everything would be clear from the getgo. In this instance, mod_push does not really correspond to a pool for mongoosepush service.
There was a problem hiding this comment.
- Previously
?MODULEwas used as the pool's type which doesn't fit nicely to the convention introduced and explained in Worker pool unification, iteration 2 #1959. - On one hand using ?MODULE as a tag name would be handy, on the other hand in the future the tag name is supposed to be configurable, which makes greping even more difficult.
- I'm ok with using
?MODULEas a tag for pool which are not started viaoutgoing_poolsoption.
@fenek I think your input on this is needed.
There was a problem hiding this comment.
I'd be fine with merging this as-is if we only change the mod_push tag to something a little more fitting.
There was a problem hiding this comment.
Done, is it any better now @kzemek?
| RiakPBOpts = [auto_reconnect, keepalive], | ||
| WorkerArgs = maybe_add_additional_opts(RiakPBOpts, SecurityOpts), | ||
| Worker = {riakc_pb_socket, [RiakAddr, RiakPort, WorkerArgs]}, | ||
| {_, Workers} = mongoose_wpool_riak:get_riak_opt(pool_size, RiakOpts, {pool_size, 20}), | ||
| %% TODO (Bartek Gorny): improve worker_pool, then use available_worker strategy here |
There was a problem hiding this comment.
@bartekgorny I feel this comment has not aged well. What does it mean? Why would we want to use available_worker?
src/mongoose_wpool.erl
Outdated
| Opts = lists:append(Opts0), | ||
| case wpool:start_sup_pool(make_pool_name(Type, Host, Tag), WpoolOpts) of | ||
| %WpoolOpts = maybe_get_wpool_opts_from_callback(Type, WpoolOptsIn, ConnOpts), |
There was a problem hiding this comment.
Yes, leftovers :)
src/mongoose_wpool.erl
Outdated
| try | ||
| CallbackModule = make_callback_module_name(Type), | ||
| CallbackModule:stop(Host, Tag) | ||
| catch E:R -> |
There was a problem hiding this comment.
Feels like this try/catch dance could be deduplicated
There was a problem hiding this comment.
I'll try to deduplicate it.
|
5603.1 / Erlang 19.3 / small_tests / 55bc875 5603.2 / Erlang 19.3 / internal_mnesia / 55bc875 5603.3 / Erlang 19.3 / mysql_redis / 55bc875 5603.4 / Erlang 19.3 / odbc_mssql_mnesia / 55bc875 5603.6 / Erlang 19.3 / elasticsearch_and_cassandra_mnesia / 55bc875 5603.5 / Erlang 19.3 / ldap_mnesia / 55bc875 5603.8 / Erlang 20.0 / pgsql_mnesia / 55bc875 5603.9 / Erlang 21.0 / riak_mnesia / 55bc875 |
|
5605.1 / Erlang 19.3 / small_tests / ac51ace 5605.5 / Erlang 19.3 / ldap_mnesia / ac51ace 5605.3 / Erlang 19.3 / mysql_redis / ac51ace 5605.6 / Erlang 19.3 / elasticsearch_and_cassandra_mnesia / ac51ace 5605.2 / Erlang 19.3 / internal_mnesia / ac51ace 5605.4 / Erlang 19.3 / odbc_mssql_mnesia / ac51ace |
Codecov Report
@@ Coverage Diff @@
## master #2077 +/- ##
==========================================
- Coverage 75% 73.25% -1.76%
==========================================
Files 315 320 +5
Lines 28630 28695 +65
==========================================
- Hits 21474 21020 -454
- Misses 7156 7675 +519
Continue to review full report at Codecov.
|
|
5610.1 / Erlang 19.3 / small_tests / 8936dc5 5610.5 / Erlang 19.3 / ldap_mnesia / 8936dc5 5610.6 / Erlang 19.3 / elasticsearch_and_cassandra_mnesia / 8936dc5 5610.3 / Erlang 19.3 / mysql_redis / 8936dc5 5610.2 / Erlang 19.3 / internal_mnesia / 8936dc5 5610.4 / Erlang 19.3 / odbc_mssql_mnesia / 8936dc5 5610.8 / Erlang 20.0 / pgsql_mnesia / 8936dc5 5610.9 / Erlang 21.0 / riak_mnesia / 8936dc5 |
| @@ -0,0 +1,208 @@ | |||
| # Outgoing connections | |||
|
|
|||
| MongooseIM can be configured to talk to external service like databases or HTTP servers in order to get or set required data. | |||
There was a problem hiding this comment.
MongooseIM can be configured to talk to external service like databases or HTTP servers in order to get or set the required data.
| * `cassandra` - pool of connections to cassandra cluster | ||
| * `riak` - pool of connections to riak cluster | ||
| * `redis` - pool of connections to redis server | ||
| * `http` - pool of connections to various HTTP(S) server MongooseIM can talk to, in example HTTP auth backend or HTTP notifications |
There was a problem hiding this comment.
http - pool of connections to various HTTP(S) servers MongooseIM can talk to, via an example HTTP auth backend or HTTP notifications ?
|
|
||
| All the above pools are managed by [inaka/worker_pool](https://github.com/inaka/worker_pool) library. | ||
|
|
||
| Every entry in the `outgoing_pools` is a 5 element tuple like below: |
There was a problem hiding this comment.
Every entry in the outgoing_pools is a 5 element tuple:
| * `Tag` is a name to distinguish pools with the same `Type` and `Host` parameter. | ||
| * `PoolOptions` is a list of `{key, value}` pairs as defined in [worker_pool doc](https://github.com/inaka/worker_pool#choosing-a-strategy) | ||
| with the following exception: | ||
| * `strategy` - specifies worker selection strategy for the given pool, default is `best_worker`, |
There was a problem hiding this comment.
strategy - specifies the worker selection strategy for the given pool, default is best_worker,
| * `strategy` - specifies worker selection strategy for the given pool, default is `best_worker`, | ||
| more details on this can be found in [Choosing strategy in worker_pool doc](https://github.com/inaka/worker_pool#choosing-a-strategy) | ||
| * `call_timeout` - specifies the timeout for a call operation to the pool | ||
| * `ConnectionOptions` - list options passed to the `start` function of the pool type |
There was a problem hiding this comment.
ConnectionOptions - lists options passed to the start function of the pool type ?
| ## Riak connection setup | ||
|
|
||
| Currently only one Riak connection pool can exist for each supported XMPP host. | ||
| It is configured with the following tuple inside `outgoing_pools` config option. |
There was a problem hiding this comment.
It is configured with the following tuple inside the outgoing_pools config option.
|
|
||
| ## Cassandra connection setup | ||
|
|
||
| The minimum pool definition for cassandra workers looks like below: |
There was a problem hiding this comment.
The minimum pool definition for cassandra workers looks as follows:
| ]}. | ||
| ``` | ||
|
|
||
| In this case MongooseIM will take by default try to connect to Cassandra server on "localhost" and port 9042. |
There was a problem hiding this comment.
In this case MongooseIM will by default try to connect to the Cassandra server on "localhost" and port 9042.
The keyspace used in queries will be mongooseim.
|
|
||
| * `Type` is one of the types listed above | ||
| * `Host` can be set to: | ||
| * `global` - meaning the pool will started once despited the XMPP hosts served by MongooseIM |
There was a problem hiding this comment.
global - meaning the pool will be started once no matter how many XMPP hosts are served by MongooseIM
This PR makes it possible to start following pools:
via
outgoing_poolsconfig option as described in #1959. The issue is not full covered by this PR yet. Remaining work relates to rdbms and elasticsearch pools.Currently
big_testsstarts only riak and cassandra pools via theoutgoing_poolsoption. Other pools while possible to start in this way are still started directly by modules using them.