From 9a4635b1d3d626c15a11842c9785846ffe02627b Mon Sep 17 00:00:00 2001 From: Omri Rotem Date: Wed, 5 Dec 2018 18:56:43 +0200 Subject: [PATCH 1/4] add waiting to status, as it is an essential part of the status --- src/poolboy.erl | 10 ++++++++-- test/poolboy_eqc.erl | 3 ++- test/poolboy_tests.erl | 28 ++++++++++++++-------------- 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/src/poolboy.erl b/src/poolboy.erl index db4973b..b038a47 100644 --- a/src/poolboy.erl +++ b/src/poolboy.erl @@ -203,9 +203,15 @@ handle_call({checkout, CRef, Block}, {FromPid, _} = From, State) -> handle_call(status, _From, State) -> #state{workers = Workers, monitors = Monitors, - overflow = Overflow} = State, + overflow = Overflow, + waiting = Waiting} = State, StateName = state_name(State), - {reply, {StateName, length(Workers), Overflow, ets:info(Monitors, size)}, State}; + Status = {StateName, + length(Workers), + Overflow, + ets:info(Monitors, size), + queue:len(Waiting)}, + {reply, Status, State}; handle_call(get_avail_workers, _From, State) -> Workers = State#state.workers, {reply, Workers, State}; diff --git a/test/poolboy_eqc.erl b/test/poolboy_eqc.erl index 59031d4..046a013 100644 --- a/test/poolboy_eqc.erl +++ b/test/poolboy_eqc.erl @@ -129,9 +129,10 @@ invariant(S = #state{pid=Pid},_) when Pid /= undefined -> Workers = max(0, S#state.size - length(S#state.checked_out)), OverFlow = max(0, length(S#state.checked_out) - S#state.size), Monitors = length(S#state.checked_out), + Waiting = queue:len(S#state.waiting), RealStatus = gen_server:call(Pid, status), - case RealStatus == {State, Workers, OverFlow, Monitors} of + case RealStatus == {State, Workers, OverFlow, Monitors, Waiting} of true -> true; _ -> diff --git a/test/poolboy_tests.erl b/test/poolboy_tests.erl index b0f3b39..967ab6c 100644 --- a/test/poolboy_tests.erl +++ b/test/poolboy_tests.erl @@ -95,7 +95,7 @@ checkin_worker(Pid, Worker) -> transaction_timeout_without_exit() -> {ok, Pid} = new_pool(1, 0), - ?assertEqual({ready,1,0,0}, pool_call(Pid, status)), + ?assertEqual({ready,1,0,0,0}, pool_call(Pid, status)), WorkerList = pool_call(Pid, get_all_workers), ?assertMatch([_], WorkerList), spawn(poolboy, transaction, [Pid, @@ -105,12 +105,12 @@ transaction_timeout_without_exit() -> 0]), timer:sleep(100), ?assertEqual(WorkerList, pool_call(Pid, get_all_workers)), - ?assertEqual({ready,1,0,0}, pool_call(Pid, status)). + ?assertEqual({ready,1,0,0,0}, pool_call(Pid, status)). transaction_timeout() -> {ok, Pid} = new_pool(1, 0), - ?assertEqual({ready,1,0,0}, pool_call(Pid, status)), + ?assertEqual({ready,1,0,0,0}, pool_call(Pid, status)), WorkerList = pool_call(Pid, get_all_workers), ?assertMatch([_], WorkerList), ?assertExit( @@ -121,7 +121,7 @@ transaction_timeout() -> end, 0)), ?assertEqual(WorkerList, pool_call(Pid, get_all_workers)), - ?assertEqual({ready,1,0,0}, pool_call(Pid, status)). + ?assertEqual({ready,1,0,0,0}, pool_call(Pid, status)). pool_startup() -> @@ -419,31 +419,31 @@ checkin_after_exception_in_transaction() -> pool_returns_status() -> {ok, Pool} = new_pool(2, 0), - ?assertEqual({ready, 2, 0, 0}, poolboy:status(Pool)), + ?assertEqual({ready, 2, 0, 0, 0}, poolboy:status(Pool)), poolboy:checkout(Pool), - ?assertEqual({ready, 1, 0, 1}, poolboy:status(Pool)), + ?assertEqual({ready, 1, 0, 1, 0}, poolboy:status(Pool)), poolboy:checkout(Pool), - ?assertEqual({full, 0, 0, 2}, poolboy:status(Pool)), + ?assertEqual({full, 0, 0, 2, 0}, poolboy:status(Pool)), ok = pool_call(Pool, stop), {ok, Pool2} = new_pool(1, 1), - ?assertEqual({ready, 1, 0, 0}, poolboy:status(Pool2)), + ?assertEqual({ready, 1, 0, 0, 0}, poolboy:status(Pool2)), poolboy:checkout(Pool2), - ?assertEqual({overflow, 0, 0, 1}, poolboy:status(Pool2)), + ?assertEqual({overflow, 0, 0, 1, 0}, poolboy:status(Pool2)), poolboy:checkout(Pool2), - ?assertEqual({full, 0, 1, 2}, poolboy:status(Pool2)), + ?assertEqual({full, 0, 1, 2, 0}, poolboy:status(Pool2)), ok = pool_call(Pool2, stop), {ok, Pool3} = new_pool(0, 2), - ?assertEqual({overflow, 0, 0, 0}, poolboy:status(Pool3)), + ?assertEqual({overflow, 0, 0, 0, 0}, poolboy:status(Pool3)), poolboy:checkout(Pool3), - ?assertEqual({overflow, 0, 1, 1}, poolboy:status(Pool3)), + ?assertEqual({overflow, 0, 1, 1, 0}, poolboy:status(Pool3)), poolboy:checkout(Pool3), - ?assertEqual({full, 0, 2, 2}, poolboy:status(Pool3)), + ?assertEqual({full, 0, 2, 2, 0}, poolboy:status(Pool3)), ok = pool_call(Pool3, stop), {ok, Pool4} = new_pool(0, 0), - ?assertEqual({full, 0, 0, 0}, poolboy:status(Pool4)), + ?assertEqual({full, 0, 0, 0, 0}, poolboy:status(Pool4)), ok = pool_call(Pool4, stop). demonitors_previously_waiting_processes() -> From 38a46fa0d5238ac7a8d3c723106ee1bc629af28c Mon Sep 17 00:00:00 2001 From: Omri Rotem Date: Wed, 5 Dec 2018 19:12:31 +0200 Subject: [PATCH 2/4] adds estatus as extended status for poolboy including waiting procs length --- src/poolboy.erl | 12 ++++++++- test/poolboy_tests.erl | 59 ++++++++++++++++++++++++++++++++---------- 2 files changed, 56 insertions(+), 15 deletions(-) diff --git a/src/poolboy.erl b/src/poolboy.erl index b038a47..d7a634e 100644 --- a/src/poolboy.erl +++ b/src/poolboy.erl @@ -5,7 +5,7 @@ -export([checkout/1, checkout/2, checkout/3, checkin/2, transaction/2, transaction/3, child_spec/2, child_spec/3, start/1, start/2, - start_link/1, start_link/2, stop/1, status/1]). + start_link/1, start_link/2, stop/1, status/1, estatus/1]). -export([init/1, handle_call/3, handle_cast/2, handle_info/2, terminate/2, code_change/3]). -export_type([pool/0]). @@ -122,6 +122,10 @@ stop(Pool) -> status(Pool) -> gen_server:call(Pool, status). +-spec estatus(Pool :: pool()) -> {atom(), integer(), integer(), integer()}. +estatus(Pool) -> + gen_server:call(Pool, estatus). + init({PoolArgs, WorkerArgs}) -> process_flag(trap_exit, true), Waiting = queue:new(), @@ -201,6 +205,12 @@ handle_call({checkout, CRef, Block}, {FromPid, _} = From, State) -> end; handle_call(status, _From, State) -> + #state{workers = Workers, + monitors = Monitors, + overflow = Overflow} = State, + StateName = state_name(State), + {reply, {StateName, length(Workers), Overflow, ets:info(Monitors, size)}, State}; +handle_call(estatus, _From, State) -> #state{workers = Workers, monitors = Monitors, overflow = Overflow, diff --git a/test/poolboy_tests.erl b/test/poolboy_tests.erl index 967ab6c..127359c 100644 --- a/test/poolboy_tests.erl +++ b/test/poolboy_tests.erl @@ -50,6 +50,8 @@ pool_test_() -> }, {<<"Pool returns status">>, fun pool_returns_status/0 + },{<<"Pool returns extended status">>, + fun pool_returns_estatus/0 }, {<<"Pool demonitors previously waiting processes">>, fun demonitors_previously_waiting_processes/0 @@ -95,7 +97,7 @@ checkin_worker(Pid, Worker) -> transaction_timeout_without_exit() -> {ok, Pid} = new_pool(1, 0), - ?assertEqual({ready,1,0,0,0}, pool_call(Pid, status)), + ?assertEqual({ready,1,0,0}, pool_call(Pid, status)), WorkerList = pool_call(Pid, get_all_workers), ?assertMatch([_], WorkerList), spawn(poolboy, transaction, [Pid, @@ -105,12 +107,12 @@ transaction_timeout_without_exit() -> 0]), timer:sleep(100), ?assertEqual(WorkerList, pool_call(Pid, get_all_workers)), - ?assertEqual({ready,1,0,0,0}, pool_call(Pid, status)). + ?assertEqual({ready,1,0,0}, pool_call(Pid, status)). transaction_timeout() -> {ok, Pid} = new_pool(1, 0), - ?assertEqual({ready,1,0,0,0}, pool_call(Pid, status)), + ?assertEqual({ready,1,0,0}, pool_call(Pid, status)), WorkerList = pool_call(Pid, get_all_workers), ?assertMatch([_], WorkerList), ?assertExit( @@ -121,7 +123,7 @@ transaction_timeout() -> end, 0)), ?assertEqual(WorkerList, pool_call(Pid, get_all_workers)), - ?assertEqual({ready,1,0,0,0}, pool_call(Pid, status)). + ?assertEqual({ready,1,0,0}, pool_call(Pid, status)). pool_startup() -> @@ -419,31 +421,60 @@ checkin_after_exception_in_transaction() -> pool_returns_status() -> {ok, Pool} = new_pool(2, 0), - ?assertEqual({ready, 2, 0, 0, 0}, poolboy:status(Pool)), + ?assertEqual({ready, 2, 0, 0}, poolboy:status(Pool)), poolboy:checkout(Pool), - ?assertEqual({ready, 1, 0, 1, 0}, poolboy:status(Pool)), + ?assertEqual({ready, 1, 0, 1}, poolboy:status(Pool)), poolboy:checkout(Pool), - ?assertEqual({full, 0, 0, 2, 0}, poolboy:status(Pool)), + ?assertEqual({full, 0, 0, 2}, poolboy:status(Pool)), ok = pool_call(Pool, stop), {ok, Pool2} = new_pool(1, 1), - ?assertEqual({ready, 1, 0, 0, 0}, poolboy:status(Pool2)), + ?assertEqual({ready, 1, 0, 0}, poolboy:status(Pool2)), poolboy:checkout(Pool2), - ?assertEqual({overflow, 0, 0, 1, 0}, poolboy:status(Pool2)), + ?assertEqual({overflow, 0, 0, 1}, poolboy:status(Pool2)), poolboy:checkout(Pool2), - ?assertEqual({full, 0, 1, 2, 0}, poolboy:status(Pool2)), + ?assertEqual({full, 0, 1, 2}, poolboy:status(Pool2)), ok = pool_call(Pool2, stop), {ok, Pool3} = new_pool(0, 2), - ?assertEqual({overflow, 0, 0, 0, 0}, poolboy:status(Pool3)), + ?assertEqual({overflow, 0, 0, 0}, poolboy:status(Pool3)), poolboy:checkout(Pool3), - ?assertEqual({overflow, 0, 1, 1, 0}, poolboy:status(Pool3)), + ?assertEqual({overflow, 0, 1, 1}, poolboy:status(Pool3)), poolboy:checkout(Pool3), - ?assertEqual({full, 0, 2, 2, 0}, poolboy:status(Pool3)), + ?assertEqual({full, 0, 2, 2}, poolboy:status(Pool3)), ok = pool_call(Pool3, stop), {ok, Pool4} = new_pool(0, 0), - ?assertEqual({full, 0, 0, 0, 0}, poolboy:status(Pool4)), + ?assertEqual({full, 0, 0, 0}, poolboy:status(Pool4)), + ok = pool_call(Pool4, stop). + +pool_returns_estatus() -> + {ok, Pool} = new_pool(2, 0), + ?assertEqual({ready, 2, 0, 0, 0}, poolboy:estatus(Pool)), + poolboy:checkout(Pool), + ?assertEqual({ready, 1, 0, 1, 0}, poolboy:estatus(Pool)), + poolboy:checkout(Pool), + ?assertEqual({full, 0, 0, 2, 0}, poolboy:estatus(Pool)), + ok = pool_call(Pool, stop), + + {ok, Pool2} = new_pool(1, 1), + ?assertEqual({ready, 1, 0, 0, 0}, poolboy:estatus(Pool2)), + poolboy:checkout(Pool2), + ?assertEqual({overflow, 0, 0, 1, 0}, poolboy:estatus(Pool2)), + poolboy:checkout(Pool2), + ?assertEqual({full, 0, 1, 2, 0}, poolboy:estatus(Pool2)), + ok = pool_call(Pool2, stop), + + {ok, Pool3} = new_pool(0, 2), + ?assertEqual({overflow, 0, 0, 0, 0}, poolboy:estatus(Pool3)), + poolboy:checkout(Pool3), + ?assertEqual({overflow, 0, 1, 1, 0}, poolboy:estatus(Pool3)), + poolboy:checkout(Pool3), + ?assertEqual({full, 0, 2, 2, 0}, poolboy:estatus(Pool3)), + ok = pool_call(Pool3, stop), + + {ok, Pool4} = new_pool(0, 0), + ?assertEqual({full, 0, 0, 0, 0}, poolboy:estatus(Pool4)), ok = pool_call(Pool4, stop). demonitors_previously_waiting_processes() -> From eb219f8b6d831eb5d7e4975c7a2b07535a89b3ca Mon Sep 17 00:00:00 2001 From: Omri Rotem Date: Sun, 9 Dec 2018 13:36:31 +0200 Subject: [PATCH 3/4] test that the waiting status is reported correctly --- test/poolboy_tests.erl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/poolboy_tests.erl b/test/poolboy_tests.erl index 127359c..cf5d3c5 100644 --- a/test/poolboy_tests.erl +++ b/test/poolboy_tests.erl @@ -455,6 +455,9 @@ pool_returns_estatus() -> ?assertEqual({ready, 1, 0, 1, 0}, poolboy:estatus(Pool)), poolboy:checkout(Pool), ?assertEqual({full, 0, 0, 2, 0}, poolboy:estatus(Pool)), + spawn(fun() -> poolboy:checkout(Pool) end), + timer:sleep(50), + ?assertEqual({full, 0, 0, 2, 1}, poolboy:estatus(Pool)), ok = pool_call(Pool, stop), {ok, Pool2} = new_pool(1, 1), From 31ec825de10b9efc5242965bb528709d31215e21 Mon Sep 17 00:00:00 2001 From: Omri Rotem Date: Sun, 9 Dec 2018 13:42:22 +0200 Subject: [PATCH 4/4] revert tests for eqc --- test/poolboy_eqc.erl | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/poolboy_eqc.erl b/test/poolboy_eqc.erl index 046a013..59031d4 100644 --- a/test/poolboy_eqc.erl +++ b/test/poolboy_eqc.erl @@ -129,10 +129,9 @@ invariant(S = #state{pid=Pid},_) when Pid /= undefined -> Workers = max(0, S#state.size - length(S#state.checked_out)), OverFlow = max(0, length(S#state.checked_out) - S#state.size), Monitors = length(S#state.checked_out), - Waiting = queue:len(S#state.waiting), RealStatus = gen_server:call(Pid, status), - case RealStatus == {State, Workers, OverFlow, Monitors, Waiting} of + case RealStatus == {State, Workers, OverFlow, Monitors} of true -> true; _ ->