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/04/22 17:17:05 UTC

[couchdb-erlfdb] branch master updated (523a738 -> 01f79db)

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

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


    from 523a738  Merge pull request #1 from apache/dont-create-new-tx-object-on-retries
     new b0856d7  Implement has_watches(Tx) transaction flag
     new 01f79db  Do not commit read-only transactions

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:
 c_src/main.c                                | 47 +++++++++++++++++++++++++++++
 c_src/resources.h                           |  1 +
 src/erlfdb.erl                              | 13 +++++++-
 src/erlfdb_nif.erl                          |  7 +++++
 test/erlfdb_03_transaction_options_test.erl | 23 ++++++++++++++
 5 files changed, 90 insertions(+), 1 deletion(-)


[couchdb-erlfdb] 01/02: Implement has_watches(Tx) transaction flag

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

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

commit b0856d711fd11efd6a0a6b7831ce1a626f933611
Author: Nick Vatamaniuc <va...@apache.org>
AuthorDate: Wed Apr 22 12:24:42 2020 -0400

    Implement has_watches(Tx) transaction flag
    
    Returns true if transaction has created any watches. If writes are disallowed,
    watches are also cannot be created.
---
 c_src/main.c                                | 47 +++++++++++++++++++++++++++++
 c_src/resources.h                           |  1 +
 src/erlfdb.erl                              |  8 +++++
 src/erlfdb_nif.erl                          |  7 +++++
 test/erlfdb_03_transaction_options_test.erl | 23 ++++++++++++++
 5 files changed, 86 insertions(+)

diff --git a/c_src/main.c b/c_src/main.c
index 4eb35a0..674b92b 100644
--- a/c_src/main.c
+++ b/c_src/main.c
@@ -818,6 +818,7 @@ erlfdb_database_create_transaction(
     t->txid = 0;
     t->read_only = true;
     t->writes_allowed = true;
+    t->has_watches = false;
 
     ret = enif_make_resource(env, t);
     enif_release_resource(t);
@@ -1659,6 +1660,13 @@ erlfdb_transaction_watch(
         return enif_make_badarg(env);
     }
 
+    // In order for the watches to fire the transaction must commit, even if it
+    // is a read-only transaction. So if writes are explicitly disallowed, also
+    // do not allow setting any watches.
+    if(!t->writes_allowed) {
+        return enif_raise_exception(env, ATOM_writes_not_allowed);
+    }
+
     if(!enif_inspect_binary(env, argv[1], &key)) {
         return enif_make_badarg(env);
     }
@@ -1669,6 +1677,7 @@ erlfdb_transaction_watch(
             key.size
         );
 
+    t->has_watches = true;
     return erlfdb_create_future(env, future, ErlFDB_FT_VOID);
 }
 
@@ -1748,6 +1757,7 @@ erlfdb_transaction_reset(
 
     t->txid = 0;
     t->read_only = true;
+    t->has_watches = false;
 
     return ATOM_ok;
 }
@@ -1967,6 +1977,42 @@ erlfdb_transaction_is_read_only(
 
 
 static ERL_NIF_TERM
+erlfdb_transaction_has_watches(
+        ErlNifEnv* env,
+        int argc,
+        const ERL_NIF_TERM argv[]
+    )
+{
+    ErlFDBSt* st = (ErlFDBSt*) enif_priv_data(env);
+    ErlFDBTransaction* t;
+    void* res;
+
+    if(st->lib_state != ErlFDB_CONNECTED) {
+        return enif_make_badarg(env);
+    }
+
+    if(argc != 1) {
+        return enif_make_badarg(env);
+    }
+
+    if(!enif_get_resource(env, argv[0], ErlFDBTransactionRes, &res)) {
+        return enif_make_badarg(env);
+    }
+    t = (ErlFDBTransaction*) res;
+
+    if(!erlfdb_transaction_is_owner(env, t)) {
+        return enif_make_badarg(env);
+    }
+
+    if(t->has_watches) {
+        return ATOM_true;
+    } else {
+        return ATOM_false;
+    }
+}
+
+
+static ERL_NIF_TERM
 erlfdb_transaction_get_writes_allowed(
         ErlNifEnv* env,
         int argc,
@@ -2111,6 +2157,7 @@ static ErlNifFunc funcs[] =
     NIF_FUNC(erlfdb_transaction_get_approximate_size, 1),
     NIF_FUNC(erlfdb_transaction_get_next_tx_id, 1),
     NIF_FUNC(erlfdb_transaction_is_read_only, 1),
+    NIF_FUNC(erlfdb_transaction_has_watches, 1),
     NIF_FUNC(erlfdb_transaction_get_writes_allowed, 1),
 
     NIF_FUNC(erlfdb_get_error, 1),
diff --git a/c_src/resources.h b/c_src/resources.h
index bee88f6..6694a99 100644
--- a/c_src/resources.h
+++ b/c_src/resources.h
@@ -61,6 +61,7 @@ typedef struct _ErlFDBTransaction
     unsigned int txid;
     bool read_only;
     bool writes_allowed;
+    bool has_watches;
 } ErlFDBTransaction;
 
 
diff --git a/src/erlfdb.erl b/src/erlfdb.erl
index 2f6460e..2b62b91 100644
--- a/src/erlfdb.erl
+++ b/src/erlfdb.erl
@@ -109,6 +109,7 @@
     % Transaction status
     get_next_tx_id/1,
     is_read_only/1,
+    has_watches/1,
     get_writes_allowed/1,
 
     % Locality
@@ -594,6 +595,13 @@ is_read_only(?IS_SS = SS) ->
     is_read_only(?GET_TX(SS)).
 
 
+has_watches(?IS_TX = Tx) ->
+    erlfdb_nif:transaction_has_watches(Tx);
+
+has_watches(?IS_SS = SS) ->
+    has_watches(?GET_TX(SS)).
+
+
 get_writes_allowed(?IS_TX = Tx) ->
     erlfdb_nif:transaction_get_writes_allowed(Tx);
 
diff --git a/src/erlfdb_nif.erl b/src/erlfdb_nif.erl
index 755fe7c..2c0a944 100644
--- a/src/erlfdb_nif.erl
+++ b/src/erlfdb_nif.erl
@@ -52,6 +52,7 @@
     transaction_add_conflict_range/4,
     transaction_get_next_tx_id/1,
     transaction_is_read_only/1,
+    transaction_has_watches/1,
     transaction_get_writes_allowed/1,
     transaction_get_approximate_size/1,
 
@@ -430,6 +431,11 @@ transaction_is_read_only({erlfdb_transaction, Tx}) ->
     erlfdb_transaction_is_read_only(Tx).
 
 
+-spec transaction_has_watches(transaction()) -> true | false.
+transaction_has_watches({erlfdb_transaction, Tx}) ->
+    erlfdb_transaction_has_watches(Tx).
+
+
 -spec transaction_get_writes_allowed(transaction()) -> true | false.
 transaction_get_writes_allowed({erlfdb_transaction, Tx}) ->
     erlfdb_transaction_get_writes_allowed(Tx).
@@ -575,6 +581,7 @@ erlfdb_transaction_add_conflict_range(
     ) -> ?NOT_LOADED.
 erlfdb_transaction_get_next_tx_id(_Transaction) -> ?NOT_LOADED.
 erlfdb_transaction_is_read_only(_Transaction) -> ?NOT_LOADED.
+erlfdb_transaction_has_watches(_Transaction) -> ?NOT_LOADED.
 erlfdb_transaction_get_writes_allowed(_Transaction) -> ?NOT_LOADED.
 erlfdb_transaction_get_approximate_size(_Transaction) -> ?NOT_LOADED.
 
diff --git a/test/erlfdb_03_transaction_options_test.erl b/test/erlfdb_03_transaction_options_test.erl
index 7a68074..83c7fe7 100644
--- a/test/erlfdb_03_transaction_options_test.erl
+++ b/test/erlfdb_03_transaction_options_test.erl
@@ -59,5 +59,28 @@ once_writes_happend_cannot_disallow_them_test() ->
     end)).
 
 
+has_watches_test() ->
+    Db1 = erlfdb_util:get_test_db(),
+    {Before, After, AfterReset} = (erlfdb:transactional(Db1, fun(Tx) ->
+        Before = erlfdb:has_watches(Tx),
+        erlfdb:watch(Tx, gen(10)),
+        After = erlfdb:has_watches(Tx),
+        erlfdb:reset(Tx),
+        AfterReset = erlfdb:has_watches(Tx),
+        {Before, After, AfterReset}
+    end)),
+    ?assert(not Before),
+    ?assert(After),
+    ?assert(not AfterReset).
+
+
+cannot_set_watches_if_writes_disallowed_test() ->
+    Db1 = erlfdb_util:get_test_db(),
+    ?assertError(writes_not_allowed, erlfdb:transactional(Db1, fun(Tx) ->
+        erlfdb:set_option(Tx, disallow_writes),
+        erlfdb:watch(Tx, gen(10))
+    end)).
+
+
 gen(Size) ->
     crypto:strong_rand_bytes(Size).


[couchdb-erlfdb] 02/02: Do not commit read-only transactions

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

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

commit 01f79dbb3938ceac4638752953b84a396d78c0ce
Author: Nick Vatamaniuc <va...@apache.org>
AuthorDate: Wed Apr 22 12:25:31 2020 -0400

    Do not commit read-only transactions
    
    This might save a round-trip to the network thread [1]. It also follows the
    recommendation in the C api docs [2].
    
    [1] https://forums.foundationdb.org/t/performance-of-read-only-transactions/1998
    [2] https://apple.github.io/foundationdb/api-c.html#c.fdb_transaction_commit
    
    However, it turns out in order for the watches to fire the read-only
    transaction still has to commit, so avoid this optimization if has_watches(Tx)
    is true.
---
 src/erlfdb.erl | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/erlfdb.erl b/src/erlfdb.erl
index 2b62b91..b50f843 100644
--- a/src/erlfdb.erl
+++ b/src/erlfdb.erl
@@ -653,7 +653,10 @@ clear_erlfdb_error() ->
 do_transaction(?IS_TX = Tx, UserFun) ->
     try
         Ret = UserFun(Tx),
-        wait(commit(Tx)),
+        case is_read_only(Tx) andalso not has_watches(Tx) of
+            true -> ok;
+            false -> wait(commit(Tx))
+        end,
         Ret
     catch error:{erlfdb_error, Code} ->
         put(?ERLFDB_ERROR, Code),