Fixed bug with connection req completion Algorithm change had bug where ets:select return value was incorrectly assumed to be the object key and not the entire object causing the following delete attempt based on a matchspec and the key to fail.
This meant that ets was not updated to reflect the completed requests on each connection and causing exaustion of pipelines event though connections were idle. Functional test added which demonstrated the problem. Project: http://git-wip-us.apache.org/repos/asf/couchdb-ibrowse/repo Commit: http://git-wip-us.apache.org/repos/asf/couchdb-ibrowse/commit/247dd563 Tree: http://git-wip-us.apache.org/repos/asf/couchdb-ibrowse/tree/247dd563 Diff: http://git-wip-us.apache.org/repos/asf/couchdb-ibrowse/diff/247dd563 Branch: refs/heads/upstream Commit: 247dd563fbf5b11a54bfefe96b6cd9bc2efa5a1b Parents: 86ccc45 Author: benjaminplee <[email protected]> Authored: Fri Nov 21 04:33:08 2014 +0000 Committer: benjaminplee <[email protected]> Committed: Fri Nov 21 04:35:52 2014 +0000 ---------------------------------------------------------------------- src/ibrowse_lb.erl | 2 +- test/ibrowse_functional_tests.erl | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/couchdb-ibrowse/blob/247dd563/src/ibrowse_lb.erl ---------------------------------------------------------------------- diff --git a/src/ibrowse_lb.erl b/src/ibrowse_lb.erl index 656adda..656c3f9 100644 --- a/src/ibrowse_lb.erl +++ b/src/ibrowse_lb.erl @@ -259,7 +259,7 @@ report_request_complete(_Tid, 0) -> report_request_complete(Tid, RemainingRetries) -> %% Don't cascade errors since Tid is really managed by other process catch case ets:select(Tid, ?KEY_MATCHSPEC_BY_PID(self())) of - [MatchKey] -> + [{MatchKey, _}] -> case ets:select_delete(Tid, ?KEY_MATCHSPEC_FOR_DELETE(MatchKey)) of 1 -> ets:insert(Tid, {decremented(MatchKey), undefined}), http://git-wip-us.apache.org/repos/asf/couchdb-ibrowse/blob/247dd563/test/ibrowse_functional_tests.erl ---------------------------------------------------------------------- diff --git a/test/ibrowse_functional_tests.erl b/test/ibrowse_functional_tests.erl index 8b75e3d..e55c5b2 100644 --- a/test/ibrowse_functional_tests.erl +++ b/test/ibrowse_functional_tests.erl @@ -38,6 +38,7 @@ running_server_fixture_test_() -> ?TIMEDTEST("Simple request can be honored", simple_request), ?TIMEDTEST("Slow server causes timeout", slow_server_timeout), ?TIMEDTEST("Pipeline depth goes down with responses", pipeline_depth), + ?TIMEDTEST("Pipelines refill", pipeline_refill), ?TIMEDTEST("Timeout closes pipe", closing_pipes), ?TIMEDTEST("Requests are balanced over connections", balanced_connections), ?TIMEDTEST("Pipeline too small signals retries", small_pipeline), @@ -68,6 +69,26 @@ pipeline_depth() -> ?assertEqual(MaxSessions, length(Counts)), ?assertEqual(lists:duplicate(MaxSessions, EmptyPipelineDepth), Counts). +pipeline_refill() -> + MaxSessions = 2, + MaxPipeline = 2, + RequestsToFill = MaxSessions * MaxPipeline, + + %% Send off enough requests to fill sessions and pipelines in rappid succession + Fun = fun() -> ibrowse:send_req(?BASE_URL, [], get, [], [{max_sessions, MaxSessions}, {max_pipeline_size, MaxPipeline}], ?SHORT_TIMEOUT_MS) end, + times(RequestsToFill, fun() -> spawn_link(Fun) end), + timer:sleep(?PAUSE_FOR_CONNECTIONS_MS), + + % Verify that connections properly reported their completed responses and can still accept more + ?assertMatch({ok, "200", _, _}, ibrowse:send_req(?BASE_URL, [], get, [], [{max_sessions, MaxSessions}, {max_pipeline_size, MaxPipeline}], ?SHORT_TIMEOUT_MS)), + + % and do it again to make sure we really are clear + times(RequestsToFill, fun() -> spawn_link(Fun) end), + timer:sleep(?PAUSE_FOR_CONNECTIONS_MS), + + % Verify that connections properly reported their completed responses and can still accept more + ?assertMatch({ok, "200", _, _}, ibrowse:send_req(?BASE_URL, [], get, [], [{max_sessions, MaxSessions}, {max_pipeline_size, MaxPipeline}], ?SHORT_TIMEOUT_MS)). + closing_pipes() -> MaxSessions = 2, MaxPipeline = 2,
