From 7b70f345dda53dd0dcb05d73e4de1d1ee3baad2e Mon Sep 17 00:00:00 2001 From: Toshihiro Suzuki Date: Mon, 7 Jul 2025 18:46:34 +0900 Subject: [PATCH] Fix error handling for one-phase commit (#2826) --- .../consensuscommit/CommitHandler.java | 5 +++-- .../consensuscommit/CommitHandlerTest.java | 19 +++++++++++-------- .../CommitHandlerWithGroupCommitTest.java | 6 ++++-- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/com/scalar/db/transaction/consensuscommit/CommitHandler.java b/core/src/main/java/com/scalar/db/transaction/consensuscommit/CommitHandler.java index e775b876f7..845f351d56 100644 --- a/core/src/main/java/com/scalar/db/transaction/consensuscommit/CommitHandler.java +++ b/core/src/main/java/com/scalar/db/transaction/consensuscommit/CommitHandler.java @@ -261,7 +261,8 @@ protected void handleCommitConflict(Snapshot snapshot, Exception cause) } @VisibleForTesting - void onePhaseCommitRecords(Snapshot snapshot) throws CommitException { + void onePhaseCommitRecords(Snapshot snapshot) + throws CommitConflictException, UnknownTransactionStatusException { try { OnePhaseCommitMutationComposer composer = new OnePhaseCommitMutationComposer(snapshot.getId(), tableMetadataManager); @@ -279,7 +280,7 @@ void onePhaseCommitRecords(Snapshot snapshot) throws CommitException { e, snapshot.getId()); } catch (ExecutionException e) { - throw new CommitException( + throw new UnknownTransactionStatusException( CoreError.CONSENSUS_COMMIT_COMMITTING_RECORDS_FAILED.buildMessage(), e, 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 3a355ece42..63d84561bf 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 @@ -1089,7 +1089,7 @@ public void canOnePhaseCommit_WhenMutationsGrouperThrowsException_ShouldThrowCom @Test public void onePhaseCommitRecords_WhenSuccessful_ShouldMutateUsingComposerMutations() - throws CommitException, ExecutionException { + throws CommitConflictException, UnknownTransactionStatusException, ExecutionException { // Arrange Snapshot snapshot = spy(prepareSnapshotWithSamePartitionPut()); doNothing().when(storage).mutate(anyList()); @@ -1131,15 +1131,16 @@ public void onePhaseCommitRecords_WhenSuccessful_ShouldMutateUsingComposerMutati } @Test - public void onePhaseCommitRecords_WhenExecutionExceptionThrown_ShouldThrowCommitException() - throws ExecutionException { + public void + onePhaseCommitRecords_WhenExecutionExceptionThrown_ShouldThrowUnknownTransactionStatusException() + throws ExecutionException { // Arrange Snapshot snapshot = prepareSnapshotWithSamePartitionPut(); doThrow(ExecutionException.class).when(storage).mutate(anyList()); // Act Assert assertThatThrownBy(() -> handler.onePhaseCommitRecords(snapshot)) - .isInstanceOf(CommitException.class) + .isInstanceOf(UnknownTransactionStatusException.class) .hasCauseInstanceOf(ExecutionException.class); } @@ -1162,17 +1163,19 @@ public void commit_OnePhaseCommitted_ShouldNotThrowAnyException() } @Test - public void commit_OnePhaseCommitted_CommitExceptionThrown_ShouldThrowCommitException() - throws CommitException { + public void + commit_OnePhaseCommitted_UnknownTransactionStatusExceptionThrown_ShouldThrowUnknownTransactionStatusException() + throws CommitException, UnknownTransactionStatusException { // Arrange CommitHandler handler = spy(createCommitHandlerWithOnePhaseCommit()); Snapshot snapshot = prepareSnapshotWithSamePartitionPut(); doReturn(true).when(handler).canOnePhaseCommit(snapshot); - doThrow(CommitException.class).when(handler).onePhaseCommitRecords(snapshot); + doThrow(UnknownTransactionStatusException.class).when(handler).onePhaseCommitRecords(snapshot); // Act Assert - assertThatThrownBy(() -> handler.commit(snapshot, true)).isInstanceOf(CommitException.class); + assertThatThrownBy(() -> handler.commit(snapshot, true)) + .isInstanceOf(UnknownTransactionStatusException.class); verify(handler).onFailureBeforeCommit(snapshot); } 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 3d05640e28..9c1f190f10 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 @@ -254,7 +254,8 @@ public void onePhaseCommitRecords_WhenSuccessful_ShouldMutateUsingComposerMutati @Disabled("Enabling both one-phase commit and group commit is not supported") @Override @Test - public void onePhaseCommitRecords_WhenExecutionExceptionThrown_ShouldThrowCommitException() {} + public void + onePhaseCommitRecords_WhenExecutionExceptionThrown_ShouldThrowUnknownTransactionStatusException() {} @Disabled("Enabling both one-phase commit and group commit is not supported") @Override @@ -264,5 +265,6 @@ public void commit_OnePhaseCommitted_ShouldNotThrowAnyException() {} @Disabled("Enabling both one-phase commit and group commit is not supported") @Override @Test - public void commit_OnePhaseCommitted_CommitExceptionThrown_ShouldThrowCommitException() {} + public void + commit_OnePhaseCommitted_UnknownTransactionStatusExceptionThrown_ShouldThrowUnknownTransactionStatusException() {} }