From 037913934c5e7dca44262c280b48136300fdeb92 Mon Sep 17 00:00:00 2001 From: Mitsunori Komatsu Date: Wed, 9 Oct 2024 14:08:08 +0900 Subject: [PATCH] Rename SnapshotHandler to PreparedSnapshotHook --- .../consensuscommit/CommitHandler.java | 35 ++++++------ ...Handler.java => PreparedSnapshotHook.java} | 6 +-- .../TwoPhaseConsensusCommit.java | 2 +- .../consensuscommit/CommitHandlerTest.java | 53 +++++++++---------- 4 files changed, 48 insertions(+), 48 deletions(-) rename core/src/main/java/com/scalar/db/transaction/consensuscommit/{SnapshotHandler.java => PreparedSnapshotHook.java} (58%) 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 59584165f..ecbbede00 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 @@ -19,7 +19,7 @@ import com.scalar.db.exception.transaction.ValidationException; import com.scalar.db.transaction.consensuscommit.Coordinator.State; import com.scalar.db.transaction.consensuscommit.ParallelExecutor.ParallelExecutorTask; -import com.scalar.db.transaction.consensuscommit.SnapshotHandler.SnapshotHandleFuture; +import com.scalar.db.transaction.consensuscommit.PreparedSnapshotHook.PreparedSnapshotHookFuture; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.ArrayList; import java.util.List; @@ -37,7 +37,7 @@ public class CommitHandler { private final TransactionTableMetadataManager tableMetadataManager; private final ParallelExecutor parallelExecutor; - @LazyInit @Nullable private SnapshotHandler snapshotHandler; + @LazyInit @Nullable private PreparedSnapshotHook preparedSnapshotHook; @SuppressFBWarnings("EI_EXPOSE_REP2") public CommitHandler( @@ -56,9 +56,9 @@ protected void onPrepareFailure(Snapshot snapshot) {} protected void onValidateFailure(Snapshot snapshot) {} public void commit(Snapshot snapshot) throws CommitException, UnknownTransactionStatusException { - Optional snapshotHandleFuture; + Optional snapshotHookFuture; try { - snapshotHandleFuture = prepare(snapshot); + snapshotHookFuture = prepare(snapshot); } catch (PreparationException e) { abortState(snapshot.getId()); rollbackRecords(snapshot); @@ -85,7 +85,7 @@ public void commit(Snapshot snapshot) throws CommitException, UnknownTransaction throw e; } - snapshotHandleFuture.ifPresent(SnapshotHandleFuture::get); + snapshotHookFuture.ifPresent(PreparedSnapshotHookFuture::get); commitState(snapshot); commitRecords(snapshot); @@ -118,7 +118,8 @@ protected void handleCommitConflict(Snapshot snapshot, Exception cause) } } - public Optional prepare(Snapshot snapshot) throws PreparationException { + public Optional prepare(Snapshot snapshot) + throws PreparationException { try { return prepareRecords(snapshot); } catch (NoMutationException e) { @@ -135,14 +136,14 @@ public Optional prepare(Snapshot snapshot) throws Preparat } } - private Optional prepareRecords(Snapshot snapshot) + private Optional prepareRecords(Snapshot snapshot) throws ExecutionException, PreparationConflictException { - Optional snapshotHandleFuture; - if (snapshotHandler == null) { - snapshotHandleFuture = Optional.empty(); + Optional snapshotHookFuture; + if (preparedSnapshotHook == null) { + snapshotHookFuture = Optional.empty(); } else { - snapshotHandleFuture = Optional.of(snapshotHandler.handle(tableMetadataManager, snapshot)); + snapshotHookFuture = Optional.of(preparedSnapshotHook.handle(tableMetadataManager, snapshot)); } PrepareMutationComposer composer = new PrepareMutationComposer(snapshot.getId(), tableMetadataManager); @@ -156,7 +157,7 @@ private Optional prepareRecords(Snapshot snapshot) } parallelExecutor.prepare(tasks, snapshot.getId()); - return snapshotHandleFuture; + return snapshotHookFuture; } public void validate(Snapshot snapshot) throws ValidationException { @@ -253,13 +254,13 @@ public void rollbackRecords(Snapshot snapshot) { } /** - * Sets the {@link SnapshotHandler}. This method must be called immediately after the constructor - * is invoked. + * Sets the {@link PreparedSnapshotHook}. This method must be called immediately after the + * constructor is invoked. * - * @param snapshotHandler The snapshot handler to set. + * @param preparedSnapshotHook The snapshot hook to set. * @throws NullPointerException If the argument is null. */ - public void setSnapshotHandler(SnapshotHandler snapshotHandler) { - this.snapshotHandler = checkNotNull(snapshotHandler); + public void setPreparedSnapshotHook(PreparedSnapshotHook preparedSnapshotHook) { + this.preparedSnapshotHook = checkNotNull(preparedSnapshotHook); } } diff --git a/core/src/main/java/com/scalar/db/transaction/consensuscommit/SnapshotHandler.java b/core/src/main/java/com/scalar/db/transaction/consensuscommit/PreparedSnapshotHook.java similarity index 58% rename from core/src/main/java/com/scalar/db/transaction/consensuscommit/SnapshotHandler.java rename to core/src/main/java/com/scalar/db/transaction/consensuscommit/PreparedSnapshotHook.java index 9a83d3074..bd5dd148a 100644 --- a/core/src/main/java/com/scalar/db/transaction/consensuscommit/SnapshotHandler.java +++ b/core/src/main/java/com/scalar/db/transaction/consensuscommit/PreparedSnapshotHook.java @@ -1,10 +1,10 @@ package com.scalar.db.transaction.consensuscommit; -public interface SnapshotHandler { - SnapshotHandleFuture handle( +public interface PreparedSnapshotHook { + PreparedSnapshotHookFuture handle( TransactionTableMetadataManager transactionTableMetadataManager, Snapshot snapshot); - interface SnapshotHandleFuture { + interface PreparedSnapshotHookFuture { void get(); } } diff --git a/core/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommit.java b/core/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommit.java index 0b7c94dfb..4ec4d118b 100644 --- a/core/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommit.java +++ b/core/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommit.java @@ -223,7 +223,7 @@ public void prepare() throws PreparationException { } try { - // SnapshotHandler is not supported in two-phase commit interface. + // PreparedSnapshotHook is not supported in two-phase commit interface. commit .prepare(crud.getSnapshot()) .ifPresent( 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 37f9cb3e1..5b5e9e7fa 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 @@ -25,7 +25,7 @@ import com.scalar.db.exception.transaction.UnknownTransactionStatusException; import com.scalar.db.exception.transaction.ValidationConflictException; import com.scalar.db.io.Key; -import com.scalar.db.transaction.consensuscommit.SnapshotHandler.SnapshotHandleFuture; +import com.scalar.db.transaction.consensuscommit.PreparedSnapshotHook.PreparedSnapshotHookFuture; import java.time.Duration; import java.time.Instant; import java.util.Optional; @@ -57,8 +57,8 @@ public class CommitHandlerTest { @Mock protected Coordinator coordinator; @Mock protected TransactionTableMetadataManager tableMetadataManager; @Mock protected ConsensusCommitConfig config; - @Mock protected SnapshotHandler snapshotHandler; - @Mock protected SnapshotHandleFuture snapshotHandleFuture; + @Mock protected PreparedSnapshotHook preparedSnapshotHook; + @Mock protected PreparedSnapshotHookFuture preparedSnapshotHookFuture; private CommitHandler handler; protected ParallelExecutor parallelExecutor; @@ -153,24 +153,24 @@ private Snapshot prepareSnapshotWithSamePartitionPut() { return snapshot; } - private void setSnapshotHandlerIfNeeded(boolean withSnapshotHandler) { - if (withSnapshotHandler) { - doReturn(snapshotHandleFuture).when(snapshotHandler).handle(any(), any()); - handler.setSnapshotHandler(snapshotHandler); + private void setPreparedSnapshotHookIfNeeded(boolean withSnapshotHook) { + if (withSnapshotHook) { + doReturn(preparedSnapshotHookFuture).when(preparedSnapshotHook).handle(any(), any()); + handler.setPreparedSnapshotHook(preparedSnapshotHook); } } @ParameterizedTest @ValueSource(booleans = {false, true}) public void commit_SnapshotWithDifferentPartitionPutsGiven_ShouldCommitRespectively( - boolean withSnapshotHandler) + boolean withSnapshotHook) throws CommitException, UnknownTransactionStatusException, ExecutionException, CoordinatorException { // Arrange Snapshot snapshot = prepareSnapshotWithDifferentPartitionPut(); doNothing().when(storage).mutate(anyList()); doNothingWhenCoordinatorPutState(); - setSnapshotHandlerIfNeeded(withSnapshotHandler); + setPreparedSnapshotHookIfNeeded(withSnapshotHook); // Act handler.commit(snapshot); @@ -178,20 +178,19 @@ public void commit_SnapshotWithDifferentPartitionPutsGiven_ShouldCommitRespectiv // Assert verify(storage, times(4)).mutate(anyList()); verifyCoordinatorPutState(TransactionState.COMMITTED); - verifySnapshotHandler(withSnapshotHandler, snapshot); + verifySnapshotHook(withSnapshotHook, snapshot); } @ParameterizedTest @ValueSource(booleans = {false, true}) - public void commit_SnapshotWithSamePartitionPutsGiven_ShouldCommitAtOnce( - boolean withSnapshotHandler) + public void commit_SnapshotWithSamePartitionPutsGiven_ShouldCommitAtOnce(boolean withSnapshotHook) throws CommitException, UnknownTransactionStatusException, ExecutionException, CoordinatorException { // Arrange Snapshot snapshot = prepareSnapshotWithSamePartitionPut(); doNothing().when(storage).mutate(anyList()); doNothingWhenCoordinatorPutState(); - setSnapshotHandlerIfNeeded(withSnapshotHandler); + setPreparedSnapshotHookIfNeeded(withSnapshotHook); // Act handler.commit(snapshot); @@ -199,7 +198,7 @@ public void commit_SnapshotWithSamePartitionPutsGiven_ShouldCommitAtOnce( // Assert verify(storage, times(2)).mutate(anyList()); verifyCoordinatorPutState(TransactionState.COMMITTED); - verifySnapshotHandler(withSnapshotHandler, snapshot); + verifySnapshotHook(withSnapshotHook, snapshot); } @Test @@ -644,24 +643,24 @@ public void commit_ExceptionThrownInCoordinatorCommit_ShouldThrowUnknown() } @Test - public void commit_SnapshotHandlerGiven_ShouldWaitSnapshotHandlerFinishesBeforeCommitState() + public void commit_SnapshotHookGiven_ShouldWaitSnapshotHookFinishesBeforeCommitState() throws ExecutionException, CoordinatorException { // Arrange Snapshot snapshot = prepareSnapshotWithDifferentPartitionPut(); doNothing().when(storage).mutate(anyList()); doThrowExceptionWhenCoordinatorPutState(TransactionState.COMMITTED, CoordinatorException.class); - // Lambda can be spied... - SnapshotHandler delayedSnapshotHandler = + // Lambda can't be spied... + PreparedSnapshotHook delayedPreparedSnapshotHook = spy( - new SnapshotHandler() { + new PreparedSnapshotHook() { @Override - public SnapshotHandleFuture handle( + public PreparedSnapshotHookFuture handle( TransactionTableMetadataManager tableMetadataManager, Snapshot ss) { Uninterruptibles.sleepUninterruptibly(Duration.ofSeconds(2)); - return snapshotHandleFuture; + return preparedSnapshotHookFuture; } }); - handler.setSnapshotHandler(delayedSnapshotHandler); + handler.setPreparedSnapshotHook(delayedPreparedSnapshotHook); // Act Instant start = Instant.now(); @@ -673,8 +672,8 @@ public SnapshotHandleFuture handle( verify(storage, times(2)).mutate(anyList()); verifyCoordinatorPutState(TransactionState.COMMITTED); verify(handler, never()).rollbackRecords(snapshot); - verify(delayedSnapshotHandler).handle(tableMetadataManager, snapshot); - // This means `commit()` waited until `SnapshotHandler.handle()` is completed before throwing + verify(delayedPreparedSnapshotHook).handle(tableMetadataManager, snapshot); + // This means `commit()` waited until the callback was completed before throwing // an exception from `commitState()`. assertThat(Duration.between(start, end)).isGreaterThanOrEqualTo(Duration.ofSeconds(2)); } @@ -695,11 +694,11 @@ protected void verifyCoordinatorPutState(TransactionState expectedTransactionSta verify(coordinator).putState(new Coordinator.State(anyId(), expectedTransactionState)); } - private void verifySnapshotHandler(boolean withSnapshotHandler, Snapshot snapshot) { - if (withSnapshotHandler) { - verify(snapshotHandler).handle(eq(tableMetadataManager), eq(snapshot)); + private void verifySnapshotHook(boolean withSnapshotHook, Snapshot snapshot) { + if (withSnapshotHook) { + verify(preparedSnapshotHook).handle(eq(tableMetadataManager), eq(snapshot)); } else { - verify(snapshotHandler, never()).handle(any(), any()); + verify(preparedSnapshotHook, never()).handle(any(), any()); } } }