diff options
author | Nick Vatamaniuc <vatamane@apache.org> | 2020-03-20 23:14:48 -0400 |
---|---|---|
committer | Nick Vatamaniuc <nickva@users.noreply.github.com> | 2020-03-21 17:52:39 -0400 |
commit | e520294c7ee3f55c3e8cc7d528ff37a5a93c800f (patch) | |
tree | 28014eeae4261e7398b03cfe6e3fc1f8627eeee2 | |
parent | 15509f29c74a8dfca5126f39aaec28423b1f913e (diff) | |
download | couchdb-e520294c7ee3f55c3e8cc7d528ff37a5a93c800f.tar.gz |
Fix database re-creation
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.erl | 3 | ||||
-rw-r--r-- | src/fabric/src/fabric2_fdb.erl | 29 | ||||
-rw-r--r-- | src/fabric/src/fabric2_server.erl | 12 | ||||
-rw-r--r-- | src/fabric/test/fabric2_db_crud_tests.erl | 32 | ||||
-rw-r--r-- | src/fabric/test/fabric2_db_misc_tests.erl | 5 |
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}]), |