ptupitsyn commented on code in PR #1503:
URL: https://github.com/apache/ignite-3/pull/1503#discussion_r1066699108
##########
modules/client/src/main/java/org/apache/ignite/internal/client/tx/ClientTransaction.java:
##########
@@ -87,7 +84,9 @@ public void commit() throws TransactionException {
/** {@inheritDoc} */
@Override
public CompletableFuture<Void> commitAsync() {
- setState(STATE_COMMITTED);
+ if (!state.compareAndSet(STATE_OPEN, STATE_CLOSE)) {
Review Comment:
We can probably replace `AtomicInteger` with `AtomicBoolean` now since there
are only 2 states.
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientTransactionsTest.java:
##########
@@ -199,21 +200,23 @@ void testAccessLockedKeyTimesOut() {
}
@Test
- void testCommitRollbackSameTxThrows() {
+ void testCommitRollbackSameTxNotThrows() {
Review Comment:
```suggestion
void testCommitRollbackSameTxDoesNotThrow() {
```
##########
modules/table/src/test/java/org/apache/ignite/internal/table/TxAbstractTest.java:
##########
@@ -121,10 +122,29 @@ public abstract class TxAbstractTest extends
IgniteAbstractTest {
public abstract void before() throws Exception;
@Test
- public void testDeleteUpsertCommit() throws TransactionException {
- deleteUpsert().commit();
+ public void testCommitRollbackSameTxNotThrows() throws
TransactionException {
Review Comment:
```suggestion
public void testCommitRollbackSameTxDoesNotThrow() throws
TransactionException {
```
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientTransactionsTest.java:
##########
@@ -199,21 +200,23 @@ void testAccessLockedKeyTimesOut() {
}
@Test
- void testCommitRollbackSameTxThrows() {
+ void testCommitRollbackSameTxNotThrows() {
Transaction tx = client().transactions().begin();
tx.commit();
- TransactionException ex = assertThrows(TransactionException.class,
tx::rollback);
- assertThat(ex.getMessage(), containsString("Transaction is already
committed"));
+ assertDoesNotThrow(tx::rollback, "Unexpected exception was thrown.");
+ assertDoesNotThrow(tx::commit, "Unexpected exception was thrown.");
+ assertDoesNotThrow(tx::rollback, "Unexpected exception was thrown.");
}
@Test
- void testRollbackCommitSameTxThrows() {
+ void testRollbackCommitSameTxNotThrows() {
Review Comment:
```suggestion
void testRollbackCommitSameTxDoesNotThrow() {
```
##########
modules/client/src/main/java/org/apache/ignite/internal/client/tx/ClientTransaction.java:
##########
@@ -34,11 +34,8 @@ 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;
+ /** Close state. */
+ private static final int STATE_CLOSE = 1;
Review Comment:
```suggestion
private static final int STATE_CLOSED = 1;
```
##########
modules/table/src/test/java/org/apache/ignite/internal/table/TxAbstractTest.java:
##########
@@ -121,10 +122,29 @@ public abstract class TxAbstractTest extends
IgniteAbstractTest {
public abstract void before() throws Exception;
@Test
- public void testDeleteUpsertCommit() throws TransactionException {
- deleteUpsert().commit();
+ public void testCommitRollbackSameTxNotThrows() throws
TransactionException {
+ InternalTransaction tx = (InternalTransaction)
igniteTransactions.begin();
- assertEquals(200., accounts.recordView().get(null,
makeKey(1)).doubleValue("balance"));
+ accounts.recordView().upsert(tx, makeValue(1, 100.));
+
+ tx.commit();
+
+ assertDoesNotThrow(tx::rollback, "Unexpected exception was thrown.");
+ assertDoesNotThrow(tx::commit, "Unexpected exception was thrown.");
+ assertDoesNotThrow(tx::rollback, "Unexpected exception was thrown.");
+ }
+
+ @Test
+ public void testRollbackCommitSameTxNotThrows() throws
TransactionException {
Review Comment:
```suggestion
public void testRollbackCommitSameTxDoesNotThrow() throws
TransactionException {
```
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java:
##########
@@ -981,7 +989,7 @@ private CompletableFuture<Void>
processTxFinishAction(TxFinishReplicaRequest req
* @param commit True is the transaction is committed, false otherwise.
* @return Future to wait of the finish.
*/
- private CompletableFuture<Object>
finishTransaction(List<ReplicationGroupId> aggregatedGroupIds, UUID txId,
boolean commit) {
+ private CompletableFuture<Object>
finishTransaction(List<ReplicationGroupId> aggregatedGroupIds, UUID txId,
boolean commit) {//
Review Comment:
Temp change? Please revert.
--
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]
