Alexei,
I agree that runInTransaction is confusing and error-prone.
But we already have view.withTransaction(), which seems to be the most
boilerplate-free approach.
The example above will look like this:
public void testMixedPutGet() throws TransactionException {
RecordView<Tuple> view = accounts.recordView();
view.upsert(makeValue(1, BALANCE_1));
RecordView<Tuple> txView = view.withTransaction();
txView.getAsync(makeKey(1)).thenCompose(r ->
txView.upsertAsync(makeValue(1, r.doubleValue("balance") + DELTA),
tx)).thenCompose(txView.transaction().commitAsync()).join();
assertEquals(BALANCE_1 + DELTA,
view.get(makeKey(1)).doubleValue("balance"));
}
Is there any problem with this?
On Mon, Nov 29, 2021 at 10:45 AM Alexei Scherbakov <
alexey.scherbak...@gmail.com> wrote:
> 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
>
