Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

handle db deletion in load validation funs #1629

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/couch/src/couch_db.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
91 changes: 91 additions & 0 deletions src/couch/test/couchdb_db_tests.erl
Original file line number Diff line number Diff line change
@@ -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).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test name implies that the shard should be closed by the deletion, so I wonder if we should also add an explicit assertion to that effect? In the absence of an is_open function, we could add something like ?assertEqual([], ets:lookup(couch_dbs, DbName)) at the 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).
13 changes: 8 additions & 5 deletions src/ddoc_cache/src/ddoc_cache_entry.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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.


Expand Down
26 changes: 13 additions & 13 deletions src/ddoc_cache/test/ddoc_cache_basic_test.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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}
])
}.


Expand All @@ -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}
])
}.


Expand Down
10 changes: 5 additions & 5 deletions src/ddoc_cache/test/ddoc_cache_disabled_test.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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}
])
}.


Expand Down
18 changes: 9 additions & 9 deletions src/ddoc_cache/test/ddoc_cache_entry_test.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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}
])
}.


Expand Down
10 changes: 5 additions & 5 deletions src/ddoc_cache/test/ddoc_cache_eviction_test.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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}
])
}.


Expand Down
14 changes: 7 additions & 7 deletions src/ddoc_cache/test/ddoc_cache_lru_test.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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}
])
}.


Expand Down
6 changes: 3 additions & 3 deletions src/ddoc_cache/test/ddoc_cache_open_error_test.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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}
])
}.


Expand Down
107 changes: 107 additions & 0 deletions src/ddoc_cache/test/ddoc_cache_open_test.erl
Original file line number Diff line number Diff line change
@@ -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)).
16 changes: 8 additions & 8 deletions src/ddoc_cache/test/ddoc_cache_refresh_test.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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}
])
}.


Expand Down
Loading