You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by va...@apache.org on 2020/06/02 15:52:27 UTC
[couchdb] branch fix-on-commit created (now 0eeab22)
This is an automated email from the ASF dual-hosted git repository.
vatamane pushed a change to branch fix-on-commit
in repository https://gitbox.apache.org/repos/asf/couchdb.git.
at 0eeab22 Prevent eviction of newer handles from fabric2_server cache
This branch includes the following new commits:
new 0eeab22 Prevent eviction of newer handles from fabric2_server cache
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[couchdb] 01/01: Prevent eviction of newer handles from
fabric2_server cache
Posted by va...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
vatamane pushed a commit to branch fix-on-commit
in repository https://gitbox.apache.org/repos/asf/couchdb.git
commit 0eeab22b1f609dcfec1ea9468c4e4303cc70de7b
Author: Nick Vatamaniuc <va...@apache.org>
AuthorDate: Tue Jun 2 11:33:36 2020 -0400
Prevent eviction of newer handles from fabric2_server cache
Ensure fabric2_server operations do not evict newer handles by comparing the
metadata version to the what's already in the table. Also, remove on_commit
handler and instead update the fresh db handle right away to let concurrent
request see the updated handle as soon as possible.
---
src/fabric/include/fabric2.hrl | 1 -
src/fabric/src/fabric2_fdb.erl | 76 ++++++++++++-------------------
src/fabric/src/fabric2_server.erl | 62 +++++++++++++++++++++----
src/fabric/test/fabric2_db_misc_tests.erl | 17 +++++++
4 files changed, 100 insertions(+), 56 deletions(-)
diff --git a/src/fabric/include/fabric2.hrl b/src/fabric/include/fabric2.hrl
index 2e71787..29f3b02 100644
--- a/src/fabric/include/fabric2.hrl
+++ b/src/fabric/include/fabric2.hrl
@@ -70,7 +70,6 @@
-define(PDICT_CHECKED_MD_IS_CURRENT, '$fabric_checked_md_is_current').
-define(PDICT_TX_ID_KEY, '$fabric_tx_id').
-define(PDICT_TX_RES_KEY, '$fabric_tx_result').
--define(PDICT_ON_COMMIT_FUN, '$fabric_on_commit_fun').
-define(PDICT_FOLD_ACC_STATE, '$fabric_fold_acc_state').
% Let's keep these in ascending order
diff --git a/src/fabric/src/fabric2_fdb.erl b/src/fabric/src/fabric2_fdb.erl
index e8f6e0d..5857673 100644
--- a/src/fabric/src/fabric2_fdb.erl
+++ b/src/fabric/src/fabric2_fdb.erl
@@ -171,11 +171,7 @@ do_transaction(Fun, LayerPrefix) when is_function(Fun, 1) ->
true ->
get_previous_transaction_result();
false ->
- try
- execute_transaction(Tx, Fun, LayerPrefix)
- after
- erase({?PDICT_ON_COMMIT_FUN, Tx})
- end
+ execute_transaction(Tx, Fun, LayerPrefix)
end
end)
after
@@ -1311,18 +1307,14 @@ check_db_version(#{} = Db, CheckDbVersion) ->
} = Db,
AlreadyChecked = get(?PDICT_CHECKED_DB_IS_CURRENT),
- if not CheckDbVersion orelse AlreadyChecked == true -> Db; true ->
+ if not CheckDbVersion orelse AlreadyChecked == true -> current; true ->
DbVersionKey = erlfdb_tuple:pack({?DB_VERSION}, DbPrefix),
case erlfdb:wait(erlfdb:get(Tx, DbVersionKey)) of
DbVersion ->
put(?PDICT_CHECKED_DB_IS_CURRENT, true),
- Now = erlang:monotonic_time(millisecond),
- Db1 = Db#{check_current_ts := Now},
- on_commit(Tx, fun() -> fabric2_server:store(Db1) end),
- Db1;
+ current;
_NewDBVersion ->
- fabric2_server:remove(maps:get(name, Db)),
- throw({?MODULE, reopen})
+ stale
end
end.
@@ -1830,10 +1822,6 @@ fold_range_cb({K, V}, #fold_acc{} = Acc) ->
restart_fold(Tx, #fold_acc{} = Acc) ->
erase(?PDICT_CHECKED_MD_IS_CURRENT),
- % Not actually committing anyting so we skip on-commit handlers here. Those
- % are usually to refresh db handles in the cache. If the iterator runs for
- % a while it might be inserting a stale handle in there anyway.
- erase({?PDICT_ON_COMMIT_FUN, Tx}),
ok = erlfdb:reset(Tx),
@@ -1871,17 +1859,35 @@ ensure_current(Db) ->
ensure_current(#{} = Db0, CheckDbVersion) ->
require_transaction(Db0),
- Db2 = case check_metadata_version(Db0) of
- {current, Db1} -> Db1;
- {stale, Db1} -> check_db_version(Db1, CheckDbVersion)
+ Db3 = case check_metadata_version(Db0) of
+ {current, Db1} ->
+ Db1;
+ {stale, Db1} ->
+ % If db version is stale, then `reopen` will be thrown. If db
+ % version is current, make sure to update cache with the
+ % latest metadata so other requests can immediately see the
+ % refreshed db handle.
+ case check_db_version(Db1, CheckDbVersion) of
+ current ->
+ % If db version is current, update cache with the latest
+ % metadata so other requests can immediately see the
+ % refreshed db handle.
+ Now = erlang:monotonic_time(millisecond),
+ Db2 = Db1#{check_current_ts := Now},
+ fabric2_server:maybe_update(Db2),
+ Db2;
+ stale ->
+ fabric2_server:maybe_remove(Db1),
+ throw({?MODULE, reopen})
+ end
end,
- case maps:get(security_fun, Db2) of
+ case maps:get(security_fun, Db3) of
SecurityFun when is_function(SecurityFun, 2) ->
- #{security_doc := SecDoc} = Db2,
- ok = SecurityFun(Db2, SecDoc),
- Db2#{security_fun := undefined};
+ #{security_doc := SecDoc} = Db3,
+ ok = SecurityFun(Db3, SecDoc),
+ Db3#{security_fun := undefined};
undefined ->
- Db2
+ Db3
end.
@@ -1928,7 +1934,6 @@ execute_transaction(Tx, Fun, LayerPrefix) ->
erlfdb:set(Tx, get_transaction_id(Tx, LayerPrefix), <<>>),
put(?PDICT_TX_RES_KEY, Result)
end,
- ok = run_on_commit_fun(Tx),
Result.
@@ -1963,27 +1968,6 @@ get_transaction_id(Tx, LayerPrefix) ->
end.
-on_commit(Tx, Fun) when is_function(Fun, 0) ->
- % Here we rely on Tx objects matching. However they contain a nif resource
- % object. Before Erlang 20.0 those would have been represented as empty
- % binaries and would have compared equal to each other. See
- % http://erlang.org/doc/man/erl_nif.html for more info. We assume we run on
- % Erlang 20+ here and don't worry about that anymore.
- case get({?PDICT_ON_COMMIT_FUN, Tx}) of
- undefined -> put({?PDICT_ON_COMMIT_FUN, Tx}, Fun);
- _ -> error({?MODULE, on_commit_function_already_set})
- end.
-
-
-run_on_commit_fun(Tx) ->
- case get({?PDICT_ON_COMMIT_FUN, Tx}) of
- undefined ->
- ok;
- Fun when is_function(Fun, 0) ->
- Fun(),
- ok
- end.
-
with_span(Operation, ExtraTags, Fun) ->
case ctrace:has_span() of
true ->
diff --git a/src/fabric/src/fabric2_server.erl b/src/fabric/src/fabric2_server.erl
index 957efff..bf2c1d8 100644
--- a/src/fabric/src/fabric2_server.erl
+++ b/src/fabric/src/fabric2_server.erl
@@ -17,9 +17,15 @@
-export([
start_link/0,
+
fetch/2,
+
store/1,
+ maybe_update/1,
+
remove/1,
+ maybe_remove/1,
+
fdb_directory/0,
fdb_cluster/0
]).
@@ -66,27 +72,57 @@ start_link() ->
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
+ {undefined, [{DbName, _UUID, _, #{} = Db}]} -> Db;
+ {<<_/binary>>, [{DbName, UUID, _, #{} = Db}]} -> Db;
+ {<<_/binary>>, [{DbName, _UUID, _, #{} = _Db}]} -> undefined
end.
store(#{name := DbName} = Db0) when is_binary(DbName) ->
- Db1 = Db0#{
- tx := undefined,
- user_ctx := #user_ctx{},
- security_fun := undefined
- },
- true = ets:insert(?MODULE, {DbName, Db1}),
+ #{
+ uuid := UUID,
+ md_version := MDVer
+ } = Db0,
+ Db1 = sanitize(Db0),
+ case ets:insert_new(?MODULE, {DbName, UUID, MDVer, Db1}) of
+ true -> ok;
+ false -> maybe_update(Db1)
+ end,
ok.
+maybe_update(#{name := DbName} = Db0) when is_binary(DbName) ->
+ #{
+ uuid := UUID,
+ md_version := MDVer
+ } = Db0,
+ Db1 = sanitize(Db0),
+ Head = {DbName, UUID, '$1', '_'},
+ Guard = {'=<', '$1', MDVer},
+ Body = {DbName, UUID, MDVer, {const, Db1}},
+ try
+ 1 =:= ets:select_replace(?MODULE, [{Head, [Guard], [{Body}]}])
+ catch
+ error:badarg ->
+ false
+ end.
+
+
remove(DbName) when is_binary(DbName) ->
true = ets:delete(?MODULE, DbName),
ok.
+maybe_remove(#{name := DbName} = Db) when is_binary(DbName) ->
+ #{
+ uuid := UUID,
+ md_version := MDVer
+ } = Db,
+ Head = {DbName, UUID, '$1', '_'},
+ Guard = {'=<', '$1', MDVer},
+ 0 < ets:select_delete(?MODULE, [{Head, [Guard], [true]}]).
+
+
init(_) ->
ets:new(?MODULE, [
public,
@@ -229,3 +265,11 @@ set_option(Db, Option, Val) ->
Msg = "~p : Could not set fdb tx option ~p = ~p",
couch_log:error(Msg, [?MODULE, Option, Val])
end.
+
+
+sanitize(#{} = Db) ->
+ Db#{
+ tx := undefined,
+ user_ctx := #user_ctx{},
+ security_fun := undefined
+ }.
diff --git a/src/fabric/test/fabric2_db_misc_tests.erl b/src/fabric/test/fabric2_db_misc_tests.erl
index 9c95ca5..2fad73f 100644
--- a/src/fabric/test/fabric2_db_misc_tests.erl
+++ b/src/fabric/test/fabric2_db_misc_tests.erl
@@ -50,6 +50,7 @@ misc_test_() ->
?TDEF(ensure_full_commit),
?TDEF(metadata_bump),
?TDEF(db_version_bump),
+ ?TDEF(db_cache_doesnt_evict_newer_handles),
?TDEF(events_listener)
])
}
@@ -375,6 +376,22 @@ db_version_bump({DbName, _, _}) ->
?assertMatch(#{db_version := NewDbVersion}, Db2).
+db_cache_doesnt_evict_newer_handles({DbName, _, _}) ->
+ {ok, Db} = fabric2_db:open(DbName, [{user_ctx, ?ADMIN_USER}]),
+ CachedDb = fabric2_server:fetch(DbName, undefined),
+
+ StaleDb = Db#{md_version := <<0>>},
+
+ ok = fabric2_server:store(StaleDb),
+ ?assertEqual(CachedDb, fabric2_server:fetch(DbName, undefined)),
+
+ ?assert(not fabric2_server:maybe_update(StaleDb)),
+ ?assertEqual(CachedDb, fabric2_server:fetch(DbName, undefined)),
+
+ ?assert(not fabric2_server:maybe_remove(StaleDb)),
+ ?assertEqual(CachedDb, fabric2_server:fetch(DbName, undefined)).
+
+
events_listener({DbName, Db, _}) ->
Opts = [
{dbname, DbName},