summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNick Vatamaniuc <vatamane@apache.org>2020-03-20 23:14:48 -0400
committerNick Vatamaniuc <vatamane@apache.org>2020-03-20 23:14:48 -0400
commitd4a61b2b4cd09ab0bc28af01731d0f2e41dcc639 (patch)
tree28014eeae4261e7398b03cfe6e3fc1f8627eeee2
parent15509f29c74a8dfca5126f39aaec28423b1f913e (diff)
downloadcouchdb-handle-db-recreation-properly.tar.gz
Fix database re-creationhandle-db-recreation-properly
Previously it was possible for a database to be re-created while a `Db` handle was open and the `Db` handle would continue operating on the new db without any error. To avoid that situation ensure instance UUID is explicitly checked during open and reopen calls. This includes checking it after the metadata is loaded in `fabric2_fdb:open/2` and when fetching the handle from the cache. Also, create a `{uuid, UUID}` option to allow specifying a particular instance UUID when opening a database. If that instance doesn't exist raise a `database_does_not_exist` error.
-rw-r--r--src/fabric/src/fabric2_db.erl3
-rw-r--r--src/fabric/src/fabric2_fdb.erl29
-rw-r--r--src/fabric/src/fabric2_server.erl12
-rw-r--r--src/fabric/test/fabric2_db_crud_tests.erl32
-rw-r--r--src/fabric/test/fabric2_db_misc_tests.erl5
5 files changed, 70 insertions, 11 deletions
diff --git a/src/fabric/src/fabric2_db.erl b/src/fabric/src/fabric2_db.erl
index 4d65f306f..129dea2d7 100644
--- a/src/fabric/src/fabric2_db.erl
+++ b/src/fabric/src/fabric2_db.erl
@@ -178,7 +178,8 @@ create(DbName, Options) ->
open(DbName, Options) ->
- case fabric2_server:fetch(DbName) of
+ UUID = fabric2_util:get_value(uuid, Options),
+ case fabric2_server:fetch(DbName, UUID) of
#{} = Db ->
Db1 = maybe_set_user_ctx(Db, Options),
{ok, require_member_check(Db1)};
diff --git a/src/fabric/src/fabric2_fdb.erl b/src/fabric/src/fabric2_fdb.erl
index 6f9373936..5c72a1726 100644
--- a/src/fabric/src/fabric2_fdb.erl
+++ b/src/fabric/src/fabric2_fdb.erl
@@ -255,6 +255,9 @@ open(#{} = Db0, Options) ->
UserCtx = fabric2_util:get_value(user_ctx, Options, #user_ctx{}),
Options1 = lists:keydelete(user_ctx, 1, Options),
+ UUID = fabric2_util:get_value(uuid, Options1),
+ Options2 = lists:keydelete(uuid, 1, Options1),
+
Db2 = Db1#{
db_prefix => DbPrefix,
db_version => DbVersion,
@@ -271,16 +274,28 @@ open(#{} = Db0, Options) ->
before_doc_update => undefined,
after_doc_read => undefined,
- db_options => Options1
+ db_options => Options2
},
Db3 = load_config(Db2),
+ case {UUID, Db3} of
+ {undefined, _} -> ok;
+ {<<_/binary>>, #{uuid := UUID}} -> ok;
+ {<<_/binary>>, #{uuid := _}} -> erlang:error(database_does_not_exist)
+ end,
+
load_validate_doc_funs(Db3).
-refresh(#{tx := undefined, name := DbName, md_version := OldVer} = Db) ->
- case fabric2_server:fetch(DbName) of
+refresh(#{tx := undefined} = Db) ->
+ #{
+ name := DbName,
+ uuid := UUID,
+ md_version := OldVer
+ } = Db,
+
+ case fabric2_server:fetch(DbName, UUID) of
% Relying on these assumptions about the `md_version` value:
% - It is bumped every time `db_version` is bumped
% - Is a versionstamp, so we can check which one is newer
@@ -304,12 +319,20 @@ reopen(#{} = OldDb) ->
#{
tx := Tx,
name := DbName,
+ uuid := UUID,
db_options := Options,
user_ctx := UserCtx,
security_fun := SecurityFun
} = OldDb,
Options1 = lists:keystore(user_ctx, 1, Options, {user_ctx, UserCtx}),
NewDb = open(init_db(Tx, DbName, Options1), Options1),
+
+ % Check if database was re-created
+ case maps:get(uuid, NewDb) of
+ UUID -> ok;
+ _OtherUUID -> error(database_does_not_exist)
+ end,
+
NewDb#{security_fun := SecurityFun}.
diff --git a/src/fabric/src/fabric2_server.erl b/src/fabric/src/fabric2_server.erl
index b1c38ef55..1de60f798 100644
--- a/src/fabric/src/fabric2_server.erl
+++ b/src/fabric/src/fabric2_server.erl
@@ -17,7 +17,7 @@
-export([
start_link/0,
- fetch/1,
+ fetch/2,
store/1,
remove/1,
fdb_directory/0,
@@ -48,10 +48,12 @@ start_link() ->
gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
-fetch(DbName) when is_binary(DbName) ->
- case ets:lookup(?MODULE, DbName) of
- [{DbName, #{} = Db}] -> Db;
- [] -> undefined
+fetch(DbName, UUID) when is_binary(DbName) ->
+ case {UUID, ets:lookup(?MODULE, DbName)} of
+ {_, []} -> undefined;
+ {undefined, [{DbName, #{} = Db}]} -> Db;
+ {<<_/binary>>, [{DbName, #{uuid := UUID} = Db}]} -> Db;
+ {<<_/binary>>, [{DbName, #{} = _Db}]} -> undefined
end.
diff --git a/src/fabric/test/fabric2_db_crud_tests.erl b/src/fabric/test/fabric2_db_crud_tests.erl
index a82afb54d..f409389d6 100644
--- a/src/fabric/test/fabric2_db_crud_tests.erl
+++ b/src/fabric/test/fabric2_db_crud_tests.erl
@@ -36,6 +36,7 @@ crud_test_() ->
?TDEF_FE(create_db),
?TDEF_FE(open_db),
?TDEF_FE(delete_db),
+ ?TDEF_FE(recreate_db),
?TDEF_FE(list_dbs),
?TDEF_FE(list_dbs_user_fun),
?TDEF_FE(list_dbs_user_fun_partial),
@@ -107,6 +108,37 @@ delete_db(_) ->
?assertError(database_does_not_exist, fabric2_db:open(DbName, [])).
+recreate_db(_) ->
+ DbName = ?tempdb(),
+ ?assertMatch({ok, _}, fabric2_db:create(DbName, [])),
+
+ {ok, Db1} = fabric2_db:open(DbName, []),
+
+ ?assertEqual(ok, fabric2_db:delete(DbName, [])),
+ ?assertMatch({ok, _}, fabric2_db:create(DbName, [])),
+
+ ?assertError(database_does_not_exist, fabric2_db:get_db_info(Db1)),
+
+ ?assertEqual(ok, fabric2_db:delete(DbName, [])),
+ ?assertMatch({ok, _}, fabric2_db:create(DbName, [])),
+
+ {ok, Db2} = fabric2_db:open(DbName, []),
+
+ CurOpts = [{uuid, fabric2_db:get_uuid(Db2)}],
+ ?assertMatch({ok, #{}}, fabric2_db:open(DbName, CurOpts)),
+
+ % Remove from cache to force it to open through fabric2_fdb:open
+ fabric2_server:remove(DbName),
+ ?assertMatch({ok, #{}}, fabric2_db:open(DbName, CurOpts)),
+
+ BadOpts = [{uuid, fabric2_util:uuid()}],
+ ?assertError(database_does_not_exist, fabric2_db:open(DbName, BadOpts)),
+
+ % Remove from cache to force it to open through fabric2_fdb:open
+ fabric2_server:remove(DbName),
+ ?assertError(database_does_not_exist, fabric2_db:open(DbName, BadOpts)).
+
+
list_dbs(_) ->
DbName = ?tempdb(),
AllDbs1 = fabric2_db:list_dbs(),
diff --git a/src/fabric/test/fabric2_db_misc_tests.erl b/src/fabric/test/fabric2_db_misc_tests.erl
index 42a63e2f9..fe0ae9faa 100644
--- a/src/fabric/test/fabric2_db_misc_tests.erl
+++ b/src/fabric/test/fabric2_db_misc_tests.erl
@@ -302,7 +302,8 @@ metadata_bump({DbName, _, _}) ->
{ok, _} = fabric2_db:get_db_info(Db),
% Check that db handle in the cache got the new metadata version
- ?assertMatch(#{md_version := NewMDVersion}, fabric2_server:fetch(DbName)).
+ CachedDb = fabric2_server:fetch(DbName, undefined),
+ ?assertMatch(#{md_version := NewMDVersion}, CachedDb).
db_version_bump({DbName, _, _}) ->
@@ -326,7 +327,7 @@ db_version_bump({DbName, _, _}) ->
{ok, _} = fabric2_db:get_db_info(Db),
% After previous operation, the cache should have been cleared
- ?assertMatch(undefined, fabric2_server:fetch(DbName)),
+ ?assertMatch(undefined, fabric2_server:fetch(DbName, undefined)),
% Call open again and check that we have the latest db version
{ok, Db2} = fabric2_db:open(DbName, [{user_ctx, ?ADMIN_USER}]),