diff --git a/core/src/main/java/com/scalar/db/transaction/consensuscommit/CommitHandlerWithGroupCommit.java b/core/src/main/java/com/scalar/db/transaction/consensuscommit/CommitHandlerWithGroupCommit.java index 1cb7d721db..6918f7c3ba 100644 --- a/core/src/main/java/com/scalar/db/transaction/consensuscommit/CommitHandlerWithGroupCommit.java +++ b/core/src/main/java/com/scalar/db/transaction/consensuscommit/CommitHandlerWithGroupCommit.java @@ -57,6 +57,23 @@ public void commit(Snapshot snapshot, boolean readOnly) super.commit(snapshot, readOnly); } + @Override + boolean canOnePhaseCommit(Snapshot snapshot) throws CommitException { + try { + return super.canOnePhaseCommit(snapshot); + } catch (CommitException e) { + cancelGroupCommitIfNeeded(snapshot.getId()); + throw e; + } + } + + @Override + void onePhaseCommitRecords(Snapshot snapshot) + throws CommitConflictException, UnknownTransactionStatusException { + cancelGroupCommitIfNeeded(snapshot.getId()); + super.onePhaseCommitRecords(snapshot); + } + @Override protected void onFailureBeforeCommit(Snapshot snapshot) { cancelGroupCommitIfNeeded(snapshot.getId()); diff --git a/core/src/test/java/com/scalar/db/transaction/consensuscommit/CommitHandlerTest.java b/core/src/test/java/com/scalar/db/transaction/consensuscommit/CommitHandlerTest.java index 63d84561bf..80984ccc83 100644 --- a/core/src/test/java/com/scalar/db/transaction/consensuscommit/CommitHandlerTest.java +++ b/core/src/test/java/com/scalar/db/transaction/consensuscommit/CommitHandlerTest.java @@ -90,6 +90,11 @@ protected CommitHandler createCommitHandler(boolean coordinatorWriteOmissionOnRe false); } + protected CommitHandler createCommitHandlerWithOnePhaseCommit() { + return new CommitHandler( + storage, coordinator, tableMetadataManager, parallelExecutor, mutationsGrouper, true, true); + } + @BeforeEach void setUp() throws Exception { MockitoAnnotations.openMocks(this).close(); @@ -1068,8 +1073,9 @@ public void canOnePhaseCommit_WhenMutationsCannotBeGrouped_ShouldReturnFalse() t } @Test - public void canOnePhaseCommit_WhenMutationsGrouperThrowsException_ShouldThrowCommitException() - throws ExecutionException { + public void + canOnePhaseCommit_WhenMutationsGrouperThrowsExecutionException_ShouldThrowCommitException() + throws ExecutionException { // Arrange CommitHandler handler = createCommitHandlerWithOnePhaseCommit(); Snapshot snapshot = prepareSnapshot(); @@ -1180,11 +1186,6 @@ public void commit_OnePhaseCommitted_ShouldNotThrowAnyException() verify(handler).onFailureBeforeCommit(snapshot); } - private CommitHandler createCommitHandlerWithOnePhaseCommit() { - return new CommitHandler( - storage, coordinator, tableMetadataManager, parallelExecutor, mutationsGrouper, true, true); - } - protected void doThrowExceptionWhenCoordinatorPutState( TransactionState targetState, Class exceptionClass) throws CoordinatorException { diff --git a/core/src/test/java/com/scalar/db/transaction/consensuscommit/CommitHandlerWithGroupCommitTest.java b/core/src/test/java/com/scalar/db/transaction/consensuscommit/CommitHandlerWithGroupCommitTest.java index 9c1f190f10..f79eecae55 100644 --- a/core/src/test/java/com/scalar/db/transaction/consensuscommit/CommitHandlerWithGroupCommitTest.java +++ b/core/src/test/java/com/scalar/db/transaction/consensuscommit/CommitHandlerWithGroupCommitTest.java @@ -15,6 +15,7 @@ import com.scalar.db.api.TransactionState; import com.scalar.db.exception.storage.ExecutionException; +import com.scalar.db.exception.transaction.CommitConflictException; import com.scalar.db.exception.transaction.CommitException; import com.scalar.db.exception.transaction.UnknownTransactionStatusException; import com.scalar.db.exception.transaction.ValidationConflictException; @@ -22,7 +23,6 @@ import com.scalar.db.util.groupcommit.GroupCommitConfig; import java.util.List; import java.util.UUID; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -75,6 +75,20 @@ protected CommitHandler createCommitHandler(boolean coordinatorWriteOmissionOnRe groupCommitter); } + @Override + protected CommitHandler createCommitHandlerWithOnePhaseCommit() { + createGroupCommitterIfNotExists(); + return new CommitHandlerWithGroupCommit( + storage, + coordinator, + tableMetadataManager, + parallelExecutor, + mutationsGrouper, + true, + true, + groupCommitter); + } + private String anyGroupCommitParentId() { return parentKey; } @@ -199,72 +213,121 @@ public void commit_NoReadsInSnapshot_ShouldNotValidateRecords(boolean withSnapsh verify(groupCommitter, never()).remove(anyId()); } - @Disabled("Enabling both one-phase commit and group commit is not supported") - @Override @Test - public void canOnePhaseCommit_WhenOnePhaseCommitDisabled_ShouldReturnFalse() {} - - @Disabled("Enabling both one-phase commit and group commit is not supported") @Override - @Test - public void canOnePhaseCommit_WhenValidationRequired_ShouldReturnFalse() {} + public void onePhaseCommitRecords_WhenSuccessful_ShouldMutateUsingComposerMutations() + throws CommitConflictException, UnknownTransactionStatusException, ExecutionException { + super.onePhaseCommitRecords_WhenSuccessful_ShouldMutateUsingComposerMutations(); - @Disabled("Enabling both one-phase commit and group commit is not supported") - @Override - @Test - public void canOnePhaseCommit_WhenNoWritesAndDeletes_ShouldReturnFalse() {} + // Assert + verify(groupCommitter).remove(anyId()); + } - @Disabled("Enabling both one-phase commit and group commit is not supported") - @Override @Test - public void canOnePhaseCommit_WhenDeleteWithoutExistingRecord_ShouldReturnFalse() {} - - @Disabled("Enabling both one-phase commit and group commit is not supported") @Override - @Test - public void canOnePhaseCommit_WhenMutationsCanBeGrouped_ShouldReturnTrue() {} + public void + onePhaseCommitRecords_WhenNoMutationExceptionThrown_ShouldThrowCommitConflictException() + throws ExecutionException { + super.onePhaseCommitRecords_WhenNoMutationExceptionThrown_ShouldThrowCommitConflictException(); - @Disabled("Enabling both one-phase commit and group commit is not supported") - @Override - @Test - public void canOnePhaseCommit_WhenMutationsCannotBeGrouped_ShouldReturnFalse() {} + // Assert + verify(groupCommitter).remove(anyId()); + } - @Disabled("Enabling both one-phase commit and group commit is not supported") - @Override @Test - public void canOnePhaseCommit_WhenMutationsGrouperThrowsException_ShouldThrowCommitException() {} + @Override + public void + onePhaseCommitRecords_WhenRetriableExecutionExceptionThrown_ShouldThrowCommitConflictException() + throws ExecutionException { + super + .onePhaseCommitRecords_WhenRetriableExecutionExceptionThrown_ShouldThrowCommitConflictException(); - @Disabled("Enabling both one-phase commit and group commit is not supported") + // Assert + verify(groupCommitter).remove(anyId()); + } + + @Test @Override + public void + onePhaseCommitRecords_WhenExecutionExceptionThrown_ShouldThrowUnknownTransactionStatusException() + throws ExecutionException { + super + .onePhaseCommitRecords_WhenExecutionExceptionThrown_ShouldThrowUnknownTransactionStatusException(); + + // Assert + verify(groupCommitter).remove(anyId()); + } + @Test - public void onePhaseCommitRecords_WhenSuccessful_ShouldMutateUsingComposerMutations() {} + @Override + public void canOnePhaseCommit_WhenOnePhaseCommitDisabled_ShouldReturnFalse() throws Exception { + super.canOnePhaseCommit_WhenOnePhaseCommitDisabled_ShouldReturnFalse(); + groupCommitter.remove(anyId()); + } - @Disabled("Enabling both one-phase commit and group commit is not supported") + @Test @Override + public void canOnePhaseCommit_WhenValidationRequired_ShouldReturnFalse() throws Exception { + super.canOnePhaseCommit_WhenValidationRequired_ShouldReturnFalse(); + groupCommitter.remove(anyId()); + } + @Test - public void - onePhaseCommitRecords_WhenNoMutationExceptionThrown_ShouldThrowCommitConflictException() {} + @Override + public void canOnePhaseCommit_WhenNoWritesAndDeletes_ShouldReturnFalse() throws Exception { + super.canOnePhaseCommit_WhenNoWritesAndDeletes_ShouldReturnFalse(); + groupCommitter.remove(anyId()); + } - @Disabled("Enabling both one-phase commit and group commit is not supported") + @Test @Override + public void canOnePhaseCommit_WhenDeleteWithoutExistingRecord_ShouldReturnFalse() + throws Exception { + super.canOnePhaseCommit_WhenDeleteWithoutExistingRecord_ShouldReturnFalse(); + groupCommitter.remove(anyId()); + } + @Test - public void - onePhaseCommitRecords_WhenRetriableExecutionExceptionThrown_ShouldThrowCommitConflictException() {} + @Override + public void canOnePhaseCommit_WhenMutationsCanBeGrouped_ShouldReturnTrue() throws Exception { + super.canOnePhaseCommit_WhenMutationsCanBeGrouped_ShouldReturnTrue(); + groupCommitter.remove(anyId()); + } - @Disabled("Enabling both one-phase commit and group commit is not supported") + @Test @Override + public void canOnePhaseCommit_WhenMutationsCannotBeGrouped_ShouldReturnFalse() throws Exception { + super.canOnePhaseCommit_WhenMutationsCannotBeGrouped_ShouldReturnFalse(); + groupCommitter.remove(anyId()); + } + @Test + @Override public void - onePhaseCommitRecords_WhenExecutionExceptionThrown_ShouldThrowUnknownTransactionStatusException() {} + canOnePhaseCommit_WhenMutationsGrouperThrowsExecutionException_ShouldThrowCommitException() + throws ExecutionException { + super + .canOnePhaseCommit_WhenMutationsGrouperThrowsExecutionException_ShouldThrowCommitException(); - @Disabled("Enabling both one-phase commit and group commit is not supported") - @Override - @Test - public void commit_OnePhaseCommitted_ShouldNotThrowAnyException() {} + // Assert + verify(groupCommitter).remove(anyId()); + } - @Disabled("Enabling both one-phase commit and group commit is not supported") + @Test @Override + public void commit_OnePhaseCommitted_ShouldNotThrowAnyException() + throws CommitException, UnknownTransactionStatusException { + super.commit_OnePhaseCommitted_ShouldNotThrowAnyException(); + groupCommitter.remove(anyId()); + } + @Test + @Override public void - commit_OnePhaseCommitted_UnknownTransactionStatusExceptionThrown_ShouldThrowUnknownTransactionStatusException() {} + commit_OnePhaseCommitted_UnknownTransactionStatusExceptionThrown_ShouldThrowUnknownTransactionStatusException() + throws CommitException, UnknownTransactionStatusException { + super + .commit_OnePhaseCommitted_UnknownTransactionStatusExceptionThrown_ShouldThrowUnknownTransactionStatusException(); + groupCommitter.remove(anyId()); + } }