You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by rn...@apache.org on 2016/08/22 11:19:35 UTC

[09/50] ibrowse commit: updated refs/heads/upstream to b28542d

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 <ya...@gmail.com>
Authored: Fri Nov 21 04:33:08 2014 +0000
Committer: benjaminplee <ya...@gmail.com>
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,