diff options
author | ILYA Khlopotov <iilyak@apache.org> | 2018-09-27 03:34:01 -0700 |
---|---|---|
committer | ILYA Khlopotov <iilyak@apache.org> | 2018-11-22 12:19:09 +0000 |
commit | d35b5c9c4a5c2820e25944de2c4fde8bc866c8c1 (patch) | |
tree | 75aabb409bf1e1cb5e20ffb55bd5a25d1fe9d303 | |
parent | 4fc292bde6097a11942dbd408264c8301b2853b1 (diff) | |
download | couchdb-d35b5c9c4a5c2820e25944de2c4fde8bc866c8c1.tar.gz |
Handle db deletion in couch_db:load_validation_funs
Previously there were quite a few problems with load_validation_funs
in the case when clustered database is deleted.
- the calls to load_validation_funs were failing with `internal_server` error [1]
- the deleted database stayed opened because:
- the caller of the load_validation_funs (update_doc) stayed alive
- the main_pid of the deleted database wasn't killed either
- there was an infinite loop in ddoc_cache_entry trying to recover ddoc from deleted database
The solution is:
- do not call `recover` for deleted database
- close `main_pid`
- use `erlang:error` to crash the caller
[1] - The stack trace was:
```
{database_does_not_exist,[
{mem3_shards,load_shards_from_db,"bailey/meta",[
{file,"src/mem3_shards.erl"},{line,394}]},
{mem3_shards,load_shards_from_disk,1,[
{file,"src/mem3_shards.erl"},{line,369}]},
{mem3_shards,for_db,2,[
{file,"src/mem3_shards.erl"},{line,54}]},
{fabric_view_all_docs,go,5,[
{file,"src/fabric_view_all_docs.erl"},{line,24}]},
{ddoc_cache_entry_validation_funs,recover,1,[
{file,"src/ddoc_cache_entry_validation_funs.erl"},{line,33}]},
{ddoc_cache_entry,do_open,1,[
{file,"src/ddoc_cache_entry.erl"},{line,294}]}]}
```
-rw-r--r-- | src/couch/src/couch_db.erl | 3 | ||||
-rw-r--r-- | src/couch/test/couchdb_db_tests.erl | 91 | ||||
-rw-r--r-- | src/ddoc_cache/src/ddoc_cache_entry.erl | 13 | ||||
-rw-r--r-- | src/ddoc_cache/test/ddoc_cache_basic_test.erl | 26 | ||||
-rw-r--r-- | src/ddoc_cache/test/ddoc_cache_disabled_test.erl | 10 | ||||
-rw-r--r-- | src/ddoc_cache/test/ddoc_cache_entry_test.erl | 18 | ||||
-rw-r--r-- | src/ddoc_cache/test/ddoc_cache_eviction_test.erl | 10 | ||||
-rw-r--r-- | src/ddoc_cache/test/ddoc_cache_lru_test.erl | 14 | ||||
-rw-r--r-- | src/ddoc_cache/test/ddoc_cache_open_error_test.erl | 6 | ||||
-rw-r--r-- | src/ddoc_cache/test/ddoc_cache_open_test.erl | 107 | ||||
-rw-r--r-- | src/ddoc_cache/test/ddoc_cache_refresh_test.erl | 16 | ||||
-rw-r--r-- | src/ddoc_cache/test/ddoc_cache_remove_test.erl | 14 | ||||
-rw-r--r-- | src/ddoc_cache/test/ddoc_cache_tutil.erl | 6 |
13 files changed, 272 insertions, 62 deletions
diff --git a/src/couch/src/couch_db.erl b/src/couch/src/couch_db.erl index 60a395fd7..0d435c2ff 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 000000000..734bafb9f --- /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 79f67bd67..4cc3d7e52 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 7f6dbc9a4..b576d88bb 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 bfc08002d..d46bdde32 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 dd7a039ec..c992bea8d 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 5a02a5c12..bd61afc37 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 60605b9a5..e37f1c090 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 f3a9b10f4..c7379d26a 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 000000000..73d644f71 --- /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 261c158c7..24ae346d4 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 8787482e9..e40518529 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 6463b38d1..ec5d2db1e 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. |