vldpyatkov commented on code in PR #1503:
URL: https://github.com/apache/ignite-3/pull/1503#discussion_r1069181212
##########
modules/client/src/main/java/org/apache/ignite/internal/client/tx/ClientTransaction.java:
##########
@@ -31,23 +32,17 @@
* Client transaction.
*/
public class ClientTransaction implements Transaction {
- /** Open state. */
- private static final int STATE_OPEN = 0;
-
- /** Committed state. */
- private static final int STATE_COMMITTED = 1;
-
- /** Rolled back state. */
- private static final int STATE_ROLLED_BACK = 2;
-
/** Channel that the transaction belongs to. */
private final ClientChannel ch;
/** Transaction id. */
private final long id;
- /** State. */
- private final AtomicInteger state = new AtomicInteger(STATE_OPEN);
+ /** The future used on repeated commit/rollback. */
+ private AtomicReference<CompletableFuture<Void>> finishFut = new
AtomicReference<>();
+
+ /** {@code true} if commit is started. */
+ private volatile boolean commitState;
Review Comment:
This variable is not really required.
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/ReadWriteTransactionImpl.java:
##########
@@ -51,6 +53,12 @@ public class ReadWriteTransactionImpl extends
IgniteAbstractTransactionImpl {
/** Reference to the partition that stores the transaction state. */
private final AtomicReference<ReplicationGroupId> commitPartitionRef = new
AtomicReference<>();
+ /** The future used on repeated commit/rollback. */
+ private final AtomicReference<CompletableFuture<Void>> finishFut = new
AtomicReference<>();
+
+ /** {@code true} if commit is started. */
+ private volatile boolean commitState;
Review Comment:
The same as it is for Client transaction: the variable is not really
required.
##########
modules/client/src/main/java/org/apache/ignite/internal/client/tx/ClientTransaction.java:
##########
@@ -101,23 +105,13 @@ public void rollback() throws TransactionException {
/** {@inheritDoc} */
@Override
public CompletableFuture<Void> rollbackAsync() {
- setState(STATE_ROLLED_BACK);
-
- return ch.serviceAsync(ClientOp.TX_ROLLBACK, w ->
w.out().packLong(id), r -> null);
- }
-
- private void setState(int state) {
- int oldState = this.state.compareAndExchange(STATE_OPEN, state);
-
- if (oldState == STATE_OPEN) {
- return;
+ if (!finishFut.compareAndSet(null, new CompletableFuture<>())) {
+ return completedFuture(null);
}
- String message = oldState == STATE_COMMITTED
- ? "Transaction is already committed."
- : "Transaction is already rolled back.";
+ commitState = false;
- throw new TransactionException(message);
+ return ch.serviceAsync(ClientOp.TX_ROLLBACK, w ->
w.out().packLong(id), r -> null);
Review Comment:
Here is my variant:
```
return finishFut.updateAndGet(fut -> {
if (fut != null)
return fut;
ch.serviceAsync(ClientOp.TX_ROLLBACK, w -> w.out().packLong(id), r ->
null);
return CompletableFuture.completedFuture(null);
});
```
##########
modules/client/src/main/java/org/apache/ignite/internal/client/tx/ClientTransaction.java:
##########
@@ -101,23 +105,13 @@ public void rollback() throws TransactionException {
/** {@inheritDoc} */
@Override
public CompletableFuture<Void> rollbackAsync() {
- setState(STATE_ROLLED_BACK);
-
- return ch.serviceAsync(ClientOp.TX_ROLLBACK, w ->
w.out().packLong(id), r -> null);
- }
-
- private void setState(int state) {
- int oldState = this.state.compareAndExchange(STATE_OPEN, state);
-
- if (oldState == STATE_OPEN) {
- return;
+ if (!finishFut.compareAndSet(null, new CompletableFuture<>())) {
+ return completedFuture(null);
}
- String message = oldState == STATE_COMMITTED
- ? "Transaction is already committed."
- : "Transaction is already rolled back.";
+ commitState = false;
- throw new TransactionException(message);
+ return ch.serviceAsync(ClientOp.TX_ROLLBACK, w ->
w.out().packLong(id), r -> null);
Review Comment:
If we do not wait rollback, we shouldn't wait it at all (no ever it is a
first invocation or not)
##########
modules/client/src/main/java/org/apache/ignite/internal/client/tx/ClientTransaction.java:
##########
@@ -87,9 +82,18 @@ public void commit() throws TransactionException {
/** {@inheritDoc} */
@Override
public CompletableFuture<Void> commitAsync() {
- setState(STATE_COMMITTED);
+ if (!finishFut.compareAndSet(null, new CompletableFuture<>())) {
+ if (commitState) {
+ return finishFut.get();
+ } else {
+ return completedFuture(null);
+ }
+ }
+
+ commitState = true;
- return ch.serviceAsync(ClientOp.TX_COMMIT, w -> w.out().packLong(id),
r -> null);
+ return ch.serviceAsync(ClientOp.TX_COMMIT, w -> w.out().packLong(id),
r -> null)
+ .thenRun(() -> finishFut.get().complete(null));
Review Comment:
I'll write that like this:
```
return finishFut.updateAndGet(fut -> {
if (fut != null)
return fut;
return ch.serviceAsync(ClientOp.TX_COMMIT, w -> w.out().packLong(id), r
-> null);
});
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
