diff --git a/rebar.config b/rebar.config index 2b2d953..053219a 100644 --- a/rebar.config +++ b/rebar.config @@ -46,6 +46,6 @@ {profiles, [{ test, [{deps, [{fakeredis_cluster, - {git, "git://github.com/bjosv/fakeredis_cluster.git", {ref, "77024cf"}}}] + {git, "git://github.com/bjosv/fakeredis_cluster.git", {ref, "73e3e2f"}}}] }] }]}. diff --git a/src/eredis_cluster.erl b/src/eredis_cluster.erl index 5420458..5051077 100644 --- a/src/eredis_cluster.erl +++ b/src/eredis_cluster.erl @@ -372,6 +372,7 @@ query_noreply(Command, PoolKey) -> Transaction = fun(Worker) -> qw_noreply(Worker, Command) end, {Pool, _Version} = eredis_cluster_monitor:get_pool_by_slot(Slot), eredis_cluster_pool:transaction(Pool, Transaction), + %% TODO: Retry if pool is full? ok. query(_, _, ?REDIS_CLUSTER_REQUEST_TTL) -> @@ -407,6 +408,10 @@ handle_transaction_result(Result, Version) -> {error, tcp_closed} -> retry; + %% Pool is full + {error, full} -> + retry; + %% Other TCP issues %% See reasons: https://erlang.org/doc/man/inet.html#type-posix {error, Reason} when is_atom(Reason) -> diff --git a/src/eredis_cluster_pool_worker.erl b/src/eredis_cluster_pool_worker.erl index 4c42d26..77fad43 100644 --- a/src/eredis_cluster_pool_worker.erl +++ b/src/eredis_cluster_pool_worker.erl @@ -36,6 +36,8 @@ init(Args) -> {ok, #state{conn=Conn}}. +query(full, _Commands) -> + {error, full}; query(Worker, Commands) -> gen_server:call(Worker, {'query', Commands}). diff --git a/test/eredis_cluster_fakeredis_SUITE.erl b/test/eredis_cluster_fakeredis_SUITE.erl index 210dbe9..db04e60 100644 --- a/test/eredis_cluster_fakeredis_SUITE.erl +++ b/test/eredis_cluster_fakeredis_SUITE.erl @@ -10,6 +10,7 @@ %% Test cases -export([ t_connect/1 , t_connect_tls/1 + , t_pool_full/1 , t_redis_crash/1 ]). @@ -72,6 +73,53 @@ t_connect_tls(Config) when is_list(Config) -> ?assertMatch(ok, eredis_cluster:stop()). +t_pool_full(Config) when is_list(Config) -> + %% Pool size 1 with concurrent queries causes poolboy to return + %% 'full' and the query is retried until the first workers are + %% done. + + %% Poolboy's checkout timeout is 5000, so we make sure a worker is + %% blocked for 6000ms. The time limit for gen_server calls used + %% for performing a query is 5000 but it's OK to wait longer for + %% the pool. Thus, we need two workers each taking 3000ms to + %% block a 3rd worker for 6000ms. + + fakeredis_cluster:start_link([20001, 20002, 20003], + [{delay, 3000}]), + + application:set_env(eredis_cluster, pool_size, 1), + application:set_env(eredis_cluster, pool_max_overflow, 0), + + ct:print("Perform inital connect..."), + ?assertMatch(ok, eredis_cluster:connect([{"127.0.0.1", 20001}])), + + ct:print("Test concurrent access..."), + MainPid = self(), + Fun = fun() -> + ct:print("Test access from process ~p...", [self()]), + ?assertEqual({ok, undefined}, + eredis_cluster:q(["GET", "dummy"])), + ct:print("Access from process ~p is done.", [self()]), + MainPid ! done + end, + + StartTime = erlang:system_time(millisecond), + spawn(Fun), + spawn(Fun), + spawn(Fun), + receive done -> ok after 10000 -> error(timeout) end, + receive done -> ok after 10000 -> error(timeout) end, + receive done -> ok after 10000 -> error(timeout) end, + EndTime = erlang:system_time(millisecond), + + %% The whole sequence must have taken at least 9 + %% seconds. Otherwise, the delay mechanism in fakeredis doesn't + %% work. + ct:print("It took ~p milliseconds.", [EndTime - StartTime]), + ?assert(EndTime - StartTime >= 9000), + + ?assertMatch(ok, eredis_cluster:stop()). + t_redis_crash(Config) when is_list(Config) -> ok. %% fakeredis_cluster:start_link([20001, 20002, 20003]),