Folks,

Recently I've pushed transactions support phase 1 for Ignite 3, see [1].
Feel free to give feedback.
Current implementation attempts to automatically enlist a table into
transaction if it's started using [2] or [3] by using thread local context,
similar to Ignite 2 approach, to reduce the amount of boilerplate code.
But it turns out such an approach still has unacceptable drawbacks from a
user experience point of view.

Consider the example [4]:

public void testMixedPutGet() throws TransactionException {
        accounts.recordView().upsert(makeValue(1, BALANCE_1));

        igniteTransactions.runInTransaction(tx -> {
            var txAcc = accounts.recordView().withTransaction(tx);

            txAcc.getAsync(makeKey(1)).thenCompose(r ->
txAcc.upsertAsync(makeValue(1, r.doubleValue("balance") + DELTA))).join();
        });

        assertEquals(BALANCE_1 + DELTA,
accounts.recordView().get(makeKey(1)).doubleValue("balance"));
}

Here we *have to* to manually enlist a table if it's used in async chain
call, because the caller thread will be different and the chained operation
will be executed in separate tx.
This works similarly in Ignite 2 and is very confusing.

To avoid this, I propose to add an explicit Transaction argument to each
table API method. Null value means to start the implicit transaction
(autocommit mode). For example:

/**
     * Asynchronously inserts a record into the table if it doesn't exist
or replaces the existed one.
     *
     * @param rec A record to insert into the table. The record cannot be
{@code null}.
     * @param tx The transaction or {@code null} to auto commit.
     * @return Future representing pending completion of the operation.
     */
    @NotNull CompletableFuture<Void> upsertAsync(@NotNull R rec, @Nullable
Transaction tx);

The example [4] turns to

public void testMixedPutGet() throws TransactionException {
        RecordView<Tuple> view = accounts.recordView();

        view.upsert(makeValue(1, BALANCE_1));

        igniteTransactions.runInTransaction(tx -> {
            view.getAsync(makeKey(1), tx).thenCompose(r ->
view.upsertAsync(makeValue(1, r.doubleValue("balance") + DELTA),
tx)).join();
        });

        assertEquals(BALANCE_1 + DELTA,
view.get(makeKey(1)).doubleValue("balance"));
}

Share your thoughts.

[1] https://issues.apache.org/jira/browse/IGNITE-15085
[2] 
org.apache.ignite.tx.IgniteTransactions#runInTransaction(java.util.function.Consumer<org.apache.ignite.tx.Transaction>)
[3] 
org.apache.ignite.tx.IgniteTransactions#runInTransaction(java.util.function.Function<org.apache.ignite.tx.Transaction,T>)
[4] org.apache.ignite.internal.table.TxAbstractTest#testMixedPutGet

ср, 14 июл. 2021 г. в 14:12, Alexei Scherbakov <alexey.scherbak...@gmail.com
>:

> Andrey,
>
> 1) "As a user, I'd expect runInTransaction(closure) will create Tx for me,
> commit Tx after a successful closure call, and rollback Tx in case of
> error."
> - I'm ok with this behavior, and will alter javadoc.
>
> 2) "Transaction tx = beginTx()" - there is no such method "beginTx" in the
> proposed API, and I'm not intending to add it.
> For the synchronous case I suggest to use "runInTransaction", which
> eliminates the need in AutoClosable.
>
>
>
> ср, 14 июл. 2021 г. в 13:21, Ivan Daschinsky <ivanda...@gmail.com>:
>
>> > yes, it is stated in the javadoc in the PR.
>> Ah, I see.
>>
>> ср, 14 июл. 2021 г. в 12:16, Alexei Scherbakov <
>> alexey.scherbak...@gmail.com
>> >:
>>
>> > Ivan,
>> >
>> > And what if I have already committed transaction? Is it safe rollback
>> > already committed transaction? Rollback will silently return and do
>> > nothing? - yes, it is stated in the javadoc in the PR.
>> >
>> > Andrey,
>> >
>> > Then using "runInTransaction", lack of commit will cause a transaction
>> to
>> > rollback automatically.
>> >
>> > There is no need for a "close" method, it just adds confusion.
>> >
>> >
>> > ср, 14 июл. 2021 г. в 11:37, Andrey Mashenkov <
>> andrey.mashen...@gmail.com
>> > >:
>> >
>> > > Agree with Ivan.
>> > >
>> > > Method runInTransaction() should try to finish the transaction if the
>> > user
>> > > forgot to commit one.
>> > > I guess it might be a common mistake among new users.
>> > >
>> > > Also, I suggest to extent all table projections for better UX.
>> > > Let's allow
>> > >     table.kvView().withTx(tx)
>> > > to user may cache kvVew instance and do
>> > >     kvView.withTx(tx)
>> > > rather than
>> > >     table.withTx(tx).kvVew()
>> > >
>> > >
>> > >
>> > > On Wed, Jul 14, 2021 at 10:13 AM Ivan Daschinsky <ivanda...@gmail.com
>> >
>> > > wrote:
>> > >
>> > > > Alexey, and is there any analogue to close() of transaction? When
>> you
>> > > start
>> > > > transaction, you should somehow to close it, if you don't catch
>> > exception
>> > > > or forget to commit.
>> > > >
>> > > > I suggest to add method closeAsync() to Transaction, so user can
>> call
>> > it
>> > > in
>> > > > handle or whenComplete, i.e.
>> > > >
>> > > > So code will looks like
>> > > >
>> > > > CacheApi cache = CacheApi.getCache("testCache");
>> > > >
>> > > > Transactions
>> > > >     .beginTransaction()
>> > > >     .thenCompose(tx -> {
>> > > >         CacheApi txCache = cache.withTx(tx);
>> > > >         CompletableFuture<Void> result = txCache.getAsync("key")
>> > > >             .thenCompose(val -> {
>> > > >                 if (val == "test") {
>> > > >                     return txCache.putAsync("key", "test1");
>> > > >                 }
>> > > >                 else
>> > > >                     return CompletableFuture.completedFuture(null);
>> > > >             })
>> > > >             .thenCompose(v -> tx.commitAsync())
>> > > >             .handle((v, ex) -> null);
>> > > >         return result.thenCompose(v -> tx.closeAsync());
>> > > >     });
>> > > >
>> > > > I also suggests to add method something like this
>> > > >
>> > > > static CompletableFuture<Void> inTxAsync(Function<Transaction,
>> > > > CompletableFuture<Void>> action) {
>> > > >     return Transactions
>> > > >         .beginTransaction()
>> > > >         .thenCompose(tx -> {
>> > > >             CompletableFuture<Object> result = action.apply(tx)
>> > > >                 .handle((v, ex) -> null);
>> > > >             return result.thenCompose(v -> tx.closeAsync());
>> > > >         });
>> > > > }
>> > > >
>> > > > Async api is not very readable, but this method can help user write
>> > code,
>> > > > this is rewritten first example:
>> > > >
>> > > > Transactions.inTxAsync(tx -> {
>> > > >     CacheApi txCache = cache.withTx(tx);
>> > > >     return txCache.getAsync("key")
>> > > >         .thenCompose(val -> {
>> > > >             if (val == "test") {
>> > > >                 return txCache.putAsync("key", "test1");
>> > > >             }
>> > > >             else
>> > > >                 return CompletableFuture.completedFuture(null);
>> > > >         })
>> > > >         .thenCompose(v -> tx.commitAsync());
>> > > > });
>> > > >
>> > > > ср, 14 июл. 2021 г. в 10:03, Alexei Scherbakov <
>> > > > alexey.scherbak...@gmail.com
>> > > > >:
>> > > >
>> > > > > Andrey,
>> > > > >
>> > > > > I suggest you look at the PR [1], if you haven't.
>> > > > >
>> > > > > A transaction [2]
>> > > > > Transactions facade [3]
>> > > > > Examples [4]
>> > > > >
>> > > > > [1] https://github.com/apache/ignite-3/pull/214/files
>> > > > > [2]
>> > > > >
>> > > > >
>> > > >
>> > >
>> >
>> https://github.com/apache/ignite-3/blob/d2122ce8c15de020e121f53509bd5a097aac9cf2/modules/api/src/main/java/org/apache/ignite/tx/Transaction.java
>> > > > > [3]
>> > > > >
>> > > > >
>> > > >
>> > >
>> >
>> https://github.com/apache/ignite-3/blob/d2122ce8c15de020e121f53509bd5a097aac9cf2/modules/api/src/main/java/org/apache/ignite/tx/IgniteTransactions.java
>> > > > > [4]
>> > > > >
>> > > > >
>> > > >
>> > >
>> >
>> https://github.com/apache/ignite-3/blob/d2122ce8c15de020e121f53509bd5a097aac9cf2/modules/table/src/test/java/org/apache/ignite/internal/table/TxTest.java
>> > > > >
>> > > > >
>> > > > > вт, 13 июл. 2021 г. в 19:41, Andrey Gura <ag...@apache.org>:
>> > > > >
>> > > > > > Alexey,
>> > > > > >
>> > > > > > could you please describe Transaction interface?
>> > > > > >
>> > > > > > Also it would be great to have a couple examples of using the
>> > > proposed
>> > > > > API.
>> > > > > >
>> > > > > > On Tue, Jul 13, 2021 at 4:43 PM Alexei Scherbakov
>> > > > > > <alexey.scherbak...@gmail.com> wrote:
>> > > > > > >
>> > > > > > > Folks,
>> > > > > > >
>> > > > > > > I've prepared a PR implementing my vision of public
>> transactions
>> > > API.
>> > > > > > >
>> > > > > > > API is very simple and similar to Ignite 2, but has some
>> > > differences.
>> > > > > > >
>> > > > > > > More details can be found here [1]
>> > > > > > >
>> > > > > > > Share your thoughts.
>> > > > > > >
>> > > > > > > [1] https://issues.apache.org/jira/browse/IGNITE-15086
>> > > > > > >
>> > > > > > > --
>> > > > > > >
>> > > > > > > Best regards,
>> > > > > > > Alexei Scherbakov
>> > > > > >
>> > > > >
>> > > > >
>> > > > > --
>> > > > >
>> > > > > Best regards,
>> > > > > Alexei Scherbakov
>> > > > >
>> > > >
>> > > >
>> > > > --
>> > > > Sincerely yours, Ivan Daschinskiy
>> > > >
>> > >
>> > >
>> > > --
>> > > Best regards,
>> > > Andrey V. Mashenkov
>> > >
>> >
>> >
>> > --
>> >
>> > Best regards,
>> > Alexei Scherbakov
>> >
>>
>>
>> --
>> Sincerely yours, Ivan Daschinskiy
>>
>
>
> --
>
> Best regards,
> Alexei Scherbakov
>


-- 

Best regards,
Alexei Scherbakov

Reply via email to