You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by va...@apache.org on 2021/01/15 21:20:01 UTC
[couchdb] branch 3.x updated: Do not return broken processes to the
query process pool
This is an automated email from the ASF dual-hosted git repository.
vatamane pushed a commit to branch 3.x
in repository https://gitbox.apache.org/repos/asf/couchdb.git
The following commit(s) were added to refs/heads/3.x by this push:
new 9231824 Do not return broken processes to the query process pool
9231824 is described below
commit 92318242b9bc5ec4c273a9be5e8c03080e4d780a
Author: Nick Vatamaniuc <va...@apache.org>
AuthorDate: Fri Jan 15 12:23:50 2021 -0500
Do not return broken processes to the query process pool
Previously, if an error was thrown in a `with_ddoc_proc/2` callback, the
process was still returned to the process pool in the `after` clause. However,
in some cases, for example when processing a _list response, the process might
end up stuck in a bad state state, such that it could not be re-used anymore.
In such a case, a subsequent user of that couch_js process would end up
throwing an error and crashing.
Fixes #2962
---
src/couch/src/couch_query_servers.erl | 11 +++--
src/couch/test/eunit/couchdb_os_proc_pool.erl | 62 ++++++++++++++++++++++++++-
2 files changed, 69 insertions(+), 4 deletions(-)
diff --git a/src/couch/src/couch_query_servers.erl b/src/couch/src/couch_query_servers.erl
index 6649df3..c6d7134 100644
--- a/src/couch/src/couch_query_servers.erl
+++ b/src/couch/src/couch_query_servers.erl
@@ -523,9 +523,14 @@ with_ddoc_proc(#doc{id=DDocId,revs={Start, [DiskRev|_]}}=DDoc, Fun) ->
Rev = couch_doc:rev_to_str({Start, DiskRev}),
DDocKey = {DDocId, Rev},
Proc = get_ddoc_process(DDoc, DDocKey),
- try Fun({Proc, DDocId})
- after
- ok = ret_os_process(Proc)
+ try Fun({Proc, DDocId}) of
+ Resp ->
+ ok = ret_os_process(Proc),
+ Resp
+ catch Tag:Err ->
+ Stack = erlang:get_stacktrace(),
+ catch proc_stop(Proc),
+ erlang:raise(Tag, Err, Stack)
end.
proc_prompt(Proc, Args) ->
diff --git a/src/couch/test/eunit/couchdb_os_proc_pool.erl b/src/couch/test/eunit/couchdb_os_proc_pool.erl
index 69f8051..b552e11 100644
--- a/src/couch/test/eunit/couchdb_os_proc_pool.erl
+++ b/src/couch/test/eunit/couchdb_os_proc_pool.erl
@@ -20,9 +20,11 @@
setup() ->
ok = couch_proc_manager:reload(),
+ meck:new(couch_os_process, [passthrough]),
ok = setup_config().
teardown(_) ->
+ meck:unload(),
ok.
os_proc_pool_test_() ->
@@ -39,7 +41,8 @@ os_proc_pool_test_() ->
should_free_slot_on_proc_unexpected_exit(),
should_reuse_known_proc(),
% should_process_waiting_queue_as_fifo(),
- should_reduce_pool_on_idle_os_procs()
+ should_reduce_pool_on_idle_os_procs(),
+ should_not_return_broken_process_to_the_pool()
]
}
}
@@ -205,6 +208,63 @@ should_reduce_pool_on_idle_os_procs() ->
end).
+should_not_return_broken_process_to_the_pool() ->
+ ?_test(begin
+ config:set("query_server_config",
+ "os_process_soft_limit", "1", false),
+ ok = confirm_config("os_process_soft_limit", "1"),
+
+ config:set("query_server_config",
+ "os_process_limit", "1", false),
+ ok = confirm_config("os_process_limit", "1"),
+
+ DDoc1 = ddoc(<<"_design/ddoc1">>),
+
+ meck:reset(couch_os_process),
+
+ ?assertEqual(0, couch_proc_manager:get_proc_count()),
+ ok = couch_query_servers:with_ddoc_proc(DDoc1, fun(_) -> ok end),
+ ?assertEqual(0, meck:num_calls(couch_os_process, stop, 1)),
+ ?assertEqual(1, couch_proc_manager:get_proc_count()),
+
+ ?assertError(bad, couch_query_servers:with_ddoc_proc(DDoc1, fun(_) ->
+ error(bad)
+ end)),
+ ?assertEqual(1, meck:num_calls(couch_os_process, stop, 1)),
+
+ WaitFun = fun() ->
+ case couch_proc_manager:get_proc_count() of
+ 0 -> ok;
+ N when is_integer(N), N > 0 -> wait
+ end
+ end,
+ case test_util:wait(WaitFun, 5000) of
+ timeout -> error(timeout);
+ _ -> ok
+ end,
+ ?assertEqual(0, couch_proc_manager:get_proc_count()),
+
+ DDoc2 = ddoc(<<"_design/ddoc2">>),
+ ok = couch_query_servers:with_ddoc_proc(DDoc2, fun(_) -> ok end),
+ ?assertEqual(1, meck:num_calls(couch_os_process, stop, 1)),
+ ?assertEqual(1, couch_proc_manager:get_proc_count())
+ end).
+
+
+ddoc(DDocId) ->
+ #doc{
+ id = DDocId,
+ revs = {1, [<<"abc">>]},
+ body = {[
+ {<<"language">>, <<"javascript">>},
+ {<<"views">>, {[
+ {<<"v1">>, {[
+ {<<"map">>, <<"function(doc) {emit(doc.value,1);}">>}
+ ]}}
+ ]}}
+ ]}
+ }.
+
setup_config() ->
config:set("native_query_servers", "enable_erlang_query_server", "true", false),
config:set("query_server_config", "os_process_limit", "3", false),