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 17:33:15 UTC

[couchdb] branch prototype/fdb-layer updated (6501709 -> 9f8b4fd)

This is an automated email from the ASF dual-hosted git repository.

vatamane pushed a change to branch prototype/fdb-layer
in repository https://gitbox.apache.org/repos/asf/couchdb.git.


    from 6501709  Guard couch_jobs:accept_loop timing out
     new 03210b0  Prevent eviction of newer handles from fabric_server cache
     new 9f8b4fd  Remove on_commit handler from fabric2_fdb

The 2 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.


Summary of changes:
 src/fabric/include/fabric2.hrl            |  1 -
 src/fabric/src/fabric2_fdb.erl            | 72 +++++++++++--------------------
 src/fabric/src/fabric2_server.erl         | 62 ++++++++++++++++++++++----
 src/fabric/test/fabric2_db_misc_tests.erl | 17 ++++++++
 4 files changed, 96 insertions(+), 56 deletions(-)


[couchdb] 02/02: Remove on_commit handler from fabric2_fdb

Posted by va...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

vatamane pushed a commit to branch prototype/fdb-layer
in repository https://gitbox.apache.org/repos/asf/couchdb.git

commit 9f8b4fdb26414b556edeaaa1d56bf13d418e0c87
Author: Nick Vatamaniuc <va...@apache.org>
AuthorDate: Tue Jun 2 12:07:44 2020 -0400

    Remove on_commit handler from fabric2_fdb
    
    Update db handles right away as soon we db verison is checked. This ensures
    concurrent requests will get access to the current handle as soon as possible
    and may avoid doing extra version checks and re-opens.
---
 src/fabric/include/fabric2.hrl |  1 -
 src/fabric/src/fabric2_fdb.erl | 72 +++++++++++++++---------------------------
 2 files changed, 26 insertions(+), 47 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..7e736b0 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,31 @@ 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} ->
+            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 +1930,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 +1964,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 ->


[couchdb] 01/02: Prevent eviction of newer handles from fabric_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 prototype/fdb-layer
in repository https://gitbox.apache.org/repos/asf/couchdb.git

commit 03210b029cbb60f906f13b6a826d8897c42102b6
Author: Nick Vatamaniuc <va...@apache.org>
AuthorDate: Tue Jun 2 12:03:58 2020 -0400

    Prevent eviction of newer handles from fabric_server cache
    
    Check metadata versions to ensure newer handles are not clobbered. The same
    thing is done for removal, `maybe_remove/1` removes handle only if there isn't
    a newer handle already there.
---
 src/fabric/src/fabric2_server.erl         | 62 ++++++++++++++++++++++++++-----
 src/fabric/test/fabric2_db_misc_tests.erl | 17 +++++++++
 2 files changed, 70 insertions(+), 9 deletions(-)

diff --git a/src/fabric/src/fabric2_server.erl b/src/fabric/src/fabric2_server.erl
index 957efff..b557da8 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},
+    1 =:= 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},