Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Remove the client ref when it dies

We have observed periods of couchjs processes spiking into the hundreds
and thousands for short periods of time since the new couch_proc_manager
was released. Today I happened to catch one in the act and poked at
couch_proc_manager's ets table. There seemed to be a few more couchjs
processes with clients than I would have expected so I skimmed the code
looking for a place where we didn't clear the client value (which would
prevent it from being reused so that it would eventually just timeout).

I found a case where if the Pid that checked out the process dies
without the OS process dying, we were forgetting to clear the client in
the ets table. This patch refactors the two places we return processes
into a single function call which clears the OS process client.
  • Loading branch information...
commit fa43486fbbe918344d66337b3b0b0d34128d5c39 1 parent a15c94a
@davisp davisp authored Robert Newson committed
Showing with 12 additions and 9 deletions.
  1. +12 −9 apps/couch/src/couch_proc_manager.erl
View
21 apps/couch/src/couch_proc_manager.erl
@@ -75,16 +75,13 @@ handle_call({get_proc, Lang}, {Client, _} = From, State) ->
{reply, {error, Reason}, State}
end;
-handle_call({ret_proc, #proc{client=Ref, pid=Pid} = Proc}, _From, State) ->
+handle_call({ret_proc, #proc{client=Ref} = Proc}, _From, State) ->
erlang:demonitor(Ref, [flush]),
% We need to check if the process is alive here, as the client could be
% handing us a #proc{} with a dead one. We would have already removed the
% #proc{} from our own table, so the alternative is to do a lookup in the
% table before the insert. Don't know which approach is cheaper.
- case is_process_alive(Pid) of true ->
- gen_server:cast(Pid, garbage_collect),
- ets:insert(State#state.tab, Proc#proc{client=nil});
- false -> ok end,
+ return_proc(State#state.tab, Proc),
{reply, true, State};
handle_call(_Call, _From, State) ->
@@ -124,10 +121,8 @@ handle_info({'DOWN', Ref, _, _, _Reason}, State) ->
case ets:match_object(State#state.tab, #proc{client=Ref, _='_'}) of
[] ->
ok;
- [#proc{pid = Pid} = Proc] ->
- case is_process_alive(Pid) of true ->
- ets:insert(State#state.tab, Proc);
- false -> ok end
+ [#proc{} = Proc] ->
+ return_proc(State#state.tab, Proc)
end,
{noreply, State};
@@ -225,6 +220,14 @@ assign_proc(Tab, Client, #proc{client=nil}=Proc0) ->
ets:insert(Tab, Proc),
Proc.
+return_proc(Tab, #proc{pid=Pid} = Proc) ->
+ case is_process_alive(Pid) of true ->
+ gen_server:cast(Pid, garbage_collect),
+ ets:insert(Tab, Proc#proc{client=nil});
+ false ->
+ ets:delete(Tab, Pid)
+ end.
+
get_query_server_config() ->
Limit = couch_config:get("query_server_config", "reduce_limit", "true"),
{[{<<"reduce_limit">>, list_to_atom(Limit)}]}.
Please sign in to comment.
Something went wrong with that request. Please try again.