You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2018/11/22 13:16:02 UTC
[GitHub] iilyak closed pull request #1629: handle db deletion in load
validation funs
iilyak closed pull request #1629: handle db deletion in load validation funs
URL: https://github.com/apache/couchdb/pull/1629
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/src/couch/src/couch_db.erl b/src/couch/src/couch_db.erl
index 60a395fd7a..0d435c2ffe 100644
--- a/src/couch/src/couch_db.erl
+++ b/src/couch/src/couch_db.erl
@@ -872,6 +872,9 @@ load_validation_funs(#db{main_pid=Pid, name = <<"shards/", _/binary>>}=Db) ->
{'DOWN', Ref, _, _, {ok, Funs}} ->
gen_server:cast(Pid, {load_validation_funs, Funs}),
Funs;
+ {'DOWN', Ref, _, _, {database_does_not_exist, _StackTrace}} ->
+ ok = couch_server:close_db_if_idle(Db#db.name),
+ erlang:error(database_does_not_exist);
{'DOWN', Ref, _, _, Reason} ->
couch_log:error("could not load validation funs ~p", [Reason]),
throw(internal_server_error)
diff --git a/src/couch/test/couchdb_db_tests.erl b/src/couch/test/couchdb_db_tests.erl
new file mode 100644
index 0000000000..734bafb9fb
--- /dev/null
+++ b/src/couch/test/couchdb_db_tests.erl
@@ -0,0 +1,91 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couchdb_db_tests).
+
+-include_lib("couch/include/couch_eunit.hrl").
+-include_lib("couch/include/couch_db.hrl").
+-include_lib("mem3/include/mem3.hrl").
+
+setup() ->
+ DbName = ?b2l(?tempdb()),
+ fabric:create_db(DbName),
+ DbName.
+
+
+teardown(DbName) ->
+ (catch fabric:delete_db(DbName)),
+ ok.
+
+
+clustered_db_test_() ->
+ {
+ "Checking clustered db API",
+ {
+ setup,
+ fun() -> test_util:start_couch([ddoc_cache, mem3]) end,
+ fun test_util:stop/1,
+ [
+ {
+ "DB deletion",
+ {
+ foreach,
+ fun setup/0, fun teardown/1,
+ [
+ fun should_close_deleted_db/1,
+ fun should_kill_caller_from_load_validation_funs_for_deleted_db/1
+ ]
+ }
+ }
+ ]
+ }
+ }.
+
+
+should_close_deleted_db(DbName) ->
+ ?_test(begin
+ [#shard{name = ShardName} | _] = mem3:shards(DbName),
+ {ok, Db} = couch_db:open(ShardName, []),
+
+ MonitorRef = couch_db:monitor(Db),
+ fabric:delete_db(DbName),
+ receive
+ {'DOWN', MonitorRef, _Type, _Pid, _Info} ->
+ ok
+ after 2000 ->
+ throw(timeout_error)
+ end,
+ test_util:wait(fun() ->
+ case ets:lookup(couch_dbs, DbName) of
+ [] -> ok;
+ _ -> wait
+ end
+ end),
+ ?assertEqual([], ets:lookup(couch_dbs, DbName))
+ end).
+
+
+should_kill_caller_from_load_validation_funs_for_deleted_db(DbName) ->
+ ?_test(begin
+ [#shard{name = ShardName} | _] = mem3:shards(DbName),
+ {ok, Db} = couch_db:open(ShardName, []),
+
+ MonitorRef = couch_db:monitor(Db),
+ fabric:delete_db(DbName),
+ receive
+ {'DOWN', MonitorRef, _Type, _Pid, _Info} ->
+ ok
+ after 2000 ->
+ throw(timeout_error)
+ end,
+ ?assertError(database_does_not_exist, couch_db:load_validation_funs(Db))
+ end).
diff --git a/src/ddoc_cache/src/ddoc_cache_entry.erl b/src/ddoc_cache/src/ddoc_cache_entry.erl
index 79f67bd671..4cc3d7e522 100644
--- a/src/ddoc_cache/src/ddoc_cache_entry.erl
+++ b/src/ddoc_cache/src/ddoc_cache_entry.erl
@@ -106,11 +106,14 @@ open(Pid, Key) ->
{open_error, {T, R, S}} ->
erlang:raise(T, R, S)
end
- catch exit:_ ->
- % Its possible that this process was evicted just
- % before we tried talking to it. Just fallback
- % to a standard recovery
- recover(Key)
+ catch
+ error:database_does_not_exist ->
+ erlang:error(database_does_not_exist);
+ exit:_ ->
+ % Its possible that this process was evicted just
+ % before we tried talking to it. Just fallback
+ % to a standard recovery
+ recover(Key)
end.
diff --git a/src/ddoc_cache/test/ddoc_cache_basic_test.erl b/src/ddoc_cache/test/ddoc_cache_basic_test.erl
index 7f6dbc9a4f..b576d88bb6 100644
--- a/src/ddoc_cache/test/ddoc_cache_basic_test.erl
+++ b/src/ddoc_cache/test/ddoc_cache_basic_test.erl
@@ -43,15 +43,15 @@ check_basic_test_() ->
setup,
fun start_couch/0,
fun stop_couch/1,
- {with, [
- fun cache_ddoc/1,
- fun cache_ddoc_rev/1,
- fun cache_vdu/1,
- fun cache_custom/1,
- fun cache_ddoc_refresher_unchanged/1,
- fun dont_cache_not_found/1,
- fun deprecated_api_works/1
- ]}
+ ddoc_cache_tutil:with([
+ {"cache_ddoc", fun cache_ddoc/1},
+ {"cache_ddoc_rev", fun cache_ddoc_rev/1},
+ {"cache_vdu", fun cache_vdu/1},
+ {"cache_custom", fun cache_custom/1},
+ {"cache_ddoc_refresher_unchanged", fun cache_ddoc_refresher_unchanged/1},
+ {"dont_cache_not_found", fun dont_cache_not_found/1},
+ {"deprecated_api_works", fun deprecated_api_works/1}
+ ])
}.
@@ -60,10 +60,10 @@ check_no_vdu_test_() ->
setup,
fun() -> ddoc_cache_tutil:start_couch([{write_ddocs, false}]) end,
fun ddoc_cache_tutil:stop_couch/1,
- {with, [
- fun cache_no_vdu_no_ddoc/1,
- fun cache_no_vdu_empty_ddoc/1
- ]}
+ ddoc_cache_tutil:with([
+ {"cache_no_vdu_no_ddoc", fun cache_no_vdu_no_ddoc/1},
+ {"cache_no_vdu_empty_ddoc", fun cache_no_vdu_empty_ddoc/1}
+ ])
}.
diff --git a/src/ddoc_cache/test/ddoc_cache_disabled_test.erl b/src/ddoc_cache/test/ddoc_cache_disabled_test.erl
index bfc08002db..d46bdde325 100644
--- a/src/ddoc_cache/test/ddoc_cache_disabled_test.erl
+++ b/src/ddoc_cache/test/ddoc_cache_disabled_test.erl
@@ -29,11 +29,11 @@ check_disabled_test_() ->
setup,
fun start_couch/0,
fun ddoc_cache_tutil:stop_couch/1,
- {with, [
- fun resp_ok/1,
- fun resp_not_found/1,
- fun check_effectively_disabled/1
- ]}
+ ddoc_cache_tutil:with([
+ {"resp_ok", fun resp_ok/1},
+ {"resp_not_found", fun resp_not_found/1},
+ {"check_effectively_disabled", fun check_effectively_disabled/1}
+ ])
}.
diff --git a/src/ddoc_cache/test/ddoc_cache_entry_test.erl b/src/ddoc_cache/test/ddoc_cache_entry_test.erl
index dd7a039ecb..c992bea8d3 100644
--- a/src/ddoc_cache/test/ddoc_cache_entry_test.erl
+++ b/src/ddoc_cache/test/ddoc_cache_entry_test.erl
@@ -46,15 +46,15 @@ check_entry_test_() ->
setup,
fun start_couch/0,
fun stop_couch/1,
- {with, [
- fun cancel_and_replace_opener/1,
- fun condenses_access_messages/1,
- fun kill_opener_on_terminate/1,
- fun evict_when_not_accessed/1,
- fun open_dead_entry/1,
- fun handles_bad_messages/1,
- fun handles_code_change/1
- ]}
+ ddoc_cache_tutil:with([
+ {"cancel_and_replace_opener", fun cancel_and_replace_opener/1},
+ {"condenses_access_messages", fun condenses_access_messages/1},
+ {"kill_opener_on_terminate", fun kill_opener_on_terminate/1},
+ {"evict_when_not_accessed", fun evict_when_not_accessed/1},
+ {"open_dead_entry", fun open_dead_entry/1},
+ {"handles_bad_messages", fun handles_bad_messages/1},
+ {"handles_code_change", fun handles_code_change/1}
+ ])
}.
diff --git a/src/ddoc_cache/test/ddoc_cache_eviction_test.erl b/src/ddoc_cache/test/ddoc_cache_eviction_test.erl
index 5a02a5c12e..bd61afc372 100644
--- a/src/ddoc_cache/test/ddoc_cache_eviction_test.erl
+++ b/src/ddoc_cache/test/ddoc_cache_eviction_test.erl
@@ -44,11 +44,11 @@ check_eviction_test_() ->
setup,
fun start_couch/0,
fun stop_couch/1,
- {with, [
- fun evict_all/1,
- fun dont_evict_all_unrelated/1,
- fun check_upgrade_clause/1
- ]}
+ ddoc_cache_tutil:with([
+ {"evict_all", fun evict_all/1},
+ {"dont_evict_all_unrelated", fun dont_evict_all_unrelated/1},
+ {"check_upgrade_clause", fun check_upgrade_clause/1}
+ ])
}.
diff --git a/src/ddoc_cache/test/ddoc_cache_lru_test.erl b/src/ddoc_cache/test/ddoc_cache_lru_test.erl
index 60605b9a59..e37f1c0907 100644
--- a/src/ddoc_cache/test/ddoc_cache_lru_test.erl
+++ b/src/ddoc_cache/test/ddoc_cache_lru_test.erl
@@ -61,13 +61,13 @@ check_lru_test_() ->
setup,
fun start_couch/0,
fun stop_couch/1,
- {with, [
- fun check_multi_start/1,
- fun check_multi_open/1,
- fun check_capped_size/1,
- fun check_cache_refill/1,
- fun check_evict_and_exit/1
- ]}
+ ddoc_cache_tutil:with([
+ {"check_multi_start", fun check_multi_start/1},
+ {"check_multi_open", fun check_multi_open/1},
+ {"check_capped_size", fun check_capped_size/1},
+ {"check_cache_refill", fun check_cache_refill/1},
+ {"check_evict_and_exit", fun check_evict_and_exit/1}
+ ])
}.
diff --git a/src/ddoc_cache/test/ddoc_cache_open_error_test.erl b/src/ddoc_cache/test/ddoc_cache_open_error_test.erl
index f3a9b10f42..c7379d26ae 100644
--- a/src/ddoc_cache/test/ddoc_cache_open_error_test.erl
+++ b/src/ddoc_cache/test/ddoc_cache_open_error_test.erl
@@ -36,9 +36,9 @@ check_open_error_test_() ->
setup,
fun start_couch/0,
fun stop_couch/1,
- {with, [
- fun handle_open_error/1
- ]}
+ ddoc_cache_tutil:with([
+ {"handle_open_error", fun handle_open_error/1}
+ ])
}.
diff --git a/src/ddoc_cache/test/ddoc_cache_open_test.erl b/src/ddoc_cache/test/ddoc_cache_open_test.erl
new file mode 100644
index 0000000000..73d644f712
--- /dev/null
+++ b/src/ddoc_cache/test/ddoc_cache_open_test.erl
@@ -0,0 +1,107 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(ddoc_cache_open_test).
+
+-export([
+ dbname/1,
+ ddocid/1,
+ recover/1,
+ insert/2
+]).
+
+-include_lib("couch/include/couch_db.hrl").
+-include_lib("eunit/include/eunit.hrl").
+-include("ddoc_cache_test.hrl").
+
+
+%% behaviour callbacks
+dbname(DbName) ->
+ DbName.
+
+
+ddocid(_) ->
+ no_ddocid.
+
+
+recover({deleted, _DbName}) ->
+ erlang:error(database_does_not_exist);
+recover(DbName) ->
+ ddoc_cache_entry_validation_funs:recover(DbName).
+
+
+insert(_, _) ->
+ ok.
+
+
+start_couch() ->
+ Ctx = ddoc_cache_tutil:start_couch(),
+ meck:new(ddoc_cache_entry_validation_funs, [passthrough]),
+ meck:expect(ddoc_cache_entry_validation_funs, recover,
+ ['_'], meck:passthrough()),
+ Ctx.
+
+
+stop_couch(Ctx) ->
+ meck:unload(),
+ ddoc_cache_tutil:stop_couch(Ctx).
+
+
+check_open_error_test_() ->
+ {
+ setup,
+ fun start_couch/0,
+ fun stop_couch/1,
+ ddoc_cache_tutil:with([
+ {"should_return_database_does_not_exist",
+ fun should_return_database_does_not_exist/1},
+ {"should_not_call_recover_when_database_does_not_exist",
+ fun should_not_call_recover_when_database_does_not_exist/1},
+ {"should_call_recover_when_needed",
+ fun should_call_recover_when_needed/1},
+ {"should_call_recover_when_needed",
+ fun should_not_crash_lru_process/1}
+ ])
+ }.
+
+
+should_return_database_does_not_exist({DbName, _}) ->
+ ?assertError(
+ database_does_not_exist,
+ ddoc_cache_lru:open({?MODULE, {deleted, DbName}})).
+
+
+should_not_call_recover_when_database_does_not_exist({DbName, _}) ->
+ meck:reset(ddoc_cache_entry_validation_funs),
+ ?assertError(
+ database_does_not_exist,
+ ddoc_cache_lru:open({?MODULE, {deleted, DbName}})),
+ ?assertError(
+ timeout,
+ meck:wait(1, ddoc_cache_entry_validation_funs, recover, '_', 100)).
+
+
+should_call_recover_when_needed({DbName, _}) ->
+ meck:reset(ddoc_cache_entry_validation_funs),
+ ddoc_cache_lru:open({?MODULE, DbName}),
+ ?assertEqual(
+ ok,
+ meck:wait(1, ddoc_cache_entry_validation_funs, recover, '_', 500)).
+
+
+should_not_crash_lru_process({DbName, _}) ->
+ LRUPid = whereis(ddoc_cache_lru),
+ ?assert(is_process_alive(LRUPid)),
+ ?assertError(
+ database_does_not_exist,
+ ddoc_cache_lru:open({?MODULE, {deleted, DbName}})),
+ ?assert(is_process_alive(LRUPid)).
diff --git a/src/ddoc_cache/test/ddoc_cache_refresh_test.erl b/src/ddoc_cache/test/ddoc_cache_refresh_test.erl
index 261c158c72..24ae346d45 100644
--- a/src/ddoc_cache/test/ddoc_cache_refresh_test.erl
+++ b/src/ddoc_cache/test/ddoc_cache_refresh_test.erl
@@ -43,14 +43,14 @@ check_refresh_test_() ->
setup,
fun start_couch/0,
fun stop_couch/1,
- {with, [
- fun refresh_ddoc/1,
- fun refresh_ddoc_rev/1,
- fun refresh_vdu/1,
- fun refresh_custom/1,
- fun refresh_multiple/1,
- fun check_upgrade_clause/1
- ]}
+ ddoc_cache_tutil:with([
+ {"refresh_ddoc", fun refresh_ddoc/1},
+ {"refresh_ddoc_rev", fun refresh_ddoc_rev/1},
+ {"refresh_vdu", fun refresh_vdu/1},
+ {"refresh_custom", fun refresh_custom/1},
+ {"refresh_multiple", fun refresh_multiple/1},
+ {"check_upgrade_clause", fun check_upgrade_clause/1}
+ ])
}.
diff --git a/src/ddoc_cache/test/ddoc_cache_remove_test.erl b/src/ddoc_cache/test/ddoc_cache_remove_test.erl
index 8787482e92..e40518529d 100644
--- a/src/ddoc_cache/test/ddoc_cache_remove_test.erl
+++ b/src/ddoc_cache/test/ddoc_cache_remove_test.erl
@@ -52,13 +52,13 @@ check_refresh_test_() ->
setup,
fun start_couch/0,
fun stop_couch/1,
- {with, [
- fun remove_ddoc/1,
- fun remove_ddoc_rev/1,
- fun remove_ddoc_rev_only/1,
- fun remove_custom_not_ok/1,
- fun remove_custom_error/1
- ]}
+ ddoc_cache_tutil:with([
+ {"remove_ddoc", fun remove_ddoc/1},
+ {"remove_ddoc_rev", fun remove_ddoc_rev/1},
+ {"remove_ddoc_rev_only", fun remove_ddoc_rev_only/1},
+ {"remove_custom_not_ok", fun remove_custom_not_ok/1},
+ {"remove_custom_error", fun remove_custom_error/1}
+ ])
}.
diff --git a/src/ddoc_cache/test/ddoc_cache_tutil.erl b/src/ddoc_cache/test/ddoc_cache_tutil.erl
index 6463b38d13..ec5d2db1ed 100644
--- a/src/ddoc_cache/test/ddoc_cache_tutil.erl
+++ b/src/ddoc_cache/test/ddoc_cache_tutil.erl
@@ -94,3 +94,9 @@ purge_modules() ->
undefined ->
ok
end.
+
+%% eunit implementation of {with, Tests} doesn't detect test name correctly
+with(Tests) ->
+ fun(ArgsTuple) ->
+ [{Name, ?_test(Fun(ArgsTuple))} || {Name, Fun} <- Tests]
+ end.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services