You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by ja...@apache.org on 2019/04/09 06:35:59 UTC

[couchdb-ibrowse] branch master updated: Get a new lb pid if existing pid is not alive

This is an automated email from the ASF dual-hosted git repository.

jaydoane pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/couchdb-ibrowse.git


The following commit(s) were added to refs/heads/master by this push:
     new 4a10e05  Get a new lb pid if existing pid is not alive
     new 6c15e3e  Merge pull request #2 from cloudant/handle-dead-ibrowse_lb-pids-backport
4a10e05 is described below

commit 4a10e0594c8ef762d9e1b7054e3ab53334488fa1
Author: Jay Doane <ja...@apache.org>
AuthorDate: Mon Apr 8 23:32:52 2019 -0700

    Get a new lb pid if existing pid is not alive
    
    It's possible for the connection process associated with a pid in the
    ibrowse_lb ets table to die, yet remain in the table, in which case
    subsequent requests to the corresponding {Host, Port} will result in an
    error like the following:
    
    (node1@127.0.0.1)9> ibrowse:send_req("http://localhost:15984", [], get).
                    ** exception exit: {noproc,
                           {gen_server,call,
                               [<0.2451.0>,
                                {spawn_connection,
                                    {url,"http://localhost:15984","localhost",15984,
                                        undefined,undefined,"/",http,hostname},
                                    10,10,
                                    {[],false},
                                    []}]}}
         in function  gen_server:call/2 (gen_server.erl, line 215)
         in call from ibrowse:try_routing_request/14 (src/ibrowse.erl, line 377)
    
    This checks whether the pid about to be returned from the table is alive,
    and if not, the entry is deleted, and a new pid is obtained.
---
 src/ibrowse.erl      | 21 +++++++++++++++------
 src/ibrowse_test.erl | 23 +++++++++++++++++++++--
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/src/ibrowse.erl b/src/ibrowse.erl
index 80a4282..ca2b5e4 100644
--- a/src/ibrowse.erl
+++ b/src/ibrowse.erl
@@ -311,12 +311,7 @@ send_req(Url, Headers, Method, Body, Options, Timeout) ->
         #url{host = Host,
              port = Port,
              protocol = Protocol} = Parsed_url ->
-            Lb_pid = case ets:lookup(ibrowse_lb, {Host, Port}) of
-                         [] ->
-                             get_lb_pid(Parsed_url);
-                         [#lb_pid{pid = Lb_pid_1}] ->
-                             Lb_pid_1
-                     end,
+            Lb_pid = lb_pid(Host, Port, Parsed_url),
             Max_sessions = get_max_sessions(Host, Port, Options),
             Max_pipeline_size = get_max_pipeline_size(Host, Port, Options),
             Options_1 = merge_options(Host, Port, Options),
@@ -335,6 +330,20 @@ send_req(Url, Headers, Method, Body, Options, Timeout) ->
             {error, {url_parsing_failed, Err}}
     end.
 
+lb_pid(Host, Port, Url) ->
+    case ets:lookup(ibrowse_lb, {Host, Port}) of
+        [] ->
+            get_lb_pid(Url);
+        [#lb_pid{pid = Pid}] ->
+            case is_process_alive(Pid) of
+                true ->
+                    Pid;
+                false ->
+                    ets:delete(ibrowse_lb, {Host, Port}),
+                    get_lb_pid(Url)
+            end
+    end.
+
 try_routing_request(Lb_pid, Parsed_url,
                     Max_sessions, 
                     Max_pipeline_size,
diff --git a/src/ibrowse_test.erl b/src/ibrowse_test.erl
index d97f76c..70db5db 100644
--- a/src/ibrowse_test.erl
+++ b/src/ibrowse_test.erl
@@ -27,7 +27,8 @@
          test_head_transfer_encoding/0,
          test_head_transfer_encoding/1,
          test_head_response_with_body/0,
-         test_head_response_with_body/1
+         test_head_response_with_body/1,
+         test_dead_lb_pid/0
 	]).
 
 test_stream_once(Url, Method, Options) ->
@@ -233,7 +234,8 @@ dump_errors(Key, Iod) ->
                     {local_test_fun, test_20122010, []},
                     {local_test_fun, test_pipeline_head_timeout, []},
                     {local_test_fun, test_head_transfer_encoding, []},
-                    {local_test_fun, test_head_response_with_body, []}
+                    {local_test_fun, test_head_response_with_body, []},
+                    {local_test_fun, test_dead_lb_pid, []}
 		   ]).
 
 unit_tests() ->
@@ -616,6 +618,23 @@ do_test_20122010_1(Expected_resp, Req_id, Acc) ->
             exit({timeout, test_failed})
     end.
 
+%%------------------------------------------------------------------------------
+%% Test that when an lb process dies, its entry is removed from the ibrowse_lb
+%% table by the next requestor and replaced with a new process
+%%------------------------------------------------------------------------------
+test_dead_lb_pid() ->
+    {Host, Port} = {"localhost", 8181},
+    Url = "http://" ++ Host ++ ":" ++ integer_to_list(Port),
+    {ok, "200", _, _} = ibrowse:send_req(Url, [], get),
+    [{lb_pid, {Host, Port}, Pid}] = ets:lookup(ibrowse_lb, {Host, Port}),
+    true = exit(Pid, kill),
+    false = is_process_alive(Pid),
+    {ok, "200", _, _} = ibrowse:send_req(Url, [], get),
+    [{lb_pid, {Host, Port}, NewPid}] = ets:lookup(ibrowse_lb, {Host, Port}),
+    true = NewPid /= Pid,
+    true = is_process_alive(NewPid),
+    success.
+
 do_trace(Fmt, Args) ->
     do_trace(get(my_trace_flag), Fmt, Args).