Browse Source

Merge pull request #92 from seriyps/map-config-and-state-is-internal

Pool config is accepted as map; `#pool{}` record is module-local now
Sergey Prokhorov 2 years ago
parent
commit
6c0ee31e26
10 changed files with 292 additions and 331 deletions
  1. 29 29
      README.org
  2. 152 35
      src/pooler.erl
  3. 0 95
      src/pooler.hrl
  4. 1 4
      src/pooler_app.erl
  5. 0 50
      src/pooler_config.erl
  6. 15 13
      src/pooler_pool_sup.erl
  7. 3 3
      src/pooler_pooled_worker_sup.erl
  8. 15 17
      src/pooler_sup.erl
  9. 19 19
      test/pooler_tests.erl
  10. 58 66
      test/prop_pooler.erl

+ 29 - 29
README.org

@@ -81,8 +81,8 @@ better support multiple independent pools.
 
 
 Pool configuration is specified in the pooler application's
 Pool configuration is specified in the pooler application's
 environment.  This can be provided in a config file using =-config= or
 environment.  This can be provided in a config file using =-config= or
-set at startup using =application:set_env(pooler, pools,
-Pools)=. Here's an example config file that creates two pools of
+set at startup using =application:set_env(pooler, pools, Pools)=.
+Here's an example config file that creates two pools of
 Riak pb clients each talking to a different node in a local cluster
 Riak pb clients each talking to a different node in a local cluster
 and one pool talking to a Postgresql database:
 and one pool talking to a Postgresql database:
 
 
@@ -94,25 +94,25 @@ and one pool talking to a Postgresql database:
   [
   [
    {pooler, [
    {pooler, [
            {pools, [
            {pools, [
-                    [{name, rc8081},
-                     {group, riak},
-                     {max_count, 5},
-                     {init_count, 2},
-                     {start_mfa,
-                      {riakc_pb_socket, start_link, ["localhost", 8081]}}],
-
-                    [{name, rc8082},
-                     {group, riak},
-                     {max_count, 5},
-                     {init_count, 2},
-                     {start_mfa,
-                      {riakc_pb_socket, start_link, ["localhost", 8082]}}],
-
-                    [{name, pg_db1},
-                     {max_count, 10},
-                     {init_count, 2},
-                     {start_mfa,
-                      {my_pg_sql_driver, start_link, ["db_host"]}}]
+                    #{name => rc8081,
+                      group => riak,
+                      max_count => 5,
+                      init_count => 2,
+                      start_mfa =>
+                       {riakc_pb_socket, start_link, ["localhost", 8081]}},
+
+                    #{name => rc8082,
+                      group => riak,
+                      max_count => 5,
+                      init_count => 2,
+                      start_mfa =>
+                       {riakc_pb_socket, start_link, ["localhost", 8082]}},
+
+                    #{name => pg_db1,
+                      max_count => 10,
+                      init_count => 2,
+                      start_mfa =>
+                       {my_pg_sql_driver, start_link, ["db_host"]}}
                    ]}
                    ]}
              %% if you want to enable metrics, set this to a module with
              %% if you want to enable metrics, set this to a module with
              %% an API conformant to the folsom_metrics module.
              %% an API conformant to the folsom_metrics module.
@@ -159,15 +159,15 @@ defaults to ={30, sec}=.
 
 
 *** Pool Configuration via =pooler:new_pool=
 *** Pool Configuration via =pooler:new_pool=
 You can create pools using =pooler:new_pool/1= when accepts a
 You can create pools using =pooler:new_pool/1= when accepts a
-proplist of pool configuration. Here's an example:
+map of pool configuration. Here's an example:
 #+BEGIN_SRC erlang
 #+BEGIN_SRC erlang
-PoolConfig = [{name, rc8081},
-              {group, riak},
-              {max_count, 5},
-              {init_count, 2},
-              {start_mfa,
-               {riakc_pb_socket,
-                start_link, ["localhost", 8081]}}],
+PoolConfig = #{
+    name => rc8081,
+    group => riak,
+    max_count => 5,
+    init_count => 2,
+    start_mfa => {riakc_pb_socket, start_link, ["localhost", 8081]}
+},
 pooler:new_pool(PoolConfig).
 pooler:new_pool(PoolConfig).
 #+END_SRC
 #+END_SRC
 *** Using pooler
 *** Using pooler

+ 152 - 35
src/pooler.erl

@@ -11,7 +11,6 @@
 -module(pooler).
 -module(pooler).
 -behaviour(gen_server).
 -behaviour(gen_server).
 
 
--include("pooler.hrl").
 -include_lib("kernel/include/logger.hrl").
 -include_lib("kernel/include/logger.hrl").
 
 
 %% ------------------------------------------------------------------
 %% ------------------------------------------------------------------
@@ -44,6 +43,7 @@
     call_free_members/2,
     call_free_members/2,
     call_free_members/3
     call_free_members/3
 ]).
 ]).
+-export([create_group_table/0, config_as_map/1]).
 
 
 %% ------------------------------------------------------------------
 %% ------------------------------------------------------------------
 %% gen_server Function Exports
 %% gen_server Function Exports
@@ -64,8 +64,98 @@
 %% ------------------------------------------------------------------
 %% ------------------------------------------------------------------
 -export_type([pool_config/0, pool_name/0, group_name/0, member_info/0, time_unit/0, time_spec/0]).
 -export_type([pool_config/0, pool_name/0, group_name/0, member_info/0, time_unit/0, time_spec/0]).
 
 
-% Internal
--export_type([members_map/0, consumers_map/0, requestor_queue/0, pool_state/0]).
+-define(DEFAULT_ADD_RETRY, 1).
+-define(DEFAULT_CULL_INTERVAL, {1, min}).
+-define(DEFAULT_MAX_AGE, {30, sec}).
+-define(DEFAULT_MEMBER_START_TIMEOUT, {1, min}).
+-define(DEFAULT_AUTO_GROW_THRESHOLD, undefined).
+-define(POOLER_GROUP_TABLE, pooler_group_table).
+-define(DEFAULT_POOLER_QUEUE_MAX, 50).
+-define(POOLER_POOL_NAME, '$pooler_pool_name').
+-define(POOLER_PID, '$pooler_pid').
+-define(DEFAULT_STOP_MFA, {supervisor, terminate_child, [?POOLER_POOL_NAME, ?POOLER_PID]}).
+
+-record(pool, {
+    name :: atom(),
+    group :: atom(),
+    max_count = 100 :: non_neg_integer(),
+    init_count = 10 :: non_neg_integer(),
+    start_mfa :: {atom(), atom(), [term()]},
+    free_pids = [] :: [pid()],
+    in_use_count = 0 :: non_neg_integer(),
+    free_count = 0 :: non_neg_integer(),
+    %% The number times to attempt adding a pool member if the
+    %% pool size is below max_count and there are no free
+    %% members. After this many tries, error_no_members will be
+    %% returned by a call to take_member. NOTE: this value
+    %% should be >= 2 or else the pool will not grow on demand
+    %% when max_count is larger than init_count.
+    add_member_retry = ?DEFAULT_ADD_RETRY :: non_neg_integer(),
+
+    %% The interval to schedule a cull message. Both
+    %% 'cull_interval' and 'max_age' are specified using a
+    %% `time_spec()' type.
+    cull_interval = ?DEFAULT_CULL_INTERVAL :: time_spec(),
+    %% The maximum age for members.
+    max_age = ?DEFAULT_MAX_AGE :: time_spec(),
+    cull_timer :: reference() | undefined,
+
+    %% The supervisor used to start new members
+    member_sup :: atom() | pid(),
+
+    %% The supervisor used to start starter servers that start
+    %% new members. This is what enables async member starts.
+    starter_sup :: atom() | pid(),
+
+    %% Maps member pid to a tuple of the form:
+    %% {MonitorRef, Status, Time},
+    %% where MonitorRef is a monitor reference for the member,,
+    %% Status is either 'free' or the consumer pid, and Time is
+    %% an Erlang timestamp that records when the member became
+    %% free.
+
+    all_members = #{} :: members_map(),
+
+    %% Maps consumer pid to a tuple of the form:
+    %% {MonitorRef, MemberList} where MonitorRef is a monitor
+    %% reference for the consumer and MemberList is a list of
+    %% members being consumed.
+    consumer_to_pid = #{} :: consumers_map(),
+
+    %% A list of `{References, Timestamp}' tuples representing
+    %% new member start requests that are in-flight. The
+    %% timestamp records when the start request was initiated
+    %% and is used to implement start timeout.
+    starting_members = [] :: [{pid(), erlang:timestamp()}],
+
+    %% The maximum amount of time to allow for member start.
+    member_start_timeout = ?DEFAULT_MEMBER_START_TIMEOUT :: time_spec(),
+
+    %% The optional threshold at which more members will be started if
+    %% free_count drops to this value.  Normally undefined, but may be
+    %% set to a non-negative integer in order to enable "anticipatory"
+    %% behavior (start members before they're actually needed).
+    auto_grow_threshold = ?DEFAULT_AUTO_GROW_THRESHOLD :: undefined | non_neg_integer(),
+
+    %% Stop callback to gracefully attempt to terminate pool members.
+    %% The list of arguments must contain the fixed atom '$pooler_pid'.
+    stop_mfa = ?DEFAULT_STOP_MFA :: {atom(), atom(), [term()]},
+
+    %% The module to use for collecting metrics. If set to
+    %% 'pooler_no_metrics', then metric sending calls do
+    %% nothing. A typical value to actually capture metrics is
+    %% folsom_metrics.
+    metrics_mod = pooler_no_metrics :: atom(),
+
+    %% The API used to call the metrics system. It supports both Folsom
+    %% and Exometer format.
+    metrics_api = folsom :: 'folsom' | 'exometer',
+
+    %% A queue of requestors for blocking take member requests
+    queued_requestors = queue:new() :: requestor_queue(),
+    %% The max depth of the queue
+    queue_max = 50 :: non_neg_integer()
+}).
 
 
 -type pool_name() :: atom().
 -type pool_name() :: atom().
 %% The name of the pool
 %% The name of the pool
@@ -75,32 +165,31 @@
 -type time_spec() :: {non_neg_integer(), time_unit()}.
 -type time_spec() :: {non_neg_integer(), time_unit()}.
 %% Human-friendly way to specify the amount of time
 %% Human-friendly way to specify the amount of time
 
 
--type pool_config() :: [
-    {name, pool_name()}
-    | {init_count, non_neg_integer()}
-    | {max_count, non_neg_integer()}
-    | {start_mfa, {module(), atom(), [any()]}}
-    | {group, group_name()}
-    | {cull_interval, time_spec()}
-    | {max_age, time_spec()}
-    | {member_start_timeout, time_spec()}
-    | {queue_max, non_neg_integer()}
-    | {metrics_api, folsom | exometer}
-    | {metrics_mod, module()}
-    | {stop_mfa, {module(), atom(), ['$pooler_pid' | any(), ...]}}
-    | {auto_grow_threshold, non_neg_integer()}
-    | {add_member_retry, non_neg_integer()}
-].
+-type pool_config() ::
+    #{
+        name := pool_name(),
+        init_count := non_neg_integer(),
+        max_count := non_neg_integer(),
+        start_mfa := {module(), atom(), [any()]},
+        group => group_name(),
+        cull_interval => time_spec(),
+        max_age => time_spec(),
+        member_start_timeout => time_spec(),
+        queue_max => non_neg_integer(),
+        metrics_api => folsom | exometer,
+        metrics_mod => module(),
+        stop_mfa => {module(), atom(), ['$pooler_pid' | any(), ...]},
+        auto_grow_threshold => non_neg_integer(),
+        add_member_retry => non_neg_integer()
+    }.
 %% See {@link pooler:new_pool/1}
 %% See {@link pooler:new_pool/1}
+-type pool_config_legacy() :: [{atom, any()}].
 
 
 -type free_member_info() :: {reference(), free, erlang:timestamp()}.
 -type free_member_info() :: {reference(), free, erlang:timestamp()}.
 -type member_info() :: {reference(), free | pid(), erlang:timestamp()}.
 -type member_info() :: {reference(), free | pid(), erlang:timestamp()}.
+
 -type members_map() :: #{pid() => member_info()}.
 -type members_map() :: #{pid() => member_info()}.
-%% Internal
 -type consumers_map() :: #{pid() => {reference(), [pid()]}}.
 -type consumers_map() :: #{pid() => {reference(), [pid()]}}.
-%% Internal
--type pool_state() :: #pool{}.
-%% Internal
 
 
 -if(?OTP_RELEASE >= 25).
 -if(?OTP_RELEASE >= 25).
 -type gen_server_from() :: gen_server:from().
 -type gen_server_from() :: gen_server:from().
@@ -137,15 +226,21 @@ stop() ->
 %% API Function Definitions
 %% API Function Definitions
 %% ------------------------------------------------------------------
 %% ------------------------------------------------------------------
 
 
-start_link(#pool{name = Name} = Pool) ->
-    gen_server:start_link({local, Name}, ?MODULE, Pool, []).
+-spec start_link(pool_config()) -> {ok, pid()} | {error, any()}.
+start_link(#{name := Name, max_count := _, init_count := _, start_mfa := _} = PoolConfig) ->
+    %% PoolConfig may include `metrics_mod' and `metrics_api' at this point
+    gen_server:start_link({local, Name}, ?MODULE, PoolConfig, []).
 
 
 manual_start() ->
 manual_start() ->
     application:start(sasl),
     application:start(sasl),
     application:start(pooler).
     application:start(pooler).
 
 
-%% @doc Start a new pool described by the proplist `PoolConfig'. The
-%% following keys are required in the proplist:
+%% @private
+create_group_table() ->
+    ets:new(?POOLER_GROUP_TABLE, [set, public, named_table, {write_concurrency, true}]).
+
+%% @doc Start a new pool described by the map `PoolConfig'. The
+%% following keys are required in the map:
 %%
 %%
 %% <dl>
 %% <dl>
 %% <dt>`name'</dt>
 %% <dt>`name'</dt>
@@ -189,9 +284,9 @@ manual_start() ->
 %% <dd>Time limit for member starts. Specified as `{Time,
 %% <dd>Time limit for member starts. Specified as `{Time,
 %% Unit}'. Defaults to `{1, min}'.</dd>
 %% Unit}'. Defaults to `{1, min}'.</dd>
 %% </dl>
 %% </dl>
--spec new_pool(pool_config()) -> {ok, pid()} | {error, {already_started, pid()}}.
+-spec new_pool(pool_config() | pool_config_legacy()) -> {ok, pid()} | {error, {already_started, pid()}}.
 new_pool(PoolConfig) ->
 new_pool(PoolConfig) ->
-    pooler_sup:new_pool(PoolConfig).
+    pooler_sup:new_pool(config_as_map(PoolConfig)).
 
 
 %% @doc Terminate the named pool.
 %% @doc Terminate the named pool.
 -spec rm_pool(pool_name()) -> ok | {error, not_found | running | restarting}.
 -spec rm_pool(pool_name()) -> ok | {error, not_found | running | restarting}.
@@ -234,9 +329,9 @@ rm_group_members(MemberPids) ->
 %% @doc Get child spec described by the proplist `PoolConfig'.
 %% @doc Get child spec described by the proplist `PoolConfig'.
 %%
 %%
 %% See {@link pooler:new_pool/1} for info about `PoolConfig'.
 %% See {@link pooler:new_pool/1} for info about `PoolConfig'.
--spec pool_child_spec(pool_config()) -> supervisor:child_spec().
+-spec pool_child_spec(pool_config() | pool_config_legacy()) -> supervisor:child_spec().
 pool_child_spec(PoolConfig) ->
 pool_child_spec(PoolConfig) ->
-    pooler_sup:pool_child_spec(PoolConfig).
+    pooler_sup:pool_child_spec(config_as_map(PoolConfig)).
 
 
 %% @doc For INTERNAL use. Adds `MemberPid' to the pool.
 %% @doc For INTERNAL use. Adds `MemberPid' to the pool.
 -spec accept_member(pool_name(), pooler_starter:start_result()) -> ok.
 -spec accept_member(pool_name(), pooler_starter:start_result()) -> ok.
@@ -406,14 +501,29 @@ call_free_members(PoolName, Fun, Timeout) when
 %% gen_server Function Definitions
 %% gen_server Function Definitions
 %% ------------------------------------------------------------------
 %% ------------------------------------------------------------------
 
 
-init(#pool{} = Pool) ->
-    #pool{init_count = N} = Pool,
-    MemberSup = pooler_pool_sup:member_sup_name(Pool),
+init(#{name := Name, max_count := MaxCount, init_count := InitCount, start_mfa := StartMFA} = P) ->
+    Pool = #pool{
+        name = Name,
+        group = maps:get(group, P, undefined),
+        max_count = MaxCount,
+        init_count = InitCount,
+        start_mfa = StartMFA,
+        add_member_retry = maps:get(add_member_retry, P, ?DEFAULT_ADD_RETRY),
+        cull_interval = maps:get(cull_interval, P, ?DEFAULT_CULL_INTERVAL),
+        max_age = maps:get(max_age, P, ?DEFAULT_MAX_AGE),
+        member_start_timeout = maps:get(member_start_timeout, P, ?DEFAULT_MEMBER_START_TIMEOUT),
+        auto_grow_threshold = maps:get(auto_grow_threshold, P, ?DEFAULT_AUTO_GROW_THRESHOLD),
+        stop_mfa = maps:get(stop_mfa, P, ?DEFAULT_STOP_MFA),
+        metrics_mod = maps:get(metrics_mod, P, pooler_no_metrics),
+        metrics_api = maps:get(metrics_api, P, folsom),
+        queue_max = maps:get(queue_max, P, ?DEFAULT_POOLER_QUEUE_MAX)
+    },
+    MemberSup = pooler_pool_sup:build_member_sup_name(Name),
     Pool1 = set_member_sup(Pool, MemberSup),
     Pool1 = set_member_sup(Pool, MemberSup),
     %% This schedules the next cull when the pool is configured for
     %% This schedules the next cull when the pool is configured for
     %% such and is otherwise a no-op.
     %% such and is otherwise a no-op.
     Pool2 = cull_members_from_pool(Pool1),
     Pool2 = cull_members_from_pool(Pool1),
-    {ok, NewPool} = init_members_sync(N, Pool2),
+    {ok, NewPool} = init_members_sync(InitCount, Pool2),
     {ok, NewPool, {continue, join_group}}.
     {ok, NewPool, {continue, join_group}}.
 
 
 handle_continue(join_group, #pool{group = undefined} = Pool) ->
 handle_continue(join_group, #pool{group = undefined} = Pool) ->
@@ -1207,6 +1317,13 @@ to_map(#pool{} = Pool) ->
         [{'$record_name', Name} | lists:zip(record_info(fields, pool), Values)]
         [{'$record_name', Name} | lists:zip(record_info(fields, pool), Values)]
     ).
     ).
 
 
+%% @private
+-spec config_as_map(pool_config() | pool_config_legacy()) -> pool_config().
+config_as_map(Conf) when is_map(Conf) ->
+    Conf;
+config_as_map(LegacyConf) when is_list(LegacyConf) ->
+    maps:from_list(LegacyConf).
+
 % >= OTP-21
 % >= OTP-21
 -ifdef(OTP_RELEASE).
 -ifdef(OTP_RELEASE).
 -if(?OTP_RELEASE >= 23).
 -if(?OTP_RELEASE >= 23).

+ 0 - 95
src/pooler.hrl

@@ -1,95 +0,0 @@
--define(DEFAULT_ADD_RETRY, 1).
--define(DEFAULT_CULL_INTERVAL, {1, min}).
--define(DEFAULT_MAX_AGE, {30, sec}).
--define(DEFAULT_MEMBER_START_TIMEOUT, {1, min}).
--define(DEFAULT_AUTO_GROW_THRESHOLD, undefined).
--define(POOLER_GROUP_TABLE, pooler_group_table).
--define(DEFAULT_POOLER_QUEUE_MAX, 50).
--define(POOLER_POOL_NAME, '$pooler_pool_name').
--define(POOLER_PID, '$pooler_pid').
--define(DEFAULT_STOP_MFA, {supervisor, terminate_child, [?POOLER_POOL_NAME, ?POOLER_PID]}).
-
--record(pool, {
-    name :: atom(),
-    group :: atom(),
-    max_count = 100 :: non_neg_integer(),
-    init_count = 10 :: non_neg_integer(),
-    start_mfa :: {atom(), atom(), [term()]},
-    free_pids = [] :: [pid()],
-    in_use_count = 0 :: non_neg_integer(),
-    free_count = 0 :: non_neg_integer(),
-    %% The number times to attempt adding a pool member if the
-    %% pool size is below max_count and there are no free
-    %% members. After this many tries, error_no_members will be
-    %% returned by a call to take_member. NOTE: this value
-    %% should be >= 2 or else the pool will not grow on demand
-    %% when max_count is larger than init_count.
-    add_member_retry = ?DEFAULT_ADD_RETRY :: non_neg_integer(),
-
-    %% The interval to schedule a cull message. Both
-    %% 'cull_interval' and 'max_age' are specified using a
-    %% `time_spec()' type.
-    cull_interval = ?DEFAULT_CULL_INTERVAL :: pooler:time_spec(),
-    %% The maximum age for members.
-    max_age = ?DEFAULT_MAX_AGE :: pooler:time_spec(),
-    cull_timer :: reference() | undefined,
-
-    %% The supervisor used to start new members
-    member_sup :: atom() | pid(),
-
-    %% The supervisor used to start starter servers that start
-    %% new members. This is what enables async member starts.
-    starter_sup :: atom() | pid(),
-
-    %% Maps member pid to a tuple of the form:
-    %% {MonitorRef, Status, Time},
-    %% where MonitorRef is a monitor reference for the member,,
-    %% Status is either 'free' or the consumer pid, and Time is
-    %% an Erlang timestamp that records when the member became
-    %% free.
-
-    all_members = #{} :: pooler:members_map(),
-
-    %% Maps consumer pid to a tuple of the form:
-    %% {MonitorRef, MemberList} where MonitorRef is a monitor
-    %% reference for the consumer and MemberList is a list of
-    %% members being consumed.
-    consumer_to_pid = #{} :: pooler:consumers_map(),
-
-    %% A list of `{References, Timestamp}' tuples representing
-    %% new member start requests that are in-flight. The
-    %% timestamp records when the start request was initiated
-    %% and is used to implement start timeout.
-    starting_members = [] :: [{pid(), erlang:timestamp()}],
-
-    %% The maximum amount of time to allow for member start.
-    member_start_timeout = ?DEFAULT_MEMBER_START_TIMEOUT :: pooler:time_spec(),
-
-    %% The optional threshold at which more members will be started if
-    %% free_count drops to this value.  Normally undefined, but may be
-    %% set to a non-negative integer in order to enable "anticipatory"
-    %% behavior (start members before they're actually needed).
-    auto_grow_threshold = ?DEFAULT_AUTO_GROW_THRESHOLD :: undefined | non_neg_integer(),
-
-    %% Stop callback to gracefully attempt to terminate pool members.
-    %% The list of arguments must contain the fixed atom '$pooler_pid'.
-    stop_mfa = ?DEFAULT_STOP_MFA :: {atom(), atom(), [term()]},
-
-    %% The module to use for collecting metrics. If set to
-    %% 'pooler_no_metrics', then metric sending calls do
-    %% nothing. A typical value to actually capture metrics is
-    %% folsom_metrics.
-    metrics_mod = pooler_no_metrics :: atom(),
-
-    %% The API used to call the metrics system. It supports both Folsom
-    %% and Exometer format.
-    metrics_api = folsom :: 'folsom' | 'exometer',
-
-    %% A queue of requestors for blocking take member requests
-    queued_requestors = queue:new() :: pooler:requestor_queue(),
-    %% The max depth of the queue
-    queue_max = 50 :: non_neg_integer()
-}).
-
--define(gv(X, Y), proplists:get_value(X, Y)).
--define(gv(X, Y, D), proplists:get_value(X, Y, D)).

+ 1 - 4
src/pooler_app.erl

@@ -10,10 +10,7 @@
 %% ===================================================================
 %% ===================================================================
 
 
 start(_StartType, _StartArgs) ->
 start(_StartType, _StartArgs) ->
-    case pooler_sup:start_link() of
-        {ok, Pid} -> {ok, Pid};
-        Other -> {error, Other}
-    end.
+    pooler_sup:start_link().
 
 
 stop(_State) ->
 stop(_State) ->
     ok.
     ok.

+ 0 - 50
src/pooler_config.erl

@@ -1,50 +0,0 @@
-%% @author Seth Falcon <seth@userprimary.net>
-%% @copyright 2012-2013 Seth Falcon
-%% @doc Helper module to transform app config proplists into pool records
-
--module(pooler_config).
-
--export([list_to_pool/1, get_name/1, get_start_mfa/1, group_table/0]).
-
--include("pooler.hrl").
-
--spec list_to_pool(pooler:pool_config()) -> #pool{}.
-list_to_pool(P) ->
-    #pool{
-        name = req(name, P),
-        group = ?gv(group, P),
-        max_count = req(max_count, P),
-        init_count = req(init_count, P),
-        start_mfa = req(start_mfa, P),
-        add_member_retry = ?gv(add_member_retry, P, ?DEFAULT_ADD_RETRY),
-        cull_interval = ?gv(cull_interval, P, ?DEFAULT_CULL_INTERVAL),
-        max_age = ?gv(max_age, P, ?DEFAULT_MAX_AGE),
-        member_start_timeout = ?gv(member_start_timeout, P, ?DEFAULT_MEMBER_START_TIMEOUT),
-        auto_grow_threshold = ?gv(auto_grow_threshold, P, ?DEFAULT_AUTO_GROW_THRESHOLD),
-        stop_mfa = ?gv(stop_mfa, P, ?DEFAULT_STOP_MFA),
-        metrics_mod = ?gv(metrics_mod, P, pooler_no_metrics),
-        metrics_api = ?gv(metrics_api, P, folsom),
-        queue_max = ?gv(queue_max, P, ?DEFAULT_POOLER_QUEUE_MAX)
-    }.
-
--spec get_name(#pool{}) -> pooler:pool_name().
-get_name(#pool{name = Name}) ->
-    Name.
-
--spec get_start_mfa(pooler:pool_state()) -> {module(), atom(), [any()]}.
-get_start_mfa(#pool{start_mfa = MFA}) ->
-    MFA.
-
--spec group_table() -> atom().
-group_table() ->
-    ?POOLER_GROUP_TABLE.
-
-%% Return `Value' for `Key' in proplist `P' or crashes with an
-%% informative message if no value is found.
-req(Key, P) ->
-    case lists:keyfind(Key, 1, P) of
-        false ->
-            error({missing_required_config, Key, P});
-        {Key, Value} ->
-            Value
-    end.

+ 15 - 13
src/pooler_pool_sup.erl

@@ -10,16 +10,16 @@
     build_member_sup_name/1
     build_member_sup_name/1
 ]).
 ]).
 
 
--spec start_link(pooler:pool_state()) -> {ok, pid()}.
-start_link(Pool) ->
-    SupName = pool_sup_name(Pool),
-    supervisor:start_link({local, SupName}, ?MODULE, Pool).
-
-init(Pool) ->
-    PoolerSpec = {pooler, {pooler, start_link, [Pool]}, transient, 5000, worker, [pooler]},
-    MemberSupName = member_sup_name(Pool),
+-spec start_link(pooler:pool_config()) -> {ok, pid()}.
+start_link(PoolConf) ->
+    SupName = pool_sup_name(PoolConf),
+    supervisor:start_link({local, SupName}, ?MODULE, PoolConf).
+
+init(PoolConf) ->
+    PoolerSpec = {pooler, {pooler, start_link, [PoolConf]}, transient, 5000, worker, [pooler]},
+    MemberSupName = member_sup_name(PoolConf),
     MemberSupSpec =
     MemberSupSpec =
-        {MemberSupName, {pooler_pooled_worker_sup, start_link, [Pool]}, transient, 5000, supervisor, [
+        {MemberSupName, {pooler_pooled_worker_sup, start_link, [PoolConf]}, transient, 5000, supervisor, [
             pooler_pooled_worker_sup
             pooler_pooled_worker_sup
         ]},
         ]},
 
 
@@ -27,11 +27,13 @@ init(Pool) ->
     Restart = {one_for_all, 5, 60},
     Restart = {one_for_all, 5, 60},
     {ok, {Restart, [MemberSupSpec, PoolerSpec]}}.
     {ok, {Restart, [MemberSupSpec, PoolerSpec]}}.
 
 
-member_sup_name(Pool) ->
-    build_member_sup_name(pooler_config:get_name(Pool)).
+-spec member_sup_name(pooler:pool_config()) -> atom().
+member_sup_name(#{name := Name}) ->
+    build_member_sup_name(Name).
 
 
+-spec build_member_sup_name(pooler:pool_name()) -> atom().
 build_member_sup_name(PoolName) ->
 build_member_sup_name(PoolName) ->
     list_to_atom("pooler_" ++ atom_to_list(PoolName) ++ "_member_sup").
     list_to_atom("pooler_" ++ atom_to_list(PoolName) ++ "_member_sup").
 
 
-pool_sup_name(Pool) ->
-    list_to_atom("pooler_" ++ atom_to_list(pooler_config:get_name(Pool)) ++ "_pool_sup").
+pool_sup_name(#{name := Name}) ->
+    list_to_atom("pooler_" ++ atom_to_list(Name) ++ "_pool_sup").

+ 3 - 3
src/pooler_pooled_worker_sup.erl

@@ -4,9 +4,9 @@
 
 
 -export([start_link/1, init/1]).
 -export([start_link/1, init/1]).
 
 
-start_link(Pool) ->
-    MFA = pooler_config:get_start_mfa(Pool),
-    SupName = pooler_pool_sup:member_sup_name(Pool),
+-spec start_link(pooler:pool_config()) -> {ok, pid()} | {error, any()}.
+start_link(#{start_mfa := MFA} = PoolConf) ->
+    SupName = pooler_pool_sup:member_sup_name(PoolConf),
     supervisor:start_link({local, SupName}, ?MODULE, MFA).
     supervisor:start_link({local, SupName}, ?MODULE, MFA).
 
 
 init({Mod, Fun, Args}) ->
 init({Mod, Fun, Args}) ->

+ 15 - 17
src/pooler_sup.erl

@@ -15,7 +15,7 @@ start_link() ->
 
 
 init([]) ->
 init([]) ->
     %% a list of pool configs
     %% a list of pool configs
-    Config =
+    Configs =
         case application:get_env(pooler, pools) of
         case application:get_env(pooler, pools) of
             {ok, C} ->
             {ok, C} ->
                 C;
                 C;
@@ -23,13 +23,14 @@ init([]) ->
                 []
                 []
         end,
         end,
     {MetricsApi, MetricsMod} = metrics_module(),
     {MetricsApi, MetricsMod} = metrics_module(),
-    MetricsConfig = [
-        {metrics_mod, MetricsMod},
-        {metrics_api, MetricsApi}
+    PoolSupSpecs = [
+        pool_sup_spec((pooler:config_as_map(Config))#{
+            metrics_mod => MetricsMod,
+            metrics_api => MetricsApi
+        })
+     || Config <- Configs
     ],
     ],
-    Pools = [pooler_config:list_to_pool(MetricsConfig ++ L) || L <- Config],
-    PoolSupSpecs = [pool_sup_spec(Pool) || Pool <- Pools],
-    ets:new(pooler_config:group_table(), [set, public, named_table, {write_concurrency, true}]),
+    pooler:create_group_table(),
     {ok, {{one_for_one, 5, 60}, [starter_sup_spec() | PoolSupSpecs]}}.
     {ok, {{one_for_one, 5, 60}, [starter_sup_spec() | PoolSupSpecs]}}.
 
 
 %% @doc Create a new pool from proplist pool config `PoolConfig'. The
 %% @doc Create a new pool from proplist pool config `PoolConfig'. The
@@ -42,13 +43,10 @@ new_pool(PoolConfig) ->
 %% public API for this functionality is {@link pooler:pool_child_spec/1}.
 %% public API for this functionality is {@link pooler:pool_child_spec/1}.
 pool_child_spec(PoolConfig) ->
 pool_child_spec(PoolConfig) ->
     {MetricsApi, MetricsMod} = metrics_module(),
     {MetricsApi, MetricsMod} = metrics_module(),
-    NewPool = pooler_config:list_to_pool(
-        [
-            {metrics_mod, MetricsMod},
-            {metrics_api, MetricsApi}
-        ] ++ PoolConfig
-    ),
-    pool_sup_spec(NewPool).
+    pool_sup_spec(PoolConfig#{
+        metrics_mod => MetricsMod,
+        metrics_api => MetricsApi
+    }).
 
 
 %% @doc Shutdown the named pool.
 %% @doc Shutdown the named pool.
 rm_pool(Name) ->
 rm_pool(Name) ->
@@ -65,9 +63,9 @@ rm_pool(Name) ->
 starter_sup_spec() ->
 starter_sup_spec() ->
     {pooler_starter_sup, {pooler_starter_sup, start_link, []}, transient, 5000, supervisor, [pooler_starter_sup]}.
     {pooler_starter_sup, {pooler_starter_sup, start_link, []}, transient, 5000, supervisor, [pooler_starter_sup]}.
 
 
-pool_sup_spec(Pool) ->
-    SupName = pool_sup_name(pooler_config:get_name(Pool)),
-    {SupName, {pooler_pool_sup, start_link, [Pool]}, transient, 5000, supervisor, [pooler_pool_sup]}.
+pool_sup_spec(#{name := Name} = PoolConfig) ->
+    SupName = pool_sup_name(Name),
+    {SupName, {pooler_pool_sup, start_link, [PoolConfig]}, transient, 5000, supervisor, [pooler_pool_sup]}.
 
 
 pool_sup_name(Name) ->
 pool_sup_name(Name) ->
     list_to_atom("pooler_" ++ atom_to_list(Name) ++ "_pool_sup").
     list_to_atom("pooler_" ++ atom_to_list(Name) ++ "_pool_sup").

+ 19 - 19
test/pooler_tests.erl

@@ -120,13 +120,13 @@ pooler_basics_via_config_test_() ->
             fun() ->
             fun() ->
                 error_logger_mon:reset(),
                 error_logger_mon:reset(),
                 Pools = [
                 Pools = [
-                    [
-                        {name, test_pool_1},
-                        {max_count, 3},
-                        {init_count, 2},
-                        {cull_interval, {0, min}},
-                        {start_mfa, {pooled_gs, start_link, [{"type-0"}]}}
-                    ]
+                    #{
+                        name => test_pool_1,
+                        max_count => 3,
+                        init_count => 2,
+                        cull_interval => {0, min},
+                        start_mfa => {pooled_gs, start_link, [{"type-0"}]}
+                    }
                 ],
                 ],
                 application:set_env(pooler, pools, Pools),
                 application:set_env(pooler, pools, Pools),
                 application:start(pooler)
                 application:start(pooler)
@@ -154,12 +154,12 @@ pooler_basics_dynamic_test_() ->
             % setup
             % setup
             fun() ->
             fun() ->
                 error_logger_mon:reset(),
                 error_logger_mon:reset(),
-                Pool = [
-                    {name, test_pool_1},
-                    {max_count, 3},
-                    {init_count, 2},
-                    {start_mfa, {pooled_gs, start_link, [{"type-0"}]}}
-                ],
+                Pool = #{
+                    name => test_pool_1,
+                    max_count => 3,
+                    init_count => 2,
+                    start_mfa => {pooled_gs, start_link, [{"type-0"}]}
+                },
                 application:unset_env(pooler, pools),
                 application:unset_env(pooler, pools),
                 application:start(pooler),
                 application:start(pooler),
                 pooler:new_pool(Pool)
                 pooler:new_pool(Pool)
@@ -187,12 +187,12 @@ pooler_basics_integration_to_other_supervisor_test_() ->
             % setup
             % setup
             fun() ->
             fun() ->
                 error_logger_mon:reset(),
                 error_logger_mon:reset(),
-                Pool = [
-                    {name, test_pool_1},
-                    {max_count, 3},
-                    {init_count, 2},
-                    {start_mfa, {pooled_gs, start_link, [{"type-0"}]}}
-                ],
+                Pool = #{
+                    name => test_pool_1,
+                    max_count => 3,
+                    init_count => 2,
+                    start_mfa => {pooled_gs, start_link, [{"type-0"}]}
+                },
                 application:unset_env(pooler, pools),
                 application:unset_env(pooler, pools),
                 application:start(pooler),
                 application:start(pooler),
                 supervisor:start_link(fake_external_supervisor, Pool)
                 supervisor:start_link(fake_external_supervisor, Pool)

+ 58 - 66
test/prop_pooler.erl

@@ -17,19 +17,19 @@ prop_fixed_start(doc) ->
     "Check that the pool of any fixed size can be started, internal statistics is correct".
     "Check that the pool of any fixed size can be started, internal statistics is correct".
 
 
 prop_fixed_start() ->
 prop_fixed_start() ->
-    Conf0 = [
-        {name, ?FUNCTION_NAME},
-        {start_mfa, {pooled_gs, start_link, [{?FUNCTION_NAME}]}}
-    ],
+    Conf0 =
+        #{
+            name => ?FUNCTION_NAME,
+            start_mfa => {pooled_gs, start_link, [{?FUNCTION_NAME}]}
+        },
     ?FORALL(
     ?FORALL(
         Size,
         Size,
         pos_integer(),
         pos_integer(),
         with_pool(
         with_pool(
-            [
-                {init_count, Size},
-                {max_count, Size}
-                | Conf0
-            ],
+            Conf0#{
+                init_count => Size,
+                max_count => Size
+            },
             fun() ->
             fun() ->
                 %% Pool is not utilized
                 %% Pool is not utilized
                 pool_is_free(?FUNCTION_NAME, Size),
                 pool_is_free(?FUNCTION_NAME, Size),
@@ -42,19 +42,18 @@ prop_fixed_checkout_all(doc) ->
     "Can take all members from fixed-size pool. Following attempts will return error. Stats is correct.".
     "Can take all members from fixed-size pool. Following attempts will return error. Stats is correct.".
 
 
 prop_fixed_checkout_all() ->
 prop_fixed_checkout_all() ->
-    Conf0 = [
-        {name, ?FUNCTION_NAME},
-        {start_mfa, {pooled_gs, start_link, [{?FUNCTION_NAME}]}}
-    ],
+    Conf0 = #{
+        name => ?FUNCTION_NAME,
+        start_mfa => {pooled_gs, start_link, [{?FUNCTION_NAME}]}
+    },
     ?FORALL(
     ?FORALL(
         Size,
         Size,
         pos_integer(),
         pos_integer(),
         with_pool(
         with_pool(
-            [
-                {init_count, Size},
-                {max_count, Size}
-                | Conf0
-            ],
+            Conf0#{
+                init_count => Size,
+                max_count => Size
+            },
             fun() ->
             fun() ->
                 ?assert(
                 ?assert(
                     lists:all(
                     lists:all(
@@ -75,20 +74,19 @@ prop_dynamic_checkout(doc) ->
     "It's possible to take all fixed and then all dynamic members, but no more than max_count; stats is correct".
     "It's possible to take all fixed and then all dynamic members, but no more than max_count; stats is correct".
 
 
 prop_dynamic_checkout() ->
 prop_dynamic_checkout() ->
-    Conf0 = [
-        {name, ?FUNCTION_NAME},
-        {max_age, {1, min}},
-        {start_mfa, {pooled_gs, start_link, [{?FUNCTION_NAME}]}}
-    ],
+    Conf0 = #{
+        name => ?FUNCTION_NAME,
+        max_age => {1, min},
+        start_mfa => {pooled_gs, start_link, [{?FUNCTION_NAME}]}
+    },
     ?FORALL(
     ?FORALL(
         {Size, Extra},
         {Size, Extra},
         {pos_integer(), pos_integer()},
         {pos_integer(), pos_integer()},
         with_pool(
         with_pool(
-            [
-                {init_count, Size},
-                {max_count, Size + Extra}
-                | Conf0
-            ],
+            Conf0#{
+                init_count => Size,
+                max_count => Size + Extra
+            },
             fun() ->
             fun() ->
                 MaxCount = Size + Extra,
                 MaxCount = Size + Extra,
                 ?assert(
                 ?assert(
@@ -119,10 +117,10 @@ prop_fixed_take_return(doc) ->
     "The state of the pool is same before all members are taken and after they are returned".
     "The state of the pool is same before all members are taken and after they are returned".
 
 
 prop_fixed_take_return() ->
 prop_fixed_take_return() ->
-    Conf0 = [
-        {name, ?FUNCTION_NAME},
-        {start_mfa, {pooled_gs, start_link, [{?FUNCTION_NAME}]}}
-    ],
+    Conf0 = #{
+        name => ?FUNCTION_NAME,
+        start_mfa => {pooled_gs, start_link, [{?FUNCTION_NAME}]}
+    },
     Stats = fun() ->
     Stats = fun() ->
         lists:sort([{Pid, State} || {Pid, {_, State, _}} <- pooler:pool_stats(?FUNCTION_NAME)])
         lists:sort([{Pid, State} || {Pid, {_, State, _}} <- pooler:pool_stats(?FUNCTION_NAME)])
     end,
     end,
@@ -130,11 +128,10 @@ prop_fixed_take_return() ->
         Size,
         Size,
         pos_integer(),
         pos_integer(),
         with_pool(
         with_pool(
-            [
-                {init_count, Size},
-                {max_count, Size}
-                | Conf0
-            ],
+            Conf0#{
+                init_count => Size,
+                max_count => Size
+            },
             fun() ->
             fun() ->
                 UtilizationBefore = utilization(?FUNCTION_NAME),
                 UtilizationBefore = utilization(?FUNCTION_NAME),
                 StatsBefore = Stats(),
                 StatsBefore = Stats(),
@@ -156,10 +153,10 @@ prop_fixed_take_return_broken(doc) ->
     "Pool recovers to initial state when all members are returned with 'fail' flag, but workers are replaced".
     "Pool recovers to initial state when all members are returned with 'fail' flag, but workers are replaced".
 
 
 prop_fixed_take_return_broken() ->
 prop_fixed_take_return_broken() ->
-    Conf0 = [
-        {name, ?FUNCTION_NAME},
-        {start_mfa, {pooled_gs, start_link, [{?FUNCTION_NAME}]}}
-    ],
+    Conf0 = #{
+        name => ?FUNCTION_NAME,
+        start_mfa => {pooled_gs, start_link, [{?FUNCTION_NAME}]}
+    },
     Stats = fun() ->
     Stats = fun() ->
         lists:sort([{Pid, State} || {Pid, {_, State, _}} <- pooler:pool_stats(?FUNCTION_NAME)])
         lists:sort([{Pid, State} || {Pid, {_, State, _}} <- pooler:pool_stats(?FUNCTION_NAME)])
     end,
     end,
@@ -167,11 +164,10 @@ prop_fixed_take_return_broken() ->
         Size,
         Size,
         pos_integer(),
         pos_integer(),
         with_pool(
         with_pool(
-            [
-                {init_count, Size},
-                {max_count, Size}
-                | Conf0
-            ],
+            Conf0#{
+                init_count => Size,
+                max_count => Size
+            },
             fun() ->
             fun() ->
                 UtilizationBefore = utilization(?FUNCTION_NAME),
                 UtilizationBefore = utilization(?FUNCTION_NAME),
                 StatsBefore = Stats(),
                 StatsBefore = Stats(),
@@ -206,10 +202,10 @@ prop_fixed_client_died(doc) ->
     "Pool recovers to initial state when client that have taken processes have died with reason 'normal'".
     "Pool recovers to initial state when client that have taken processes have died with reason 'normal'".
 
 
 prop_fixed_client_died() ->
 prop_fixed_client_died() ->
-    Conf0 = [
-        {name, ?FUNCTION_NAME},
-        {start_mfa, {pooled_gs, start_link, [{?FUNCTION_NAME}]}}
-    ],
+    Conf0 = #{
+        name => ?FUNCTION_NAME,
+        start_mfa => {pooled_gs, start_link, [{?FUNCTION_NAME}]}
+    },
     Stats = fun() ->
     Stats = fun() ->
         lists:sort([{Pid, State} || {Pid, {_, State, _}} <- pooler:pool_stats(?FUNCTION_NAME)])
         lists:sort([{Pid, State} || {Pid, {_, State, _}} <- pooler:pool_stats(?FUNCTION_NAME)])
     end,
     end,
@@ -217,11 +213,10 @@ prop_fixed_client_died() ->
         Size,
         Size,
         pos_integer(),
         pos_integer(),
         with_pool(
         with_pool(
-            [
-                {init_count, Size},
-                {max_count, Size}
-                | Conf0
-            ],
+            Conf0#{
+                init_count => Size,
+                max_count => Size
+            },
             fun() ->
             fun() ->
                 Main = self(),
                 Main = self(),
                 UtilizationBefore = utilization(?FUNCTION_NAME),
                 UtilizationBefore = utilization(?FUNCTION_NAME),
@@ -277,9 +272,7 @@ prop_group_take_return(doc) ->
     "Take all workers from all group members - no more workers can be taken. Return them - pools are free.".
     "Take all workers from all group members - no more workers can be taken. Return them - pools are free.".
 
 
 prop_group_take_return() ->
 prop_group_take_return() ->
-    Conf0 = [
-        {start_mfa, {pooled_gs, start_link, [{?FUNCTION_NAME}]}}
-    ],
+    Conf0 = #{start_mfa => {pooled_gs, start_link, [{?FUNCTION_NAME}]}},
     PoolName = fun(I) -> list_to_atom(atom_to_list(?FUNCTION_NAME) ++ integer_to_list(I)) end,
     PoolName = fun(I) -> list_to_atom(atom_to_list(?FUNCTION_NAME) ++ integer_to_list(I)) end,
     ?FORALL(
     ?FORALL(
         {NumWorkers, NumPools},
         {NumWorkers, NumPools},
@@ -287,13 +280,12 @@ prop_group_take_return() ->
         begin
         begin
             with_pools(
             with_pools(
                 [
                 [
-                    [
-                        {name, PoolName(I)},
-                        {init_count, NumWorkers},
-                        {max_count, NumWorkers},
-                        {group, ?FUNCTION_NAME}
-                        | Conf0
-                    ]
+                    Conf0#{
+                        name => PoolName(I),
+                        init_count => NumWorkers,
+                        max_count => NumWorkers,
+                        group => ?FUNCTION_NAME
+                    }
                  || I <- lists:seq(1, NumPools)
                  || I <- lists:seq(1, NumPools)
                 ],
                 ],
                 fun() ->
                 fun() ->
@@ -348,7 +340,7 @@ with_pools(Confs, Fun) ->
         {ok, _} = application:ensure_all_started(pooler),
         {ok, _} = application:ensure_all_started(pooler),
         [{ok, _} = pooler:new_pool(Conf) || Conf <- Confs],
         [{ok, _} = pooler:new_pool(Conf) || Conf <- Confs],
         Res = Fun(),
         Res = Fun(),
-        [ok = pooler:rm_pool(proplists:get_value(name, Conf)) || Conf <- Confs],
+        [ok = pooler:rm_pool(maps:get(name, Conf)) || Conf <- Confs],
         Res
         Res
     after
     after
         application:stop(pooler)
         application:stop(pooler)