Skip to content
Browse files

Fixing the error checkout behaviour

In the current version of the application, returning
{error, Reason, NewState} from the checkout callback wouldn't
reset the counter for this worker, effectively taking it offline.

This fix makes it that every {error, Reason, NewState} call
resets the counter and keeps the watcher alive.
  • Loading branch information...
1 parent 2f56d2b commit dd589b1fe097c9e0996198e0ce89b47b4f733cbd @ferd committed Feb 20, 2012
Showing with 53 additions and 3 deletions.
  1. +2 −0 .gitignore
  2. +3 −1 src/dispcount_watcher.erl
  3. +21 −2 test/dispcount_SUITE.erl
  4. +27 −0 test/ref_dispatch_error.erl
View
2 .gitignore
@@ -2,3 +2,5 @@
*.beam
*.dump
*.COVER.*
+logs/*
+ebin/*
View
4 src/dispcount_watcher.erl
@@ -52,12 +52,14 @@ init({Id,C=#config{watcher_type=named,dispatch_table=Tid},{M,A}}) ->
ets:insert(Tid, {Id, 0}),
init(Id,C,M,A).
-handle_call({get, Pid}, _From, S=#state{callback=M, callback_state=CS, ref=undefined}) ->
+handle_call({get, Pid}, _From, S=#state{callback=M, callback_state=CS, ref=undefined, config=Conf, id=Id}) ->
try M:checkout(Pid, CS) of
{ok, Res, NewCS} ->
MonRef = erlang:monitor(process, Pid),
{reply, {ok, {self(),MonRef}, Res}, S#state{callback_state=NewCS, ref=MonRef}};
{error, Reason, NewCS} ->
+ #config{dispatch_table=DTid} = Conf,
+ set_free(DTid, Id),
{reply, {error, Reason}, S#state{callback_state=NewCS}};
{stop, Reason, NewCS} ->
M:terminate(Reason, NewCS),
View
23 test/dispcount_SUITE.erl
@@ -2,9 +2,9 @@
-include_lib("common_test/include/ct.hrl").
-export([all/0, init_per_suite/1, end_per_suite/1,
init_per_testcase/2, end_per_testcase/2]).
--export([starting/1, stopping/1, overload/1, dead/1]).
+-export([starting/1, stopping/1, overload/1, dead/1, error/1]).
-all() -> [starting, stopping, overload, dead].
+all() -> [starting, stopping, overload, dead, error].
init_per_suite(Config) ->
application:start(dispcount),
@@ -31,6 +31,15 @@ init_per_testcase(dead, Config) ->
),
{ok, Info} = dispcount:dispatcher_info(ref_dead_dispatcher),
[{info, Info} | Config];
+init_per_testcase(error, Config) ->
+ ok = dispcount:start_dispatch(
+ ref_error_dispatcher,
+ {ref_dispatch_error, []},
+ [{restart,permanent},{shutdown,4000},
+ {maxr,10},{maxt,60},{resources,1}]
+ ),
+ {ok, Info} = dispcount:dispatcher_info(ref_error_dispatcher),
+ [{info, Info} | Config];
init_per_testcase(_, Config) ->
Config.
@@ -106,3 +115,13 @@ dead(Config) ->
{error, busy} = dispcount:checkout(Info),
timer:sleep(500),
{ok, _Ref, _Res} = dispcount:checkout(Info).
+
+%% an error being returned resets the counter
+error(Config) ->
+ %% The dispatcher has 1 resource available
+ Info = ?config(info, Config),
+ %% returning a custom error (as done in the dispatch callback module for
+ %% this test) should reset the counter.
+ {error, denied} = dispcount:checkout(Info),
+ %% if we get {error, busy}, this is an error.
+ {error, denied} = dispcount:checkout(Info).
View
27 test/ref_dispatch_error.erl
@@ -0,0 +1,27 @@
+-module(ref_dispatch_error).
+-behaviour(dispcount).
+-export([init/1, checkout/2, checkin/2, handle_info/2, dead/1,
+ terminate/2, code_change/3]).
+
+init([]) ->
+ {ok, make_ref()}.
+
+checkout(_From, Ref) ->
+ {error, denied, Ref}.
+
+checkin(Ref, undefined) ->
+ {ok, Ref};
+checkin(SomeRef, Ref) ->
+ {ignore, Ref}.
+
+dead(undefined) ->
+ {ok, make_ref()}.
+
+handle_info(_Msg, State) ->
+ {ok, State}.
+
+terminate(_Reason, _State) ->
+ ok.
+
+code_change(_OldVsn, State, _Extra) ->
+ {ok, State}.

0 comments on commit dd589b1

Please sign in to comment.
Something went wrong with that request. Please try again.