fix performance issue

The sockets wasn't correctly passed to the pool and prematuraly closed. This change fix it. While I'm here improve the socket pool handling to mot use more sockets than the max wanted and remove premature optimisations. The default max connections number have been increased to ease the change. fix #104.
benoitc · Nov 10, 2014 · ec5b90c · ec5b90c
1 parent 3a4446b
commit ec5b90c
Show file tree

Hide file tree

Showing 7 changed files with 165 additions and 73 deletions.
diff --git a/src/hackney.app.src b/src/hackney.app.src
@@ -16,7 +16,7 @@
         {included_applications, []},
         {mod, { hackney_app, []}},
         {env, [{timeout, 150000},
-               {max_connections, 25},
+               {max_connections, 50},
                {restart, permanent},
                {shutdown, 10000},
                {maxr, 10},

diff --git a/src/hackney_client/hackney_manager.erl b/src/hackney_client/hackney_manager.erl
@@ -48,8 +48,6 @@ new_request(InitialState) ->
     ok = gen_server:call(?MODULE, {new_request, self(), Ref}),
     Ref.
 
-
-
 cancel_request(#client{request_ref=Ref}) ->
     cancel_request(Ref);
 cancel_request(Ref) when is_reference(Ref) ->
@@ -79,33 +77,36 @@ cancel_request(Ref) when is_reference(Ref) ->
             end
     end.
 
-close_request(#client{request_ref=Ref}) ->
-    close_request(Ref);
+close_request(#client{}=Client) ->
+    #client{transport=Transport,
+            socket=Socket,
+            state=Status,
+            request_ref=Ref} = Client,
+
+    %% remove the request
+    erase(Ref),
 
+    %% stop to monitor the request
+    Reply = gen_server:call(?MODULE, {cancel_request, Ref}),
+
+    case Status of
+        done -> ok;
+        _ when Socket /= nil ->
+            catch Transport:controlling_process(Socket, self()),
+            catch Transport:close(Socket),
+            ok;
+        _ -> ok
+    end,
+    Reply;
 close_request(Ref) ->
     case get_state(Ref) of
         req_not_found ->
             req_not_found;
         Client ->
-            #client{transport=Transport, socket=Socket, state=Status} = Client,
-
-            %% remove the request
-            erase(Ref),
-
-            %% stop to monitor the request
-            Reply = gen_server:call(?MODULE, {cancel_request, Ref}),
-
-            case Status of
-                done -> ok;
-                _ when Socket /= nil ->
-                    catch Transport:controlling_process(Socket, self()),
-                    catch Transport:close(Socket),
-                    ok;
-                _ -> ok
-            end,
-            Reply
+            close_request(Client)
     end.
 
+
 controlling_process(Ref, Pid) ->
     case get(Ref) of
         undefined ->
@@ -211,11 +212,7 @@ store_state(Ref, NState) ->
 
 take_control(Ref, NState) ->
     %% maybe delete the state from ets
-    case ets:lookup(?MODULE, Ref) of
-        [] -> ok;
-        [{Ref, _Req}] ->
-            ets:delete(?MODULE, Ref)
-    end,
+    ets:delete(?MODULE, Ref),
     %% add the state to the current context
     put(Ref, NState),
     ok.

diff --git a/src/hackney_client/hackney_response.erl b/src/hackney_client/hackney_response.erl
@@ -139,6 +139,7 @@ stream_body(Client=#client{parser=Parser}) ->
 
 stream_body(Data, #client{parser=Parser}=Client) ->
     stream_body1(hackney_http:execute(Parser, Data), Client).
+
 stream_body1({more, Parser, Buffer}, Client) ->
     stream_body_recv(Buffer, Client#client{parser=Parser});
 stream_body1({ok, Data, Parser}, Client) ->
@@ -272,7 +273,6 @@ skip_body(Client) ->
 		{error, Reason} -> {error, Reason}
 	end.
 
-
 end_stream_body(Rest, Client0) ->
     Client = Client0#client{response_state=done,
                             body_state=done,
@@ -282,13 +282,15 @@ end_stream_body(Rest, Client0) ->
                             async=false},
 
     Pool = hackney_connect:is_pool(Client),
+
     case maybe_close(Client) of
         true ->
             close(Client);
         false when Pool /= false ->
             #client{socket=Socket,
                     socket_ref=Ref,
                     pool_handler=Handler}=Client,
+
             Handler:checkin(Ref, Socket),
             Client#client{state=closed, socket=nil, socket_ref=nil,
                           buffer = <<>>};
@@ -315,6 +317,8 @@ read_body(_MaxLength, Client, Acc) ->
 	{ok, Acc, Client}.
 
 
+maybe_close(#client{socket=nil}) ->
+    true;
 maybe_close(#client{version={Min,Maj}, connection=Connection}) ->
     case Connection of
         <<"close">> -> true;

diff --git a/src/hackney_connect/hackney_connect.erl b/src/hackney_connect/hackney_connect.erl
@@ -190,6 +190,7 @@ socket_from_pool(Host, Port, Transport, #client{request_ref=ReqRef0}=Client) ->
 
 do_connect(Host, Port, Transport, #client{options=Opts,
                                           request_ref=Ref0}=Client) ->
+
     ConnectOpts0 = proplists:get_value(connect_options, Opts, []),
     ConnectTimeout = proplists:get_value(connect_timeout, Opts, 8000),
 
@@ -208,7 +209,7 @@ do_connect(Host, Port, Transport, #client{options=Opts,
                     ConnectOpts1 ++ [{verify, verify_none},
                              {reuse_sessions, true}];
                 _ ->
-                    ConnectOpts1
+                       ConnectOpts1
             end;
         {hackney_ssl_transport, SslOpts} ->
             ConnectOpts1 ++ SslOpts;