From d4531826d5c1593883be386207c2bb26dfb2fda8 Mon Sep 17 00:00:00 2001 From: Toshihiro Suzuki Date: Wed, 15 Oct 2025 05:21:06 +0000 Subject: [PATCH 1/2] Empty commit [skip ci] From ed873e51c11540be61be125346f7da9506bb1bec Mon Sep 17 00:00:00 2001 From: Toshihiro Suzuki Date: Wed, 15 Oct 2025 14:20:51 +0900 Subject: [PATCH 2/2] Introduce TransactionContext in Consensus Commit (#3039) --- .../java/com/scalar/db/common/CoreError.java | 4 +- ...otHook.java => BeforePreparationHook.java} | 5 +- .../consensuscommit/CommitHandler.java | 205 ++--- .../CommitHandlerWithGroupCommit.java | 51 +- .../consensuscommit/ConsensusCommit.java | 47 +- .../ConsensusCommitManager.java | 36 +- .../ConsensusCommitScanner.java | 7 + .../CoordinatorGroupCommitter.java | 2 +- .../consensuscommit/CrudHandler.java | 368 ++++----- .../MutationConditionsValidator.java | 41 +- .../transaction/consensuscommit/Snapshot.java | 105 +-- .../consensuscommit/TransactionContext.java | 64 ++ .../TwoPhaseConsensusCommit.java | 56 +- .../TwoPhaseConsensusCommitManager.java | 35 +- .../consensuscommit/CommitHandlerTest.java | 391 +++++---- .../CommitHandlerWithGroupCommitTest.java | 26 +- .../ConsensusCommitManagerTest.java | 104 +-- .../consensuscommit/ConsensusCommitTest.java | 151 ++-- .../CoordinatorGroupCommitterTest.java | 54 +- .../consensuscommit/CrudHandlerTest.java | 778 ++++++++---------- .../MutationConditionsValidatorTest.java | 47 +- .../consensuscommit/SnapshotTest.java | 212 +---- .../TwoPhaseConsensusCommitManagerTest.java | 68 +- .../TwoPhaseConsensusCommitTest.java | 152 ++-- ...CommitNullMetadataIntegrationTestBase.java | 21 +- ...nsusCommitSpecificIntegrationTestBase.java | 19 +- ...nsusCommitSpecificIntegrationTestBase.java | 4 +- 27 files changed, 1500 insertions(+), 1553 deletions(-) rename core/src/main/java/com/scalar/db/transaction/consensuscommit/{BeforePreparationSnapshotHook.java => BeforePreparationHook.java} (63%) create mode 100644 core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitScanner.java create mode 100644 core/src/main/java/com/scalar/db/transaction/consensuscommit/TransactionContext.java diff --git a/core/src/main/java/com/scalar/db/common/CoreError.java b/core/src/main/java/com/scalar/db/common/CoreError.java index 91762de22f..a90085c480 100644 --- a/core/src/main/java/com/scalar/db/common/CoreError.java +++ b/core/src/main/java/com/scalar/db/common/CoreError.java @@ -925,10 +925,10 @@ public enum CoreError implements ScalarDbError { Category.INTERNAL_ERROR, "0044", "The Upsert operation failed. Details: %s", "", ""), JDBC_TRANSACTION_UPDATE_OPERATION_FAILED( Category.INTERNAL_ERROR, "0045", "The Update operation failed. Details: %s", "", ""), - HANDLING_BEFORE_PREPARATION_SNAPSHOT_HOOK_FAILED( + CONSENSUS_COMMIT_HANDLING_BEFORE_PREPARATION_HOOK_FAILED( Category.INTERNAL_ERROR, "0046", - "Handling the before-preparation snapshot hook failed. Details: %s", + "Handling the before-preparation hook failed. Details: %s", "", ""), JDBC_TRANSACTION_GETTING_SCANNER_FAILED( diff --git a/core/src/main/java/com/scalar/db/transaction/consensuscommit/BeforePreparationSnapshotHook.java b/core/src/main/java/com/scalar/db/transaction/consensuscommit/BeforePreparationHook.java similarity index 63% rename from core/src/main/java/com/scalar/db/transaction/consensuscommit/BeforePreparationSnapshotHook.java rename to core/src/main/java/com/scalar/db/transaction/consensuscommit/BeforePreparationHook.java index c8aa62804d..d2d7832771 100644 --- a/core/src/main/java/com/scalar/db/transaction/consensuscommit/BeforePreparationSnapshotHook.java +++ b/core/src/main/java/com/scalar/db/transaction/consensuscommit/BeforePreparationHook.java @@ -2,8 +2,7 @@ import java.util.concurrent.Future; -public interface BeforePreparationSnapshotHook { +public interface BeforePreparationHook { Future handle( - TransactionTableMetadataManager transactionTableMetadataManager, - Snapshot.ReadWriteSets readWriteSets); + TransactionTableMetadataManager transactionTableMetadataManager, TransactionContext context); } 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 845f351d56..5bfbb3078c 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 @@ -23,7 +23,9 @@ import com.scalar.db.transaction.consensuscommit.ParallelExecutor.ParallelExecutorTask; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.ArrayList; +import java.util.Collection; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.concurrent.Future; import java.util.stream.Collectors; @@ -44,7 +46,7 @@ public class CommitHandler { protected final boolean coordinatorWriteOmissionOnReadOnlyEnabled; private final boolean onePhaseCommitEnabled; - @LazyInit @Nullable private BeforePreparationSnapshotHook beforePreparationSnapshotHook; + @LazyInit @Nullable private BeforePreparationHook beforePreparationHook; @SuppressFBWarnings("EI_EXPOSE_REP2") public CommitHandler( @@ -67,104 +69,106 @@ public CommitHandler( /** * A callback invoked when any exception occurs before committing transactions. * - * @param snapshot the failed snapshot. + * @param context the transaction context */ - protected void onFailureBeforeCommit(Snapshot snapshot) {} + protected void onFailureBeforeCommit(TransactionContext context) {} - private void safelyCallOnFailureBeforeCommit(Snapshot snapshot) { + private void safelyCallOnFailureBeforeCommit(TransactionContext context) { try { - onFailureBeforeCommit(snapshot); + onFailureBeforeCommit(context); } catch (Exception e) { - logger.warn("Failed to call the callback. Transaction ID: {}", snapshot.getId(), e); + logger.warn("Failed to call the callback. Transaction ID: {}", context.transactionId, e); } } - private Optional> invokeBeforePreparationSnapshotHook(Snapshot snapshot) + private Optional> invokeBeforePreparationHook(TransactionContext context) throws UnknownTransactionStatusException, CommitException { - if (beforePreparationSnapshotHook == null) { + if (beforePreparationHook == null) { return Optional.empty(); } try { - return Optional.of( - beforePreparationSnapshotHook.handle(tableMetadataManager, snapshot.getReadWriteSets())); + return Optional.of(beforePreparationHook.handle(tableMetadataManager, context)); } catch (Exception e) { - safelyCallOnFailureBeforeCommit(snapshot); - abortState(snapshot.getId()); - rollbackRecords(snapshot); + safelyCallOnFailureBeforeCommit(context); + abortState(context.transactionId); + rollbackRecords(context); throw new CommitException( - CoreError.HANDLING_BEFORE_PREPARATION_SNAPSHOT_HOOK_FAILED.buildMessage(e.getMessage()), + CoreError.CONSENSUS_COMMIT_HANDLING_BEFORE_PREPARATION_HOOK_FAILED.buildMessage( + e.getMessage()), e, - snapshot.getId()); + context.transactionId); } } - private void waitBeforePreparationSnapshotHookFuture( - Snapshot snapshot, @Nullable Future snapshotHookFuture) + private void waitBeforePreparationHookFuture( + TransactionContext context, @Nullable Future beforePreparationHookFuture) throws UnknownTransactionStatusException, CommitException { - if (snapshotHookFuture == null) { + if (beforePreparationHookFuture == null) { return; } try { - snapshotHookFuture.get(); + beforePreparationHookFuture.get(); } catch (Exception e) { - safelyCallOnFailureBeforeCommit(snapshot); - abortState(snapshot.getId()); - rollbackRecords(snapshot); + safelyCallOnFailureBeforeCommit(context); + abortState(context.transactionId); + rollbackRecords(context); throw new CommitException( - CoreError.HANDLING_BEFORE_PREPARATION_SNAPSHOT_HOOK_FAILED.buildMessage(e.getMessage()), + CoreError.CONSENSUS_COMMIT_HANDLING_BEFORE_PREPARATION_HOOK_FAILED.buildMessage( + e.getMessage()), e, - snapshot.getId()); + context.transactionId); } } - public void commit(Snapshot snapshot, boolean readOnly) + public void commit(TransactionContext context) throws CommitException, UnknownTransactionStatusException { - boolean hasWritesOrDeletesInSnapshot = !readOnly && snapshot.hasWritesOrDeletes(); + boolean hasWritesOrDeletesInSnapshot = + !context.readOnly && context.snapshot.hasWritesOrDeletes(); - Optional> snapshotHookFuture = invokeBeforePreparationSnapshotHook(snapshot); + Optional> beforePreparationHookFuture = invokeBeforePreparationHook(context); - if (canOnePhaseCommit(snapshot)) { + if (canOnePhaseCommit(context)) { try { - onePhaseCommitRecords(snapshot); + onePhaseCommitRecords(context); return; } catch (Exception e) { - safelyCallOnFailureBeforeCommit(snapshot); + safelyCallOnFailureBeforeCommit(context); throw e; } } if (hasWritesOrDeletesInSnapshot) { try { - prepareRecords(snapshot); + prepareRecords(context); } catch (PreparationException e) { - safelyCallOnFailureBeforeCommit(snapshot); - abortState(snapshot.getId()); - rollbackRecords(snapshot); + safelyCallOnFailureBeforeCommit(context); + abortState(context.transactionId); + rollbackRecords(context); if (e instanceof PreparationConflictException) { throw new CommitConflictException(e.getMessage(), e, e.getTransactionId().orElse(null)); } throw new CommitException(e.getMessage(), e, e.getTransactionId().orElse(null)); } catch (Exception e) { - safelyCallOnFailureBeforeCommit(snapshot); + safelyCallOnFailureBeforeCommit(context); throw e; } } - if (snapshot.hasReads()) { + if (context.snapshot.hasReads()) { try { - validateRecords(snapshot); + validateRecords(context); } catch (ValidationException e) { - safelyCallOnFailureBeforeCommit(snapshot); + safelyCallOnFailureBeforeCommit(context); // If the transaction has no writes and deletes, we don't need to abort-state and // rollback-records since there are no changes to be made. if (hasWritesOrDeletesInSnapshot || !coordinatorWriteOmissionOnReadOnlyEnabled) { - abortState(snapshot.getId()); + abortState(context.transactionId); } if (hasWritesOrDeletesInSnapshot) { - rollbackRecords(snapshot); + rollbackRecords(context); } if (e instanceof ValidationConflictException) { @@ -172,45 +176,47 @@ public void commit(Snapshot snapshot, boolean readOnly) } throw new CommitException(e.getMessage(), e, e.getTransactionId().orElse(null)); } catch (Exception e) { - safelyCallOnFailureBeforeCommit(snapshot); + safelyCallOnFailureBeforeCommit(context); throw e; } } - waitBeforePreparationSnapshotHookFuture(snapshot, snapshotHookFuture.orElse(null)); + waitBeforePreparationHookFuture(context, beforePreparationHookFuture.orElse(null)); if (hasWritesOrDeletesInSnapshot || !coordinatorWriteOmissionOnReadOnlyEnabled) { - commitState(snapshot); + commitState(context); } if (hasWritesOrDeletesInSnapshot) { - commitRecords(snapshot); + commitRecords(context); } } @VisibleForTesting - boolean canOnePhaseCommit(Snapshot snapshot) throws CommitException { + boolean canOnePhaseCommit(TransactionContext context) throws CommitException { if (!onePhaseCommitEnabled) { return false; } // If validation is required (in SERIALIZABLE isolation), we cannot one-phase commit the // transaction - if (snapshot.isValidationRequired()) { + if (context.isValidationRequired()) { return false; } // If the snapshot has no write and deletes, we do not one-phase commit the transaction - if (!snapshot.hasWritesOrDeletes()) { + if (!context.snapshot.hasWritesOrDeletes()) { return false; } - List deletesInDeleteSet = snapshot.getDeletesInDeleteSet(); + Collection> deleteSetEntries = context.snapshot.getDeleteSet(); // If a record corresponding to a delete in the delete set does not exist in the storage,　we // cannot one-phase commit the transaction. This is because the storage does not support // delete-if-not-exists semantics, so we cannot detect conflicts with other transactions. - for (Delete delete : deletesInDeleteSet) { - Optional result = snapshot.getFromReadSet(new Snapshot.Key(delete)); + for (Map.Entry entry : deleteSetEntries) { + Delete delete = entry.getValue(); + Optional result = + context.snapshot.getFromReadSet(new Snapshot.Key(delete)); // For deletes, we always perform implicit pre-reads if the result does not exit in the read // set. So the result should always exist in the read set. @@ -225,26 +231,30 @@ boolean canOnePhaseCommit(Snapshot snapshot) throws CommitException { // If the mutations can be grouped altogether, the mutations can be done in a single mutate // API call, so we can one-phase commit the transaction return mutationsGrouper.canBeGroupedAltogether( - Stream.concat(snapshot.getPutsInWriteSet().stream(), deletesInDeleteSet.stream()) + Stream.concat( + context.snapshot.getWriteSet().stream().map(Map.Entry::getValue), + deleteSetEntries.stream().map(Map.Entry::getValue)) .collect(Collectors.toList())); } catch (ExecutionException e) { throw new CommitException( - CoreError.CONSENSUS_COMMIT_COMMITTING_RECORDS_FAILED.buildMessage(), e, snapshot.getId()); + CoreError.CONSENSUS_COMMIT_COMMITTING_RECORDS_FAILED.buildMessage(e.getMessage()), + e, + context.transactionId); } } - protected void handleCommitConflict(Snapshot snapshot, Exception cause) + protected void handleCommitConflict(TransactionContext context, Exception cause) throws CommitConflictException, UnknownTransactionStatusException { try { - Optional s = coordinator.getState(snapshot.getId()); + Optional s = coordinator.getState(context.transactionId); if (s.isPresent()) { TransactionState state = s.get().getState(); if (state.equals(TransactionState.ABORTED)) { - rollbackRecords(snapshot); + rollbackRecords(context); throw new CommitConflictException( CoreError.CONSENSUS_COMMIT_CONFLICT_OCCURRED_WHEN_COMMITTING_STATE.buildMessage(), cause, - snapshot.getId()); + context.transactionId); } } else { throw new UnknownTransactionStatusException( @@ -252,105 +262,113 @@ protected void handleCommitConflict(Snapshot snapshot, Exception cause) .CONSENSUS_COMMIT_COMMITTING_STATE_FAILED_WITH_NO_MUTATION_EXCEPTION_BUT_COORDINATOR_STATUS_DOES_NOT_EXIST .buildMessage(), cause, - snapshot.getId()); + context.transactionId); } } catch (CoordinatorException ex) { throw new UnknownTransactionStatusException( - CoreError.CONSENSUS_COMMIT_CANNOT_GET_STATE.buildMessage(), ex, snapshot.getId()); + CoreError.CONSENSUS_COMMIT_CANNOT_GET_STATE.buildMessage(), ex, context.transactionId); } } @VisibleForTesting - void onePhaseCommitRecords(Snapshot snapshot) + void onePhaseCommitRecords(TransactionContext context) throws CommitConflictException, UnknownTransactionStatusException { try { OnePhaseCommitMutationComposer composer = - new OnePhaseCommitMutationComposer(snapshot.getId(), tableMetadataManager); - snapshot.to(composer); + new OnePhaseCommitMutationComposer(context.transactionId, tableMetadataManager); + context.snapshot.to(composer); // One-phase commit does not require grouping mutations and using the parallel executor since // it is always executed in a single mutate API call. storage.mutate(composer.get()); } catch (NoMutationException e) { throw new CommitConflictException( - CoreError.CONSENSUS_COMMIT_PREPARING_RECORD_EXISTS.buildMessage(), e, snapshot.getId()); + CoreError.CONSENSUS_COMMIT_PREPARING_RECORD_EXISTS.buildMessage(), + e, + context.transactionId); } catch (RetriableExecutionException e) { throw new CommitConflictException( CoreError.CONSENSUS_COMMIT_CONFLICT_OCCURRED_WHEN_COMMITTING_RECORDS.buildMessage(), e, - snapshot.getId()); + context.transactionId); } catch (ExecutionException e) { throw new UnknownTransactionStatusException( - CoreError.CONSENSUS_COMMIT_COMMITTING_RECORDS_FAILED.buildMessage(), e, snapshot.getId()); + CoreError.CONSENSUS_COMMIT_COMMITTING_RECORDS_FAILED.buildMessage(), + e, + context.transactionId); } } - public void prepareRecords(Snapshot snapshot) throws PreparationException { + public void prepareRecords(TransactionContext context) throws PreparationException { try { PrepareMutationComposer composer = - new PrepareMutationComposer(snapshot.getId(), tableMetadataManager); - snapshot.to(composer); + new PrepareMutationComposer(context.transactionId, tableMetadataManager); + context.snapshot.to(composer); List> groupedMutations = mutationsGrouper.groupMutations(composer.get()); List tasks = new ArrayList<>(groupedMutations.size()); for (List mutations : groupedMutations) { tasks.add(() -> storage.mutate(mutations)); } - parallelExecutor.prepareRecords(tasks, snapshot.getId()); + parallelExecutor.prepareRecords(tasks, context.transactionId); } catch (NoMutationException e) { throw new PreparationConflictException( - CoreError.CONSENSUS_COMMIT_PREPARING_RECORD_EXISTS.buildMessage(), e, snapshot.getId()); + CoreError.CONSENSUS_COMMIT_PREPARING_RECORD_EXISTS.buildMessage(), + e, + context.transactionId); } catch (RetriableExecutionException e) { throw new PreparationConflictException( CoreError.CONSENSUS_COMMIT_CONFLICT_OCCURRED_WHEN_PREPARING_RECORDS.buildMessage(), e, - snapshot.getId()); + context.transactionId); } catch (ExecutionException e) { throw new PreparationException( - CoreError.CONSENSUS_COMMIT_PREPARING_RECORDS_FAILED.buildMessage(), e, snapshot.getId()); + CoreError.CONSENSUS_COMMIT_PREPARING_RECORDS_FAILED.buildMessage(), + e, + context.transactionId); } } - public void validateRecords(Snapshot snapshot) throws ValidationException { + public void validateRecords(TransactionContext context) throws ValidationException { try { // validation is executed when SERIALIZABLE is chosen. - snapshot.toSerializable(storage); + context.snapshot.toSerializable(storage); } catch (ExecutionException e) { throw new ValidationException( - CoreError.CONSENSUS_COMMIT_VALIDATION_FAILED.buildMessage(), e, snapshot.getId()); + CoreError.CONSENSUS_COMMIT_VALIDATION_FAILED.buildMessage(), e, context.transactionId); } } - public void commitState(Snapshot snapshot) + public void commitState(TransactionContext context) throws CommitConflictException, UnknownTransactionStatusException { - String id = snapshot.getId(); + String id = context.transactionId; try { Coordinator.State state = new Coordinator.State(id, TransactionState.COMMITTED); coordinator.putState(state); logger.debug( "Transaction {} is committed successfully at {}", id, System.currentTimeMillis()); } catch (CoordinatorConflictException e) { - handleCommitConflict(snapshot, e); + handleCommitConflict(context, e); } catch (CoordinatorException e) { throw new UnknownTransactionStatusException( CoreError.CONSENSUS_COMMIT_UNKNOWN_COORDINATOR_STATUS.buildMessage(), e, id); } } - public void commitRecords(Snapshot snapshot) { + public void commitRecords(TransactionContext context) { try { CommitMutationComposer composer = - new CommitMutationComposer(snapshot.getId(), tableMetadataManager); - snapshot.to(composer); + new CommitMutationComposer(context.transactionId, tableMetadataManager); + context.snapshot.to(composer); List> groupedMutations = mutationsGrouper.groupMutations(composer.get()); List tasks = new ArrayList<>(groupedMutations.size()); for (List mutations : groupedMutations) { tasks.add(() -> storage.mutate(mutations)); } - parallelExecutor.commitRecords(tasks, snapshot.getId()); + parallelExecutor.commitRecords(tasks, context.transactionId); } catch (Exception e) { - logger.warn("Committing records failed. Transaction ID: {}", snapshot.getId(), e); + logger.info("Committing records failed. Transaction ID: {}", context.transactionId, e); // ignore since records are recovered lazily } } @@ -383,34 +401,33 @@ public TransactionState abortState(String id) throws UnknownTransactionStatusExc } } - public void rollbackRecords(Snapshot snapshot) { - logger.debug("Rollback from snapshot for {}", snapshot.getId()); + public void rollbackRecords(TransactionContext context) { + logger.debug("Rollback from snapshot for {}", context.transactionId); try { RollbackMutationComposer composer = - new RollbackMutationComposer(snapshot.getId(), storage, tableMetadataManager); - snapshot.to(composer); + new RollbackMutationComposer(context.transactionId, storage, tableMetadataManager); + context.snapshot.to(composer); List> groupedMutations = mutationsGrouper.groupMutations(composer.get()); List tasks = new ArrayList<>(groupedMutations.size()); for (List mutations : groupedMutations) { tasks.add(() -> storage.mutate(mutations)); } - parallelExecutor.rollbackRecords(tasks, snapshot.getId()); + parallelExecutor.rollbackRecords(tasks, context.transactionId); } catch (Exception e) { - logger.warn("Rolling back records failed. Transaction ID: {}", snapshot.getId(), e); + logger.info("Rolling back records failed. Transaction ID: {}", context.transactionId, e); // ignore since records are recovered lazily } } /** - * Sets the {@link BeforePreparationSnapshotHook}. This method must be called immediately after - * the constructor is invoked. + * Sets the {@link BeforePreparationHook}. This method must be called immediately after the + * constructor is invoked. * - * @param beforePreparationSnapshotHook The snapshot hook to set. + * @param beforePreparationHook The before-preparation hook to set. * @throws NullPointerException If the argument is null. */ - public void setBeforePreparationSnapshotHook( - BeforePreparationSnapshotHook beforePreparationSnapshotHook) { - this.beforePreparationSnapshotHook = checkNotNull(beforePreparationSnapshotHook); + public void setBeforePreparationHook(BeforePreparationHook beforePreparationHook) { + this.beforePreparationHook = checkNotNull(beforePreparationHook); } } 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 6918f7c3ba..e84eff3711 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 @@ -48,55 +48,57 @@ public CommitHandlerWithGroupCommit( } @Override - public void commit(Snapshot snapshot, boolean readOnly) + public void commit(TransactionContext context) throws CommitException, UnknownTransactionStatusException { - if (!readOnly && !snapshot.hasWritesOrDeletes() && coordinatorWriteOmissionOnReadOnlyEnabled) { - cancelGroupCommitIfNeeded(snapshot.getId()); + if (!context.readOnly + && !context.snapshot.hasWritesOrDeletes() + && coordinatorWriteOmissionOnReadOnlyEnabled) { + cancelGroupCommitIfNeeded(context.transactionId); } - super.commit(snapshot, readOnly); + super.commit(context); } @Override - boolean canOnePhaseCommit(Snapshot snapshot) throws CommitException { + boolean canOnePhaseCommit(TransactionContext context) throws CommitException { try { - return super.canOnePhaseCommit(snapshot); + return super.canOnePhaseCommit(context); } catch (CommitException e) { - cancelGroupCommitIfNeeded(snapshot.getId()); + cancelGroupCommitIfNeeded(context.transactionId); throw e; } } @Override - void onePhaseCommitRecords(Snapshot snapshot) + void onePhaseCommitRecords(TransactionContext context) throws CommitConflictException, UnknownTransactionStatusException { - cancelGroupCommitIfNeeded(snapshot.getId()); - super.onePhaseCommitRecords(snapshot); + cancelGroupCommitIfNeeded(context.transactionId); + super.onePhaseCommitRecords(context); } @Override - protected void onFailureBeforeCommit(Snapshot snapshot) { - cancelGroupCommitIfNeeded(snapshot.getId()); + protected void onFailureBeforeCommit(TransactionContext context) { + cancelGroupCommitIfNeeded(context.transactionId); } - private void commitStateViaGroupCommit(Snapshot snapshot) + private void commitStateViaGroupCommit(TransactionContext context) throws CommitConflictException, UnknownTransactionStatusException { - String id = snapshot.getId(); + String id = context.transactionId; try { // Group commit the state by internally calling `groupCommitState()` via the emitter. - groupCommitter.ready(id, snapshot); + groupCommitter.ready(id, context); logger.debug( "Transaction {} is committed successfully at {}", id, System.currentTimeMillis()); } catch (GroupCommitConflictException e) { cancelGroupCommitIfNeeded(id); // Throw a proper exception from this method if needed. - handleCommitConflict(snapshot, e); + handleCommitConflict(context, e); } catch (GroupCommitException e) { cancelGroupCommitIfNeeded(id); Throwable cause = e.getCause(); if (cause instanceof CoordinatorConflictException) { // Throw a proper exception from this method if needed. - handleCommitConflict(snapshot, (CoordinatorConflictException) cause); + handleCommitConflict(context, (CoordinatorConflictException) cause); } else { // Failed to access the coordinator state. The state is unknown. throw new UnknownTransactionStatusException("Coordinator status is unknown", cause, id); @@ -118,9 +120,9 @@ private void cancelGroupCommitIfNeeded(String id) { } @Override - public void commitState(Snapshot snapshot) + public void commitState(TransactionContext context) throws CommitConflictException, UnknownTransactionStatusException { - commitStateViaGroupCommit(snapshot); + commitStateViaGroupCommit(context); } @Override @@ -129,7 +131,7 @@ public TransactionState abortState(String id) throws UnknownTransactionStatusExc return super.abortState(id); } - private static class Emitter implements Emittable { + private static class Emitter implements Emittable { private final Coordinator coordinator; public Emitter(Coordinator coordinator) { @@ -137,9 +139,9 @@ public Emitter(Coordinator coordinator) { } @Override - public void emitNormalGroup(String parentId, List snapshots) + public void emitNormalGroup(String parentId, List contexts) throws CoordinatorException { - if (snapshots.isEmpty()) { + if (contexts.isEmpty()) { // This means all buffered transactions were manually rolled back. Nothing to do. return; } @@ -147,7 +149,7 @@ public void emitNormalGroup(String parentId, List snapshots) // These transactions are contained in a normal group that has multiple transactions. // Therefore, the transaction states should be put together in Coordinator.State. List transactionIds = - snapshots.stream().map(Snapshot::getId).collect(Collectors.toList()); + contexts.stream().map(c -> c.transactionId).collect(Collectors.toList()); coordinator.putStateForGroupCommit( parentId, transactionIds, TransactionState.COMMITTED, System.currentTimeMillis()); @@ -159,7 +161,8 @@ public void emitNormalGroup(String parentId, List snapshots) } @Override - public void emitDelayedGroup(String fullId, Snapshot snapshot) throws CoordinatorException { + public void emitDelayedGroup(String fullId, TransactionContext context) + throws CoordinatorException { // This transaction is contained in a delayed group that has only a single transaction. // Therefore, the transaction state can be committed as if it's a normal commit (not a // group commit). diff --git a/core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommit.java b/core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommit.java index a94fb33307..7e89239d88 100644 --- a/core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommit.java +++ b/core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommit.java @@ -47,6 +47,7 @@ @NotThreadSafe public class ConsensusCommit extends AbstractDistributedTransaction { private static final Logger logger = LoggerFactory.getLogger(ConsensusCommit.class); + private final TransactionContext context; private final CrudHandler crud; private final CommitHandler commit; private final ConsensusCommitMutationOperationChecker mutationOperationChecker; @@ -54,10 +55,12 @@ public class ConsensusCommit extends AbstractDistributedTransaction { @SuppressFBWarnings("EI_EXPOSE_REP2") public ConsensusCommit( + TransactionContext context, CrudHandler crud, CommitHandler commit, ConsensusCommitMutationOperationChecker mutationOperationChecker, @Nullable CoordinatorGroupCommitter groupCommitter) { + this.context = checkNotNull(context); this.crud = checkNotNull(crud); this.commit = checkNotNull(commit); this.mutationOperationChecker = mutationOperationChecker; @@ -66,23 +69,23 @@ public ConsensusCommit( @Override public String getId() { - return crud.getSnapshot().getId(); + return context.transactionId; } @Override public Optional get(Get get) throws CrudException { - return crud.get(copyAndSetTargetToIfNot(get)); + return crud.get(copyAndSetTargetToIfNot(get), context); } @Override public List scan(Scan scan) throws CrudException { - return crud.scan(copyAndSetTargetToIfNot(scan)); + return crud.scan(copyAndSetTargetToIfNot(scan), context); } @Override public Scanner getScanner(Scan scan) throws CrudException { scan = copyAndSetTargetToIfNot(scan); - return crud.getScanner(scan); + return crud.getScanner(scan, context); } /** @deprecated As of release 3.13.0. Will be removed in release 5.0.0. */ @@ -91,7 +94,7 @@ public Scanner getScanner(Scan scan) throws CrudException { public void put(Put put) throws CrudException { put = copyAndSetTargetToIfNot(put); checkMutation(put); - crud.put(put); + crud.put(put, context); } /** @deprecated As of release 3.13.0. Will be removed in release 5.0.0. */ @@ -108,7 +111,7 @@ public void put(List puts) throws CrudException { public void delete(Delete delete) throws CrudException { delete = copyAndSetTargetToIfNot(delete); checkMutation(delete); - crud.delete(delete); + crud.delete(delete, context); } /** @deprecated As of release 3.13.0. Will be removed in release 5.0.0. */ @@ -126,7 +129,7 @@ public void insert(Insert insert) throws CrudException { insert = copyAndSetTargetToIfNot(insert); Put put = ConsensusCommitUtils.createPutForInsert(insert); checkMutation(put); - crud.put(put); + crud.put(put, context); } @Override @@ -134,7 +137,7 @@ public void upsert(Upsert upsert) throws CrudException { upsert = copyAndSetTargetToIfNot(upsert); Put put = ConsensusCommitUtils.createPutForUpsert(upsert); checkMutation(put); - crud.put(put); + crud.put(put, context); } @Override @@ -144,13 +147,13 @@ public void update(Update update) throws CrudException { Put put = ConsensusCommitUtils.createPutForUpdate(update); checkMutation(put); try { - crud.put(put); + crud.put(put, context); } catch (UnsatisfiedConditionException e) { if (update.getCondition().isPresent()) { throw new UnsatisfiedConditionException( ConsensusCommitUtils.convertUnsatisfiedConditionExceptionMessageForUpdate( e, update.getCondition().get()), - crud.getSnapshot().getId()); + getId()); } // If the condition is not specified, it means that the record does not exist. In this case, @@ -179,13 +182,13 @@ public void mutate(List mutations) throws CrudException { @Override public void commit() throws CommitException, UnknownTransactionStatusException { - if (!crud.areAllScannersClosed()) { + if (!context.areAllScannersClosed()) { throw new IllegalStateException(CoreError.CONSENSUS_COMMIT_SCANNER_NOT_CLOSED.buildMessage()); } // Execute implicit pre-read try { - crud.readIfImplicitPreReadEnabled(); + crud.readIfImplicitPreReadEnabled(context); } catch (CrudConflictException e) { throw new CommitConflictException( CoreError.CONSENSUS_COMMIT_CONFLICT_OCCURRED_WHILE_IMPLICIT_PRE_READ.buildMessage(), @@ -197,29 +200,34 @@ public void commit() throws CommitException, UnknownTransactionStatusException { } try { - crud.waitForRecoveryCompletionIfNecessary(); + crud.waitForRecoveryCompletionIfNecessary(context); } catch (CrudConflictException e) { throw new CommitConflictException(e.getMessage(), e, getId()); } catch (CrudException e) { throw new CommitException(e.getMessage(), e, getId()); } - commit.commit(crud.getSnapshot(), crud.isReadOnly()); + commit.commit(context); } @Override public void rollback() { try { - crud.closeScanners(); + context.closeScanners(); } catch (CrudException e) { logger.warn("Failed to close the scanner", e); } - if (groupCommitter != null && !crud.isReadOnly()) { - groupCommitter.remove(crud.getSnapshot().getId()); + if (groupCommitter != null && !context.readOnly) { + groupCommitter.remove(getId()); } } + @VisibleForTesting + TransactionContext getTransactionContext() { + return context; + } + @VisibleForTesting CrudHandler getCrudHandler() { return crud; @@ -230,6 +238,11 @@ CommitHandler getCommitHandler() { return commit; } + @VisibleForTesting + void waitForRecoveryCompletion() throws CrudException { + crud.waitForRecoveryCompletion(context); + } + private void checkMutation(Mutation mutation) throws CrudException { try { mutationOperationChecker.check(mutation); diff --git a/core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitManager.java b/core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitManager.java index bf8c78249c..12b4875f15 100644 --- a/core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitManager.java +++ b/core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitManager.java @@ -53,9 +53,9 @@ public class ConsensusCommitManager extends AbstractDistributedTransactionManage private final Coordinator coordinator; private final ParallelExecutor parallelExecutor; private final RecoveryExecutor recoveryExecutor; + private final CrudHandler crud; protected final CommitHandler commit; private final Isolation isolation; - private final boolean isIncludeMetadataEnabled; private final ConsensusCommitMutationOperationChecker mutationOperationChecker; @Nullable private final CoordinatorGroupCommitter groupCommitter; @@ -75,9 +75,15 @@ public ConsensusCommitManager( RecoveryHandler recovery = new RecoveryHandler(storage, coordinator, tableMetadataManager); recoveryExecutor = new RecoveryExecutor(coordinator, recovery, tableMetadataManager); groupCommitter = CoordinatorGroupCommitter.from(config).orElse(null); + crud = + new CrudHandler( + storage, + recoveryExecutor, + tableMetadataManager, + config.isIncludeMetadataEnabled(), + parallelExecutor); commit = createCommitHandler(config); isolation = config.getIsolation(); - isIncludeMetadataEnabled = config.isIncludeMetadataEnabled(); mutationOperationChecker = new ConsensusCommitMutationOperationChecker(tableMetadataManager); } @@ -96,9 +102,15 @@ protected ConsensusCommitManager(DatabaseConfig databaseConfig) { RecoveryHandler recovery = new RecoveryHandler(storage, coordinator, tableMetadataManager); recoveryExecutor = new RecoveryExecutor(coordinator, recovery, tableMetadataManager); groupCommitter = CoordinatorGroupCommitter.from(config).orElse(null); + crud = + new CrudHandler( + storage, + recoveryExecutor, + tableMetadataManager, + config.isIncludeMetadataEnabled(), + parallelExecutor); commit = createCommitHandler(config); isolation = config.getIsolation(); - isIncludeMetadataEnabled = config.isIncludeMetadataEnabled(); mutationOperationChecker = new ConsensusCommitMutationOperationChecker(tableMetadataManager); } @@ -111,9 +123,9 @@ protected ConsensusCommitManager(DatabaseConfig databaseConfig) { Coordinator coordinator, ParallelExecutor parallelExecutor, RecoveryExecutor recoveryExecutor, + CrudHandler crud, CommitHandler commit, Isolation isolation, - boolean isIncludeMetadataEnabled, @Nullable CoordinatorGroupCommitter groupCommitter) { super(databaseConfig); this.storage = storage; @@ -124,10 +136,10 @@ protected ConsensusCommitManager(DatabaseConfig databaseConfig) { this.coordinator = coordinator; this.parallelExecutor = parallelExecutor; this.recoveryExecutor = recoveryExecutor; + this.crud = crud; this.commit = commit; this.groupCommitter = groupCommitter; this.isolation = isolation; - this.isIncludeMetadataEnabled = isIncludeMetadataEnabled; this.mutationOperationChecker = new ConsensusCommitMutationOperationChecker(tableMetadataManager); } @@ -244,18 +256,10 @@ DistributedTransaction begin( + "anomalies"); } Snapshot snapshot = new Snapshot(txId, isolation, tableMetadataManager, parallelExecutor); - CrudHandler crud = - new CrudHandler( - storage, - snapshot, - recoveryExecutor, - tableMetadataManager, - isIncludeMetadataEnabled, - parallelExecutor, - readOnly, - oneOperation); + TransactionContext context = + new TransactionContext(txId, snapshot, isolation, readOnly, oneOperation); DistributedTransaction transaction = - new ConsensusCommit(crud, commit, mutationOperationChecker, groupCommitter); + new ConsensusCommit(context, crud, commit, mutationOperationChecker, groupCommitter); if (readOnly) { transaction = new ReadOnlyDistributedTransaction(transaction); } diff --git a/core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitScanner.java b/core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitScanner.java new file mode 100644 index 0000000000..fcedfe034a --- /dev/null +++ b/core/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitScanner.java @@ -0,0 +1,7 @@ +package com.scalar.db.transaction.consensuscommit; + +import com.scalar.db.api.TransactionCrudOperable; + +public interface ConsensusCommitScanner extends TransactionCrudOperable.Scanner { + boolean isClosed(); +} diff --git a/core/src/main/java/com/scalar/db/transaction/consensuscommit/CoordinatorGroupCommitter.java b/core/src/main/java/com/scalar/db/transaction/consensuscommit/CoordinatorGroupCommitter.java index 3ceb64c5f8..606ce19b35 100644 --- a/core/src/main/java/com/scalar/db/transaction/consensuscommit/CoordinatorGroupCommitter.java +++ b/core/src/main/java/com/scalar/db/transaction/consensuscommit/CoordinatorGroupCommitter.java @@ -6,7 +6,7 @@ import java.util.Optional; public class CoordinatorGroupCommitter - extends GroupCommitter { + extends GroupCommitter { CoordinatorGroupCommitter(GroupCommitConfig config) { super("coordinator", config, new CoordinatorGroupCommitKeyManipulator()); } diff --git a/core/src/main/java/com/scalar/db/transaction/consensuscommit/CrudHandler.java b/core/src/main/java/com/scalar/db/transaction/consensuscommit/CrudHandler.java index 0c34613e8c..9f6a21bfad 100644 --- a/core/src/main/java/com/scalar/db/transaction/consensuscommit/CrudHandler.java +++ b/core/src/main/java/com/scalar/db/transaction/consensuscommit/CrudHandler.java @@ -43,79 +43,56 @@ import java.util.stream.Collectors; import javax.annotation.Nullable; import javax.annotation.concurrent.NotThreadSafe; +import javax.annotation.concurrent.ThreadSafe; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -@NotThreadSafe +@ThreadSafe public class CrudHandler { private static final Logger logger = LoggerFactory.getLogger(CrudHandler.class); private final DistributedStorage storage; - private final Snapshot snapshot; private final RecoveryExecutor recoveryExecutor; private final TransactionTableMetadataManager tableMetadataManager; private final boolean isIncludeMetadataEnabled; private final MutationConditionsValidator mutationConditionsValidator; private final ParallelExecutor parallelExecutor; - // Whether the transaction is in read-only mode or not. - private final boolean readOnly; - - // Whether the transaction is in one-operation mode or not. One-operation mode refers to executing - // a CRUD operation directly through `DistributedTransactionManager` without explicitly beginning - // a transaction. - private final boolean oneOperation; - - private final List scanners = new ArrayList<>(); - private final List recoveryResults = new ArrayList<>(); - @SuppressFBWarnings("EI_EXPOSE_REP2") public CrudHandler( DistributedStorage storage, - Snapshot snapshot, RecoveryExecutor recoveryExecutor, TransactionTableMetadataManager tableMetadataManager, boolean isIncludeMetadataEnabled, - ParallelExecutor parallelExecutor, - boolean readOnly, - boolean oneOperation) { + ParallelExecutor parallelExecutor) { this.storage = checkNotNull(storage); - this.snapshot = checkNotNull(snapshot); this.recoveryExecutor = checkNotNull(recoveryExecutor); this.tableMetadataManager = checkNotNull(tableMetadataManager); this.isIncludeMetadataEnabled = isIncludeMetadataEnabled; - this.mutationConditionsValidator = new MutationConditionsValidator(snapshot.getId()); + this.mutationConditionsValidator = new MutationConditionsValidator(); this.parallelExecutor = checkNotNull(parallelExecutor); - this.readOnly = readOnly; - this.oneOperation = oneOperation; } @VisibleForTesting CrudHandler( DistributedStorage storage, - Snapshot snapshot, RecoveryExecutor recoveryExecutor, TransactionTableMetadataManager tableMetadataManager, boolean isIncludeMetadataEnabled, MutationConditionsValidator mutationConditionsValidator, - ParallelExecutor parallelExecutor, - boolean readOnly, - boolean oneOperation) { + ParallelExecutor parallelExecutor) { this.storage = checkNotNull(storage); - this.snapshot = checkNotNull(snapshot); this.recoveryExecutor = checkNotNull(recoveryExecutor); this.tableMetadataManager = checkNotNull(tableMetadataManager); this.isIncludeMetadataEnabled = isIncludeMetadataEnabled; this.mutationConditionsValidator = checkNotNull(mutationConditionsValidator); this.parallelExecutor = checkNotNull(parallelExecutor); - this.readOnly = readOnly; - this.oneOperation = oneOperation; } - public Optional get(Get originalGet) throws CrudException { + public Optional get(Get originalGet, TransactionContext context) throws CrudException { List originalProjections = new ArrayList<>(originalGet.getProjections()); Get get = (Get) prepareStorageSelection(originalGet); - TableMetadata metadata = getTableMetadata(get); + TableMetadata metadata = getTableMetadata(get, context.transactionId); Snapshot.Key key; if (ScalarDbUtils.isSecondaryIndexSpecified(get, metadata)) { @@ -125,14 +102,16 @@ public Optional get(Get originalGet) throws CrudException { key = new Snapshot.Key(get); } - if (isSnapshotReadRequired()) { - readUnread(key, get); - return snapshot + if (isSnapshotReadRequired(context)) { + readUnread(key, get, context); + return context + .snapshot .getResult(key, get) .map(r -> new FilteredResult(r, originalProjections, metadata, isIncludeMetadataEnabled)); } else { - Optional result = read(key, get); - return snapshot + Optional result = read(key, get, context); + return context + .snapshot .mergeResult(key, result, get.getConjunctions()) .map(r -> new FilteredResult(r, originalProjections, metadata, isIncludeMetadataEnabled)); } @@ -140,27 +119,30 @@ public Optional get(Get originalGet) throws CrudException { // Only for a Get with index, the argument `key` is null @VisibleForTesting - void readUnread(@Nullable Snapshot.Key key, Get get) throws CrudException { - if (!snapshot.containsKeyInGetSet(get)) { - read(key, get); + void readUnread(@Nullable Snapshot.Key key, Get get, TransactionContext context) + throws CrudException { + if (!context.snapshot.containsKeyInGetSet(get)) { + read(key, get, context); } } // Although this class is not thread-safe, this method is actually thread-safe, so we call it // concurrently in the implicit pre-read @VisibleForTesting - Optional read(@Nullable Snapshot.Key key, Get get) throws CrudException { - Optional result = getFromStorage(get); + Optional read(@Nullable Snapshot.Key key, Get get, TransactionContext context) + throws CrudException { + Optional result = getFromStorage(get, context); if (result.isPresent() && !result.get().isCommitted()) { // Lazy recovery if (key == null) { // Only for a Get with index, the argument `key` is null. In that case, create a key from // the result - key = new Snapshot.Key(get, result.get()); + TableMetadata tableMetadata = getTableMetadata(get, context.transactionId); + key = new Snapshot.Key(get, result.get(), tableMetadata); } - result = executeRecovery(key, get, result.get()); + result = executeRecovery(key, get, result.get(), context); } if (!get.getConjunctions().isEmpty()) { @@ -179,29 +161,31 @@ Optional read(@Nullable Snapshot.Key key, Get get) throws Cru // due to the conjunction. if (key != null) { - putIntoReadSetInSnapshot(key, result); + putIntoReadSetInSnapshot(key, result, context); } else { // Only for a Get with index, the argument `key` is null if (result.isPresent()) { // Only when we can get the record with the Get with index, we can put it into the read // set - key = new Snapshot.Key(get, result.get()); - putIntoReadSetInSnapshot(key, result); + TableMetadata tableMetadata = getTableMetadata(get, context.transactionId); + key = new Snapshot.Key(get, result.get(), tableMetadata); + putIntoReadSetInSnapshot(key, result, context); } } } - putIntoGetSetInSnapshot(get, result); + putIntoGetSetInSnapshot(get, result, context); return result; } private Optional executeRecovery( - Snapshot.Key key, Selection selection, TransactionResult result) throws CrudException { + Snapshot.Key key, Selection selection, TransactionResult result, TransactionContext context) + throws CrudException { RecoveryExecutor.RecoveryType recoveryType; - if (snapshot.getIsolation() == Isolation.READ_COMMITTED) { + if (context.isolation == Isolation.READ_COMMITTED) { // In READ_COMMITTED isolation - if (readOnly) { + if (context.readOnly) { // In read-only mode, we don't recover the record, but return the committed result recoveryType = RecoveryExecutor.RecoveryType.RETURN_COMMITTED_RESULT_AND_NOT_RECOVER; } else { @@ -215,28 +199,28 @@ private Optional executeRecovery( } RecoveryExecutor.Result recoveryResult = - recoveryExecutor.execute(key, selection, result, snapshot.getId(), recoveryType); + recoveryExecutor.execute(key, selection, result, context.transactionId, recoveryType); - recoveryResults.add(recoveryResult); + context.recoveryResults.add(recoveryResult); return recoveryResult.recoveredResult; } - public List scan(Scan originalScan) throws CrudException { + public List scan(Scan originalScan, TransactionContext context) throws CrudException { List originalProjections = new ArrayList<>(originalScan.getProjections()); Scan scan = (Scan) prepareStorageSelection(originalScan); - LinkedHashMap results = scanInternal(scan); - verifyNoOverlap(scan, results); + LinkedHashMap results = scanInternal(scan, context); + verifyNoOverlap(scan, results, context); - TableMetadata metadata = getTableMetadata(scan); + TableMetadata metadata = getTableMetadata(scan, context.transactionId); return results.values().stream() .map(r -> new FilteredResult(r, originalProjections, metadata, isIncludeMetadataEnabled)) .collect(Collectors.toList()); } - private LinkedHashMap scanInternal(Scan scan) - throws CrudException { + private LinkedHashMap scanInternal( + Scan scan, TransactionContext context) throws CrudException { Optional> resultsInSnapshot = - snapshot.getResults(scan); + context.snapshot.getResults(scan); if (resultsInSnapshot.isPresent()) { return resultsInSnapshot.get(); } @@ -248,15 +232,17 @@ private LinkedHashMap scanInternal(Scan scan) if (scan.getLimit() > 0) { // Since recovery and conjunctions may delete some records from the scan result, it is // necessary to perform the scan without a limit. - scanner = scanFromStorage(Scan.newBuilder(scan).limit(0).build()); + scanner = scanFromStorage(Scan.newBuilder(scan).limit(0).build(), context); } else { - scanner = scanFromStorage(scan); + scanner = scanFromStorage(scan, context); } for (Result r : scanner) { TransactionResult result = new TransactionResult(r); - Snapshot.Key key = new Snapshot.Key(scan, r); - Optional processedScanResult = processScanResult(key, scan, result); + TableMetadata tableMetadata = getTableMetadata(scan, context.transactionId); + Snapshot.Key key = new Snapshot.Key(scan, r, tableMetadata); + Optional processedScanResult = + processScanResult(key, scan, result, context); processedScanResult.ifPresent(res -> results.put(key, res)); if (scan.getLimit() > 0 && results.size() >= scan.getLimit()) { @@ -274,28 +260,29 @@ private LinkedHashMap scanInternal(Scan scan) throw new CrudException( CoreError.CONSENSUS_COMMIT_SCANNING_RECORDS_FROM_STORAGE_FAILED.buildMessage(), exception, - snapshot.getId()); + context.transactionId); } finally { if (scanner != null) { try { scanner.close(); } catch (IOException e) { - logger.warn("Failed to close the scanner", e); + logger.warn("Failed to close the scanner. Transaction ID: {}", context.transactionId, e); } } } - putIntoScanSetInSnapshot(scan, results); + putIntoScanSetInSnapshot(scan, results, context); return results; } private Optional processScanResult( - Snapshot.Key key, Scan scan, TransactionResult result) throws CrudException { + Snapshot.Key key, Scan scan, TransactionResult result, TransactionContext context) + throws CrudException { Optional ret; if (!result.isCommitted()) { // Lazy recovery - ret = executeRecovery(key, scan, result); + ret = executeRecovery(key, scan, result, context); } else { ret = Optional.of(result); } @@ -311,100 +298,97 @@ private Optional processScanResult( } if (ret.isPresent()) { - putIntoReadSetInSnapshot(key, ret); + putIntoReadSetInSnapshot(key, ret, context); } return ret; } - public TransactionCrudOperable.Scanner getScanner(Scan originalScan) throws CrudException { + public TransactionCrudOperable.Scanner getScanner(Scan originalScan, TransactionContext context) + throws CrudException { List originalProjections = new ArrayList<>(originalScan.getProjections()); Scan scan = (Scan) prepareStorageSelection(originalScan); ConsensusCommitScanner scanner; Optional> resultsInSnapshot = - snapshot.getResults(scan); + context.snapshot.getResults(scan); if (resultsInSnapshot.isPresent()) { scanner = - new ConsensusCommitSnapshotScanner(scan, originalProjections, resultsInSnapshot.get()); + new ConsensusCommitSnapshotScanner( + scan, originalProjections, context, resultsInSnapshot.get()); } else { - scanner = new ConsensusCommitStorageScanner(scan, originalProjections); + scanner = new ConsensusCommitStorageScanner(scan, originalProjections, context); } - scanners.add(scanner); + context.scanners.add(scanner); return scanner; } - public boolean areAllScannersClosed() { - return scanners.stream().allMatch(ConsensusCommitScanner::isClosed); - } - - public void closeScanners() throws CrudException { - for (ConsensusCommitScanner scanner : scanners) { - if (!scanner.isClosed()) { - scanner.close(); - } - } - } - - private void putIntoReadSetInSnapshot(Snapshot.Key key, Optional result) { + private void putIntoReadSetInSnapshot( + Snapshot.Key key, Optional result, TransactionContext context) { // In read-only mode, we don't need to put the result into the read set - if (!readOnly && !snapshot.containsKeyInReadSet(key)) { - snapshot.putIntoReadSet(key, result); + if (!context.readOnly && !context.snapshot.containsKeyInReadSet(key)) { + context.snapshot.putIntoReadSet(key, result); } } - private boolean isSnapshotReadRequired() { + private boolean isSnapshotReadRequired(TransactionContext context) { // In one-operation mode, we don't need snapshot reads - return !oneOperation && snapshot.isSnapshotReadRequired(); + return !context.oneOperation && context.isSnapshotReadRequired(); } - private boolean isValidationOrSnapshotReadRequired() { - return snapshot.isValidationRequired() || isSnapshotReadRequired(); + private boolean isValidationOrSnapshotReadRequired(TransactionContext context) { + return context.isValidationRequired() || isSnapshotReadRequired(context); } - private void putIntoGetSetInSnapshot(Get get, Optional result) { + private void putIntoGetSetInSnapshot( + Get get, Optional result, TransactionContext context) { // If neither validation nor snapshot reads are required, we don't need to put the result into // the get set - if (isValidationOrSnapshotReadRequired()) { - snapshot.putIntoGetSet(get, result); + if (isValidationOrSnapshotReadRequired(context)) { + context.snapshot.putIntoGetSet(get, result); } } private void putIntoScanSetInSnapshot( - Scan scan, LinkedHashMap results) { + Scan scan, + LinkedHashMap results, + TransactionContext context) { // If neither validation nor snapshot reads are required, we don't need to put the results into // the scan set - if (isValidationOrSnapshotReadRequired()) { - snapshot.putIntoScanSet(scan, results); + if (isValidationOrSnapshotReadRequired(context)) { + context.snapshot.putIntoScanSet(scan, results); } } private void putIntoScannerSetInSnapshot( - Scan scan, LinkedHashMap results) { + Scan scan, + LinkedHashMap results, + TransactionContext context) { // if validation is not required, we don't need to put the results into the scanner set - if (snapshot.isValidationRequired()) { - snapshot.putIntoScannerSet(scan, results); + if (context.isValidationRequired()) { + context.snapshot.putIntoScannerSet(scan, results); } } - private void verifyNoOverlap(Scan scan, Map results) { - if (isOverlapVerificationRequired()) { - snapshot.verifyNoOverlap(scan, results); + private void verifyNoOverlap( + Scan scan, Map results, TransactionContext context) { + if (isOverlapVerificationRequired(context)) { + context.snapshot.verifyNoOverlap(scan, results); } } - private boolean isOverlapVerificationRequired() { + private boolean isOverlapVerificationRequired(TransactionContext context) { // In either read-only mode or one-operation mode, we don't need to verify overlap - return !readOnly && !oneOperation; + return !context.readOnly && !context.oneOperation; } - public void put(Put put) throws CrudException { + public void put(Put put, TransactionContext context) throws CrudException { Snapshot.Key key = new Snapshot.Key(put); if (put.getCondition().isPresent() - && (!isImplicitPreReadEnabled(put) && !snapshot.containsKeyInReadSet(key))) { + && (!isImplicitPreReadEnabled(put) && !context.snapshot.containsKeyInReadSet(key))) { throw new IllegalArgumentException( CoreError .CONSENSUS_COMMIT_PUT_CANNOT_HAVE_CONDITION_WHEN_TARGET_RECORD_UNREAD_AND_IMPLICIT_PRE_READ_DISABLED @@ -412,54 +396,55 @@ public void put(Put put) throws CrudException { } if (put.getCondition().isPresent()) { - if (isImplicitPreReadEnabled(put) && !snapshot.containsKeyInReadSet(key)) { - read(key, createGet(key)); + if (isImplicitPreReadEnabled(put) && !context.snapshot.containsKeyInReadSet(key)) { + read(key, createGet(key), context); } mutationConditionsValidator.checkIfConditionIsSatisfied( - put, snapshot.getResult(key).orElse(null)); + put, context.snapshot.getResult(key).orElse(null), context); } - snapshot.putIntoWriteSet(key, put); + context.snapshot.putIntoWriteSet(key, put); } - public void delete(Delete delete) throws CrudException { + public void delete(Delete delete, TransactionContext context) throws CrudException { Snapshot.Key key = new Snapshot.Key(delete); if (delete.getCondition().isPresent()) { - if (!snapshot.containsKeyInReadSet(key)) { - read(key, createGet(key)); + if (!context.snapshot.containsKeyInReadSet(key)) { + read(key, createGet(key), context); } mutationConditionsValidator.checkIfConditionIsSatisfied( - delete, snapshot.getResult(key).orElse(null)); + delete, context.snapshot.getResult(key).orElse(null), context); } - snapshot.putIntoDeleteSet(key, delete); + context.snapshot.putIntoDeleteSet(key, delete); } - public void readIfImplicitPreReadEnabled() throws CrudException { + public void readIfImplicitPreReadEnabled(TransactionContext context) throws CrudException { List tasks = new ArrayList<>(); // For each put in the write set, if implicit pre-read is enabled and the record is not read // yet, read the record - for (Put put : snapshot.getPutsInWriteSet()) { + for (Map.Entry entry : context.snapshot.getWriteSet()) { + Put put = entry.getValue(); if (isImplicitPreReadEnabled(put)) { - Snapshot.Key key = new Snapshot.Key(put); - if (!snapshot.containsKeyInReadSet(key)) { - tasks.add(() -> read(key, createGet(key))); + Snapshot.Key key = entry.getKey(); + if (!context.snapshot.containsKeyInReadSet(key)) { + tasks.add(() -> read(key, createGet(key), context)); } } } // For each delete in the write set, if the record is not read yet, read the record - for (Delete delete : snapshot.getDeletesInDeleteSet()) { - Snapshot.Key key = new Snapshot.Key(delete); - if (!snapshot.containsKeyInReadSet(key)) { - tasks.add(() -> read(key, createGet(key))); + for (Map.Entry entry : context.snapshot.getDeleteSet()) { + Snapshot.Key key = entry.getKey(); + if (!context.snapshot.containsKeyInReadSet(key)) { + tasks.add(() -> read(key, createGet(key), context)); } } if (!tasks.isEmpty()) { - parallelExecutor.executeImplicitPreRead(tasks, snapshot.getId()); + parallelExecutor.executeImplicitPreRead(tasks, context.transactionId); } } @@ -499,58 +484,58 @@ private Get createGet(Snapshot.Key key) { * complete, the validation could fail due to records with PREPARED or DELETED status. * * + * @param context the transaction context * @throws CrudConflictException if any recovery task fails due to a conflict * @throws CrudException if any recovery task fails */ - public void waitForRecoveryCompletionIfNecessary() throws CrudException { - for (RecoveryExecutor.Result recoveryResult : recoveryResults) { + public void waitForRecoveryCompletionIfNecessary(TransactionContext context) + throws CrudException { + for (RecoveryExecutor.Result recoveryResult : context.recoveryResults) { try { - if (snapshot.containsKeyInWriteSet(recoveryResult.key) - || snapshot.containsKeyInDeleteSet(recoveryResult.key) - || snapshot.isValidationRequired()) { + if (context.snapshot.containsKeyInWriteSet(recoveryResult.key) + || context.snapshot.containsKeyInDeleteSet(recoveryResult.key) + || context.isValidationRequired()) { recoveryResult.recoveryFuture.get(); } } catch (java.util.concurrent.ExecutionException e) { - if (e.getCause() instanceof CrudConflictException) { - throw new CrudConflictException( - e.getCause().getMessage(), e.getCause(), snapshot.getId()); + Throwable cause = e.getCause(); + if (cause instanceof CrudConflictException) { + throw new CrudConflictException(cause.getMessage(), cause, context.transactionId); } throw new CrudException( - CoreError.CONSENSUS_COMMIT_RECOVERING_RECORDS_FAILED.buildMessage( - e.getCause().getMessage()), - e.getCause(), - snapshot.getId()); + CoreError.CONSENSUS_COMMIT_RECOVERING_RECORDS_FAILED.buildMessage(cause.getMessage()), + cause, + context.transactionId); } catch (Exception e) { throw new CrudException( CoreError.CONSENSUS_COMMIT_RECOVERING_RECORDS_FAILED.buildMessage(e.getMessage()), e, - snapshot.getId()); + context.transactionId); } } } @VisibleForTesting - void waitForRecoveryCompletion() throws CrudException { - for (RecoveryExecutor.Result recoveryResult : recoveryResults) { + void waitForRecoveryCompletion(TransactionContext context) throws CrudException { + for (RecoveryExecutor.Result recoveryResult : context.recoveryResults) { try { recoveryResult.recoveryFuture.get(); } catch (java.util.concurrent.ExecutionException e) { - if (e.getCause() instanceof CrudConflictException) { - throw new CrudConflictException( - e.getCause().getMessage(), e.getCause(), snapshot.getId()); + Throwable cause = e.getCause(); + if (cause instanceof CrudConflictException) { + throw new CrudConflictException(cause.getMessage(), cause, context.transactionId); } throw new CrudException( - CoreError.CONSENSUS_COMMIT_RECOVERING_RECORDS_FAILED.buildMessage( - e.getCause().getMessage()), - e.getCause(), - snapshot.getId()); + CoreError.CONSENSUS_COMMIT_RECOVERING_RECORDS_FAILED.buildMessage(cause.getMessage()), + cause, + context.transactionId); } catch (Exception e) { throw new CrudException( CoreError.CONSENSUS_COMMIT_RECOVERING_RECORDS_FAILED.buildMessage(e.getMessage()), e, - snapshot.getId()); + context.transactionId); } } } @@ -558,7 +543,8 @@ void waitForRecoveryCompletion() throws CrudException { // Although this class is not thread-safe, this method is actually thread-safe because the storage // is thread-safe @VisibleForTesting - Optional getFromStorage(Get get) throws CrudException { + Optional getFromStorage(Get get, TransactionContext context) + throws CrudException { try { if (get.getConjunctions().isEmpty()) { // If there are no conjunctions, we can read the record directly @@ -566,7 +552,7 @@ Optional getFromStorage(Get get) throws CrudException { } else { // If there are conjunctions, we need to convert them to include conditions on the before // image - Set converted = convertConjunctions(get, get.getConjunctions()); + Set converted = convertConjunctions(get, get.getConjunctions(), context); Get convertedGet = Get.newBuilder(get).clearConditions().whereOr(converted).build(); return storage.get(convertedGet).map(TransactionResult::new); } @@ -574,11 +560,11 @@ Optional getFromStorage(Get get) throws CrudException { throw new CrudException( CoreError.CONSENSUS_COMMIT_READING_RECORD_FROM_STORAGE_FAILED.buildMessage(), e, - snapshot.getId()); + context.transactionId); } } - private Scanner scanFromStorage(Scan scan) throws CrudException { + private Scanner scanFromStorage(Scan scan, TransactionContext context) throws CrudException { try { if (scan.getConjunctions().isEmpty()) { // If there are no conjunctions, we can read the record directly @@ -586,7 +572,7 @@ private Scanner scanFromStorage(Scan scan) throws CrudException { } else { // If there are conjunctions, we need to convert them to include conditions on the before // image - Set converted = convertConjunctions(scan, scan.getConjunctions()); + Set converted = convertConjunctions(scan, scan.getConjunctions(), context); Scan convertedScan = Scan.newBuilder(scan).clearConditions().whereOr(converted).build(); return storage.scan(convertedScan); } @@ -594,7 +580,7 @@ private Scanner scanFromStorage(Scan scan) throws CrudException { throw new CrudException( CoreError.CONSENSUS_COMMIT_SCANNING_RECORDS_FROM_STORAGE_FAILED.buildMessage(), e, - snapshot.getId()); + context.transactionId); } } @@ -661,11 +647,13 @@ private Scanner scanFromStorage(Scan scan) throws CrudException { * * @param selection the selection to convert * @param conjunctions the conjunctions to convert + * @param context the transaction context * @return the converted conjunctions */ private Set convertConjunctions( - Selection selection, Set conjunctions) throws CrudException { - TableMetadata metadata = getTableMetadata(selection); + Selection selection, Set conjunctions, TransactionContext context) + throws CrudException { + TableMetadata metadata = getTableMetadata(selection, context.transactionId); Set converted = new HashSet<>(conjunctions.size() * 2); @@ -717,40 +705,33 @@ private Selection prepareStorageSelection(Selection selection) { return selection; } - private TransactionTableMetadata getTransactionTableMetadata(Operation operation) - throws CrudException { + private TransactionTableMetadata getTransactionTableMetadata( + Operation operation, String transactionId) throws CrudException { + assert operation.forFullTableName().isPresent(); + try { return ConsensusCommitUtils.getTransactionTableMetadata(tableMetadataManager, operation); } catch (ExecutionException e) { throw new CrudException( - CoreError.GETTING_TABLE_METADATA_FAILED.buildMessage(), e, snapshot.getId()); + CoreError.GETTING_TABLE_METADATA_FAILED.buildMessage(operation.forFullTableName().get()), + e, + transactionId); } } - private TableMetadata getTableMetadata(Operation operation) throws CrudException { - TransactionTableMetadata metadata = getTransactionTableMetadata(operation); + private TableMetadata getTableMetadata(Operation operation, String transactionId) + throws CrudException { + TransactionTableMetadata metadata = getTransactionTableMetadata(operation, transactionId); return metadata.getTableMetadata(); } - @SuppressFBWarnings("EI_EXPOSE_REP") - public Snapshot getSnapshot() { - return snapshot; - } - - public boolean isReadOnly() { - return readOnly; - } - - private interface ConsensusCommitScanner extends TransactionCrudOperable.Scanner { - boolean isClosed(); - } - @NotThreadSafe private class ConsensusCommitStorageScanner extends AbstractTransactionCrudOperableScanner implements ConsensusCommitScanner { private final Scan scan; private final List originalProjections; + private final TransactionContext context; private final Scanner scanner; @Nullable private final LinkedHashMap results; @@ -758,20 +739,22 @@ private class ConsensusCommitStorageScanner extends AbstractTransactionCrudOpera private final AtomicBoolean fullyScanned = new AtomicBoolean(); private final AtomicBoolean closed = new AtomicBoolean(); - public ConsensusCommitStorageScanner(Scan scan, List originalProjections) + public ConsensusCommitStorageScanner( + Scan scan, List originalProjections, TransactionContext context) throws CrudException { this.scan = scan; this.originalProjections = originalProjections; + this.context = context; if (scan.getLimit() > 0) { // Since recovery and conjunctions may delete some records, it is necessary to perform the // scan without a limit. - scanner = scanFromStorage(Scan.newBuilder(scan).limit(0).build()); + scanner = scanFromStorage(Scan.newBuilder(scan).limit(0).build(), context); } else { - scanner = scanFromStorage(scan); + scanner = scanFromStorage(scan, context); } - if (isValidationOrSnapshotReadRequired() || isOverlapVerificationRequired()) { + if (isValidationOrSnapshotReadRequired(context) || isOverlapVerificationRequired(context)) { results = new LinkedHashMap<>(); } else { // If neither validation nor snapshot reads are required, we don't need to put the results @@ -795,10 +778,12 @@ public Optional one() throws CrudException { return Optional.empty(); } - Snapshot.Key key = new Snapshot.Key(scan, r.get()); + TableMetadata tableMetadata = getTableMetadata(scan, context.transactionId); + Snapshot.Key key = new Snapshot.Key(scan, r.get(), tableMetadata); TransactionResult result = new TransactionResult(r.get()); - Optional processedScanResult = processScanResult(key, scan, result); + Optional processedScanResult = + processScanResult(key, scan, result, context); if (!processedScanResult.isPresent()) { continue; } @@ -813,7 +798,7 @@ public Optional one() throws CrudException { fullyScanned.set(true); } - TableMetadata metadata = getTableMetadata(scan); + TableMetadata metadata = getTableMetadata(scan, context.transactionId); return Optional.of( new FilteredResult( processedScanResult.get(), @@ -826,7 +811,7 @@ public Optional one() throws CrudException { throw new CrudException( CoreError.CONSENSUS_COMMIT_SCANNING_RECORDS_FROM_STORAGE_FAILED.buildMessage(), e, - snapshot.getId()); + context.transactionId); } catch (CrudException e) { closeScanner(); throw e; @@ -859,13 +844,13 @@ public void close() { if (fullyScanned.get()) { // If the scanner is fully scanned, we can treat it as a normal scan, and put the results // into the scan set - putIntoScanSetInSnapshot(scan, results); + putIntoScanSetInSnapshot(scan, results, context); } else { // If the scanner is not fully scanned, put the results into the scanner set - putIntoScannerSetInSnapshot(scan, results); + putIntoScannerSetInSnapshot(scan, results, context); } - verifyNoOverlap(scan, results); + verifyNoOverlap(scan, results, context); } @Override @@ -878,7 +863,7 @@ private void closeScanner() { try { scanner.close(); } catch (IOException e) { - logger.warn("Failed to close the scanner", e); + logger.warn("Failed to close the scanner. Transaction ID: {}", context.transactionId, e); } } } @@ -889,6 +874,7 @@ private class ConsensusCommitSnapshotScanner extends AbstractTransactionCrudOper private final Scan scan; private final List originalProjections; + private final TransactionContext context; private final Iterator> resultsIterator; private final LinkedHashMap results = new LinkedHashMap<>(); @@ -897,9 +883,11 @@ private class ConsensusCommitSnapshotScanner extends AbstractTransactionCrudOper public ConsensusCommitSnapshotScanner( Scan scan, List originalProjections, + TransactionContext context, LinkedHashMap resultsInSnapshot) { this.scan = scan; this.originalProjections = originalProjections; + this.context = context; resultsIterator = resultsInSnapshot.entrySet().iterator(); } @@ -912,7 +900,7 @@ public Optional one() throws CrudException { Map.Entry entry = resultsIterator.next(); results.put(entry.getKey(), entry.getValue()); - TableMetadata metadata = getTableMetadata(scan); + TableMetadata metadata = getTableMetadata(scan, context.transactionId); return Optional.of( new FilteredResult( entry.getValue(), originalProjections, metadata, isIncludeMetadataEnabled)); @@ -936,7 +924,7 @@ public List all() throws CrudException { @Override public void close() { closed = true; - verifyNoOverlap(scan, results); + verifyNoOverlap(scan, results, context); } @Override diff --git a/core/src/main/java/com/scalar/db/transaction/consensuscommit/MutationConditionsValidator.java b/core/src/main/java/com/scalar/db/transaction/consensuscommit/MutationConditionsValidator.java index 2d165cf577..5e9b4c5767 100644 --- a/core/src/main/java/com/scalar/db/transaction/consensuscommit/MutationConditionsValidator.java +++ b/core/src/main/java/com/scalar/db/transaction/consensuscommit/MutationConditionsValidator.java @@ -24,11 +24,6 @@ */ @ThreadSafe public class MutationConditionsValidator { - private final String transactionId; - - public MutationConditionsValidator(String transactionId) { - this.transactionId = transactionId; - } /** * This checks if the condition of the specified Put operation is satisfied for the specified @@ -36,26 +31,28 @@ public MutationConditionsValidator(String transactionId) { * * @param put a Put operation * @param existingRecord the current value of the record targeted by the mutation, if any + * @param context the transaction context * @throws UnsatisfiedConditionException if the condition is not satisfied */ - public void checkIfConditionIsSatisfied(Put put, @Nullable TransactionResult existingRecord) + public void checkIfConditionIsSatisfied( + Put put, @Nullable TransactionResult existingRecord, TransactionContext context) throws UnsatisfiedConditionException { assert put.getCondition().isPresent(); MutationCondition condition = put.getCondition().get(); boolean recordExists = existingRecord != null; if (condition instanceof PutIf) { if (recordExists) { - validateConditionalExpressions(condition.getExpressions(), existingRecord); + validateConditionalExpressions(condition.getExpressions(), existingRecord, context); } else { - throwWhenRecordDoesNotExist(condition); + throwWhenRecordDoesNotExist(condition, context); } } else if (condition instanceof PutIfExists) { if (!recordExists) { - throwWhenRecordDoesNotExist(condition); + throwWhenRecordDoesNotExist(condition, context); } } else if (condition instanceof PutIfNotExists) { if (recordExists) { - throwWhenRecordExists(condition); + throwWhenRecordExists(condition, context); } } else { throw new AssertionError(); @@ -68,46 +65,50 @@ public void checkIfConditionIsSatisfied(Put put, @Nullable TransactionResult exi * * @param delete a Delete operation * @param existingRecord the current value of the record targeted by the mutation, if any + * @param context the transaction context * @throws UnsatisfiedConditionException if the condition is not satisfied */ - public void checkIfConditionIsSatisfied(Delete delete, @Nullable TransactionResult existingRecord) + public void checkIfConditionIsSatisfied( + Delete delete, @Nullable TransactionResult existingRecord, TransactionContext context) throws UnsatisfiedConditionException { assert delete.getCondition().isPresent(); MutationCondition condition = delete.getCondition().get(); boolean recordExists = existingRecord != null; if (condition instanceof DeleteIf) { if (recordExists) { - validateConditionalExpressions(condition.getExpressions(), existingRecord); + validateConditionalExpressions(condition.getExpressions(), existingRecord, context); } else { - throwWhenRecordDoesNotExist(condition); + throwWhenRecordDoesNotExist(condition, context); } } else if (condition instanceof DeleteIfExists) { if (!recordExists) { - throwWhenRecordDoesNotExist(condition); + throwWhenRecordDoesNotExist(condition, context); } } else { throw new AssertionError(); } } - private void throwWhenRecordDoesNotExist(MutationCondition condition) + private void throwWhenRecordDoesNotExist(MutationCondition condition, TransactionContext context) throws UnsatisfiedConditionException { throw new UnsatisfiedConditionException( CoreError.CONSENSUS_COMMIT_CONDITION_NOT_SATISFIED_BECAUSE_RECORD_NOT_EXISTS.buildMessage( condition.getClass().getSimpleName()), - transactionId); + context.transactionId); } - private void throwWhenRecordExists(MutationCondition condition) + private void throwWhenRecordExists(MutationCondition condition, TransactionContext context) throws UnsatisfiedConditionException { throw new UnsatisfiedConditionException( CoreError.CONSENSUS_COMMIT_CONDITION_NOT_SATISFIED_BECAUSE_RECORD_EXISTS.buildMessage( condition.getClass().getSimpleName()), - transactionId); + context.transactionId); } private void validateConditionalExpressions( - List conditionalExpressions, TransactionResult existingRecord) + List conditionalExpressions, + TransactionResult existingRecord, + TransactionContext context) throws UnsatisfiedConditionException { for (ConditionalExpression conditionalExpression : conditionalExpressions) { if (!shouldMutate( @@ -117,7 +118,7 @@ private void validateConditionalExpressions( throw new UnsatisfiedConditionException( CoreError.CONSENSUS_COMMIT_CONDITION_NOT_SATISFIED.buildMessage( conditionalExpression.getColumn().getName()), - transactionId); + context.transactionId); } } } diff --git a/core/src/main/java/com/scalar/db/transaction/consensuscommit/Snapshot.java b/core/src/main/java/com/scalar/db/transaction/consensuscommit/Snapshot.java index a76ad4acbc..e520678d31 100644 --- a/core/src/main/java/com/scalar/db/transaction/consensuscommit/Snapshot.java +++ b/core/src/main/java/com/scalar/db/transaction/consensuscommit/Snapshot.java @@ -45,7 +45,6 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.stream.Collectors; -import javax.annotation.Nonnull; import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; import javax.annotation.concurrent.NotThreadSafe; @@ -123,16 +122,6 @@ public Snapshot( this.scannerSet = scannerSet; } - @Nonnull - public String getId() { - return id; - } - - @Nonnull - public Isolation getIsolation() { - return isolation; - } - // Although this class is not thread-safe, this method is actually thread-safe because the readSet // is a concurrent map public void putIntoReadSet(Key key, Optional result) { @@ -197,16 +186,24 @@ public void putIntoScannerSet(Scan scan, LinkedHashMap r scannerSet.add(new ScannerInfo(scan, results)); } - public List getPutsInWriteSet() { - return new ArrayList<>(writeSet.values()); + public Collection> getWriteSet() { + return new ArrayList<>(writeSet.entrySet()); + } + + public Collection> getDeleteSet() { + return new ArrayList<>(deleteSet.entrySet()); + } + + public Collection>> getScanSet() { + return new ArrayList<>(scanSet.entrySet()); } - public List getDeletesInDeleteSet() { - return new ArrayList<>(deleteSet.values()); + public Collection getScannerSet() { + return new ArrayList<>(scannerSet); } - public ReadWriteSets getReadWriteSets() { - return new ReadWriteSets(id, readSet, writeSet.entrySet(), deleteSet.entrySet()); + public Collection>> getGetSet() { + return new ArrayList<>(getSet.entrySet()); } public boolean containsKeyInReadSet(Key key) { @@ -248,7 +245,7 @@ public Optional getResult(Key key, Get get) throws CrudExcept return mergeResult(key, result, get.getConjunctions()); } - public Optional> getResults(Scan scan) { + public Optional> getResults(Scan scan) { if (!scanSet.containsKey(scan)) { return Optional.empty(); } @@ -524,7 +521,7 @@ private Map> getAllColumns(Put put) { @VisibleForTesting void toSerializable(DistributedStorage storage) throws ExecutionException, ValidationConflictException { - if (!isSerializable()) { + if (isolation != Isolation.SERIALIZABLE) { return; } @@ -559,7 +556,7 @@ void toSerializable(DistributedStorage storage) } } - parallelExecutor.validateRecords(tasks, getId()); + parallelExecutor.validateRecords(tasks, id); } /** @@ -592,6 +589,8 @@ private void validateScanResults( throws ExecutionException, ValidationConflictException { Scanner scanner = null; try { + TableMetadata tableMetadata = getTableMetadata(scan); + // Only get tx_id and primary key columns because we use only them to compare scan.clearProjections(); scan.withProjection(Attribute.ID); @@ -616,7 +615,7 @@ private void validateScanResults( // Compare the records of the iterators while (latestResult.isPresent() && originalResultEntry != null) { TransactionResult latestTxResult = new TransactionResult(latestResult.get()); - Key key = new Key(scan, latestTxResult); + Key key = new Key(scan, latestTxResult, tableMetadata); if (latestTxResult.getId() != null && latestTxResult.getId().equals(id)) { // The record is inserted/deleted/updated by this transaction @@ -706,7 +705,7 @@ private void validateScanResults( try { scanner.close(); } catch (IOException e) { - logger.warn("Failed to close the scanner", e); + logger.warn("Failed to close the scanner. Transaction ID: {}", id, e); } } } @@ -734,7 +733,9 @@ private void validateGetWithIndexResult( .build(); LinkedHashMap results = new LinkedHashMap<>(1); - originalResult.ifPresent(r -> results.put(new Snapshot.Key(scanWithIndex, r), r)); + TableMetadata tableMetadata = getTableMetadata(scanWithIndex); + originalResult.ifPresent( + r -> results.put(new Snapshot.Key(scanWithIndex, r, tableMetadata), r)); // Validate the result to check if there is no anti-dependency validateScanResults(storage, scanWithIndex, results, false); @@ -780,18 +781,6 @@ private void throwExceptionDueToAntiDependency() throws ValidationConflictExcept CoreError.CONSENSUS_COMMIT_ANTI_DEPENDENCY_FOUND.buildMessage(), id); } - private boolean isSerializable() { - return isolation == Isolation.SERIALIZABLE; - } - - public boolean isSnapshotReadRequired() { - return isolation != Isolation.READ_COMMITTED; - } - - public boolean isValidationRequired() { - return isSerializable(); - } - @Immutable public static final class Key implements Comparable { private final String namespace; @@ -803,11 +792,11 @@ public Key(Get get) { this((Operation) get); } - public Key(Get get, Result result) { + public Key(Get get, Result result, TableMetadata tableMetadata) { this.namespace = get.forNamespace().get(); this.table = get.forTable().get(); - this.partitionKey = result.getPartitionKey().get(); - this.clusteringKey = result.getClusteringKey(); + this.partitionKey = ScalarDbUtils.getPartitionKey(result, tableMetadata); + this.clusteringKey = ScalarDbUtils.getClusteringKey(result, tableMetadata); } public Key(Put put) { @@ -818,11 +807,11 @@ public Key(Delete delete) { this((Operation) delete); } - public Key(Scan scan, Result result) { + public Key(Scan scan, Result result, TableMetadata tableMetadata) { this.namespace = scan.forNamespace().get(); this.table = scan.forTable().get(); - this.partitionKey = result.getPartitionKey().get(); - this.clusteringKey = result.getClusteringKey(); + this.partitionKey = ScalarDbUtils.getPartitionKey(result, tableMetadata); + this.clusteringKey = ScalarDbUtils.getClusteringKey(result, tableMetadata); } private Key(Operation operation) { @@ -892,40 +881,6 @@ public String toString() { } } - public static class ReadWriteSets { - public final String transactionId; - public final Map> readSetMap; - public final List> writeSet; - public final List> deleteSet; - - public ReadWriteSets( - String transactionId, - Map> readSetMap, - Collection> writeSet, - Collection> deleteSet) { - this.transactionId = transactionId; - this.readSetMap = new HashMap<>(readSetMap); - this.writeSet = new ArrayList<>(writeSet); - this.deleteSet = new ArrayList<>(deleteSet); - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (!(o instanceof ReadWriteSets)) return false; - ReadWriteSets that = (ReadWriteSets) o; - return Objects.equals(transactionId, that.transactionId) - && Objects.equals(readSetMap, that.readSetMap) - && Objects.equals(writeSet, that.writeSet) - && Objects.equals(deleteSet, that.deleteSet); - } - - @Override - public int hashCode() { - return Objects.hash(transactionId, readSetMap, writeSet, deleteSet); - } - } - @VisibleForTesting static class ScannerInfo { public final Scan scan; diff --git a/core/src/main/java/com/scalar/db/transaction/consensuscommit/TransactionContext.java b/core/src/main/java/com/scalar/db/transaction/consensuscommit/TransactionContext.java new file mode 100644 index 0000000000..db14198263 --- /dev/null +++ b/core/src/main/java/com/scalar/db/transaction/consensuscommit/TransactionContext.java @@ -0,0 +1,64 @@ +package com.scalar.db.transaction.consensuscommit; + +import com.scalar.db.exception.transaction.CrudException; +import java.util.ArrayList; +import java.util.List; + +public class TransactionContext { + + // The transaction ID + public final String transactionId; + + // The snapshot of the transaction + public final Snapshot snapshot; + + // The isolation level of the transaction + public final Isolation isolation; + + // Whether the transaction is in read-only mode or not. + public final boolean readOnly; + + // Whether the transaction is in one-operation mode or not. One-operation mode refers to executing + // a CRUD operation directly through `DistributedTransactionManager` without explicitly beginning + // a transaction. + public final boolean oneOperation; + + // A list of scanners opened in the transaction + public final List scanners = new ArrayList<>(); + + // A list of recovery results performed in the transaction + public final List recoveryResults = new ArrayList<>(); + + public TransactionContext( + String transactionId, + Snapshot snapshot, + Isolation isolation, + boolean readOnly, + boolean oneOperation) { + this.transactionId = transactionId; + this.snapshot = snapshot; + this.isolation = isolation; + this.readOnly = readOnly; + this.oneOperation = oneOperation; + } + + public boolean isSnapshotReadRequired() { + return isolation != Isolation.READ_COMMITTED; + } + + public boolean isValidationRequired() { + return isolation == Isolation.SERIALIZABLE; + } + + public boolean areAllScannersClosed() { + return scanners.stream().allMatch(ConsensusCommitScanner::isClosed); + } + + public void closeScanners() throws CrudException { + for (ConsensusCommitScanner scanner : scanners) { + if (!scanner.isClosed()) { + scanner.close(); + } + } + } +} 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 f7ba7b3ab2..a231ef9a39 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 @@ -36,7 +36,7 @@ @NotThreadSafe public class TwoPhaseConsensusCommit extends AbstractTwoPhaseCommitTransaction { private static final Logger logger = LoggerFactory.getLogger(TwoPhaseConsensusCommit.class); - + private final TransactionContext context; private final CrudHandler crud; private final CommitHandler commit; private final ConsensusCommitMutationOperationChecker mutationOperationChecker; @@ -45,9 +45,11 @@ public class TwoPhaseConsensusCommit extends AbstractTwoPhaseCommitTransaction { @SuppressFBWarnings("EI_EXPOSE_REP2") public TwoPhaseConsensusCommit( + TransactionContext context, CrudHandler crud, CommitHandler commit, ConsensusCommitMutationOperationChecker mutationOperationChecker) { + this.context = context; this.crud = crud; this.commit = commit; this.mutationOperationChecker = mutationOperationChecker; @@ -55,22 +57,22 @@ public TwoPhaseConsensusCommit( @Override public String getId() { - return crud.getSnapshot().getId(); + return context.transactionId; } @Override public Optional get(Get get) throws CrudException { - return crud.get(copyAndSetTargetToIfNot(get)); + return crud.get(copyAndSetTargetToIfNot(get), context); } @Override public List scan(Scan scan) throws CrudException { - return crud.scan(copyAndSetTargetToIfNot(scan)); + return crud.scan(copyAndSetTargetToIfNot(scan), context); } @Override public Scanner getScanner(Scan scan) throws CrudException { - return crud.getScanner(copyAndSetTargetToIfNot(scan)); + return crud.getScanner(copyAndSetTargetToIfNot(scan), context); } /** @deprecated As of release 3.13.0. Will be removed in release 5.0.0. */ @@ -93,7 +95,7 @@ public void put(List puts) throws CrudException { private void putInternal(Put put) throws CrudException { put = copyAndSetTargetToIfNot(put); checkMutation(put); - crud.put(put); + crud.put(put, context); } @Override @@ -114,7 +116,7 @@ public void delete(List deletes) throws CrudException { private void deleteInternal(Delete delete) throws CrudException { delete = copyAndSetTargetToIfNot(delete); checkMutation(delete); - crud.delete(delete); + crud.delete(delete, context); } @Override @@ -122,7 +124,7 @@ public void insert(Insert insert) throws CrudException { insert = copyAndSetTargetToIfNot(insert); Put put = ConsensusCommitUtils.createPutForInsert(insert); checkMutation(put); - crud.put(put); + crud.put(put, context); } @Override @@ -130,7 +132,7 @@ public void upsert(Upsert upsert) throws CrudException { upsert = copyAndSetTargetToIfNot(upsert); Put put = ConsensusCommitUtils.createPutForUpsert(upsert); checkMutation(put); - crud.put(put); + crud.put(put, context); } @Override @@ -140,13 +142,13 @@ public void update(Update update) throws CrudException { Put put = ConsensusCommitUtils.createPutForUpdate(update); checkMutation(put); try { - crud.put(put); + crud.put(put, context); } catch (UnsatisfiedConditionException e) { if (update.getCondition().isPresent()) { throw new UnsatisfiedConditionException( ConsensusCommitUtils.convertUnsatisfiedConditionExceptionMessageForUpdate( e, update.getCondition().get()), - crud.getSnapshot().getId()); + getId()); } // If the condition is not specified, it means that the record does not exist. In this case, @@ -175,14 +177,14 @@ public void mutate(List mutations) throws CrudException { @Override public void prepare() throws PreparationException { - if (!crud.areAllScannersClosed()) { + if (!context.areAllScannersClosed()) { throw new IllegalStateException( CoreError.TWO_PHASE_CONSENSUS_COMMIT_SCANNER_NOT_CLOSED.buildMessage()); } // Execute implicit pre-read try { - crud.readIfImplicitPreReadEnabled(); + crud.readIfImplicitPreReadEnabled(context); } catch (CrudConflictException e) { throw new PreparationConflictException( CoreError.CONSENSUS_COMMIT_CONFLICT_OCCURRED_WHILE_IMPLICIT_PRE_READ.buildMessage(), @@ -194,7 +196,7 @@ public void prepare() throws PreparationException { } try { - crud.waitForRecoveryCompletionIfNecessary(); + crud.waitForRecoveryCompletionIfNecessary(context); } catch (CrudConflictException e) { throw new PreparationConflictException(e.getMessage(), e, getId()); } catch (CrudException e) { @@ -202,7 +204,7 @@ public void prepare() throws PreparationException { } try { - commit.prepareRecords(crud.getSnapshot()); + commit.prepareRecords(context); } finally { needRollback = true; } @@ -210,19 +212,19 @@ public void prepare() throws PreparationException { @Override public void validate() throws ValidationException { - commit.validateRecords(crud.getSnapshot()); + commit.validateRecords(context); validated = true; } @Override public void commit() throws CommitConflictException, UnknownTransactionStatusException { - if (crud.getSnapshot().isValidationRequired() && !validated) { + if (context.isValidationRequired() && !validated) { throw new IllegalStateException( CoreError.CONSENSUS_COMMIT_TRANSACTION_NOT_VALIDATED_IN_SERIALIZABLE.buildMessage()); } try { - commit.commitState(crud.getSnapshot()); + commit.commitState(context); } catch (CommitConflictException | UnknownTransactionStatusException e) { // no need to rollback because the transaction has already been rolled back needRollback = false; @@ -230,13 +232,13 @@ public void commit() throws CommitConflictException, UnknownTransactionStatusExc throw e; } - commit.commitRecords(crud.getSnapshot()); + commit.commitRecords(context); } @Override public void rollback() throws RollbackException { try { - crud.closeScanners(); + context.closeScanners(); } catch (CrudException e) { logger.warn("Failed to close the scanner", e); } @@ -246,7 +248,7 @@ public void rollback() throws RollbackException { } try { - TransactionState state = commit.abortState(crud.getSnapshot().getId()); + TransactionState state = commit.abortState(getId()); if (state == TransactionState.COMMITTED) { throw new RollbackException( CoreError.CONSENSUS_COMMIT_ROLLBACK_FAILED_BECAUSE_TRANSACTION_ALREADY_COMMITTED @@ -258,7 +260,12 @@ public void rollback() throws RollbackException { CoreError.CONSENSUS_COMMIT_ROLLBACK_FAILED.buildMessage(), e, getId()); } - commit.rollbackRecords(crud.getSnapshot()); + commit.rollbackRecords(context); + } + + @VisibleForTesting + TransactionContext getTransactionContext() { + return context; } @VisibleForTesting @@ -271,6 +278,11 @@ CommitHandler getCommitHandler() { return commit; } + @VisibleForTesting + void waitForRecoveryCompletion() throws CrudException { + crud.waitForRecoveryCompletion(context); + } + private void checkMutation(Mutation mutation) throws CrudException { try { mutationOperationChecker.check(mutation); diff --git a/core/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitManager.java b/core/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitManager.java index 4a82b5d9ad..8634d8f45d 100644 --- a/core/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitManager.java +++ b/core/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitManager.java @@ -57,8 +57,8 @@ public class TwoPhaseConsensusCommitManager extends AbstractTwoPhaseCommitTransa private final Coordinator coordinator; private final ParallelExecutor parallelExecutor; private final RecoveryExecutor recoveryExecutor; + private final CrudHandler crud; private final CommitHandler commit; - private final boolean isIncludeMetadataEnabled; private final ConsensusCommitMutationOperationChecker mutationOperationChecker; @SuppressFBWarnings("EI_EXPOSE_REP2") @@ -76,6 +76,13 @@ public TwoPhaseConsensusCommitManager( parallelExecutor = new ParallelExecutor(config); RecoveryHandler recovery = new RecoveryHandler(storage, coordinator, tableMetadataManager); recoveryExecutor = new RecoveryExecutor(coordinator, recovery, tableMetadataManager); + crud = + new CrudHandler( + storage, + recoveryExecutor, + tableMetadataManager, + config.isIncludeMetadataEnabled(), + parallelExecutor); commit = new CommitHandler( storage, @@ -85,7 +92,6 @@ public TwoPhaseConsensusCommitManager( new MutationsGrouper(new StorageInfoProvider(admin)), config.isCoordinatorWriteOmissionOnReadOnlyEnabled(), config.isOnePhaseCommitEnabled()); - isIncludeMetadataEnabled = config.isIncludeMetadataEnabled(); mutationOperationChecker = new ConsensusCommitMutationOperationChecker(tableMetadataManager); } @@ -102,6 +108,13 @@ public TwoPhaseConsensusCommitManager(DatabaseConfig databaseConfig) { parallelExecutor = new ParallelExecutor(config); RecoveryHandler recovery = new RecoveryHandler(storage, coordinator, tableMetadataManager); recoveryExecutor = new RecoveryExecutor(coordinator, recovery, tableMetadataManager); + crud = + new CrudHandler( + storage, + recoveryExecutor, + tableMetadataManager, + config.isIncludeMetadataEnabled(), + parallelExecutor); commit = new CommitHandler( storage, @@ -111,7 +124,6 @@ public TwoPhaseConsensusCommitManager(DatabaseConfig databaseConfig) { new MutationsGrouper(new StorageInfoProvider(admin)), config.isCoordinatorWriteOmissionOnReadOnlyEnabled(), config.isOnePhaseCommitEnabled()); - isIncludeMetadataEnabled = config.isIncludeMetadataEnabled(); mutationOperationChecker = new ConsensusCommitMutationOperationChecker(tableMetadataManager); } @@ -125,6 +137,7 @@ public TwoPhaseConsensusCommitManager(DatabaseConfig databaseConfig) { Coordinator coordinator, ParallelExecutor parallelExecutor, RecoveryExecutor recoveryExecutor, + CrudHandler crud, CommitHandler commit) { super(databaseConfig); this.storage = storage; @@ -136,8 +149,8 @@ public TwoPhaseConsensusCommitManager(DatabaseConfig databaseConfig) { this.coordinator = coordinator; this.parallelExecutor = parallelExecutor; this.recoveryExecutor = recoveryExecutor; + this.crud = crud; this.commit = commit; - isIncludeMetadataEnabled = config.isIncludeMetadataEnabled(); mutationOperationChecker = new ConsensusCommitMutationOperationChecker(tableMetadataManager); } @@ -188,18 +201,10 @@ TwoPhaseCommitTransaction join(String txId, Isolation isolation) { TwoPhaseCommitTransaction begin( String txId, Isolation isolation, boolean readOnly, boolean oneOperation) { Snapshot snapshot = new Snapshot(txId, isolation, tableMetadataManager, parallelExecutor); - CrudHandler crud = - new CrudHandler( - storage, - snapshot, - recoveryExecutor, - tableMetadataManager, - isIncludeMetadataEnabled, - parallelExecutor, - readOnly, - oneOperation); + TransactionContext context = + new TransactionContext(txId, snapshot, isolation, readOnly, oneOperation); TwoPhaseConsensusCommit transaction = - new TwoPhaseConsensusCommit(crud, commit, mutationOperationChecker); + new TwoPhaseConsensusCommit(context, crud, commit, mutationOperationChecker); getNamespace().ifPresent(transaction::withNamespace); getTable().ifPresent(transaction::withTable); return transaction; 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 80984ccc83..7b5ec4a641 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 @@ -32,7 +32,6 @@ 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.Snapshot.ReadWriteSets; import java.time.Duration; import java.time.Instant; import java.util.Optional; @@ -64,8 +63,8 @@ public class CommitHandlerTest { @Mock protected TransactionTableMetadataManager tableMetadataManager; @Mock protected StorageInfoProvider storageInfoProvider; @Mock protected ConsensusCommitConfig config; - @Mock protected BeforePreparationSnapshotHook beforePreparationSnapshotHook; - @Mock protected Future beforePreparationSnapshotHookFuture; + @Mock protected BeforePreparationHook beforePreparationHook; + @Mock protected Future beforePreparationHookFuture; private CommitHandler handler; protected ParallelExecutor parallelExecutor; @@ -226,81 +225,83 @@ private Snapshot prepareSnapshot() { anyId(), Isolation.SNAPSHOT, tableMetadataManager, new ParallelExecutor(config)); } - private void setBeforePreparationSnapshotHookIfNeeded(boolean withSnapshotHook) { - if (withSnapshotHook) { - doReturn(beforePreparationSnapshotHookFuture) - .when(beforePreparationSnapshotHook) - .handle(any(), any()); - handler.setBeforePreparationSnapshotHook(beforePreparationSnapshotHook); + private void setBeforePreparationHookIfNeeded(boolean withBeforePreparationHook) { + if (withBeforePreparationHook) { + doReturn(beforePreparationHookFuture).when(beforePreparationHook).handle(any(), any()); + handler.setBeforePreparationHook(beforePreparationHook); } } @ParameterizedTest @ValueSource(booleans = {false, true}) public void commit_SnapshotWithDifferentPartitionPutsGiven_ShouldCommitRespectively( - boolean withSnapshotHook) + boolean withBeforePreparationHook) throws CommitException, UnknownTransactionStatusException, ExecutionException, CoordinatorException, ValidationConflictException { // Arrange Snapshot snapshot = spy(prepareSnapshotWithDifferentPartitionPut()); - ReadWriteSets readWriteSets = snapshot.getReadWriteSets(); doNothing().when(storage).mutate(anyList()); doNothingWhenCoordinatorPutState(); - setBeforePreparationSnapshotHookIfNeeded(withSnapshotHook); + setBeforePreparationHookIfNeeded(withBeforePreparationHook); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - handler.commit(snapshot, false); + handler.commit(context); // Assert verify(storage, times(4)).mutate(anyList()); verify(snapshot).toSerializable(storage); verifyCoordinatorPutState(TransactionState.COMMITTED); - verifySnapshotHook(withSnapshotHook, readWriteSets); + verifyBeforePreparationHook(withBeforePreparationHook, context); verify(handler, never()).onFailureBeforeCommit(any()); } @ParameterizedTest @ValueSource(booleans = {false, true}) - public void commit_SnapshotWithSamePartitionPutsGiven_ShouldCommitAtOnce(boolean withSnapshotHook) + public void commit_SnapshotWithSamePartitionPutsGiven_ShouldCommitAtOnce( + boolean withBeforePreparationHook) throws CommitException, UnknownTransactionStatusException, ExecutionException, CoordinatorException, ValidationConflictException { // Arrange Snapshot snapshot = spy(prepareSnapshotWithSamePartitionPut()); - ReadWriteSets readWriteSets = snapshot.getReadWriteSets(); doNothing().when(storage).mutate(anyList()); doNothingWhenCoordinatorPutState(); - setBeforePreparationSnapshotHookIfNeeded(withSnapshotHook); + setBeforePreparationHookIfNeeded(withBeforePreparationHook); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - handler.commit(snapshot, false); + handler.commit(context); // Assert verify(storage, times(2)).mutate(anyList()); verify(snapshot).toSerializable(storage); verifyCoordinatorPutState(TransactionState.COMMITTED); - verifySnapshotHook(withSnapshotHook, readWriteSets); + verifyBeforePreparationHook(withBeforePreparationHook, context); verify(handler, never()).onFailureBeforeCommit(any()); } @ParameterizedTest @ValueSource(booleans = {false, true}) public void commit_InReadOnlyMode_ShouldNotPrepareRecordsAndCommitStateAndCommitRecords( - boolean withSnapshotHook) + boolean withBeforePreparationHook) throws CommitException, UnknownTransactionStatusException, ExecutionException, CoordinatorException, ValidationConflictException { // Arrange Snapshot snapshot = spy(prepareSnapshotWithoutWrites()); - ReadWriteSets readWriteSets = snapshot.getReadWriteSets(); - setBeforePreparationSnapshotHookIfNeeded(withSnapshotHook); + setBeforePreparationHookIfNeeded(withBeforePreparationHook); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, true, false); // Act - handler.commit(snapshot, true); + handler.commit(context); // Assert verify(storage, never()).mutate(anyList()); verify(snapshot).toSerializable(storage); verify(coordinator, never()).putState(any()); - verifySnapshotHook(withSnapshotHook, readWriteSets); + verifyBeforePreparationHook(withBeforePreparationHook, context); verify(handler, never()).onFailureBeforeCommit(any()); } @@ -308,22 +309,23 @@ public void commit_InReadOnlyMode_ShouldNotPrepareRecordsAndCommitStateAndCommit @ValueSource(booleans = {false, true}) public void commit_NoWritesAndDeletesInSnapshot_ShouldNotPrepareRecordsAndCommitStateAndCommitRecords( - boolean withSnapshotHook) + boolean withBeforePreparationHook) throws CommitException, UnknownTransactionStatusException, ExecutionException, CoordinatorException, ValidationConflictException { // Arrange Snapshot snapshot = spy(prepareSnapshotWithoutWrites()); - ReadWriteSets readWriteSets = snapshot.getReadWriteSets(); - setBeforePreparationSnapshotHookIfNeeded(withSnapshotHook); + setBeforePreparationHookIfNeeded(withBeforePreparationHook); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - handler.commit(snapshot, false); + handler.commit(context); // Assert verify(storage, never()).mutate(anyList()); verify(snapshot).toSerializable(storage); verify(coordinator, never()).putState(any()); - verifySnapshotHook(withSnapshotHook, readWriteSets); + verifyBeforePreparationHook(withBeforePreparationHook, context); verify(handler, never()).onFailureBeforeCommit(any()); } @@ -331,23 +333,24 @@ public void commit_InReadOnlyMode_ShouldNotPrepareRecordsAndCommitStateAndCommit @ValueSource(booleans = {false, true}) public void commit_NoWritesAndDeletesInSnapshot_CoordinatorWriteOmissionOnReadOnlyDisabled_ShouldNotPrepareRecordsAndCommitRecordsButShouldCommitState( - boolean withSnapshotHook) + boolean withBeforePreparationHook) throws CommitException, UnknownTransactionStatusException, ExecutionException, CoordinatorException, ValidationConflictException { // Arrange handler = spy(createCommitHandler(false)); Snapshot snapshot = spy(prepareSnapshotWithoutWrites()); - ReadWriteSets readWriteSets = snapshot.getReadWriteSets(); - setBeforePreparationSnapshotHookIfNeeded(withSnapshotHook); + setBeforePreparationHookIfNeeded(withBeforePreparationHook); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - handler.commit(snapshot, false); + handler.commit(context); // Assert verify(storage, never()).mutate(anyList()); verify(snapshot).toSerializable(storage); verifyCoordinatorPutState(TransactionState.COMMITTED); - verifySnapshotHook(withSnapshotHook, readWriteSets); + verifyBeforePreparationHook(withBeforePreparationHook, context); verify(handler, never()).onFailureBeforeCommit(any()); } @@ -355,23 +358,23 @@ public void commit_InReadOnlyMode_ShouldNotPrepareRecordsAndCommitStateAndCommit @ValueSource(booleans = {false, true}) public void commit_NoWritesAndDeletesInSnapshot_ValidationFailed_ShouldNotPrepareRecordsAndAbortStateAndRollbackRecords( - boolean withSnapshotHook) + boolean withBeforePreparationHook) throws ExecutionException, CoordinatorException, ValidationConflictException { // Arrange Snapshot snapshot = spy(prepareSnapshotWithoutWrites()); - ReadWriteSets readWriteSets = snapshot.getReadWriteSets(); - setBeforePreparationSnapshotHookIfNeeded(withSnapshotHook); + setBeforePreparationHookIfNeeded(withBeforePreparationHook); doThrow(ValidationConflictException.class).when(snapshot).toSerializable(storage); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act Assert - assertThatThrownBy(() -> handler.commit(snapshot, false)) - .isInstanceOf(CommitConflictException.class); + assertThatThrownBy(() -> handler.commit(context)).isInstanceOf(CommitConflictException.class); // Assert verify(storage, never()).mutate(anyList()); verify(snapshot).toSerializable(storage); verify(coordinator, never()).putState(any()); - verifySnapshotHook(withSnapshotHook, readWriteSets); + verifyBeforePreparationHook(withBeforePreparationHook, context); verify(handler).onFailureBeforeCommit(any()); } @@ -379,47 +382,48 @@ public void commit_InReadOnlyMode_ShouldNotPrepareRecordsAndCommitStateAndCommit @ValueSource(booleans = {false, true}) public void commit_NoWritesAndDeletesInSnapshot_ValidationFailed_CoordinatorWriteOmissionOnReadOnlyDisabled_ShouldNotPrepareRecordsAndRollbackRecordsButShouldAbortState( - boolean withSnapshotHook) + boolean withBeforePreparationHook) throws ExecutionException, CoordinatorException, ValidationConflictException { // Arrange handler = spy(createCommitHandler(false)); Snapshot snapshot = spy(prepareSnapshotWithoutWrites()); - ReadWriteSets readWriteSets = snapshot.getReadWriteSets(); - setBeforePreparationSnapshotHookIfNeeded(withSnapshotHook); + setBeforePreparationHookIfNeeded(withBeforePreparationHook); doThrow(ValidationConflictException.class).when(snapshot).toSerializable(storage); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act Assert - assertThatThrownBy(() -> handler.commit(snapshot, false)) - .isInstanceOf(CommitConflictException.class); + assertThatThrownBy(() -> handler.commit(context)).isInstanceOf(CommitConflictException.class); // Assert verify(storage, never()).mutate(anyList()); verify(snapshot).toSerializable(storage); verify(coordinator).putState(any()); - verifySnapshotHook(withSnapshotHook, readWriteSets); + verifyBeforePreparationHook(withBeforePreparationHook, context); verify(handler).onFailureBeforeCommit(any()); } @ParameterizedTest @ValueSource(booleans = {false, true}) - public void commit_NoReadsInSnapshot_ShouldNotValidateRecords(boolean withSnapshotHook) + public void commit_NoReadsInSnapshot_ShouldNotValidateRecords(boolean withBeforePreparationHook) throws CommitException, UnknownTransactionStatusException, ExecutionException, CoordinatorException, ValidationConflictException { // Arrange Snapshot snapshot = spy(prepareSnapshotWithoutReads()); - ReadWriteSets readWriteSets = snapshot.getReadWriteSets(); doNothing().when(storage).mutate(anyList()); doNothingWhenCoordinatorPutState(); - setBeforePreparationSnapshotHookIfNeeded(withSnapshotHook); + setBeforePreparationHookIfNeeded(withBeforePreparationHook); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - handler.commit(snapshot, false); + handler.commit(context); // Assert verify(storage, times(2)).mutate(anyList()); verify(snapshot, never()).toSerializable(storage); verifyCoordinatorPutState(TransactionState.COMMITTED); - verifySnapshotHook(withSnapshotHook, readWriteSets); + verifyBeforePreparationHook(withBeforePreparationHook, context); verify(handler, never()).onFailureBeforeCommit(any()); } @@ -430,11 +434,12 @@ public void commit_NoMutationExceptionThrownInPrepareRecords_ShouldThrowCCExcept Snapshot snapshot = prepareSnapshotWithDifferentPartitionPut(); doThrow(NoMutationException.class).when(storage).mutate(anyList()); doNothing().when(coordinator).putState(any(Coordinator.State.class)); - doNothing().when(handler).rollbackRecords(any(Snapshot.class)); + doNothing().when(handler).rollbackRecords(any(TransactionContext.class)); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - assertThatThrownBy(() -> handler.commit(snapshot, false)) - .isInstanceOf(CommitConflictException.class); + assertThatThrownBy(() -> handler.commit(context)).isInstanceOf(CommitConflictException.class); // Assert @@ -443,8 +448,8 @@ public void commit_NoMutationExceptionThrownInPrepareRecords_ShouldThrowCCExcept verify(coordinator).putState(new Coordinator.State(anyId(), TransactionState.ABORTED)); verify(coordinator, never()) .putState(new Coordinator.State(anyId(), TransactionState.COMMITTED)); - verify(handler).rollbackRecords(snapshot); - verify(handler).onFailureBeforeCommit(snapshot); + verify(handler).rollbackRecords(context); + verify(handler).onFailureBeforeCommit(context); } @Test @@ -454,11 +459,12 @@ public void commit_RetriableExecutionExceptionThrownInPrepareRecords_ShouldThrow Snapshot snapshot = prepareSnapshotWithDifferentPartitionPut(); doThrow(RetriableExecutionException.class).when(storage).mutate(anyList()); doNothing().when(coordinator).putState(any(Coordinator.State.class)); - doNothing().when(handler).rollbackRecords(any(Snapshot.class)); + doNothing().when(handler).rollbackRecords(any(TransactionContext.class)); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - assertThatThrownBy(() -> handler.commit(snapshot, false)) - .isInstanceOf(CommitConflictException.class); + assertThatThrownBy(() -> handler.commit(context)).isInstanceOf(CommitConflictException.class); // Assert @@ -467,8 +473,8 @@ public void commit_RetriableExecutionExceptionThrownInPrepareRecords_ShouldThrow verify(coordinator).putState(new Coordinator.State(anyId(), TransactionState.ABORTED)); verify(coordinator, never()) .putState(new Coordinator.State(anyId(), TransactionState.COMMITTED)); - verify(handler).rollbackRecords(snapshot); - verify(handler).onFailureBeforeCommit(snapshot); + verify(handler).rollbackRecords(context); + verify(handler).onFailureBeforeCommit(context); } @Test @@ -478,10 +484,12 @@ public void commit_ExceptionThrownInPrepareRecords_ShouldAbortAndRollbackRecords Snapshot snapshot = prepareSnapshotWithDifferentPartitionPut(); doThrow(ExecutionException.class).when(storage).mutate(anyList()); doNothing().when(coordinator).putState(any(Coordinator.State.class)); - doNothing().when(handler).rollbackRecords(any(Snapshot.class)); + doNothing().when(handler).rollbackRecords(any(TransactionContext.class)); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - assertThatThrownBy(() -> handler.commit(snapshot, false)).isInstanceOf(CommitException.class); + assertThatThrownBy(() -> handler.commit(context)).isInstanceOf(CommitException.class); // Assert @@ -490,8 +498,8 @@ public void commit_ExceptionThrownInPrepareRecords_ShouldAbortAndRollbackRecords verify(coordinator).putState(new Coordinator.State(anyId(), TransactionState.ABORTED)); verify(coordinator, never()) .putState(new Coordinator.State(anyId(), TransactionState.COMMITTED)); - verify(handler).rollbackRecords(snapshot); - verify(handler).onFailureBeforeCommit(snapshot); + verify(handler).rollbackRecords(context); + verify(handler).onFailureBeforeCommit(context); } @Test @@ -507,10 +515,12 @@ public void commit_ExceptionThrownInPrepareRecords_ShouldAbortAndRollbackRecords doReturn(Optional.of(new Coordinator.State(anyId(), TransactionState.ABORTED))) .when(coordinator) .getState(anyId()); - doNothing().when(handler).rollbackRecords(any(Snapshot.class)); + doNothing().when(handler).rollbackRecords(any(TransactionContext.class)); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - assertThatThrownBy(() -> handler.commit(snapshot, false)).isInstanceOf(CommitException.class); + assertThatThrownBy(() -> handler.commit(context)).isInstanceOf(CommitException.class); // Assert @@ -520,8 +530,8 @@ public void commit_ExceptionThrownInPrepareRecords_ShouldAbortAndRollbackRecords verify(coordinator, never()) .putState(new Coordinator.State(anyId(), TransactionState.COMMITTED)); verify(coordinator).getState(anyId()); - verify(handler).rollbackRecords(snapshot); - verify(handler).onFailureBeforeCommit(snapshot); + verify(handler).rollbackRecords(context); + verify(handler).onFailureBeforeCommit(context); } @Test @@ -535,9 +545,11 @@ public void commit_ExceptionThrownInPrepareRecords_ShouldAbortAndRollbackRecords .when(coordinator) .putState(new Coordinator.State(anyId(), TransactionState.ABORTED)); doReturn(Optional.empty()).when(coordinator).getState(anyId()); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - assertThatThrownBy(() -> handler.commit(snapshot, false)) + assertThatThrownBy(() -> handler.commit(context)) .isInstanceOf(UnknownTransactionStatusException.class); // Assert @@ -548,8 +560,8 @@ public void commit_ExceptionThrownInPrepareRecords_ShouldAbortAndRollbackRecords verify(coordinator, never()) .putState(new Coordinator.State(anyId(), TransactionState.COMMITTED)); verify(coordinator).getState(anyId()); - verify(handler, never()).rollbackRecords(snapshot); - verify(handler).onFailureBeforeCommit(snapshot); + verify(handler, never()).rollbackRecords(context); + verify(handler).onFailureBeforeCommit(context); } @Test @@ -563,9 +575,11 @@ public void commit_ExceptionThrownInPrepareRecords_ShouldAbortAndRollbackRecords .when(coordinator) .putState(new Coordinator.State(anyId(), TransactionState.ABORTED)); doThrow(CoordinatorException.class).when(coordinator).getState(anyId()); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - assertThatThrownBy(() -> handler.commit(snapshot, false)) + assertThatThrownBy(() -> handler.commit(context)) .isInstanceOf(UnknownTransactionStatusException.class); // Assert @@ -576,8 +590,8 @@ public void commit_ExceptionThrownInPrepareRecords_ShouldAbortAndRollbackRecords verify(coordinator, never()) .putState(new Coordinator.State(anyId(), TransactionState.COMMITTED)); verify(coordinator).getState(anyId()); - verify(handler, never()).rollbackRecords(snapshot); - verify(handler).onFailureBeforeCommit(snapshot); + verify(handler, never()).rollbackRecords(context); + verify(handler).onFailureBeforeCommit(context); } @Test @@ -590,9 +604,11 @@ public void commit_ExceptionThrownInPrepareRecords_ShouldAbortAndRollbackRecords doThrow(CoordinatorException.class) .when(coordinator) .putState(new Coordinator.State(anyId(), TransactionState.ABORTED)); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - assertThatThrownBy(() -> handler.commit(snapshot, false)) + assertThatThrownBy(() -> handler.commit(context)) .isInstanceOf(UnknownTransactionStatusException.class); // Assert @@ -602,8 +618,8 @@ public void commit_ExceptionThrownInPrepareRecords_ShouldAbortAndRollbackRecords verify(coordinator).putState(new Coordinator.State(anyId(), TransactionState.ABORTED)); verify(coordinator, never()) .putState(new Coordinator.State(anyId(), TransactionState.COMMITTED)); - verify(handler, never()).rollbackRecords(snapshot); - verify(handler).onFailureBeforeCommit(snapshot); + verify(handler, never()).rollbackRecords(context); + verify(handler).onFailureBeforeCommit(context); } @Test @@ -614,10 +630,12 @@ public void commit_ValidationConflictExceptionThrownInValidation_ShouldAbortAndR doNothing().when(storage).mutate(anyList()); doThrow(ValidationConflictException.class).when(snapshot).toSerializable(storage); doNothing().when(coordinator).putState(any(Coordinator.State.class)); - doNothing().when(handler).rollbackRecords(any(Snapshot.class)); + doNothing().when(handler).rollbackRecords(any(TransactionContext.class)); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - assertThatThrownBy(() -> handler.commit(snapshot, false)).isInstanceOf(CommitException.class); + assertThatThrownBy(() -> handler.commit(context)).isInstanceOf(CommitException.class); // Assert @@ -626,8 +644,8 @@ public void commit_ValidationConflictExceptionThrownInValidation_ShouldAbortAndR verify(coordinator).putState(new Coordinator.State(anyId(), TransactionState.ABORTED)); verify(coordinator, never()) .putState(new Coordinator.State(anyId(), TransactionState.COMMITTED)); - verify(handler).rollbackRecords(snapshot); - verify(handler).onFailureBeforeCommit(snapshot); + verify(handler).rollbackRecords(context); + verify(handler).onFailureBeforeCommit(context); } @Test @@ -638,10 +656,12 @@ public void commit_ExceptionThrownInValidation_ShouldAbortAndRollbackRecords() doNothing().when(storage).mutate(anyList()); doThrow(ExecutionException.class).when(snapshot).toSerializable(storage); doNothing().when(coordinator).putState(any(Coordinator.State.class)); - doNothing().when(handler).rollbackRecords(any(Snapshot.class)); + doNothing().when(handler).rollbackRecords(any(TransactionContext.class)); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - assertThatThrownBy(() -> handler.commit(snapshot, false)).isInstanceOf(CommitException.class); + assertThatThrownBy(() -> handler.commit(context)).isInstanceOf(CommitException.class); // Assert @@ -650,8 +670,8 @@ public void commit_ExceptionThrownInValidation_ShouldAbortAndRollbackRecords() verify(coordinator).putState(new Coordinator.State(anyId(), TransactionState.ABORTED)); verify(coordinator, never()) .putState(new Coordinator.State(anyId(), TransactionState.COMMITTED)); - verify(handler).rollbackRecords(snapshot); - verify(handler).onFailureBeforeCommit(snapshot); + verify(handler).rollbackRecords(context); + verify(handler).onFailureBeforeCommit(context); } @Test @@ -668,10 +688,12 @@ public void commit_ExceptionThrownInValidation_ShouldAbortAndRollbackRecords() doReturn(Optional.of(new Coordinator.State(anyId(), TransactionState.ABORTED))) .when(coordinator) .getState(anyId()); - doNothing().when(handler).rollbackRecords(any(Snapshot.class)); + doNothing().when(handler).rollbackRecords(any(TransactionContext.class)); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - assertThatThrownBy(() -> handler.commit(snapshot, false)).isInstanceOf(CommitException.class); + assertThatThrownBy(() -> handler.commit(context)).isInstanceOf(CommitException.class); // Assert @@ -681,8 +703,8 @@ public void commit_ExceptionThrownInValidation_ShouldAbortAndRollbackRecords() verify(coordinator, never()) .putState(new Coordinator.State(anyId(), TransactionState.COMMITTED)); verify(coordinator).getState(anyId()); - verify(handler).rollbackRecords(snapshot); - verify(handler).onFailureBeforeCommit(snapshot); + verify(handler).rollbackRecords(context); + verify(handler).onFailureBeforeCommit(context); } @Test @@ -697,9 +719,11 @@ public void commit_ExceptionThrownInValidation_ShouldAbortAndRollbackRecords() .when(coordinator) .putState(new Coordinator.State(anyId(), TransactionState.ABORTED)); doReturn(Optional.empty()).when(coordinator).getState(anyId()); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - assertThatThrownBy(() -> handler.commit(snapshot, false)) + assertThatThrownBy(() -> handler.commit(context)) .isInstanceOf(UnknownTransactionStatusException.class); // Assert @@ -710,8 +734,8 @@ public void commit_ExceptionThrownInValidation_ShouldAbortAndRollbackRecords() verify(coordinator, never()) .putState(new Coordinator.State(anyId(), TransactionState.COMMITTED)); verify(coordinator).getState(anyId()); - verify(handler, never()).rollbackRecords(snapshot); - verify(handler).onFailureBeforeCommit(snapshot); + verify(handler, never()).rollbackRecords(context); + verify(handler).onFailureBeforeCommit(context); } @Test @@ -726,9 +750,11 @@ public void commit_ExceptionThrownInValidation_ShouldAbortAndRollbackRecords() .when(coordinator) .putState(new Coordinator.State(anyId(), TransactionState.ABORTED)); doThrow(CoordinatorException.class).when(coordinator).getState(anyId()); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - assertThatThrownBy(() -> handler.commit(snapshot, false)) + assertThatThrownBy(() -> handler.commit(context)) .isInstanceOf(UnknownTransactionStatusException.class); // Assert @@ -739,8 +765,8 @@ public void commit_ExceptionThrownInValidation_ShouldAbortAndRollbackRecords() verify(coordinator, never()) .putState(new Coordinator.State(anyId(), TransactionState.COMMITTED)); verify(coordinator).getState(anyId()); - verify(handler, never()).rollbackRecords(snapshot); - verify(handler).onFailureBeforeCommit(snapshot); + verify(handler, never()).rollbackRecords(context); + verify(handler).onFailureBeforeCommit(context); } @Test @@ -754,9 +780,11 @@ public void commit_ExceptionThrownInValidation_ShouldAbortAndRollbackRecords() doThrow(CoordinatorException.class) .when(coordinator) .putState(new Coordinator.State(anyId(), TransactionState.ABORTED)); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - assertThatThrownBy(() -> handler.commit(snapshot, false)) + assertThatThrownBy(() -> handler.commit(context)) .isInstanceOf(UnknownTransactionStatusException.class); // Assert @@ -766,8 +794,8 @@ public void commit_ExceptionThrownInValidation_ShouldAbortAndRollbackRecords() verify(coordinator).putState(new Coordinator.State(anyId(), TransactionState.ABORTED)); verify(coordinator, never()) .putState(new Coordinator.State(anyId(), TransactionState.COMMITTED)); - verify(handler, never()).rollbackRecords(snapshot); - verify(handler).onFailureBeforeCommit(snapshot); + verify(handler, never()).rollbackRecords(context); + verify(handler).onFailureBeforeCommit(context); } @Test @@ -783,15 +811,17 @@ public void commit_ExceptionThrownInValidation_ShouldAbortAndRollbackRecords() doReturn(Optional.of(new Coordinator.State(anyId(), TransactionState.COMMITTED))) .when(coordinator) .getState(anyId()); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - handler.commit(snapshot, false); + handler.commit(context); // Assert verify(storage, times(4)).mutate(anyList()); verifyCoordinatorPutState(TransactionState.COMMITTED); verify(coordinator).getState(anyId()); - verify(handler, never()).rollbackRecords(snapshot); + verify(handler, never()).rollbackRecords(context); verify(handler, never()).onFailureBeforeCommit(any()); } @@ -807,15 +837,17 @@ public void commit_ExceptionThrownInValidation_ShouldAbortAndRollbackRecords() doReturn(Optional.of(new Coordinator.State(anyId(), TransactionState.ABORTED))) .when(coordinator) .getState(anyId()); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - assertThatThrownBy(() -> handler.commit(snapshot, false)).isInstanceOf(CommitException.class); + assertThatThrownBy(() -> handler.commit(context)).isInstanceOf(CommitException.class); // Assert verify(storage, times(2)).mutate(anyList()); verifyCoordinatorPutState(TransactionState.COMMITTED); verify(coordinator).getState(anyId()); - verify(handler).rollbackRecords(snapshot); + verify(handler).rollbackRecords(context); verify(handler, never()).onFailureBeforeCommit(any()); } @@ -829,16 +861,18 @@ public void commit_ExceptionThrownInValidation_ShouldAbortAndRollbackRecords() doThrowExceptionWhenCoordinatorPutState( TransactionState.COMMITTED, CoordinatorConflictException.class); doReturn(Optional.empty()).when(coordinator).getState(anyId()); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - assertThatThrownBy(() -> handler.commit(snapshot, false)) + assertThatThrownBy(() -> handler.commit(context)) .isInstanceOf(UnknownTransactionStatusException.class); // Assert verify(storage, times(2)).mutate(anyList()); verifyCoordinatorPutState(TransactionState.COMMITTED); verify(coordinator).getState(anyId()); - verify(handler, never()).rollbackRecords(snapshot); + verify(handler, never()).rollbackRecords(context); verify(handler, never()).onFailureBeforeCommit(any()); } @@ -852,16 +886,18 @@ public void commit_ExceptionThrownInValidation_ShouldAbortAndRollbackRecords() doThrowExceptionWhenCoordinatorPutState( TransactionState.COMMITTED, CoordinatorConflictException.class); doThrow(CoordinatorException.class).when(coordinator).getState(anyId()); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - assertThatThrownBy(() -> handler.commit(snapshot, false)) + assertThatThrownBy(() -> handler.commit(context)) .isInstanceOf(UnknownTransactionStatusException.class); // Assert verify(storage, times(2)).mutate(anyList()); verifyCoordinatorPutState(TransactionState.COMMITTED); verify(coordinator).getState(anyId()); - verify(handler, never()).rollbackRecords(snapshot); + verify(handler, never()).rollbackRecords(context); verify(handler, never()).onFailureBeforeCommit(any()); } @@ -872,51 +908,55 @@ public void commit_ExceptionThrownInCoordinatorCommit_ShouldThrowUnknown() Snapshot snapshot = prepareSnapshotWithDifferentPartitionPut(); doNothing().when(storage).mutate(anyList()); doThrowExceptionWhenCoordinatorPutState(TransactionState.COMMITTED, CoordinatorException.class); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - assertThatThrownBy(() -> handler.commit(snapshot, false)) + assertThatThrownBy(() -> handler.commit(context)) .isInstanceOf(UnknownTransactionStatusException.class); // Assert verify(storage, times(2)).mutate(anyList()); verifyCoordinatorPutState(TransactionState.COMMITTED); - verify(handler, never()).rollbackRecords(snapshot); + verify(handler, never()).rollbackRecords(context); verify(handler, never()).onFailureBeforeCommit(any()); } @Test - public void commit_SnapshotHookGiven_ShouldWaitSnapshotHookFinishesBeforeCommitState() - throws ExecutionException, CoordinatorException { + public void + commit_BeforePreparationHookGiven_ShouldWaitBeforePreparationHookFinishesBeforeCommitState() + throws ExecutionException, CoordinatorException { // Arrange Snapshot snapshot = prepareSnapshotWithDifferentPartitionPut(); - ReadWriteSets readWriteSets = snapshot.getReadWriteSets(); doNothing().when(storage).mutate(anyList()); doThrowExceptionWhenCoordinatorPutState(TransactionState.COMMITTED, CoordinatorException.class); // Lambda can't be spied... - BeforePreparationSnapshotHook delayedBeforePreparationSnapshotHook = + BeforePreparationHook delayedBeforePreparationHook = spy( - new BeforePreparationSnapshotHook() { + new BeforePreparationHook() { @Override public Future handle( TransactionTableMetadataManager tableMetadataManager, - Snapshot.ReadWriteSets readWriteSets) { + TransactionContext context) { Uninterruptibles.sleepUninterruptibly(Duration.ofSeconds(2)); - return beforePreparationSnapshotHookFuture; + return beforePreparationHookFuture; } }); - handler.setBeforePreparationSnapshotHook(delayedBeforePreparationSnapshotHook); + handler.setBeforePreparationHook(delayedBeforePreparationHook); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act Instant start = Instant.now(); - assertThatThrownBy(() -> handler.commit(snapshot, false)) + assertThatThrownBy(() -> handler.commit(context)) .isInstanceOf(UnknownTransactionStatusException.class); Instant end = Instant.now(); // Assert verify(storage, times(2)).mutate(anyList()); verifyCoordinatorPutState(TransactionState.COMMITTED); - verify(handler, never()).rollbackRecords(snapshot); - verify(delayedBeforePreparationSnapshotHook).handle(tableMetadataManager, readWriteSets); + verify(handler, never()).rollbackRecords(context); + verify(delayedBeforePreparationHook).handle(tableMetadataManager, context); // This means `commit()` waited until the callback was completed before throwing // an exception from `commitState()`. assertThat(Duration.between(start, end)).isGreaterThanOrEqualTo(Duration.ofSeconds(2)); @@ -924,58 +964,62 @@ public Future handle( } @Test - public void commit_FailingSnapshotHookGiven_ShouldThrowCommitException() + public void commit_FailingBeforePreparationHookGiven_ShouldThrowCommitException() throws ExecutionException, CoordinatorException { // Arrange Snapshot snapshot = prepareSnapshotWithDifferentPartitionPut(); doThrow(new RuntimeException("Something is wrong")) - .when(beforePreparationSnapshotHook) + .when(beforePreparationHook) .handle(any(), any()); - handler.setBeforePreparationSnapshotHook(beforePreparationSnapshotHook); + handler.setBeforePreparationHook(beforePreparationHook); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - assertThatThrownBy(() -> handler.commit(snapshot, false)).isInstanceOf(CommitException.class); + assertThatThrownBy(() -> handler.commit(context)).isInstanceOf(CommitException.class); // Assert verify(storage, never()).mutate(anyList()); verify(coordinator).putState(new Coordinator.State(anyId(), TransactionState.ABORTED)); verify(coordinator, never()) .putState(new Coordinator.State(anyId(), TransactionState.COMMITTED)); - verify(handler).rollbackRecords(snapshot); + verify(handler).rollbackRecords(context); verify(handler).onFailureBeforeCommit(any()); } @Test - public void commit_FailingSnapshotHookFutureGiven_ShouldThrowCommitException() + public void commit_FailingBeforePreparationHookFutureGiven_ShouldThrowCommitException() throws ExecutionException, CoordinatorException, java.util.concurrent.ExecutionException, InterruptedException { // Arrange Snapshot snapshot = prepareSnapshotWithDifferentPartitionPut(); doNothing().when(storage).mutate(anyList()); - doThrow(new RuntimeException("Something is wrong")) - .when(beforePreparationSnapshotHookFuture) - .get(); - setBeforePreparationSnapshotHookIfNeeded(true); + doThrow(new RuntimeException("Something is wrong")).when(beforePreparationHookFuture).get(); + setBeforePreparationHookIfNeeded(true); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - assertThatThrownBy(() -> handler.commit(snapshot, false)).isInstanceOf(CommitException.class); + assertThatThrownBy(() -> handler.commit(context)).isInstanceOf(CommitException.class); // Assert verify(storage, times(2)).mutate(anyList()); verify(coordinator).putState(new Coordinator.State(anyId(), TransactionState.ABORTED)); verify(coordinator, never()) .putState(new Coordinator.State(anyId(), TransactionState.COMMITTED)); - verify(handler).rollbackRecords(snapshot); - verify(handler).onFailureBeforeCommit(snapshot); + verify(handler).rollbackRecords(context); + verify(handler).onFailureBeforeCommit(context); } @Test public void canOnePhaseCommit_WhenOnePhaseCommitDisabled_ShouldReturnFalse() throws Exception { // Arrange Snapshot snapshot = prepareSnapshot(); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - boolean actual = handler.canOnePhaseCommit(snapshot); + boolean actual = handler.canOnePhaseCommit(context); // Assert assertThat(actual).isFalse(); @@ -987,9 +1031,11 @@ public void canOnePhaseCommit_WhenValidationRequired_ShouldReturnFalse() throws // Arrange CommitHandler handler = createCommitHandlerWithOnePhaseCommit(); Snapshot snapshot = prepareSnapshotWithIsolation(Isolation.SERIALIZABLE); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SERIALIZABLE, false, false); // Act - boolean actual = handler.canOnePhaseCommit(snapshot); + boolean actual = handler.canOnePhaseCommit(context); // Assert assertThat(actual).isFalse(); @@ -1001,9 +1047,11 @@ public void canOnePhaseCommit_WhenNoWritesAndDeletes_ShouldReturnFalse() throws // Arrange CommitHandler handler = createCommitHandlerWithOnePhaseCommit(); Snapshot snapshot = prepareSnapshotWithoutWrites(); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - boolean actual = handler.canOnePhaseCommit(snapshot); + boolean actual = handler.canOnePhaseCommit(context); // Assert assertThat(actual).isFalse(); @@ -1022,8 +1070,11 @@ public void canOnePhaseCommit_WhenDeleteWithoutExistingRecord_ShouldReturnFalse( snapshot.putIntoDeleteSet(new Snapshot.Key(delete), delete); snapshot.putIntoReadSet(new Snapshot.Key(delete), Optional.empty()); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); + // Act - boolean actual = handler.canOnePhaseCommit(snapshot); + boolean actual = handler.canOnePhaseCommit(context); // Assert assertThat(actual).isFalse(); @@ -1043,8 +1094,11 @@ public void canOnePhaseCommit_WhenMutationsCanBeGrouped_ShouldReturnTrue() throw doReturn(true).when(mutationsGrouper).canBeGroupedAltogether(anyList()); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); + // Act - boolean actual = handler.canOnePhaseCommit(snapshot); + boolean actual = handler.canOnePhaseCommit(context); // Assert assertThat(actual).isTrue(); @@ -1064,8 +1118,11 @@ public void canOnePhaseCommit_WhenMutationsCannotBeGrouped_ShouldReturnFalse() t doReturn(false).when(mutationsGrouper).canBeGroupedAltogether(anyList()); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); + // Act - boolean actual = handler.canOnePhaseCommit(snapshot); + boolean actual = handler.canOnePhaseCommit(context); // Assert assertThat(actual).isFalse(); @@ -1087,8 +1144,11 @@ public void canOnePhaseCommit_WhenMutationsCannotBeGrouped_ShouldReturnFalse() t doThrow(ExecutionException.class).when(mutationsGrouper).canBeGroupedAltogether(anyList()); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); + // Act Assert - assertThatThrownBy(() -> handler.canOnePhaseCommit(snapshot)) + assertThatThrownBy(() -> handler.canOnePhaseCommit(context)) .isInstanceOf(CommitException.class) .hasCauseInstanceOf(ExecutionException.class); } @@ -1099,9 +1159,11 @@ public void onePhaseCommitRecords_WhenSuccessful_ShouldMutateUsingComposerMutati // Arrange Snapshot snapshot = spy(prepareSnapshotWithSamePartitionPut()); doNothing().when(storage).mutate(anyList()); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act - handler.onePhaseCommitRecords(snapshot); + handler.onePhaseCommitRecords(context); // Assert verify(storage).mutate(anyList()); @@ -1115,9 +1177,11 @@ public void onePhaseCommitRecords_WhenSuccessful_ShouldMutateUsingComposerMutati // Arrange Snapshot snapshot = prepareSnapshotWithSamePartitionPut(); doThrow(NoMutationException.class).when(storage).mutate(anyList()); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act Assert - assertThatThrownBy(() -> handler.onePhaseCommitRecords(snapshot)) + assertThatThrownBy(() -> handler.onePhaseCommitRecords(context)) .isInstanceOf(CommitConflictException.class) .hasCauseInstanceOf(NoMutationException.class); } @@ -1129,9 +1193,11 @@ public void onePhaseCommitRecords_WhenSuccessful_ShouldMutateUsingComposerMutati // Arrange Snapshot snapshot = prepareSnapshotWithSamePartitionPut(); doThrow(RetriableExecutionException.class).when(storage).mutate(anyList()); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act Assert - assertThatThrownBy(() -> handler.onePhaseCommitRecords(snapshot)) + assertThatThrownBy(() -> handler.onePhaseCommitRecords(context)) .isInstanceOf(CommitConflictException.class) .hasCauseInstanceOf(RetriableExecutionException.class); } @@ -1143,9 +1209,11 @@ public void onePhaseCommitRecords_WhenSuccessful_ShouldMutateUsingComposerMutati // Arrange Snapshot snapshot = prepareSnapshotWithSamePartitionPut(); doThrow(ExecutionException.class).when(storage).mutate(anyList()); + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, false, false); // Act Assert - assertThatThrownBy(() -> handler.onePhaseCommitRecords(snapshot)) + assertThatThrownBy(() -> handler.onePhaseCommitRecords(context)) .isInstanceOf(UnknownTransactionStatusException.class) .hasCauseInstanceOf(ExecutionException.class); } @@ -1157,15 +1225,18 @@ public void commit_OnePhaseCommitted_ShouldNotThrowAnyException() CommitHandler handler = spy(createCommitHandlerWithOnePhaseCommit()); Snapshot snapshot = prepareSnapshotWithSamePartitionPut(); - doReturn(true).when(handler).canOnePhaseCommit(snapshot); - doNothing().when(handler).onePhaseCommitRecords(snapshot); + doReturn(true).when(handler).canOnePhaseCommit(any(TransactionContext.class)); + doNothing().when(handler).onePhaseCommitRecords(any(TransactionContext.class)); + + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, true, false); // Act - handler.commit(snapshot, true); + handler.commit(context); // Assert - verify(handler).canOnePhaseCommit(snapshot); - verify(handler).onePhaseCommitRecords(snapshot); + verify(handler).canOnePhaseCommit(context); + verify(handler).onePhaseCommitRecords(context); } @Test @@ -1176,14 +1247,19 @@ public void commit_OnePhaseCommitted_ShouldNotThrowAnyException() CommitHandler handler = spy(createCommitHandlerWithOnePhaseCommit()); Snapshot snapshot = prepareSnapshotWithSamePartitionPut(); - doReturn(true).when(handler).canOnePhaseCommit(snapshot); - doThrow(UnknownTransactionStatusException.class).when(handler).onePhaseCommitRecords(snapshot); + doReturn(true).when(handler).canOnePhaseCommit(any(TransactionContext.class)); + doThrow(UnknownTransactionStatusException.class) + .when(handler) + .onePhaseCommitRecords(any(TransactionContext.class)); + + TransactionContext context = + new TransactionContext(anyId(), snapshot, Isolation.SNAPSHOT, true, false); // Act Assert - assertThatThrownBy(() -> handler.commit(snapshot, true)) + assertThatThrownBy(() -> handler.commit(context)) .isInstanceOf(UnknownTransactionStatusException.class); - verify(handler).onFailureBeforeCommit(snapshot); + verify(handler).onFailureBeforeCommit(context); } protected void doThrowExceptionWhenCoordinatorPutState( @@ -1201,11 +1277,12 @@ protected void verifyCoordinatorPutState(TransactionState expectedTransactionSta verify(coordinator).putState(new Coordinator.State(anyId(), expectedTransactionState)); } - private void verifySnapshotHook(boolean withSnapshotHook, Snapshot.ReadWriteSets readWriteSets) { - if (withSnapshotHook) { - verify(beforePreparationSnapshotHook).handle(eq(tableMetadataManager), eq(readWriteSets)); + private void verifyBeforePreparationHook( + boolean withBeforePreparationHook, TransactionContext context) { + if (withBeforePreparationHook) { + verify(beforePreparationHook).handle(eq(tableMetadataManager), eq(context)); } else { - verify(beforePreparationSnapshotHook, never()).handle(any(), any()); + verify(beforePreparationHook, never()).handle(any(), any()); } } } 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 f79eecae55..3dca1951d3 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 @@ -130,10 +130,11 @@ protected void verifyCoordinatorPutState(TransactionState expectedTransactionSta @ValueSource(booleans = {false, true}) @Override public void commit_SnapshotWithDifferentPartitionPutsGiven_ShouldCommitRespectively( - boolean withSnapshotHook) + boolean withBeforePreparationHook) throws CommitException, UnknownTransactionStatusException, ExecutionException, CoordinatorException, ValidationConflictException { - super.commit_SnapshotWithDifferentPartitionPutsGiven_ShouldCommitRespectively(withSnapshotHook); + super.commit_SnapshotWithDifferentPartitionPutsGiven_ShouldCommitRespectively( + withBeforePreparationHook); // Assert verify(groupCommitter, never()).remove(anyId()); @@ -142,10 +143,11 @@ public void commit_SnapshotWithDifferentPartitionPutsGiven_ShouldCommitRespectiv @ParameterizedTest @ValueSource(booleans = {false, true}) @Override - public void commit_SnapshotWithSamePartitionPutsGiven_ShouldCommitAtOnce(boolean withSnapshotHook) + public void commit_SnapshotWithSamePartitionPutsGiven_ShouldCommitAtOnce( + boolean withBeforePreparationHook) throws CommitException, UnknownTransactionStatusException, ExecutionException, CoordinatorException, ValidationConflictException { - super.commit_SnapshotWithSamePartitionPutsGiven_ShouldCommitAtOnce(withSnapshotHook); + super.commit_SnapshotWithSamePartitionPutsGiven_ShouldCommitAtOnce(withBeforePreparationHook); // Assert verify(groupCommitter, never()).remove(anyId()); @@ -155,7 +157,7 @@ public void commit_SnapshotWithSamePartitionPutsGiven_ShouldCommitAtOnce(boolean @ValueSource(booleans = {false, true}) @Override public void commit_InReadOnlyMode_ShouldNotPrepareRecordsAndCommitStateAndCommitRecords( - boolean withSnapshotHook) + boolean withBeforePreparationHook) throws CommitException, UnknownTransactionStatusException, ExecutionException, CoordinatorException, ValidationConflictException { // Arrange @@ -163,7 +165,7 @@ public void commit_InReadOnlyMode_ShouldNotPrepareRecordsAndCommitStateAndCommit clearInvocations(groupCommitter); super.commit_InReadOnlyMode_ShouldNotPrepareRecordsAndCommitStateAndCommitRecords( - withSnapshotHook); + withBeforePreparationHook); // Assert verify(groupCommitter, never()).remove(anyId()); @@ -174,12 +176,12 @@ public void commit_InReadOnlyMode_ShouldNotPrepareRecordsAndCommitStateAndCommit @Override public void commit_NoWritesAndDeletesInSnapshot_ShouldNotPrepareRecordsAndCommitStateAndCommitRecords( - boolean withSnapshotHook) + boolean withBeforePreparationHook) throws CommitException, UnknownTransactionStatusException, ExecutionException, CoordinatorException, ValidationConflictException { super.commit_NoWritesAndDeletesInSnapshot_ShouldNotPrepareRecordsAndCommitStateAndCommitRecords( - withSnapshotHook); + withBeforePreparationHook); // Assert verify(groupCommitter).remove(anyId()); @@ -190,12 +192,12 @@ public void commit_InReadOnlyMode_ShouldNotPrepareRecordsAndCommitStateAndCommit @Override public void commit_NoWritesAndDeletesInSnapshot_CoordinatorWriteOmissionOnReadOnlyDisabled_ShouldNotPrepareRecordsAndCommitRecordsButShouldCommitState( - boolean withSnapshotHook) + boolean withBeforePreparationHook) throws CommitException, UnknownTransactionStatusException, ExecutionException, CoordinatorException, ValidationConflictException { super .commit_NoWritesAndDeletesInSnapshot_CoordinatorWriteOmissionOnReadOnlyDisabled_ShouldNotPrepareRecordsAndCommitRecordsButShouldCommitState( - withSnapshotHook); + withBeforePreparationHook); // Assert verify(groupCommitter, never()).remove(anyId()); @@ -204,10 +206,10 @@ public void commit_InReadOnlyMode_ShouldNotPrepareRecordsAndCommitStateAndCommit @ParameterizedTest @ValueSource(booleans = {false, true}) @Override - public void commit_NoReadsInSnapshot_ShouldNotValidateRecords(boolean withSnapshotHook) + public void commit_NoReadsInSnapshot_ShouldNotValidateRecords(boolean withBeforePreparationHook) throws CommitException, UnknownTransactionStatusException, ExecutionException, CoordinatorException, ValidationConflictException { - super.commit_NoReadsInSnapshot_ShouldNotValidateRecords(withSnapshotHook); + super.commit_NoReadsInSnapshot_ShouldNotValidateRecords(withBeforePreparationHook); // Assert verify(groupCommitter, never()).remove(anyId()); diff --git a/core/src/test/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitManagerTest.java b/core/src/test/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitManagerTest.java index 6d26ef4fde..ab4840a739 100644 --- a/core/src/test/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitManagerTest.java +++ b/core/src/test/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitManagerTest.java @@ -3,7 +3,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; @@ -63,6 +62,7 @@ public class ConsensusCommitManagerTest { @Mock private Coordinator coordinator; @Mock private ParallelExecutor parallelExecutor; @Mock private RecoveryExecutor recoveryExecutor; + @Mock private CrudHandler crud; @Mock private CommitHandler commit; private ConsensusCommitManager manager; @@ -79,9 +79,9 @@ public void setUp() throws Exception { coordinator, parallelExecutor, recoveryExecutor, + crud, commit, Isolation.SNAPSHOT, - false, null); } @@ -93,9 +93,11 @@ public void begin_NoArgumentGiven_ReturnConsensusCommitWithSomeTxIdAndSnapshotIs ConsensusCommit transaction = (ConsensusCommit) manager.begin(); // Assert - assertThat(transaction.getCrudHandler().getSnapshot().getId()).isNotNull(); - assertThat(transaction.getCrudHandler().getSnapshot().getIsolation()) - .isEqualTo(Isolation.SNAPSHOT); + assertThat(transaction.getTransactionContext().transactionId).isNotNull(); + assertThat(transaction.getTransactionContext().isolation).isEqualTo(Isolation.SNAPSHOT); + assertThat(transaction.getTransactionContext().readOnly).isFalse(); + assertThat(transaction.getCrudHandler()).isEqualTo(crud); + assertThat(transaction.getCommitHandler()).isEqualTo(commit); } @Test @@ -106,9 +108,11 @@ public void begin_TxIdGiven_ReturnWithSpecifiedTxIdAndSnapshotIsolation() { ConsensusCommit transaction = (ConsensusCommit) manager.begin(ANY_TX_ID); // Assert - assertThat(transaction.getCrudHandler().getSnapshot().getId()).isEqualTo(ANY_TX_ID); - assertThat(transaction.getCrudHandler().getSnapshot().getIsolation()) - .isEqualTo(Isolation.SNAPSHOT); + assertThat(transaction.getTransactionContext().transactionId).isEqualTo(ANY_TX_ID); + assertThat(transaction.getTransactionContext().isolation).isEqualTo(Isolation.SNAPSHOT); + assertThat(transaction.getTransactionContext().readOnly).isFalse(); + assertThat(transaction.getCrudHandler()).isEqualTo(crud); + assertThat(transaction.getCommitHandler()).isEqualTo(commit); } @Test @@ -129,21 +133,22 @@ public void begin_TxIdGiven_ReturnWithSpecifiedTxIdAndSnapshotIsolation() { coordinator, parallelExecutor, recoveryExecutor, + crud, commit, Isolation.SNAPSHOT, - false, groupCommitter); // Act ConsensusCommit transaction = (ConsensusCommit) managerWithGroupCommit.begin(ANY_TX_ID); // Assert - assertThat(transaction.getId()).isEqualTo(fullKey); - Snapshot snapshot = transaction.getCrudHandler().getSnapshot(); - assertThat(snapshot.getId()).isEqualTo(fullKey); + assertThat(transaction.getTransactionContext().transactionId).isEqualTo(fullKey); + assertThat(transaction.getTransactionContext().isolation).isEqualTo(Isolation.SNAPSHOT); + assertThat(transaction.getTransactionContext().readOnly).isFalse(); + assertThat(transaction.getCrudHandler()).isEqualTo(crud); + assertThat(transaction.getCommitHandler()).isEqualTo(commit); assertThat(keyManipulator.isFullKey(transaction.getId())).isTrue(); verify(groupCommitter).reserve(ANY_TX_ID); - assertThat(snapshot.getIsolation()).isEqualTo(Isolation.SNAPSHOT); } @Test @@ -161,28 +166,29 @@ public void begin_TxIdGiven_ReturnWithSpecifiedTxIdAndSnapshotIsolation() { coordinator, parallelExecutor, recoveryExecutor, + crud, commit, Isolation.SNAPSHOT, - false, groupCommitter); // Act - DistributedTransaction transaction = managerWithGroupCommit.beginReadOnly(ANY_TX_ID); + ConsensusCommit transaction = + (ConsensusCommit) + ((DecoratedDistributedTransaction) managerWithGroupCommit.beginReadOnly(ANY_TX_ID)) + .getOriginalTransaction(); // Assert - assertThat(transaction.getId()).isEqualTo(ANY_TX_ID); - Snapshot snapshot = - ((ConsensusCommit) ((DecoratedDistributedTransaction) transaction).getOriginalTransaction()) - .getCrudHandler() - .getSnapshot(); - assertThat(snapshot.getId()).isEqualTo(ANY_TX_ID); + assertThat(transaction.getTransactionContext().transactionId).isEqualTo(ANY_TX_ID); + assertThat(transaction.getTransactionContext().isolation).isEqualTo(Isolation.SNAPSHOT); + assertThat(transaction.getTransactionContext().readOnly).isTrue(); + assertThat(transaction.getCrudHandler()).isEqualTo(crud); + assertThat(transaction.getCommitHandler()).isEqualTo(commit); assertThat(keyManipulator.isFullKey(transaction.getId())).isFalse(); verify(groupCommitter, never()).reserve(ANY_TX_ID); - assertThat(snapshot.getIsolation()).isEqualTo(Isolation.SNAPSHOT); } @Test - public void begin_CalledTwice_ReturnRespectiveConsensusCommitWithSharedCommitAndRecovery() { + public void begin_CalledTwice_ShouldReturnTransactionsWithSharedHandlers() { // Arrange // Act @@ -190,12 +196,11 @@ public void begin_CalledTwice_ReturnRespectiveConsensusCommitWithSharedCommitAnd ConsensusCommit transaction2 = (ConsensusCommit) manager.begin(); // Assert - assertThat(transaction1.getCrudHandler()).isNotEqualTo(transaction2.getCrudHandler()); - assertThat(transaction1.getCrudHandler().getSnapshot().getId()) - .isNotEqualTo(transaction2.getCrudHandler().getSnapshot().getId()); - assertThat(transaction1.getCommitHandler()) - .isEqualTo(transaction2.getCommitHandler()) - .isEqualTo(commit); + assertThat(transaction1.getCrudHandler()).isSameAs(transaction2.getCrudHandler()); + assertThat(transaction1.getCommitHandler()).isSameAs(transaction2.getCommitHandler()); + assertThat(transaction1.getTransactionContext()) + .isNotSameAs(transaction2.getTransactionContext()); + assertThat(transaction1.getId()).isNotEqualTo(transaction2.getId()); } @Test @@ -248,9 +253,11 @@ public void start_NoArgumentGiven_ReturnConsensusCommitWithSomeTxIdAndSnapshotIs ConsensusCommit transaction = (ConsensusCommit) manager.start(); // Assert - assertThat(transaction.getCrudHandler().getSnapshot().getId()).isNotNull(); - assertThat(transaction.getCrudHandler().getSnapshot().getIsolation()) - .isEqualTo(Isolation.SNAPSHOT); + assertThat(transaction.getTransactionContext().transactionId).isNotNull(); + assertThat(transaction.getTransactionContext().isolation).isEqualTo(Isolation.SNAPSHOT); + assertThat(transaction.getTransactionContext().readOnly).isFalse(); + assertThat(transaction.getCrudHandler()).isEqualTo(crud); + assertThat(transaction.getCommitHandler()).isEqualTo(commit); } @Test @@ -262,9 +269,11 @@ public void start_TxIdGiven_ReturnWithSpecifiedTxIdAndSnapshotIsolation() ConsensusCommit transaction = (ConsensusCommit) manager.start(ANY_TX_ID); // Assert - assertThat(transaction.getCrudHandler().getSnapshot().getId()).isEqualTo(ANY_TX_ID); - assertThat(transaction.getCrudHandler().getSnapshot().getIsolation()) - .isEqualTo(Isolation.SNAPSHOT); + assertThat(transaction.getTransactionContext().transactionId).isEqualTo(ANY_TX_ID); + assertThat(transaction.getTransactionContext().isolation).isEqualTo(Isolation.SNAPSHOT); + assertThat(transaction.getTransactionContext().readOnly).isFalse(); + assertThat(transaction.getCrudHandler()).isEqualTo(crud); + assertThat(transaction.getCommitHandler()).isEqualTo(commit); } @Test @@ -276,9 +285,11 @@ public void start_SerializableGiven_ReturnConsensusCommitWithSomeTxIdAndSerializ (ConsensusCommit) manager.start(com.scalar.db.api.Isolation.SERIALIZABLE); // Assert - assertThat(transaction.getCrudHandler().getSnapshot().getId()).isNotNull(); - assertThat(transaction.getCrudHandler().getSnapshot().getIsolation()) - .isEqualTo(Isolation.SERIALIZABLE); + assertThat(transaction.getTransactionContext().transactionId).isNotNull(); + assertThat(transaction.getTransactionContext().isolation).isEqualTo(Isolation.SERIALIZABLE); + assertThat(transaction.getTransactionContext().readOnly).isFalse(); + assertThat(transaction.getCrudHandler()).isEqualTo(crud); + assertThat(transaction.getCommitHandler()).isEqualTo(commit); } @Test @@ -291,7 +302,7 @@ public void start_NullIsolationGiven_ThrowNullPointerExceptionException() { } @Test - public void start_CalledTwice_ReturnRespectiveConsensusCommitWithSharedCommitAndRecovery() + public void start_CalledTwice_ShouldReturnTransactionsWithSharedHandlers() throws TransactionException { // Arrange @@ -300,12 +311,11 @@ public void start_CalledTwice_ReturnRespectiveConsensusCommitWithSharedCommitAnd ConsensusCommit transaction2 = (ConsensusCommit) manager.start(); // Assert - assertThat(transaction1.getCrudHandler()).isNotEqualTo(transaction2.getCrudHandler()); - assertThat(transaction1.getCrudHandler().getSnapshot().getId()) - .isNotEqualTo(transaction2.getCrudHandler().getSnapshot().getId()); - assertThat(transaction1.getCommitHandler()) - .isEqualTo(transaction2.getCommitHandler()) - .isEqualTo(commit); + assertThat(transaction1.getCrudHandler()).isSameAs(transaction2.getCrudHandler()); + assertThat(transaction1.getCommitHandler()).isSameAs(transaction2.getCommitHandler()); + assertThat(transaction1.getTransactionContext()) + .isNotSameAs(transaction2.getTransactionContext()); + assertThat(transaction1.getId()).isNotEqualTo(transaction2.getId()); } @Test @@ -399,7 +409,7 @@ public void resume_CalledWithBeginAndCommit_CommitExceptionThrown_ReturnSameTran DistributedTransactionManager manager = new ActiveTransactionManagedDistributedTransactionManager(this.manager, -1); - doThrow(CommitException.class).when(commit).commit(any(), anyBoolean()); + doThrow(CommitException.class).when(commit).commit(any(TransactionContext.class)); DistributedTransaction transaction1 = manager.begin(ANY_TX_ID); try { @@ -478,7 +488,7 @@ public void join_CalledWithBeginAndCommit_CommitExceptionThrown_ReturnSameTransa DistributedTransactionManager manager = new ActiveTransactionManagedDistributedTransactionManager(this.manager, -1); - doThrow(CommitException.class).when(commit).commit(any(), anyBoolean()); + doThrow(CommitException.class).when(commit).commit(any(TransactionContext.class)); DistributedTransaction transaction1 = manager.begin(ANY_TX_ID); try { diff --git a/core/src/test/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitTest.java b/core/src/test/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitTest.java index 9398fe29a4..bdb5e2df16 100644 --- a/core/src/test/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitTest.java +++ b/core/src/test/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitTest.java @@ -4,13 +4,12 @@ import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.doNothing; -import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -39,7 +38,6 @@ import java.util.Optional; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @@ -53,25 +51,23 @@ public class ConsensusCommitTest { private static final String ANY_TEXT_2 = "text2"; private static final String ANY_TEXT_3 = "text3"; private static final String ANY_TEXT_4 = "text4"; + private static final String ANY_ID = "id"; + private TransactionContext context; @Mock private Snapshot snapshot; @Mock private CrudHandler crud; @Mock private CommitHandler commit; - - @SuppressWarnings("unused") - @Mock - private ConsensusCommitManager manager; - @Mock private ConsensusCommitMutationOperationChecker mutationOperationChecker; - @InjectMocks private ConsensusCommit consensus; + private ConsensusCommit consensus; @BeforeEach public void setUp() throws Exception { MockitoAnnotations.openMocks(this).close(); // Arrange - when(crud.areAllScannersClosed()).thenReturn(true); + context = spy(new TransactionContext(ANY_ID, snapshot, Isolation.SNAPSHOT, false, false)); + consensus = new ConsensusCommit(context, crud, commit, mutationOperationChecker, null); } private Get prepareGet() { @@ -109,14 +105,14 @@ public void get_GetGiven_ShouldCallCrudHandlerGet() throws CrudException { // Arrange Get get = prepareGet(); TransactionResult result = mock(TransactionResult.class); - when(crud.get(get)).thenReturn(Optional.of(result)); + when(crud.get(get, context)).thenReturn(Optional.of(result)); // Act Optional actual = consensus.get(get); // Assert assertThat(actual).isPresent(); - verify(crud).get(get); + verify(crud).get(get, context); } @Test @@ -125,14 +121,14 @@ public void scan_ScanGiven_ShouldCallCrudHandlerScan() throws CrudException { Scan scan = prepareScan(); TransactionResult result = mock(TransactionResult.class); List results = Collections.singletonList(result); - when(crud.scan(scan)).thenReturn(results); + when(crud.scan(scan, context)).thenReturn(results); // Act List actual = consensus.scan(scan); // Assert assertThat(actual.size()).isEqualTo(1); - verify(crud).scan(scan); + verify(crud).scan(scan, context); } @Test @@ -143,7 +139,7 @@ public void getScannerAndScannerOne_ShouldCallCrudHandlerGetScannerAndScannerOne TransactionCrudOperable.Scanner scanner = mock(TransactionCrudOperable.Scanner.class); Result result = mock(Result.class); when(scanner.one()).thenReturn(Optional.of(result)); - when(crud.getScanner(scan)).thenReturn(scanner); + when(crud.getScanner(scan, context)).thenReturn(scanner); // Act TransactionCrudOperable.Scanner actualScanner = consensus.getScanner(scan); @@ -151,7 +147,7 @@ public void getScannerAndScannerOne_ShouldCallCrudHandlerGetScannerAndScannerOne // Assert assertThat(actualResult).hasValue(result); - verify(crud).getScanner(scan); + verify(crud).getScanner(scan, context); verify(scanner).one(); } @@ -164,7 +160,7 @@ public void getScannerAndScannerAll_ShouldCallCrudHandlerGetScannerAndScannerAll Result result1 = mock(Result.class); Result result2 = mock(Result.class); when(scanner.all()).thenReturn(Arrays.asList(result1, result2)); - when(crud.getScanner(scan)).thenReturn(scanner); + when(crud.getScanner(scan, context)).thenReturn(scanner); // Act TransactionCrudOperable.Scanner actualScanner = consensus.getScanner(scan); @@ -172,7 +168,7 @@ public void getScannerAndScannerAll_ShouldCallCrudHandlerGetScannerAndScannerAll // Assert assertThat(actualResults).containsExactly(result1, result2); - verify(crud).getScanner(scan); + verify(crud).getScanner(scan, context); verify(scanner).all(); } @@ -180,13 +176,13 @@ public void getScannerAndScannerAll_ShouldCallCrudHandlerGetScannerAndScannerAll public void put_PutGiven_ShouldCallCrudHandlerPut() throws ExecutionException, CrudException { // Arrange Put put = preparePut(); - doNothing().when(crud).put(put); + doNothing().when(crud).put(put, context); // Act consensus.put(put); // Assert - verify(crud).put(put); + verify(crud).put(put, context); verify(mutationOperationChecker).check(put); } @@ -195,13 +191,13 @@ public void put_TwoPutsGiven_ShouldCallCrudHandlerPutTwice() throws ExecutionException, CrudException { // Arrange Put put = preparePut(); - doNothing().when(crud).put(put); + doNothing().when(crud).put(put, context); // Act consensus.put(Arrays.asList(put, put)); // Assert - verify(crud, times(2)).put(put); + verify(crud, times(2)).put(put, context); verify(mutationOperationChecker, times(2)).check(put); } @@ -210,13 +206,13 @@ public void delete_DeleteGiven_ShouldCallCrudHandlerDelete() throws CrudException, ExecutionException { // Arrange Delete delete = prepareDelete(); - doNothing().when(crud).delete(delete); + doNothing().when(crud).delete(delete, context); // Act consensus.delete(delete); // Assert - verify(crud).delete(delete); + verify(crud).delete(delete, context); verify(mutationOperationChecker).check(delete); } @@ -225,13 +221,13 @@ public void delete_TwoDeletesGiven_ShouldCallCrudHandlerDeleteTwice() throws ExecutionException, CrudException { // Arrange Delete delete = prepareDelete(); - doNothing().when(crud).delete(delete); + doNothing().when(crud).delete(delete, context); // Act consensus.delete(Arrays.asList(delete, delete)); // Assert - verify(crud, times(2)).delete(delete); + verify(crud, times(2)).delete(delete, context); verify(mutationOperationChecker, times(2)).check(delete); } @@ -261,7 +257,7 @@ public void insert_InsertGiven_ShouldCallCrudHandlerPut() .textValue(ANY_NAME_3, ANY_TEXT_3) .enableInsertMode() .build(); - verify(crud).put(expectedPut); + verify(crud).put(expectedPut, context); verify(mutationOperationChecker).check(expectedPut); } @@ -291,7 +287,7 @@ public void upsert_UpsertGiven_ShouldCallCrudHandlerPut() .textValue(ANY_NAME_3, ANY_TEXT_3) .enableImplicitPreRead() .build(); - verify(crud).put(expectedPut); + verify(crud).put(expectedPut, context); verify(mutationOperationChecker).check(expectedPut); } @@ -322,7 +318,7 @@ public void update_UpdateWithoutConditionGiven_ShouldCallCrudHandlerPut() .condition(ConditionBuilder.putIfExists()) .enableImplicitPreRead() .build(); - verify(crud).put(expectedPut); + verify(crud).put(expectedPut, context); verify(mutationOperationChecker).check(expectedPut); } @@ -360,7 +356,7 @@ public void update_UpdateWithConditionGiven_ShouldCallCrudHandlerPut() .build()) .enableImplicitPreRead() .build(); - verify(crud).put(expectedPut); + verify(crud).put(expectedPut, context); verify(mutationOperationChecker).check(expectedPut); } @@ -388,10 +384,7 @@ public void update_UpdateWithConditionGiven_ShouldCallCrudHandlerPut() .enableImplicitPreRead() .build(); - when(crud.getSnapshot()).thenReturn(snapshot); - when(snapshot.getId()).thenReturn("id"); - - doThrow(UnsatisfiedConditionException.class).when(crud).put(put); + doThrow(UnsatisfiedConditionException.class).when(crud).put(put, context); // Act Assert assertThatCode(() -> consensus.update(update)).doesNotThrowAnyException(); @@ -428,13 +421,10 @@ public void update_UpdateWithConditionGiven_ShouldCallCrudHandlerPut() .enableImplicitPreRead() .build(); - when(crud.getSnapshot()).thenReturn(snapshot); - when(snapshot.getId()).thenReturn("id"); - UnsatisfiedConditionException unsatisfiedConditionException = mock(UnsatisfiedConditionException.class); when(unsatisfiedConditionException.getMessage()).thenReturn("PutIf"); - doThrow(unsatisfiedConditionException).when(crud).put(put); + doThrow(unsatisfiedConditionException).when(crud).put(put, context); // Act Assert assertThatThrownBy(() -> consensus.update(update)) @@ -468,13 +458,10 @@ public void update_UpdateWithConditionGiven_ShouldCallCrudHandlerPut() .enableImplicitPreRead() .build(); - when(crud.getSnapshot()).thenReturn(snapshot); - when(snapshot.getId()).thenReturn("id"); - UnsatisfiedConditionException unsatisfiedConditionException = mock(UnsatisfiedConditionException.class); when(unsatisfiedConditionException.getMessage()).thenReturn("PutIfExists"); - doThrow(unsatisfiedConditionException).when(crud).put(put); + doThrow(unsatisfiedConditionException).when(crud).put(put, context); // Act Assert assertThatThrownBy(() -> consensus.update(update)) @@ -489,15 +476,15 @@ public void mutate_PutAndDeleteGiven_ShouldCallCrudHandlerPutAndDelete() // Arrange Put put = preparePut(); Delete delete = prepareDelete(); - doNothing().when(crud).put(put); - doNothing().when(crud).delete(delete); + doNothing().when(crud).put(put, context); + doNothing().when(crud).delete(delete, context); // Act Assert consensus.mutate(Arrays.asList(put, delete)); // Assert - verify(crud).put(put); - verify(crud).delete(delete); + verify(crud).put(put, context); + verify(crud).delete(delete, context); verify(mutationOperationChecker).check(put); verify(mutationOperationChecker).check(delete); } @@ -506,36 +493,34 @@ public void mutate_PutAndDeleteGiven_ShouldCallCrudHandlerPutAndDelete() public void commit_ProcessedCrudGiven_ShouldCommitWithSnapshot() throws CommitException, UnknownTransactionStatusException, CrudException { // Arrange - doNothing().when(commit).commit(any(Snapshot.class), anyBoolean()); - when(crud.getSnapshot()).thenReturn(snapshot); - when(crud.isReadOnly()).thenReturn(false); + doNothing().when(commit).commit(any(TransactionContext.class)); // Act consensus.commit(); // Assert - verify(crud).areAllScannersClosed(); - verify(crud).readIfImplicitPreReadEnabled(); - verify(crud).waitForRecoveryCompletionIfNecessary(); - verify(commit).commit(snapshot, false); + verify(context).areAllScannersClosed(); + verify(crud).readIfImplicitPreReadEnabled(context); + verify(crud).waitForRecoveryCompletionIfNecessary(context); + verify(commit).commit(context); } @Test public void commit_ProcessedCrudGiven_InReadOnlyMode_ShouldCommitWithSnapshot() throws CommitException, UnknownTransactionStatusException, CrudException { // Arrange - doNothing().when(commit).commit(any(Snapshot.class), anyBoolean()); - when(crud.getSnapshot()).thenReturn(snapshot); - when(crud.isReadOnly()).thenReturn(true); + doNothing().when(commit).commit(any(TransactionContext.class)); + context = spy(new TransactionContext(ANY_ID, snapshot, Isolation.SNAPSHOT, true, false)); + consensus = new ConsensusCommit(context, crud, commit, mutationOperationChecker, null); // Act consensus.commit(); // Assert - verify(crud).areAllScannersClosed(); - verify(crud).readIfImplicitPreReadEnabled(); - verify(crud).waitForRecoveryCompletionIfNecessary(); - verify(commit).commit(snapshot, true); + verify(context).areAllScannersClosed(); + verify(crud).readIfImplicitPreReadEnabled(context); + verify(crud).waitForRecoveryCompletionIfNecessary(context); + verify(commit).commit(context); } @Test @@ -543,8 +528,7 @@ public void commit_ProcessedCrudGiven_InReadOnlyMode_ShouldCommitWithSnapshot() commit_ProcessedCrudGiven_CrudConflictExceptionThrownWhileImplicitPreRead_ShouldThrowCommitConflictException() throws CrudException { // Arrange - when(crud.getSnapshot()).thenReturn(snapshot); - doThrow(CrudConflictException.class).when(crud).readIfImplicitPreReadEnabled(); + doThrow(CrudConflictException.class).when(crud).readIfImplicitPreReadEnabled(context); // Act Assert assertThatThrownBy(() -> consensus.commit()).isInstanceOf(CommitConflictException.class); @@ -555,8 +539,7 @@ public void commit_ProcessedCrudGiven_InReadOnlyMode_ShouldCommitWithSnapshot() commit_ProcessedCrudGiven_CrudExceptionThrownWhileImplicitPreRead_ShouldThrowCommitException() throws CrudException { // Arrange - when(crud.getSnapshot()).thenReturn(snapshot); - doThrow(CrudException.class).when(crud).readIfImplicitPreReadEnabled(); + doThrow(CrudException.class).when(crud).readIfImplicitPreReadEnabled(context); // Act Assert assertThatThrownBy(() -> consensus.commit()).isInstanceOf(CommitException.class); @@ -565,7 +548,7 @@ public void commit_ProcessedCrudGiven_InReadOnlyMode_ShouldCommitWithSnapshot() @Test public void commit_ScannerNotClosed_ShouldThrowIllegalStateException() { // Arrange - when(crud.areAllScannersClosed()).thenReturn(false); + when(context.areAllScannersClosed()).thenReturn(false); // Act Assert assertThatThrownBy(() -> consensus.commit()).isInstanceOf(IllegalStateException.class); @@ -576,8 +559,9 @@ public void commit_ScannerNotClosed_ShouldThrowIllegalStateException() { commit_CrudConflictExceptionThrownByCrudHandlerWaitForRecoveryCompletionIfNecessary_ShouldThrowCommitConflictException() throws CrudException { // Arrange - when(crud.getSnapshot()).thenReturn(snapshot); - doThrow(CrudConflictException.class).when(crud).waitForRecoveryCompletionIfNecessary(); + CrudConflictException crudConflictException = mock(CrudConflictException.class); + when(crudConflictException.getMessage()).thenReturn("error"); + doThrow(crudConflictException).when(crud).waitForRecoveryCompletionIfNecessary(context); // Act Assert assertThatThrownBy(() -> consensus.commit()).isInstanceOf(CommitConflictException.class); @@ -588,8 +572,9 @@ public void commit_ScannerNotClosed_ShouldThrowIllegalStateException() { commit_CrudExceptionThrownByCrudHandlerWaitForRecoveryCompletionIfNecessary_ShouldThrowCommitException() throws CrudException { // Arrange - when(crud.getSnapshot()).thenReturn(snapshot); - doThrow(CrudException.class).when(crud).waitForRecoveryCompletionIfNecessary(); + CrudException crudException = mock(CrudException.class); + when(crudException.getMessage()).thenReturn("error"); + doThrow(crudException).when(crud).waitForRecoveryCompletionIfNecessary(context); // Act Assert assertThatThrownBy(() -> consensus.commit()).isInstanceOf(CommitException.class); @@ -603,8 +588,8 @@ public void rollback_ShouldDoNothing() throws CrudException, UnknownTransactionS consensus.rollback(); // Assert - verify(crud).closeScanners(); - verify(commit, never()).rollbackRecords(any(Snapshot.class)); + verify(context).closeScanners(); + verify(commit, never()).rollbackRecords(any(TransactionContext.class)); verify(commit, never()).abortState(anyString()); } @@ -612,21 +597,17 @@ public void rollback_ShouldDoNothing() throws CrudException, UnknownTransactionS public void rollback_WithGroupCommitter_ShouldRemoveTxFromGroupCommitter() throws CrudException, UnknownTransactionStatusException { // Arrange - String txId = "tx-id"; - Snapshot snapshot = mock(Snapshot.class); - doReturn(txId).when(snapshot).getId(); - doReturn(snapshot).when(crud).getSnapshot(); CoordinatorGroupCommitter groupCommitter = mock(CoordinatorGroupCommitter.class); ConsensusCommit consensusWithGroupCommit = - new ConsensusCommit(crud, commit, mutationOperationChecker, groupCommitter); + new ConsensusCommit(context, crud, commit, mutationOperationChecker, groupCommitter); // Act consensusWithGroupCommit.rollback(); // Assert - verify(crud).closeScanners(); - verify(groupCommitter).remove(txId); - verify(commit, never()).rollbackRecords(any(Snapshot.class)); + verify(context).closeScanners(); + verify(groupCommitter).remove(ANY_ID); + verify(commit, never()).rollbackRecords(context); verify(commit, never()).abortState(anyString()); } @@ -634,22 +615,18 @@ public void rollback_WithGroupCommitter_ShouldRemoveTxFromGroupCommitter() public void rollback_WithGroupCommitter_InReadOnlyMode_ShouldNotRemoveTxFromGroupCommitter() throws CrudException, UnknownTransactionStatusException { // Arrange - String txId = "tx-id"; - Snapshot snapshot = mock(Snapshot.class); - doReturn(txId).when(snapshot).getId(); - doReturn(snapshot).when(crud).getSnapshot(); - doReturn(true).when(crud).isReadOnly(); + context = spy(new TransactionContext(ANY_ID, snapshot, Isolation.SNAPSHOT, true, false)); CoordinatorGroupCommitter groupCommitter = mock(CoordinatorGroupCommitter.class); ConsensusCommit consensusWithGroupCommit = - new ConsensusCommit(crud, commit, mutationOperationChecker, groupCommitter); + new ConsensusCommit(context, crud, commit, mutationOperationChecker, groupCommitter); // Act consensusWithGroupCommit.rollback(); // Assert - verify(crud).closeScanners(); + verify(context).closeScanners(); verify(groupCommitter, never()).remove(anyString()); - verify(commit, never()).rollbackRecords(any(Snapshot.class)); + verify(commit, never()).rollbackRecords(any(TransactionContext.class)); verify(commit, never()).abortState(anyString()); } } diff --git a/core/src/test/java/com/scalar/db/transaction/consensuscommit/CoordinatorGroupCommitterTest.java b/core/src/test/java/com/scalar/db/transaction/consensuscommit/CoordinatorGroupCommitterTest.java index 0d123cd3ae..4dc4a64ba9 100644 --- a/core/src/test/java/com/scalar/db/transaction/consensuscommit/CoordinatorGroupCommitterTest.java +++ b/core/src/test/java/com/scalar/db/transaction/consensuscommit/CoordinatorGroupCommitterTest.java @@ -31,9 +31,9 @@ class CoordinatorGroupCommitterTest { private final CoordinatorGroupCommitKeyManipulator keyManipulator = new CoordinatorGroupCommitKeyManipulator(); - @Mock private Emittable emitter; - @Captor private ArgumentCaptor> snapshotsArgumentCaptor; - @Captor private ArgumentCaptor snapshotArgumentCaptor; + @Mock private Emittable emitter; + @Captor private ArgumentCaptor> contextsArgumentCaptor; + @Captor private ArgumentCaptor contextArgumentCaptor; @Test void reserve_GivenArbitraryChildTxId_ShouldReturnFullTxId() throws Exception { @@ -93,10 +93,10 @@ void ready_GivenArbitrarySnapshot_ShouldWaitUntilGroupCommitted() throws Excepti String fullTxId3 = groupCommitter.reserve(childTxId3); String fullTxId4 = groupCommitter.reserve(childTxId4); - Snapshot snapshot1 = mock(Snapshot.class); - Snapshot snapshot2 = mock(Snapshot.class); - Snapshot snapshot3 = mock(Snapshot.class); - Snapshot snapshot4 = mock(Snapshot.class); + TransactionContext context1 = mock(TransactionContext.class); + TransactionContext context2 = mock(TransactionContext.class); + TransactionContext context3 = mock(TransactionContext.class); + TransactionContext context4 = mock(TransactionContext.class); // Act ExecutorService executorService = Executors.newCachedThreadPool(); @@ -104,25 +104,25 @@ void ready_GivenArbitrarySnapshot_ShouldWaitUntilGroupCommitted() throws Excepti futures.add( executorService.submit( () -> { - groupCommitter.ready(fullTxId1, snapshot1); + groupCommitter.ready(fullTxId1, context1); return null; })); futures.add( executorService.submit( () -> { - groupCommitter.ready(fullTxId2, snapshot2); + groupCommitter.ready(fullTxId2, context2); return null; })); futures.add( executorService.submit( () -> { - groupCommitter.ready(fullTxId3, snapshot3); + groupCommitter.ready(fullTxId3, context3); return null; })); futures.add( executorService.submit( () -> { - groupCommitter.ready(fullTxId4, snapshot4); + groupCommitter.ready(fullTxId4, context4); return null; })); executorService.shutdown(); @@ -134,13 +134,13 @@ void ready_GivenArbitrarySnapshot_ShouldWaitUntilGroupCommitted() throws Excepti verify(emitter) .emitNormalGroup( eq(keyManipulator.keysFromFullKey(fullTxId1).parentKey), - snapshotsArgumentCaptor.capture()); - assertThat(snapshotsArgumentCaptor.getValue()).containsOnly(snapshot1, snapshot2); + contextsArgumentCaptor.capture()); + assertThat(contextsArgumentCaptor.getValue()).containsOnly(context1, context2); verify(emitter) .emitNormalGroup( eq(keyManipulator.keysFromFullKey(fullTxId3).parentKey), - snapshotsArgumentCaptor.capture()); - assertThat(snapshotsArgumentCaptor.getValue()).containsOnly(snapshot3, snapshot4); + contextsArgumentCaptor.capture()); + assertThat(contextsArgumentCaptor.getValue()).containsOnly(context3, context4); verify(emitter, never()).emitDelayedGroup(any(), any()); } } @@ -158,8 +158,8 @@ void ready_GivenArbitrarySnapshotWithSomeDelay_ShouldWaitUntilSeparatelyGroupCom String fullTxId1 = groupCommitter.reserve(childTxId1); String fullTxId2 = groupCommitter.reserve(childTxId2); - Snapshot snapshot1 = mock(Snapshot.class); - Snapshot snapshot2 = mock(Snapshot.class); + TransactionContext context1 = mock(TransactionContext.class); + TransactionContext context2 = mock(TransactionContext.class); // Act ExecutorService executorService = Executors.newCachedThreadPool(); @@ -167,7 +167,7 @@ void ready_GivenArbitrarySnapshotWithSomeDelay_ShouldWaitUntilSeparatelyGroupCom futures.add( executorService.submit( () -> { - groupCommitter.ready(fullTxId1, snapshot1); + groupCommitter.ready(fullTxId1, context1); return null; })); // Sleep to trigger some timeouts in the group commit. @@ -175,7 +175,7 @@ void ready_GivenArbitrarySnapshotWithSomeDelay_ShouldWaitUntilSeparatelyGroupCom futures.add( executorService.submit( () -> { - groupCommitter.ready(fullTxId2, snapshot2); + groupCommitter.ready(fullTxId2, context2); return null; })); executorService.shutdown(); @@ -187,10 +187,10 @@ void ready_GivenArbitrarySnapshotWithSomeDelay_ShouldWaitUntilSeparatelyGroupCom verify(emitter) .emitNormalGroup( eq(keyManipulator.keysFromFullKey(fullTxId1).parentKey), - snapshotsArgumentCaptor.capture()); - assertThat(snapshotsArgumentCaptor.getValue()).containsOnly(snapshot1); - verify(emitter).emitDelayedGroup(eq(fullTxId2), snapshotArgumentCaptor.capture()); - assertThat(snapshotArgumentCaptor.getValue()).isEqualTo(snapshot2); + contextsArgumentCaptor.capture()); + assertThat(contextsArgumentCaptor.getValue()).containsOnly(context1); + verify(emitter).emitDelayedGroup(eq(fullTxId2), contextArgumentCaptor.capture()); + assertThat(contextArgumentCaptor.getValue()).isEqualTo(context2); } } @@ -206,14 +206,14 @@ void remove_GivenOneOfFullTxIds_ShouldRemoveItAndProceedTheOther() throws Except String fullTxId1 = groupCommitter.reserve(childTxId1); String fullTxId2 = groupCommitter.reserve(childTxId2); - Snapshot snapshot = mock(Snapshot.class); + TransactionContext context = mock(TransactionContext.class); ExecutorService executorService = Executors.newCachedThreadPool(); List> futures = new ArrayList<>(); futures.add( executorService.submit( () -> { - groupCommitter.ready(fullTxId2, snapshot); + groupCommitter.ready(fullTxId2, context); return null; })); executorService.shutdown(); @@ -229,8 +229,8 @@ void remove_GivenOneOfFullTxIds_ShouldRemoveItAndProceedTheOther() throws Except verify(emitter) .emitNormalGroup( eq(keyManipulator.keysFromFullKey(fullTxId2).parentKey), - snapshotsArgumentCaptor.capture()); - assertThat(snapshotsArgumentCaptor.getValue()).containsOnly(snapshot); + contextsArgumentCaptor.capture()); + assertThat(contextsArgumentCaptor.getValue()).containsOnly(context); verify(emitter, never()).emitDelayedGroup(any(), any()); } } diff --git a/core/src/test/java/com/scalar/db/transaction/consensuscommit/CrudHandlerTest.java b/core/src/test/java/com/scalar/db/transaction/consensuscommit/CrudHandlerTest.java index aeabe2e77c..18aaf365d3 100644 --- a/core/src/test/java/com/scalar/db/transaction/consensuscommit/CrudHandlerTest.java +++ b/core/src/test/java/com/scalar/db/transaction/consensuscommit/CrudHandlerTest.java @@ -76,7 +76,6 @@ public class CrudHandlerTest { private static final String ANY_TEXT_3 = "text3"; private static final String ANY_TEXT_4 = "text4"; private static final String ANY_TEXT_5 = "text5"; - private static final String ANY_TX_ID = "tx_id"; private static final TableMetadata TABLE_METADATA = ConsensusCommitUtils.buildTransactionTableMetadata( @@ -92,13 +91,14 @@ public class CrudHandlerTest { private CrudHandler handler; @Mock private DistributedStorage storage; - @Mock private Snapshot snapshot; @Mock private RecoveryExecutor recoveryExecutor; @Mock private TransactionTableMetadataManager tableMetadataManager; + @Mock private MutationConditionsValidator mutationConditionsValidator; @Mock private ParallelExecutor parallelExecutor; + + @Mock private Snapshot snapshot; @Mock private Scanner scanner; @Mock private Result result; - @Mock private MutationConditionsValidator mutationConditionsValidator; @BeforeEach public void setUp() throws Exception { @@ -106,25 +106,17 @@ public void setUp() throws Exception { handler = new CrudHandler( storage, - snapshot, recoveryExecutor, tableMetadataManager, false, mutationConditionsValidator, - parallelExecutor, - false, - false); + parallelExecutor); // Arrange when(tableMetadataManager.getTransactionTableMetadata(any())) .thenReturn(new TransactionTableMetadata(TABLE_METADATA)); when(tableMetadataManager.getTransactionTableMetadata(any(), any())) .thenReturn(new TransactionTableMetadata(TABLE_METADATA)); - - // Default behavior for snapshot isolation - when(snapshot.isSnapshotReadRequired()).thenReturn(true); - when(snapshot.isValidationRequired()).thenReturn(false); - when(snapshot.getIsolation()).thenReturn(Isolation.SNAPSHOT); } private Get prepareGet() { @@ -190,9 +182,11 @@ public void get_GetExistsInSnapshot_ShouldReturnFromSnapshot() throws CrudExcept Optional expected = Optional.of(prepareResult(TransactionState.COMMITTED)); when(snapshot.containsKeyInGetSet(getForStorage)).thenReturn(true); when(snapshot.getResult(key, getForStorage)).thenReturn(expected); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); // Act - Optional actual = handler.get(get); + Optional actual = handler.get(get, context); // Assert assertThat(actual) @@ -215,9 +209,11 @@ public void get_GetExistsInSnapshot_ShouldReturnFromSnapshot() throws CrudExcept when(snapshot.containsKeyInGetSet(getForStorage)).thenReturn(false); when(storage.get(getForStorage)).thenReturn(expected); when(snapshot.getResult(key, getForStorage)).thenReturn(transactionResult); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); // Act - Optional result = handler.get(get); + Optional result = handler.get(get, context); // Assert assertThat(result) @@ -235,18 +231,6 @@ public void get_GetExistsInSnapshot_ShouldReturnFromSnapshot() throws CrudExcept get_GetNotExistsInSnapshotAndRecordInStorageCommitted_InReadOnlyMode_ShouldReturnFromStorageAndUpdateSnapshot() throws CrudException, ExecutionException { // Arrange - handler = - new CrudHandler( - storage, - snapshot, - recoveryExecutor, - tableMetadataManager, - false, - mutationConditionsValidator, - parallelExecutor, - true, - false); - Get get = prepareGet(); Get getForStorage = toGetForStorageFrom(get); Optional expected = Optional.of(prepareResult(TransactionState.COMMITTED)); @@ -255,9 +239,11 @@ public void get_GetExistsInSnapshot_ShouldReturnFromSnapshot() throws CrudExcept when(snapshot.containsKeyInGetSet(getForStorage)).thenReturn(false); when(storage.get(getForStorage)).thenReturn(expected); when(snapshot.getResult(key, getForStorage)).thenReturn(transactionResult); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, true, false); // Act - Optional result = handler.get(get); + Optional result = handler.get(get, context); // Assert assertThat(result) @@ -275,19 +261,6 @@ public void get_GetExistsInSnapshot_ShouldReturnFromSnapshot() throws CrudExcept get_GetNotExistsInSnapshotAndRecordInStorageCommitted_InOneOperationMode_ValidationNotRequired_ShouldReturnFromStorageAndUpdateSnapshot() throws CrudException, ExecutionException { // Arrange - handler = - new CrudHandler( - storage, - snapshot, - recoveryExecutor, - tableMetadataManager, - false, - mutationConditionsValidator, - parallelExecutor, - true, - true); - when(snapshot.isValidationRequired()).thenReturn(false); - Get get = prepareGet(); Get getForStorage = toGetForStorageFrom(get); Optional expected = Optional.of(prepareResult(TransactionState.COMMITTED)); @@ -297,9 +270,11 @@ public void get_GetExistsInSnapshot_ShouldReturnFromSnapshot() throws CrudExcept when(storage.get(getForStorage)).thenReturn(expected); when(snapshot.mergeResult(key, transactionResult, getForStorage.getConjunctions())) .thenReturn(transactionResult); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, true, true); // Act - Optional result = handler.get(get); + Optional result = handler.get(get, context); // Assert assertThat(result) @@ -317,19 +292,6 @@ public void get_GetExistsInSnapshot_ShouldReturnFromSnapshot() throws CrudExcept get_GetNotExistsInSnapshotAndRecordInStorageCommitted_InOneOperationMode_ValidationRequired_ShouldReturnFromStorageAndUpdateSnapshot() throws CrudException, ExecutionException { // Arrange - handler = - new CrudHandler( - storage, - snapshot, - recoveryExecutor, - tableMetadataManager, - false, - mutationConditionsValidator, - parallelExecutor, - true, - true); - when(snapshot.isValidationRequired()).thenReturn(true); - Get get = prepareGet(); Get getForStorage = toGetForStorageFrom(get); Optional expected = Optional.of(prepareResult(TransactionState.COMMITTED)); @@ -339,9 +301,11 @@ public void get_GetExistsInSnapshot_ShouldReturnFromSnapshot() throws CrudExcept when(storage.get(getForStorage)).thenReturn(expected); when(snapshot.mergeResult(key, transactionResult, getForStorage.getConjunctions())) .thenReturn(transactionResult); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SERIALIZABLE, true, true); // Act - Optional result = handler.get(get); + Optional result = handler.get(get, context); // Assert assertThat(result) @@ -359,23 +323,9 @@ public void get_GetExistsInSnapshot_ShouldReturnFromSnapshot() throws CrudExcept get_GetWithConjunction_GetNotExistsInSnapshotAndRecordInStorageCommitted_InOneOperationMode_ValidationRequired_ShouldReturnFromStorageAndUpdateSnapshot() throws CrudException, ExecutionException { // Arrange - handler = - new CrudHandler( - storage, - snapshot, - recoveryExecutor, - tableMetadataManager, - false, - mutationConditionsValidator, - parallelExecutor, - true, - true); - when(snapshot.isValidationRequired()).thenReturn(true); - ConditionalExpression condition = column(ANY_NAME_3).isEqualToText(ANY_TEXT_3); Get get = Get.newBuilder(prepareGet()).where(condition).build(); Get getForStorage = toGetForStorageFrom(get); - Optional expected = Optional.of(prepareResult(TransactionState.COMMITTED)); Optional transactionResult = expected.map(e -> (TransactionResult) e); Snapshot.Key key = new Snapshot.Key(getForStorage); @@ -384,9 +334,11 @@ public void get_GetExistsInSnapshot_ShouldReturnFromSnapshot() throws CrudExcept when(snapshot.mergeResult( key, transactionResult, Collections.singleton(Selection.Conjunction.of(condition)))) .thenReturn(transactionResult); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SERIALIZABLE, true, true); // Act - Optional result = handler.get(get); + Optional result = handler.get(get, context); // Assert assertThat(result) @@ -410,14 +362,8 @@ public void get_GetExistsInSnapshot_ShouldReturnFromSnapshot() throws CrudExcept get_GetNotExistsInSnapshotAndRecordInStorageCommitted_ReadCommittedIsolation_ShouldReturnFromStorageAndUpdateSnapshot() throws CrudException, ExecutionException { // Arrange - - // For READ_COMMITTED isolation - when(snapshot.isSnapshotReadRequired()).thenReturn(false); - when(snapshot.getIsolation()).thenReturn(Isolation.READ_COMMITTED); - Get get = Get.newBuilder(prepareGet()).build(); Get getForStorage = toGetForStorageFrom(get); - Optional expected = Optional.of(prepareResult(TransactionState.COMMITTED)); Optional transactionResult = expected.map(e -> (TransactionResult) e); Snapshot.Key key = new Snapshot.Key(getForStorage); @@ -425,9 +371,11 @@ public void get_GetExistsInSnapshot_ShouldReturnFromSnapshot() throws CrudExcept when(storage.get(any())).thenReturn(expected); when(snapshot.mergeResult(key, transactionResult, getForStorage.getConjunctions())) .thenReturn(transactionResult); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.READ_COMMITTED, false, false); // Act - Optional result = handler.get(get); + Optional result = handler.get(get, context); // Assert assertThat(result) @@ -445,22 +393,6 @@ public void get_GetExistsInSnapshot_ShouldReturnFromSnapshot() throws CrudExcept get_GetNotExistsInSnapshotAndRecordInStorageCommitted_ReadCommittedIsolation_InReadOnlyMode_ShouldReturnFromStorageAndNotUpdateSnapshot() throws CrudException, ExecutionException { // Arrange - handler = - new CrudHandler( - storage, - snapshot, - recoveryExecutor, - tableMetadataManager, - false, - mutationConditionsValidator, - parallelExecutor, - true, - false); - - // For READ_COMMITTED isolation - when(snapshot.isSnapshotReadRequired()).thenReturn(false); - when(snapshot.getIsolation()).thenReturn(Isolation.READ_COMMITTED); - Get get = Get.newBuilder(prepareGet()).build(); Get getForStorage = toGetForStorageFrom(get); @@ -471,9 +403,11 @@ public void get_GetExistsInSnapshot_ShouldReturnFromSnapshot() throws CrudExcept when(storage.get(any())).thenReturn(expected); when(snapshot.mergeResult(key, transactionResult, getForStorage.getConjunctions())) .thenReturn(transactionResult); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.READ_COMMITTED, true, false); // Act - Optional result = handler.get(get); + Optional result = handler.get(get, context); // Assert assertThat(result) @@ -497,7 +431,6 @@ public void get_GetExistsInSnapshot_ShouldReturnFromSnapshot() throws CrudExcept result = prepareResult(TransactionState.PREPARED); when(storage.get(getForStorage)).thenReturn(Optional.of(result)); when(snapshot.containsKeyInGetSet(getForStorage)).thenReturn(false); - when(snapshot.getId()).thenReturn(ANY_ID_1); TransactionResult expected = mock(TransactionResult.class); when(expected.getContainedColumnNames()).thenReturn(Collections.singleton(ANY_NAME_1)); @@ -517,8 +450,11 @@ public void get_GetExistsInSnapshot_ShouldReturnFromSnapshot() throws CrudExcept RecoveryExecutor.RecoveryType.RETURN_LATEST_RESULT_AND_RECOVER)) .thenReturn(new RecoveryExecutor.Result(key, Optional.of(recoveredResult), recoveryFuture)); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); + // Act - Optional actual = handler.get(get); + Optional actual = handler.get(get, context); // Assert verify(storage).get(getForStorage); @@ -540,32 +476,15 @@ public void get_GetExistsInSnapshot_ShouldReturnFromSnapshot() throws CrudExcept @Test public void - get_GetNotExistsInSnapshotAndRecordInStorageNotCommitted_ReadCommittedIsolation_ShouldCallRecoveryExecutorWithReturnCommittedResultAndNotRecover() + get_GetNotExistsInSnapshotAndRecordInStorageNotCommitted_ReadCommittedIsolation_ShouldCallRecoveryExecutorWithReturnCommittedResultAndRecover() throws ExecutionException, CrudException { // Arrange - handler = - new CrudHandler( - storage, - snapshot, - recoveryExecutor, - tableMetadataManager, - false, - mutationConditionsValidator, - parallelExecutor, - true, - false); - - // For READ_COMMITTED isolation - when(snapshot.isSnapshotReadRequired()).thenReturn(false); - when(snapshot.getIsolation()).thenReturn(Isolation.READ_COMMITTED); - Get get = prepareGet(); Snapshot.Key key = new Snapshot.Key(get); Get getForStorage = toGetForStorageFrom(get); result = prepareResult(TransactionState.PREPARED); when(storage.get(getForStorage)).thenReturn(Optional.of(result)); when(snapshot.containsKeyInGetSet(getForStorage)).thenReturn(false); - when(snapshot.getId()).thenReturn(ANY_ID_1); TransactionResult expected = mock(TransactionResult.class); when(expected.getContainedColumnNames()).thenReturn(Collections.singleton(ANY_NAME_1)); @@ -582,14 +501,17 @@ public void get_GetExistsInSnapshot_ShouldReturnFromSnapshot() throws CrudExcept getForStorage, transactionResult, ANY_ID_1, - RecoveryExecutor.RecoveryType.RETURN_COMMITTED_RESULT_AND_NOT_RECOVER)) + RecoveryExecutor.RecoveryType.RETURN_COMMITTED_RESULT_AND_RECOVER)) .thenReturn(new RecoveryExecutor.Result(key, Optional.of(recoveredResult), recoveryFuture)); when(snapshot.mergeResult(key, Optional.of(recoveredResult), getForStorage.getConjunctions())) .thenReturn(Optional.of(expected)); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.READ_COMMITTED, false, false); + // Act - Optional actual = handler.get(get); + Optional actual = handler.get(get, context); // Assert verify(storage).get(getForStorage); @@ -599,8 +521,8 @@ public void get_GetExistsInSnapshot_ShouldReturnFromSnapshot() throws CrudExcept getForStorage, transactionResult, ANY_ID_1, - RecoveryExecutor.RecoveryType.RETURN_COMMITTED_RESULT_AND_NOT_RECOVER); - verify(snapshot, never()).putIntoReadSet(any(), any()); + RecoveryExecutor.RecoveryType.RETURN_COMMITTED_RESULT_AND_RECOVER); + verify(snapshot).putIntoReadSet(key, Optional.of(recoveredResult)); verify(snapshot, never()).putIntoGetSet(any(), any()); assertThat(actual) @@ -611,21 +533,15 @@ public void get_GetExistsInSnapshot_ShouldReturnFromSnapshot() throws CrudExcept @Test public void - get_GetNotExistsInSnapshotAndRecordInStorageNotCommitted_ReadCommittedIsolation_InReadOnlyMode_ShouldCallRecoveryExecutorWithReturnCommittedResultAndRecover() + get_GetNotExistsInSnapshotAndRecordInStorageNotCommitted_ReadCommittedIsolation_InReadOnlyMode_ShouldCallRecoveryExecutorWithReturnCommittedResultAndNotRecover() throws ExecutionException, CrudException { // Arrange - - // For READ_COMMITTED isolation - when(snapshot.isSnapshotReadRequired()).thenReturn(false); - when(snapshot.getIsolation()).thenReturn(Isolation.READ_COMMITTED); - Get get = prepareGet(); Snapshot.Key key = new Snapshot.Key(get); Get getForStorage = toGetForStorageFrom(get); result = prepareResult(TransactionState.PREPARED); when(storage.get(getForStorage)).thenReturn(Optional.of(result)); when(snapshot.containsKeyInGetSet(getForStorage)).thenReturn(false); - when(snapshot.getId()).thenReturn(ANY_ID_1); TransactionResult expected = mock(TransactionResult.class); when(expected.getContainedColumnNames()).thenReturn(Collections.singleton(ANY_NAME_1)); @@ -642,14 +558,17 @@ public void get_GetExistsInSnapshot_ShouldReturnFromSnapshot() throws CrudExcept getForStorage, transactionResult, ANY_ID_1, - RecoveryExecutor.RecoveryType.RETURN_COMMITTED_RESULT_AND_RECOVER)) + RecoveryExecutor.RecoveryType.RETURN_COMMITTED_RESULT_AND_NOT_RECOVER)) .thenReturn(new RecoveryExecutor.Result(key, Optional.of(recoveredResult), recoveryFuture)); when(snapshot.mergeResult(key, Optional.of(recoveredResult), getForStorage.getConjunctions())) .thenReturn(Optional.of(expected)); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.READ_COMMITTED, true, false); + // Act - Optional actual = handler.get(get); + Optional actual = handler.get(get, context); // Assert verify(storage).get(getForStorage); @@ -659,8 +578,8 @@ public void get_GetExistsInSnapshot_ShouldReturnFromSnapshot() throws CrudExcept getForStorage, transactionResult, ANY_ID_1, - RecoveryExecutor.RecoveryType.RETURN_COMMITTED_RESULT_AND_RECOVER); - verify(snapshot).putIntoReadSet(key, Optional.of(recoveredResult)); + RecoveryExecutor.RecoveryType.RETURN_COMMITTED_RESULT_AND_NOT_RECOVER); + verify(snapshot, never()).putIntoReadSet(any(), any()); verify(snapshot, never()).putIntoGetSet(any(), any()); assertThat(actual) @@ -677,9 +596,11 @@ public void get_GetNotExistsInSnapshotAndRecordNotExistsInStorage_ShouldReturnEm Get getForStorage = toGetForStorageFrom(get); when(snapshot.containsKeyInGetSet(getForStorage)).thenReturn(false); when(storage.get(getForStorage)).thenReturn(Optional.empty()); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); // Act - Optional result = handler.get(get); + Optional result = handler.get(get, context); // Assert assertThat(result.isPresent()).isFalse(); @@ -694,9 +615,13 @@ public void get_GetNotExistsInSnapshotAndExceptionThrownInStorage_ShouldThrowCru when(snapshot.containsKeyInGetSet(getForStorage)).thenReturn(false); ExecutionException toThrow = mock(ExecutionException.class); when(storage.get(getForStorage)).thenThrow(toThrow); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); // Act Assert - assertThatThrownBy(() -> handler.get(get)).isInstanceOf(CrudException.class).hasCause(toThrow); + assertThatThrownBy(() -> handler.get(get, context)) + .isInstanceOf(CrudException.class) + .hasCause(toThrow); } @Test @@ -713,10 +638,12 @@ public void get_CalledTwice_SecondTimeShouldReturnTheSameFromSnapshot() when(snapshot.containsKeyInGetSet(getForStorage)).thenReturn(false).thenReturn(true); when(snapshot.getResult(key, getForStorage)).thenReturn(expected).thenReturn(expected); when(storage.get(getForStorage)).thenReturn(Optional.of(result)); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); // Act - Optional results1 = handler.get(get1); - Optional results2 = handler.get(get2); + Optional results1 = handler.get(get1, context); + Optional results2 = handler.get(get2, context); // Assert verify(snapshot).putIntoReadSet(key, expected); @@ -734,11 +661,6 @@ public void get_CalledTwice_SecondTimeShouldReturnTheSameFromSnapshot() public void get_CalledTwice_ReadCommittedIsolation_BothShouldReturnFromStorage() throws ExecutionException, CrudException { // Arrange - - // For READ_COMMITTED isolation - when(snapshot.isSnapshotReadRequired()).thenReturn(false); - when(snapshot.getIsolation()).thenReturn(Isolation.READ_COMMITTED); - Get originalGet = prepareGet(); Get getForStorage = toGetForStorageFrom(originalGet); Get get1 = prepareGet(); @@ -751,10 +673,12 @@ public void get_CalledTwice_ReadCommittedIsolation_BothShouldReturnFromStorage() key, Optional.of(new TransactionResult(result)), getForStorage.getConjunctions())) .thenReturn(expected); when(storage.get(getForStorage)).thenReturn(Optional.of(result)); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.READ_COMMITTED, false, false); // Act - Optional results1 = handler.get(get1); - Optional results2 = handler.get(get2); + Optional results1 = handler.get(get1, context); + Optional results2 = handler.get(get2, context); // Assert verify(storage, times(2)).get(getForStorage); @@ -777,22 +701,14 @@ public void get_CalledTwiceUnderRealSnapshot_SecondTimeShouldReturnTheSameFromSn Get get2 = prepareGet(); Result result = prepareResult(TransactionState.COMMITTED); Optional expected = Optional.of(new TransactionResult(result)); - snapshot = new Snapshot(ANY_TX_ID, Isolation.SNAPSHOT, tableMetadataManager, parallelExecutor); - handler = - new CrudHandler( - storage, - snapshot, - recoveryExecutor, - tableMetadataManager, - false, - parallelExecutor, - false, - false); + snapshot = new Snapshot(ANY_ID_1, Isolation.SNAPSHOT, tableMetadataManager, parallelExecutor); when(storage.get(getForStorage)).thenReturn(Optional.of(result)); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); // Act - Optional results1 = handler.get(get1); - Optional results2 = handler.get(get2); + Optional results1 = handler.get(get1, context); + Optional results2 = handler.get(get2, context); // Assert assertThat(results1) @@ -816,22 +732,14 @@ public void get_CalledTwiceUnderRealSnapshot_ReadCommittedIsolation_BothShouldRe Result result = prepareResult(TransactionState.COMMITTED); Optional expected = Optional.of(new TransactionResult(result)); snapshot = - new Snapshot(ANY_TX_ID, Isolation.READ_COMMITTED, tableMetadataManager, parallelExecutor); - handler = - new CrudHandler( - storage, - snapshot, - recoveryExecutor, - tableMetadataManager, - false, - parallelExecutor, - false, - false); + new Snapshot(ANY_ID_1, Isolation.READ_COMMITTED, tableMetadataManager, parallelExecutor); when(storage.get(getForStorage)).thenReturn(Optional.of(result)); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.READ_COMMITTED, false, false); // Act - Optional results1 = handler.get(get1); - Optional results2 = handler.get(get2); + Optional results1 = handler.get(get1, context); + Optional results2 = handler.get(get2, context); // Assert verify(storage, times(2)).get(getForStorage); @@ -859,8 +767,12 @@ public void get_ForNonExistingTable_ShouldThrowIllegalArgumentException() when(tableMetadataManager.getTransactionTableMetadata(get)).thenReturn(null); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); + // Act Assert - assertThatThrownBy(() -> handler.get(get)).isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> handler.get(get, context)) + .isInstanceOf(IllegalArgumentException.class); } @Test @@ -884,10 +796,12 @@ public void get_DifferentGetButSameRecordReturned_ShouldNotOverwriteReadSet() when(snapshot.getResult(any(), any())).thenReturn(expected).thenReturn(expected); when(snapshot.containsKeyInReadSet(key)).thenReturn(false).thenReturn(true); when(storage.get(any())).thenReturn(Optional.of(result)); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); // Act - Optional results1 = handler.get(get1); - Optional results2 = handler.get(get2); + Optional results1 = handler.get(get1, context); + Optional results2 = handler.get(get2, context); // Assert assertThat(results1) @@ -909,7 +823,7 @@ void scanOrGetScanner_ResultGivenFromStorage_ShouldUpdateSnapshotAndReturn(ScanT Scan scan = prepareScan(); Scan scanForStorage = toScanForStorageFrom(scan); result = prepareResult(TransactionState.COMMITTED); - Snapshot.Key key = new Snapshot.Key(scan, result); + Snapshot.Key key = new Snapshot.Key(scan, result, TABLE_METADATA); TransactionResult expected = new TransactionResult(result); if (scanType == ScanType.SCAN) { when(scanner.iterator()).thenReturn(Collections.singletonList(result).iterator()); @@ -917,9 +831,11 @@ void scanOrGetScanner_ResultGivenFromStorage_ShouldUpdateSnapshotAndReturn(ScanT when(scanner.one()).thenReturn(Optional.of(result)).thenReturn(Optional.empty()); } when(storage.scan(scanForStorage)).thenReturn(scanner); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); // Act - List results = scanOrGetScanner(scan, scanType); + List results = scanOrGetScanner(scan, scanType, context); // Assert verify(scanner).close(); @@ -936,22 +852,10 @@ void scanOrGetScanner_ResultGivenFromStorage_ShouldUpdateSnapshotAndReturn(ScanT void scanOrGetScanner_ResultGivenFromStorage_InReadOnlyMode_ShouldUpdateSnapshotAndReturn( ScanType scanType) throws ExecutionException, CrudException { // Arrange - handler = - new CrudHandler( - storage, - snapshot, - recoveryExecutor, - tableMetadataManager, - false, - mutationConditionsValidator, - parallelExecutor, - true, - false); - Scan scan = prepareScan(); Scan scanForStorage = toScanForStorageFrom(scan); result = prepareResult(TransactionState.COMMITTED); - Snapshot.Key key = new Snapshot.Key(scan, result); + Snapshot.Key key = new Snapshot.Key(scan, result, TABLE_METADATA); TransactionResult expected = new TransactionResult(result); if (scanType == ScanType.SCAN) { when(scanner.iterator()).thenReturn(Collections.singletonList(result).iterator()); @@ -959,9 +863,11 @@ void scanOrGetScanner_ResultGivenFromStorage_InReadOnlyMode_ShouldUpdateSnapshot when(scanner.one()).thenReturn(Optional.of(result)).thenReturn(Optional.empty()); } when(storage.scan(scanForStorage)).thenReturn(scanner); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, true, false); // Act - List results = scanOrGetScanner(scan, scanType); + List results = scanOrGetScanner(scan, scanType, context); // Assert verify(snapshot, never()).putIntoReadSet(any(), any()); @@ -978,19 +884,6 @@ void scanOrGetScanner_ResultGivenFromStorage_InReadOnlyMode_ShouldUpdateSnapshot scanOrGetScanner_ResultGivenFromStorage_InOneOperationMode_ValidationNotRequired_ShouldUpdateSnapshotAndReturn( ScanType scanType) throws ExecutionException, CrudException { // Arrange - handler = - new CrudHandler( - storage, - snapshot, - recoveryExecutor, - tableMetadataManager, - false, - mutationConditionsValidator, - parallelExecutor, - true, - true); - when(snapshot.isValidationRequired()).thenReturn(false); - Scan scan = prepareScan(); Scan scanForStorage = toScanForStorageFrom(scan); result = prepareResult(TransactionState.COMMITTED); @@ -1001,9 +894,11 @@ void scanOrGetScanner_ResultGivenFromStorage_InReadOnlyMode_ShouldUpdateSnapshot when(scanner.one()).thenReturn(Optional.of(result)).thenReturn(Optional.empty()); } when(storage.scan(scanForStorage)).thenReturn(scanner); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, true, true); // Act - List results = scanOrGetScanner(scan, scanType); + List results = scanOrGetScanner(scan, scanType, context); // Assert verify(snapshot, never()).putIntoReadSet(any(), any()); @@ -1020,23 +915,10 @@ void scanOrGetScanner_ResultGivenFromStorage_InReadOnlyMode_ShouldUpdateSnapshot scanOrGetScanner_ResultGivenFromStorage_InOneOperationMode_ValidationRequired_ShouldUpdateSnapshotAndReturn( ScanType scanType) throws ExecutionException, CrudException { // Arrange - handler = - new CrudHandler( - storage, - snapshot, - recoveryExecutor, - tableMetadataManager, - false, - mutationConditionsValidator, - parallelExecutor, - true, - true); - when(snapshot.isValidationRequired()).thenReturn(true); - Scan scan = prepareScan(); Scan scanForStorage = toScanForStorageFrom(scan); result = prepareResult(TransactionState.COMMITTED); - Snapshot.Key key = new Snapshot.Key(scan, result); + Snapshot.Key key = new Snapshot.Key(scan, result, TABLE_METADATA); TransactionResult expected = new TransactionResult(result); if (scanType == ScanType.SCAN) { when(scanner.iterator()).thenReturn(Collections.singletonList(result).iterator()); @@ -1044,9 +926,11 @@ void scanOrGetScanner_ResultGivenFromStorage_InReadOnlyMode_ShouldUpdateSnapshot when(scanner.one()).thenReturn(Optional.of(result)).thenReturn(Optional.empty()); } when(storage.scan(scanForStorage)).thenReturn(scanner); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SERIALIZABLE, true, true); // Act - List results = scanOrGetScanner(scan, scanType); + List results = scanOrGetScanner(scan, scanType, context); // Assert verify(snapshot, never()).putIntoReadSet(any(), any()); @@ -1074,11 +958,10 @@ void scanOrGetScanner_ResultGivenFromStorage_InReadOnlyMode_ShouldUpdateSnapshot } when(storage.scan(scanForStorage)).thenReturn(scanner); - Snapshot.Key key = new Snapshot.Key(scan, result); - - when(snapshot.getId()).thenReturn(ANY_ID_1); + Snapshot.Key key = new Snapshot.Key(scan, result, TABLE_METADATA); TransactionResult recoveredResult = mock(TransactionResult.class); + when(recoveredResult.getContainedColumnNames()).thenReturn(Collections.singleton(ANY_NAME_1)); when(recoveredResult.getAsObject(ANY_NAME_1)).thenReturn(ANY_TEXT_1); @@ -1093,8 +976,11 @@ void scanOrGetScanner_ResultGivenFromStorage_InReadOnlyMode_ShouldUpdateSnapshot RecoveryExecutor.RecoveryType.RETURN_LATEST_RESULT_AND_RECOVER)) .thenReturn(new RecoveryExecutor.Result(key, Optional.of(recoveredResult), recoveryFuture)); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); + // Act - List results = scanOrGetScanner(scan, scanType); + List results = scanOrGetScanner(scan, scanType, context); // Assert verify(scanner).close(); @@ -1115,11 +1001,6 @@ void scanOrGetScanner_ResultGivenFromStorage_InReadOnlyMode_ShouldUpdateSnapshot scanOrGetScanner_PreparedResultGivenFromStorage_ReadCommittedIsolation_ShouldCallRecoveryExecutorWithReturnCommittedResultAndRecover( ScanType scanType) throws ExecutionException, IOException, CrudException { // Arrange - - // For READ_COMMITTED isolation - when(snapshot.isSnapshotReadRequired()).thenReturn(false); - when(snapshot.getIsolation()).thenReturn(Isolation.READ_COMMITTED); - Scan scan = prepareScan(); Scan scanForStorage = toScanForStorageFrom(scan); @@ -1131,11 +1012,10 @@ void scanOrGetScanner_ResultGivenFromStorage_InReadOnlyMode_ShouldUpdateSnapshot } when(storage.scan(scanForStorage)).thenReturn(scanner); - Snapshot.Key key = new Snapshot.Key(scan, result); - - when(snapshot.getId()).thenReturn(ANY_ID_1); + Snapshot.Key key = new Snapshot.Key(scan, result, TABLE_METADATA); TransactionResult recoveredResult = mock(TransactionResult.class); + when(recoveredResult.getContainedColumnNames()).thenReturn(Collections.singleton(ANY_NAME_1)); when(recoveredResult.getAsObject(ANY_NAME_1)).thenReturn(ANY_TEXT_1); @@ -1150,8 +1030,11 @@ void scanOrGetScanner_ResultGivenFromStorage_InReadOnlyMode_ShouldUpdateSnapshot RecoveryExecutor.RecoveryType.RETURN_COMMITTED_RESULT_AND_RECOVER)) .thenReturn(new RecoveryExecutor.Result(key, Optional.of(recoveredResult), recoveryFuture)); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.READ_COMMITTED, false, false); + // Act - List results = scanOrGetScanner(scan, scanType); + List results = scanOrGetScanner(scan, scanType, context); // Assert verify(scanner).close(); @@ -1170,22 +1053,6 @@ void scanOrGetScanner_ResultGivenFromStorage_InReadOnlyMode_ShouldUpdateSnapshot scanOrGetScanner_PreparedResultGivenFromStorage_ReadCommittedIsolation_InReadOnlyMode_ShouldCallRecoveryExecutorWithReturnCommittedResultAndNotRecover( ScanType scanType) throws ExecutionException, IOException, CrudException { // Arrange - handler = - new CrudHandler( - storage, - snapshot, - recoveryExecutor, - tableMetadataManager, - false, - mutationConditionsValidator, - parallelExecutor, - true, - false); - - // For READ_COMMITTED isolation - when(snapshot.isSnapshotReadRequired()).thenReturn(false); - when(snapshot.getIsolation()).thenReturn(Isolation.READ_COMMITTED); - Scan scan = prepareScan(); Scan scanForStorage = toScanForStorageFrom(scan); @@ -1197,11 +1064,10 @@ void scanOrGetScanner_ResultGivenFromStorage_InReadOnlyMode_ShouldUpdateSnapshot } when(storage.scan(scanForStorage)).thenReturn(scanner); - Snapshot.Key key = new Snapshot.Key(scan, result); - - when(snapshot.getId()).thenReturn(ANY_ID_1); + Snapshot.Key key = new Snapshot.Key(scan, result, TABLE_METADATA); TransactionResult recoveredResult = mock(TransactionResult.class); + when(recoveredResult.getContainedColumnNames()).thenReturn(Collections.singleton(ANY_NAME_1)); when(recoveredResult.getAsObject(ANY_NAME_1)).thenReturn(ANY_TEXT_1); @@ -1216,8 +1082,11 @@ void scanOrGetScanner_ResultGivenFromStorage_InReadOnlyMode_ShouldUpdateSnapshot RecoveryExecutor.RecoveryType.RETURN_COMMITTED_RESULT_AND_NOT_RECOVER)) .thenReturn(new RecoveryExecutor.Result(key, Optional.of(recoveredResult), recoveryFuture)); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.READ_COMMITTED, true, false); + // Act - List results = scanOrGetScanner(scan, scanType); + List results = scanOrGetScanner(scan, scanType, context); // Assert verify(scanner).close(); @@ -1247,14 +1116,16 @@ void scanOrGetScanner_CalledTwice_SecondTimeShouldReturnTheSameFromSnapshot(Scan when(scanner.one()).thenReturn(Optional.of(result)).thenReturn(Optional.empty()); } when(storage.scan(scanForStorage)).thenReturn(scanner); - Snapshot.Key key = new Snapshot.Key(scanForStorage, result); + Snapshot.Key key = new Snapshot.Key(scanForStorage, result, TABLE_METADATA); when(snapshot.getResults(scanForStorage)) .thenReturn(Optional.empty()) .thenReturn(Optional.of(Maps.newLinkedHashMap(ImmutableMap.of(key, expected)))); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); // Act - List results1 = scanOrGetScanner(scan1, scanType); - List results2 = scanOrGetScanner(scan2, scanType); + List results1 = scanOrGetScanner(scan1, scanType, context); + List results2 = scanOrGetScanner(scan2, scanType, context); // Assert verify(scanner).close(); @@ -1280,27 +1151,19 @@ void scan_CalledTwiceUnderRealSnapshot_SecondTimeShouldReturnTheSameFromSnapshot Scan scan2 = prepareScan(); result = prepareResult(TransactionState.COMMITTED); TransactionResult expected = new TransactionResult(result); - snapshot = new Snapshot(ANY_TX_ID, Isolation.SNAPSHOT, tableMetadataManager, parallelExecutor); - handler = - new CrudHandler( - storage, - snapshot, - recoveryExecutor, - tableMetadataManager, - false, - parallelExecutor, - false, - false); + snapshot = new Snapshot(ANY_ID_1, Isolation.SNAPSHOT, tableMetadataManager, parallelExecutor); if (scanType == ScanType.SCAN) { when(scanner.iterator()).thenReturn(Collections.singletonList(result).iterator()); } else { when(scanner.one()).thenReturn(Optional.of(result)).thenReturn(Optional.empty()); } when(storage.scan(scanForStorage)).thenReturn(scanner); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); // Act - List results1 = scanOrGetScanner(scan1, scanType); - List results2 = scanOrGetScanner(scan2, scanType); + List results1 = scanOrGetScanner(scan1, scanType, context); + List results2 = scanOrGetScanner(scan2, scanType, context); // Assert assertThat(results1.size()).isEqualTo(1); @@ -1334,10 +1197,12 @@ void scanOrGetScanner_GetCalledAfterScan_ShouldReturnFromStorage(ScanType scanTy when(storage.get(getForStorage)).thenReturn(Optional.of(result)); when(snapshot.getResult(key, get)).thenReturn(transactionResult); when(snapshot.getResult(key)).thenReturn(transactionResult); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); // Act - List results = scanOrGetScanner(scan, scanType); - Optional result = handler.get(get); + List results = scanOrGetScanner(scan, scanType, context); + Optional result = handler.get(get, context); // Assert verify(storage).scan(scanForStorage); @@ -1356,17 +1221,7 @@ void scanOrGetScanner_GetCalledAfterScanUnderRealSnapshot_ShouldReturnFromStorag // Arrange Scan scan = toScanForStorageFrom(prepareScan()); result = prepareResult(TransactionState.COMMITTED); - snapshot = new Snapshot(ANY_TX_ID, Isolation.SNAPSHOT, tableMetadataManager, parallelExecutor); - handler = - new CrudHandler( - storage, - snapshot, - recoveryExecutor, - tableMetadataManager, - false, - parallelExecutor, - false, - false); + snapshot = new Snapshot(ANY_ID_1, Isolation.SNAPSHOT, tableMetadataManager, parallelExecutor); if (scanType == ScanType.SCAN) { when(scanner.iterator()).thenReturn(Collections.singletonList(result).iterator()); } else { @@ -1374,11 +1229,14 @@ void scanOrGetScanner_GetCalledAfterScanUnderRealSnapshot_ShouldReturnFromStorag } when(storage.scan(scan)).thenReturn(scanner); Get get = prepareGet(); - when(storage.get(get)).thenReturn(Optional.of(result)); + Get getForStorage = toGetForStorageFrom(get); + when(storage.get(getForStorage)).thenReturn(Optional.of(result)); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); // Act - List results = scanOrGetScanner(scan, scanType); - Optional result = handler.get(get); + List results = scanOrGetScanner(scan, scanType, context); + Optional result = handler.get(get, context); // Assert verify(storage).scan(scan); @@ -1421,7 +1279,7 @@ void scanOrGetScanner_CalledAfterDeleteUnderRealSnapshot_ShouldThrowIllegalArgum Map deleteSet = new HashMap<>(); snapshot = new Snapshot( - ANY_TX_ID, + ANY_ID_1, Isolation.SNAPSHOT, tableMetadataManager, parallelExecutor, @@ -1431,16 +1289,6 @@ void scanOrGetScanner_CalledAfterDeleteUnderRealSnapshot_ShouldThrowIllegalArgum new HashMap<>(), deleteSet, new ArrayList<>()); - handler = - new CrudHandler( - storage, - snapshot, - recoveryExecutor, - tableMetadataManager, - false, - parallelExecutor, - false, - false); if (scanType == ScanType.SCAN) { when(scanner.iterator()).thenReturn(Arrays.asList(result, result2).iterator()); } else { @@ -1456,14 +1304,17 @@ void scanOrGetScanner_CalledAfterDeleteUnderRealSnapshot_ShouldThrowIllegalArgum .forNamespace(ANY_NAMESPACE_NAME) .forTable(ANY_TABLE_NAME); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); + // Act Assert - handler.delete(delete); + handler.delete(delete, context); // check the delete set assertThat(deleteSet.size()).isEqualTo(1); assertThat(deleteSet).containsKey(new Snapshot.Key(delete)); - assertThatThrownBy(() -> scanOrGetScanner(scan, scanType)) + assertThatThrownBy(() -> scanOrGetScanner(scan, scanType, context)) .isInstanceOf(IllegalArgumentException.class); verify(scanner).close(); @@ -1477,7 +1328,7 @@ void scanOrGetScanner_CalledAfterDeleteUnderRealSnapshot_ShouldThrowIllegalArgum // Arrange Scan scan = prepareCrossPartitionScan(); result = prepareResult(TransactionState.COMMITTED); - Snapshot.Key key = new Snapshot.Key(scan, result); + Snapshot.Key key = new Snapshot.Key(scan, result, TABLE_METADATA); if (scanType == ScanType.SCAN) { when(scanner.iterator()).thenReturn(Collections.singletonList(result).iterator()); } else { @@ -1485,9 +1336,11 @@ void scanOrGetScanner_CalledAfterDeleteUnderRealSnapshot_ShouldThrowIllegalArgum } when(storage.scan(any(ScanAll.class))).thenReturn(scanner); TransactionResult transactionResult = new TransactionResult(result); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); // Act - List results = scanOrGetScanner(scan, scanType); + List results = scanOrGetScanner(scan, scanType, context); // Assert verify(scanner).close(); @@ -1511,8 +1364,7 @@ void scanOrGetScanner_CalledAfterDeleteUnderRealSnapshot_ShouldThrowIllegalArgum Scan scanForStorage = toScanForStorageFrom(scan); result = prepareResult(TransactionState.PREPARED); - Snapshot.Key key = new Snapshot.Key(scanForStorage, result); - when(snapshot.getId()).thenReturn(ANY_ID_1); + Snapshot.Key key = new Snapshot.Key(scanForStorage, result, TABLE_METADATA); if (scanType == ScanType.SCAN) { when(scanner.iterator()).thenReturn(Collections.singletonList(result).iterator()); } else { @@ -1521,6 +1373,7 @@ void scanOrGetScanner_CalledAfterDeleteUnderRealSnapshot_ShouldThrowIllegalArgum when(storage.scan(any(ScanAll.class))).thenReturn(scanner); TransactionResult recoveredResult = mock(TransactionResult.class); + when(recoveredResult.getContainedColumnNames()).thenReturn(Collections.singleton(ANY_NAME_3)); when(recoveredResult.getAsObject(ANY_NAME_3)).thenReturn(ANY_TEXT_3); when(recoveredResult.getColumns()) @@ -1539,8 +1392,11 @@ void scanOrGetScanner_CalledAfterDeleteUnderRealSnapshot_ShouldThrowIllegalArgum RecoveryExecutor.RecoveryType.RETURN_LATEST_RESULT_AND_RECOVER)) .thenReturn(new RecoveryExecutor.Result(key, Optional.of(recoveredResult), recoveryFuture)); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); + // Act - List results = scanOrGetScanner(scanForStorage, scanType); + List results = scanOrGetScanner(scanForStorage, scanType, context); // Assert verify(storage) @@ -1572,8 +1428,7 @@ void scanOrGetScanner_CalledAfterDeleteUnderRealSnapshot_ShouldThrowIllegalArgum Scan scanForStorage = toScanForStorageFrom(scan); result = prepareResult(TransactionState.PREPARED); - Snapshot.Key key = new Snapshot.Key(scanForStorage, result); - when(snapshot.getId()).thenReturn(ANY_ID_1); + Snapshot.Key key = new Snapshot.Key(scanForStorage, result, TABLE_METADATA); if (scanType == ScanType.SCAN) { when(scanner.iterator()).thenReturn(Collections.singletonList(result).iterator()); } else { @@ -1582,6 +1437,7 @@ void scanOrGetScanner_CalledAfterDeleteUnderRealSnapshot_ShouldThrowIllegalArgum when(storage.scan(any(ScanAll.class))).thenReturn(scanner); TransactionResult recoveredResult = mock(TransactionResult.class); + when(recoveredResult.getContainedColumnNames()).thenReturn(Collections.singleton(ANY_NAME_3)); when(recoveredResult.getAsObject(ANY_NAME_3)).thenReturn(ANY_TEXT_4); when(recoveredResult.getColumns()) @@ -1600,8 +1456,11 @@ void scanOrGetScanner_CalledAfterDeleteUnderRealSnapshot_ShouldThrowIllegalArgum RecoveryExecutor.RecoveryType.RETURN_LATEST_RESULT_AND_RECOVER)) .thenReturn(new RecoveryExecutor.Result(key, Optional.of(recoveredResult), recoveryFuture)); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); + // Act - List results = scanOrGetScanner(scanForStorage, scanType); + List results = scanOrGetScanner(scanForStorage, scanType, context); // Assert verify(storage) @@ -1631,8 +1490,8 @@ void scanOrGetScanner_WithLimit_ShouldReturnLimitedResults(ScanType scanType) Result result1 = prepareResult(ANY_TEXT_1, ANY_TEXT_2, TransactionState.COMMITTED); Result result2 = prepareResult(ANY_TEXT_1, ANY_TEXT_3, TransactionState.COMMITTED); - Snapshot.Key key1 = new Snapshot.Key(scanWithLimit, result1); - Snapshot.Key key2 = new Snapshot.Key(scanWithLimit, result2); + Snapshot.Key key1 = new Snapshot.Key(scanWithLimit, result1, TABLE_METADATA); + Snapshot.Key key2 = new Snapshot.Key(scanWithLimit, result2, TABLE_METADATA); TransactionResult transactionResult1 = new TransactionResult(result1); TransactionResult transactionResult2 = new TransactionResult(result2); @@ -1648,8 +1507,11 @@ void scanOrGetScanner_WithLimit_ShouldReturnLimitedResults(ScanType scanType) } when(storage.scan(scanForStorage)).thenReturn(scanner); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); + // Act - List results = scanOrGetScanner(scanWithLimit, scanType); + List results = scanOrGetScanner(scanWithLimit, scanType, context); // Assert assertThat(results).hasSize(2); @@ -1685,7 +1547,7 @@ void scanOrGetScanner_WithLimitExceedingAvailableResults_ShouldReturnAllAvailabl Scan scanForStorage = toScanForStorageFrom(scanWithoutLimit); Result result = prepareResult(TransactionState.COMMITTED); - Snapshot.Key key1 = new Snapshot.Key(scanWithLimit, result); + Snapshot.Key key1 = new Snapshot.Key(scanWithLimit, result, TABLE_METADATA); TransactionResult transactionResult1 = new TransactionResult(result); // Set up mock scanner to return one result (less than limit) @@ -1696,8 +1558,11 @@ void scanOrGetScanner_WithLimitExceedingAvailableResults_ShouldReturnAllAvailabl } when(storage.scan(scanForStorage)).thenReturn(scanner); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); + // Act - List results = scanOrGetScanner(scanWithLimit, scanType); + List results = scanOrGetScanner(scanWithLimit, scanType, context); // Assert assertThat(results).hasSize(1); @@ -1720,9 +1585,9 @@ void scanOrGetScanner_WithLimitExceedingAvailableResults_ShouldReturnAllAvailabl Result uncommittedResult2 = prepareResult(ANY_TEXT_1, ANY_TEXT_3, TransactionState.PREPARED); Result uncommittedResult3 = prepareResult(ANY_TEXT_1, ANY_TEXT_4, TransactionState.PREPARED); - Snapshot.Key key1 = new Snapshot.Key(scanWithLimit, uncommittedResult1); - Snapshot.Key key2 = new Snapshot.Key(scanWithLimit, uncommittedResult2); - Snapshot.Key key3 = new Snapshot.Key(scanWithLimit, uncommittedResult3); + Snapshot.Key key1 = new Snapshot.Key(scanWithLimit, uncommittedResult1, TABLE_METADATA); + Snapshot.Key key2 = new Snapshot.Key(scanWithLimit, uncommittedResult2, TABLE_METADATA); + Snapshot.Key key3 = new Snapshot.Key(scanWithLimit, uncommittedResult3, TABLE_METADATA); // Set up mock scanner to return one committed and one uncommitted result if (scanType == ScanType.SCAN) { @@ -1738,8 +1603,6 @@ void scanOrGetScanner_WithLimitExceedingAvailableResults_ShouldReturnAllAvailabl } when(storage.scan(scanForStorageWithoutLimit)).thenReturn(scanner); - when(snapshot.getId()).thenReturn(ANY_ID_1); - TransactionResult recoveredResult1 = mock(TransactionResult.class); when(recoveredResult1.getContainedColumnNames()).thenReturn(Collections.singleton(ANY_NAME_3)); when(recoveredResult1.getAsObject(ANY_NAME_3)).thenReturn(ANY_TEXT_3); @@ -1779,8 +1642,11 @@ void scanOrGetScanner_WithLimitExceedingAvailableResults_ShouldReturnAllAvailabl .thenReturn( new RecoveryExecutor.Result(key3, Optional.of(recoveredResult2), recoveryFuture)); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); + // Act - List results = scanOrGetScanner(scanWithLimit, scanType); + List results = scanOrGetScanner(scanWithLimit, scanType, context); // Assert verify(storage).scan(scanForStorageWithoutLimit); @@ -1817,9 +1683,11 @@ void scanOrGetScanner_WithLimitExceedingAvailableResults_ShouldReturnAllAvailabl when(iterator.hasNext()).thenThrow(runtimeException); when(scanner.iterator()).thenReturn(iterator); when(storage.scan(scanForStorage)).thenReturn(scanner); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); // Act Assert - assertThatThrownBy(() -> handler.scan(scan)) + assertThatThrownBy(() -> handler.scan(scan, context)) .isInstanceOf(CrudException.class) .hasCause(executionException); @@ -1838,9 +1706,11 @@ public void scan_RuntimeExceptionThrownByIteratorHasNext_ShouldThrowCrudExceptio when(iterator.hasNext()).thenThrow(runtimeException); when(scanner.iterator()).thenReturn(iterator); when(storage.scan(scanForStorage)).thenReturn(scanner); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); // Act Assert - assertThatThrownBy(() -> handler.scan(scan)) + assertThatThrownBy(() -> handler.scan(scan, context)) .isInstanceOf(CrudException.class) .hasCause(runtimeException); @@ -1856,9 +1726,11 @@ public void getScanner_ExecutionExceptionThrownByScannerOne_ShouldThrowCrudExcep ExecutionException executionException = mock(ExecutionException.class); when(scanner.one()).thenThrow(executionException); when(storage.scan(scanForStorage)).thenReturn(scanner); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); // Act Assert - TransactionCrudOperable.Scanner actualScanner = handler.getScanner(scan); + TransactionCrudOperable.Scanner actualScanner = handler.getScanner(scan, context); assertThatThrownBy(actualScanner::one) .isInstanceOf(CrudException.class) .hasCause(executionException); @@ -1868,25 +1740,25 @@ public void getScanner_ExecutionExceptionThrownByScannerOne_ShouldThrowCrudExcep @Test public void - getScanner_ScannerNotFullyScanned_ShouldPutReadSetAndScannerSetInSnapshotAndVerifyScan() + getScanner_ScannerNotFullyScanned_ValidationRequired_ShouldPutReadSetAndScannerSetInSnapshotAndVerifyScan() throws ExecutionException, CrudException, IOException { // Arrange - when(snapshot.isValidationRequired()).thenReturn(true); - Scan scan = prepareScan(); Scan scanForStorage = toScanForStorageFrom(scan); Result result1 = prepareResult(TransactionState.COMMITTED); Result result2 = prepareResult(TransactionState.COMMITTED); - Snapshot.Key key1 = new Snapshot.Key(scan, result1); + Snapshot.Key key1 = new Snapshot.Key(scan, result1, TABLE_METADATA); TransactionResult txResult1 = new TransactionResult(result1); when(scanner.one()) .thenReturn(Optional.of(result1)) .thenReturn(Optional.of(result2)) .thenReturn(Optional.empty()); when(storage.scan(scanForStorage)).thenReturn(scanner); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SERIALIZABLE, false, false); // Act - TransactionCrudOperable.Scanner actualScanner = handler.getScanner(scan); + TransactionCrudOperable.Scanner actualScanner = handler.getScanner(scan, context); Optional actualResult = actualScanner.one(); actualScanner.close(); @@ -1906,19 +1778,6 @@ public void getScanner_ExecutionExceptionThrownByScannerOne_ShouldThrowCrudExcep getScanner_ScannerNotFullyScanned_InOneOperationMode_ValidationNotRequired_ShouldUpdateSnapshotProperly() throws ExecutionException, CrudException { // Arrange - handler = - new CrudHandler( - storage, - snapshot, - recoveryExecutor, - tableMetadataManager, - false, - mutationConditionsValidator, - parallelExecutor, - true, - true); - when(snapshot.isValidationRequired()).thenReturn(false); - Scan scan = prepareScan(); Scan scanForStorage = toScanForStorageFrom(scan); Result result1 = prepareResult(TransactionState.COMMITTED); @@ -1929,9 +1788,11 @@ public void getScanner_ExecutionExceptionThrownByScannerOne_ShouldThrowCrudExcep .thenReturn(Optional.of(result2)) .thenReturn(Optional.empty()); when(storage.scan(scanForStorage)).thenReturn(scanner); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, true, true); // Act - TransactionCrudOperable.Scanner actualScanner = handler.getScanner(scan); + TransactionCrudOperable.Scanner actualScanner = handler.getScanner(scan, context); Optional actualResult = actualScanner.one(); actualScanner.close(); @@ -1949,33 +1810,22 @@ public void getScanner_ExecutionExceptionThrownByScannerOne_ShouldThrowCrudExcep getScanner_ScannerNotFullyScanned_InOneOperationMode_ValidationRequired_ShouldUpdateSnapshotProperly() throws ExecutionException, CrudException { // Arrange - handler = - new CrudHandler( - storage, - snapshot, - recoveryExecutor, - tableMetadataManager, - false, - mutationConditionsValidator, - parallelExecutor, - true, - true); - when(snapshot.isValidationRequired()).thenReturn(true); - Scan scan = prepareScan(); Scan scanForStorage = toScanForStorageFrom(scan); Result result1 = prepareResult(TransactionState.COMMITTED); Result result2 = prepareResult(TransactionState.COMMITTED); - Snapshot.Key key1 = new Snapshot.Key(scan, result1); + Snapshot.Key key1 = new Snapshot.Key(scan, result1, TABLE_METADATA); TransactionResult txResult1 = new TransactionResult(result1); when(scanner.one()) .thenReturn(Optional.of(result1)) .thenReturn(Optional.of(result2)) .thenReturn(Optional.empty()); when(storage.scan(scanForStorage)).thenReturn(scanner); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SERIALIZABLE, true, true); // Act - TransactionCrudOperable.Scanner actualScanner = handler.getScanner(scan); + TransactionCrudOperable.Scanner actualScanner = handler.getScanner(scan, context); Optional actualResult = actualScanner.one(); actualScanner.close(); @@ -1994,16 +1844,18 @@ public void put_PutWithoutConditionGiven_ShouldCallAppropriateMethods() throws C // Arrange Put put = Put.newBuilder().namespace("ns").table("tbl").partitionKey(Key.ofText("c1", "foo")).build(); - CrudHandler spied = spy(handler); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); // Act - spied.put(put); + spied.put(put, context); // Assert - verify(spied, never()).readUnread(any(), any()); + verify(spied, never()).readUnread(any(), any(), any()); verify(snapshot, never()).getResult(any()); - verify(mutationConditionsValidator, never()).checkIfConditionIsSatisfied(any(Put.class), any()); + verify(mutationConditionsValidator, never()) + .checkIfConditionIsSatisfied(any(Put.class), any(), any()); verify(snapshot).putIntoWriteSet(new Snapshot.Key(put), put); } @@ -2035,13 +1887,16 @@ public void put_PutWithoutConditionGiven_ShouldCallAppropriateMethods() throws C CrudHandler spied = spy(handler); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); + // Act - spied.put(put); + spied.put(put, context); // Assert - verify(spied, never()).readUnread(key, getForKey); + verify(spied, never()).readUnread(key, getForKey, context); verify(snapshot).getResult(key); - verify(mutationConditionsValidator).checkIfConditionIsSatisfied(put, result); + verify(mutationConditionsValidator).checkIfConditionIsSatisfied(put, result, context); verify(snapshot).putIntoWriteSet(key, put); } @@ -2073,15 +1928,19 @@ public void put_PutWithoutConditionGiven_ShouldCallAppropriateMethods() throws C .build()); CrudHandler spied = spy(handler); - doReturn(Optional.empty()).when(spied).getFromStorage(getForKey); + + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); + + doReturn(Optional.empty()).when(spied).getFromStorage(getForKey, context); // Act - spied.put(put); + spied.put(put, context); // Assert - verify(spied).read(key, getForKey); + verify(spied).read(key, getForKey, context); verify(snapshot).getResult(key); - verify(mutationConditionsValidator).checkIfConditionIsSatisfied(put, result); + verify(mutationConditionsValidator).checkIfConditionIsSatisfied(put, result, context); verify(snapshot).putIntoWriteSet(key, put); } @@ -2112,13 +1971,16 @@ public void put_PutWithoutConditionGiven_ShouldCallAppropriateMethods() throws C CrudHandler spied = spy(handler); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); + // Act - spied.put(put); + spied.put(put, context); // Assert - verify(spied, never()).readUnread(key, getForKey); + verify(spied, never()).readUnread(key, getForKey, context); verify(snapshot).getResult(key); - verify(mutationConditionsValidator).checkIfConditionIsSatisfied(put, result); + verify(mutationConditionsValidator).checkIfConditionIsSatisfied(put, result, context); verify(snapshot).putIntoWriteSet(key, put); } @@ -2133,9 +1995,12 @@ public void put_PutWithoutConditionGiven_ShouldCallAppropriateMethods() throws C .partitionKey(Key.ofText("c1", "foo")) .condition(putIfExists()) .build(); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); // Act Assert - assertThatThrownBy(() -> handler.put(put)).isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> handler.put(put, context)) + .isInstanceOf(IllegalArgumentException.class); } @Test @@ -2151,14 +2016,17 @@ public void delete_DeleteWithoutConditionGiven_ShouldCallAppropriateMethods() CrudHandler spied = spy(handler); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); + // Act - spied.delete(delete); + spied.delete(delete, context); // Assert - verify(spied, never()).readUnread(any(), any()); + verify(spied, never()).readUnread(any(), any(), any()); verify(snapshot, never()).getResult(any()); verify(mutationConditionsValidator, never()) - .checkIfConditionIsSatisfied(any(Delete.class), any()); + .checkIfConditionIsSatisfied(any(Delete.class), any(), any()); verify(snapshot).putIntoDeleteSet(new Snapshot.Key(delete), delete); } @@ -2188,13 +2056,16 @@ public void delete_DeleteWithConditionGiven_WithResultInReadSet_ShouldCallApprop CrudHandler spied = spy(handler); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); + // Act - spied.delete(delete); + spied.delete(delete, context); // Assert - verify(spied, never()).readUnread(key, getForKey); + verify(spied, never()).readUnread(key, getForKey, context); verify(snapshot).getResult(key); - verify(mutationConditionsValidator).checkIfConditionIsSatisfied(delete, result); + verify(mutationConditionsValidator).checkIfConditionIsSatisfied(delete, result, context); verify(snapshot).putIntoDeleteSet(key, delete); } @@ -2222,15 +2093,19 @@ public void delete_DeleteWithConditionGiven_WithoutResultInReadSet_ShouldCallApp .build()); CrudHandler spied = spy(handler); - doReturn(Optional.empty()).when(spied).getFromStorage(getForKey); + + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); + + doReturn(Optional.empty()).when(spied).getFromStorage(getForKey, context); // Act - spied.delete(delete); + spied.delete(delete, context); // Assert - verify(spied).read(key, getForKey); + verify(spied).read(key, getForKey, context); verify(snapshot).getResult(key); - verify(mutationConditionsValidator).checkIfConditionIsSatisfied(delete, null); + verify(mutationConditionsValidator).checkIfConditionIsSatisfied(delete, null, context); verify(snapshot).putIntoDeleteSet(key, delete); } @@ -2250,9 +2125,11 @@ public void readUnread_GetContainedInGetSet_ShouldCallAppropriateMethods() .partitionKey(key.getPartitionKey()) .build(); when(snapshot.containsKeyInGetSet(getForKey)).thenReturn(true); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); // Act - handler.readUnread(key, getForKey); + handler.readUnread(key, getForKey, context); // Assert verify(storage, never()).get(any()); @@ -2277,9 +2154,11 @@ public void readUnread_GetContainedInGetSet_ShouldCallAppropriateMethods() .build(); when(snapshot.containsKeyInGetSet(getForKey)).thenReturn(false); when(storage.get(any())).thenReturn(Optional.empty()); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); // Act - handler.readUnread(key, getForKey); + handler.readUnread(key, getForKey, context); // Assert verify(storage).get(any()); @@ -2305,9 +2184,11 @@ public void readUnread_GetContainedInGetSet_ShouldCallAppropriateMethods() .build(); when(snapshot.containsKeyInGetSet(getForKey)).thenReturn(false); when(storage.get(any())).thenReturn(Optional.empty()); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); // Act - handler.readUnread(key, getForKey); + handler.readUnread(key, getForKey, context); // Assert verify(storage) @@ -2344,8 +2225,11 @@ public void readUnread_GetContainedInGetSet_ShouldCallAppropriateMethods() .build(); when(snapshot.containsKeyInGetSet(getForKey)).thenReturn(false); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); + // Act - handler.readUnread(key, getForKey); + handler.readUnread(key, getForKey, context); // Assert verify(storage).get(any()); @@ -2373,7 +2257,6 @@ public void readUnread_GetContainedInGetSet_ShouldCallAppropriateMethods() .partitionKey(key.getPartitionKey()) .build(); when(snapshot.containsKeyInGetSet(getForKey)).thenReturn(false); - when(snapshot.getId()).thenReturn(ANY_ID_1); TransactionResult recoveredResult = mock(TransactionResult.class); @SuppressWarnings("unchecked") @@ -2387,8 +2270,11 @@ public void readUnread_GetContainedInGetSet_ShouldCallAppropriateMethods() RecoveryExecutor.RecoveryType.RETURN_LATEST_RESULT_AND_RECOVER)) .thenReturn(new RecoveryExecutor.Result(key, Optional.of(recoveredResult), recoveryFuture)); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); + // Act - handler.readUnread(key, getForKey); + handler.readUnread(key, getForKey, context); // Assert verify(storage).get(getForKey); @@ -2423,7 +2309,6 @@ public void readUnread_GetContainedInGetSet_ShouldCallAppropriateMethods() .partitionKey(key.getPartitionKey()) .build(); when(snapshot.containsKeyInGetSet(getForKey)).thenReturn(false); - when(snapshot.getId()).thenReturn(ANY_ID_1); Optional recoveredRecord = Optional.empty(); @SuppressWarnings("unchecked") @@ -2437,8 +2322,11 @@ public void readUnread_GetContainedInGetSet_ShouldCallAppropriateMethods() RecoveryExecutor.RecoveryType.RETURN_LATEST_RESULT_AND_RECOVER)) .thenReturn(new RecoveryExecutor.Result(key, recoveredRecord, recoveryFuture)); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); + // Act - handler.readUnread(key, getForKey); + handler.readUnread(key, getForKey, context); // Assert verify(storage).get(getForKey); @@ -2474,9 +2362,9 @@ public void readUnread_GetContainedInGetSet_ShouldCallAppropriateMethods() .where(column(ANY_NAME_3).isEqualToText(ANY_TEXT_3)) .build(); when(snapshot.containsKeyInGetSet(getWithConjunction)).thenReturn(false); - when(snapshot.getId()).thenReturn(ANY_ID_1); TransactionResult recoveredResult = mock(TransactionResult.class); + when(recoveredResult.getContainedColumnNames()).thenReturn(Collections.singleton(ANY_NAME_3)); when(recoveredResult.getAsObject(ANY_NAME_3)).thenReturn(ANY_TEXT_3); when(recoveredResult.getColumns()) @@ -2493,8 +2381,11 @@ public void readUnread_GetContainedInGetSet_ShouldCallAppropriateMethods() RecoveryExecutor.RecoveryType.RETURN_LATEST_RESULT_AND_RECOVER)) .thenReturn(new RecoveryExecutor.Result(key, Optional.of(recoveredResult), recoveryFuture)); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); + // Act - handler.readUnread(key, getWithConjunction); + handler.readUnread(key, getWithConjunction, context); // Assert verify(storage) @@ -2536,9 +2427,9 @@ public void readUnread_GetContainedInGetSet_ShouldCallAppropriateMethods() .where(column(ANY_NAME_3).isEqualToText(ANY_TEXT_3)) .build(); when(snapshot.containsKeyInGetSet(getWithConjunction)).thenReturn(false); - when(snapshot.getId()).thenReturn(ANY_ID_1); TransactionResult recoveredResult = mock(TransactionResult.class); + when(recoveredResult.getContainedColumnNames()).thenReturn(Collections.singleton(ANY_NAME_3)); when(recoveredResult.getAsObject(ANY_NAME_3)).thenReturn(ANY_TEXT_4); when(recoveredResult.getColumns()) @@ -2555,8 +2446,11 @@ public void readUnread_GetContainedInGetSet_ShouldCallAppropriateMethods() RecoveryExecutor.RecoveryType.RETURN_LATEST_RESULT_AND_RECOVER)) .thenReturn(new RecoveryExecutor.Result(key, Optional.of(recoveredResult), recoveryFuture)); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); + // Act - handler.readUnread(key, getWithConjunction); + handler.readUnread(key, getWithConjunction, context); // Assert verify(storage) @@ -2590,9 +2484,11 @@ public void readUnread_GetContainedInGetSet_ShouldCallAppropriateMethods() .build(); when(snapshot.containsKeyInGetSet(getWithIndex)).thenReturn(false); when(storage.get(any())).thenReturn(Optional.empty()); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); // Act - handler.readUnread(null, getWithIndex); + handler.readUnread(null, getWithIndex, context); // Assert verify(storage).get(any()); @@ -2606,8 +2502,11 @@ public void readUnread_GetContainedInGetSet_ShouldCallAppropriateMethods() throws CrudException, ExecutionException { // Arrange when(result.getInt(Attribute.STATE)).thenReturn(TransactionState.COMMITTED.get()); - when(result.getPartitionKey()).thenReturn(Optional.of(Key.ofText(ANY_NAME_1, ANY_TEXT_1))); - when(result.getClusteringKey()).thenReturn(Optional.of(Key.ofText(ANY_NAME_2, ANY_TEXT_2))); + when(result.getColumns()) + .thenReturn( + ImmutableMap.of( + ANY_NAME_1, TextColumn.of(ANY_NAME_1, ANY_TEXT_1), + ANY_NAME_2, TextColumn.of(ANY_NAME_2, ANY_TEXT_2))); when(storage.get(any())).thenReturn(Optional.of(result)); Get getWithIndex = @@ -2618,14 +2517,18 @@ public void readUnread_GetContainedInGetSet_ShouldCallAppropriateMethods() .build(); when(snapshot.containsKeyInGetSet(getWithIndex)).thenReturn(false); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); + // Act - handler.readUnread(null, getWithIndex); + handler.readUnread(null, getWithIndex, context); // Assert verify(storage).get(any()); verify(snapshot) .putIntoReadSet( - new Snapshot.Key(getWithIndex, result), Optional.of(new TransactionResult(result))); + new Snapshot.Key(getWithIndex, result, TABLE_METADATA), + Optional.of(new TransactionResult(result))); verify(snapshot).putIntoGetSet(getWithIndex, Optional.of(new TransactionResult(result))); } @@ -2635,8 +2538,11 @@ public void readUnread_GetContainedInGetSet_ShouldCallAppropriateMethods() throws ExecutionException, CrudException { // Arrange when(result.getInt(Attribute.STATE)).thenReturn(TransactionState.PREPARED.get()); - when(result.getPartitionKey()).thenReturn(Optional.of(Key.ofText(ANY_NAME_1, ANY_TEXT_1))); - when(result.getClusteringKey()).thenReturn(Optional.of(Key.ofText(ANY_NAME_2, ANY_TEXT_2))); + when(result.getColumns()) + .thenReturn( + ImmutableMap.of( + ANY_NAME_1, TextColumn.of(ANY_NAME_1, ANY_TEXT_1), + ANY_NAME_2, TextColumn.of(ANY_NAME_2, ANY_TEXT_2))); when(storage.get(any())).thenReturn(Optional.of(result)); Get getWithIndex = @@ -2646,9 +2552,8 @@ public void readUnread_GetContainedInGetSet_ShouldCallAppropriateMethods() .indexKey(Key.ofText(ANY_NAME_3, ANY_TEXT_1)) .build(); when(snapshot.containsKeyInGetSet(getWithIndex)).thenReturn(false); - when(snapshot.getId()).thenReturn(ANY_ID_1); - Snapshot.Key key = new Snapshot.Key(getWithIndex, result); + Snapshot.Key key = new Snapshot.Key(getWithIndex, result, TABLE_METADATA); TransactionResult recoveredResult = mock(TransactionResult.class); @SuppressWarnings("unchecked") @@ -2662,8 +2567,11 @@ public void readUnread_GetContainedInGetSet_ShouldCallAppropriateMethods() RecoveryExecutor.RecoveryType.RETURN_LATEST_RESULT_AND_RECOVER)) .thenReturn(new RecoveryExecutor.Result(key, Optional.of(recoveredResult), recoveryFuture)); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); + // Act - handler.readUnread(key, getWithIndex); + handler.readUnread(key, getWithIndex, context); // Assert verify(storage).get(getWithIndex); @@ -2707,7 +2615,12 @@ public void readIfImplicitPreReadEnabled_ShouldCallAppropriateMethods() when(put3.forTable()).thenReturn(Optional.of(ANY_TABLE_NAME)); when(put3.getPartitionKey()).thenReturn(partitionKey3); - when(snapshot.getPutsInWriteSet()).thenReturn(Arrays.asList(put1, put2, put3)); + Map writeSet = + ImmutableMap.of( + new Snapshot.Key(put1), put1, + new Snapshot.Key(put2), put2, + new Snapshot.Key(put3), put3); + when(snapshot.getWriteSet()).thenReturn(writeSet.entrySet()); Delete delete1 = mock(Delete.class); when(delete1.forNamespace()).thenReturn(Optional.of(ANY_NAMESPACE_NAME)); @@ -2719,7 +2632,11 @@ public void readIfImplicitPreReadEnabled_ShouldCallAppropriateMethods() when(delete2.forTable()).thenReturn(Optional.of(ANY_TABLE_NAME)); when(delete2.getPartitionKey()).thenReturn(partitionKey5); - when(snapshot.getDeletesInDeleteSet()).thenReturn(Arrays.asList(delete1, delete2)); + Map deleteSet = + ImmutableMap.of( + new Snapshot.Key(delete1), delete1, + new Snapshot.Key(delete2), delete2); + when(snapshot.getDeleteSet()).thenReturn(deleteSet.entrySet()); Get get1 = toGetForStorageFrom( @@ -2778,10 +2695,11 @@ public void readIfImplicitPreReadEnabled_ShouldCallAppropriateMethods() when(storage.get(get3)).thenReturn(Optional.of(result3)); when(storage.get(get4)).thenReturn(Optional.of(result4)); - when(snapshot.getId()).thenReturn(ANY_TX_ID); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); // Act - handler.readIfImplicitPreReadEnabled(); + handler.readIfImplicitPreReadEnabled(context); // Assert @SuppressWarnings("unchecked") @@ -2817,7 +2735,7 @@ public void readIfImplicitPreReadEnabled_ShouldCallAppropriateMethods() verify(snapshot).putIntoGetSet(get3, Optional.of(new TransactionResult(result3))); verify(snapshot).putIntoGetSet(get4, Optional.of(new TransactionResult(result4))); - assertThat(transactionIdCaptor.getValue()).isEqualTo(ANY_TX_ID); + assertThat(transactionIdCaptor.getValue()).isEqualTo(ANY_ID_1); } @Test @@ -2826,6 +2744,8 @@ public void get_WithConjunctions_ShouldConvertConjunctions() // Arrange when(result.getInt(Attribute.STATE)).thenReturn(TransactionState.COMMITTED.get()); when(storage.get(any())).thenReturn(Optional.of(result)); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); // Act handler.get( @@ -2835,7 +2755,8 @@ public void get_WithConjunctions_ShouldConvertConjunctions() .partitionKey(Key.ofText(ANY_NAME_1, ANY_TEXT_1)) .clusteringKey(Key.ofText(ANY_NAME_2, ANY_TEXT_2)) .where(column(ANY_NAME_3).isEqualToText(ANY_TEXT_3)) - .build()); + .build(), + context); handler.get( Get.newBuilder() .namespace(ANY_NAMESPACE_NAME) @@ -2844,7 +2765,8 @@ public void get_WithConjunctions_ShouldConvertConjunctions() .clusteringKey(Key.ofText(ANY_NAME_2, ANY_TEXT_2)) .where(column(ANY_NAME_3).isEqualToText(ANY_TEXT_3)) .and(column(ANY_NAME_4).isEqualToInt(10)) - .build()); + .build(), + context); handler.get( Get.newBuilder() .namespace(ANY_NAMESPACE_NAME) @@ -2853,7 +2775,8 @@ public void get_WithConjunctions_ShouldConvertConjunctions() .clusteringKey(Key.ofText(ANY_NAME_2, ANY_TEXT_2)) .where(column(ANY_NAME_3).isEqualToText(ANY_TEXT_3)) .or(column(ANY_NAME_4).isEqualToInt(20)) - .build()); + .build(), + context); handler.get( Get.newBuilder() .namespace(ANY_NAMESPACE_NAME) @@ -2868,7 +2791,8 @@ public void get_WithConjunctions_ShouldConvertConjunctions() condition(column(ANY_NAME_4).isGreaterThanInt(30)) .and(column(ANY_NAME_4).isLessThanOrEqualToInt(40)) .build()) - .build()); + .build(), + context); handler.get( Get.newBuilder() .namespace(ANY_NAMESPACE_NAME) @@ -2883,7 +2807,8 @@ public void get_WithConjunctions_ShouldConvertConjunctions() condition(column(ANY_NAME_4).isLessThanOrEqualToInt(50)) .or(column(ANY_NAME_4).isGreaterThanInt(60)) .build()) - .build()); + .build(), + context); handler.get( Get.newBuilder() .namespace(ANY_NAMESPACE_NAME) @@ -2892,7 +2817,8 @@ public void get_WithConjunctions_ShouldConvertConjunctions() .clusteringKey(Key.ofText(ANY_NAME_2, ANY_TEXT_2)) .where(column(ANY_NAME_3).isLikeText(ANY_TEXT_3)) .or(column(ANY_NAME_3).isLikeText(ANY_TEXT_4)) - .build()); + .build(), + context); // Assert verify(storage) @@ -3034,10 +2960,15 @@ public void scan_WithConjunctions_ShouldConvertConjunctions() throws CrudException, ExecutionException { // Arrange when(result.getInt(Attribute.STATE)).thenReturn(TransactionState.COMMITTED.get()); - when(result.getPartitionKey()).thenReturn(Optional.of(Key.ofText(ANY_NAME_1, ANY_TEXT_1))); - when(result.getClusteringKey()).thenReturn(Optional.of(Key.ofText(ANY_NAME_2, ANY_TEXT_2))); + when(result.getColumns()) + .thenReturn( + ImmutableMap.of( + ANY_NAME_1, TextColumn.of(ANY_NAME_1, ANY_TEXT_1), + ANY_NAME_2, TextColumn.of(ANY_NAME_2, ANY_TEXT_2))); when(scanner.iterator()).thenReturn(Collections.singletonList(result).iterator()); when(storage.scan(any())).thenReturn(scanner); + TransactionContext context = + new TransactionContext(ANY_ID_1, snapshot, Isolation.SNAPSHOT, false, false); // Act handler.scan( @@ -3046,7 +2977,8 @@ public void scan_WithConjunctions_ShouldConvertConjunctions() .table(ANY_TABLE_NAME) .partitionKey(Key.ofText(ANY_NAME_1, ANY_TEXT_1)) .where(column(ANY_NAME_3).isEqualToText(ANY_TEXT_3)) - .build()); + .build(), + context); handler.scan( Scan.newBuilder() .namespace(ANY_NAMESPACE_NAME) @@ -3054,7 +2986,8 @@ public void scan_WithConjunctions_ShouldConvertConjunctions() .partitionKey(Key.ofText(ANY_NAME_1, ANY_TEXT_1)) .where(column(ANY_NAME_3).isEqualToText(ANY_TEXT_3)) .and(column(ANY_NAME_4).isEqualToInt(10)) - .build()); + .build(), + context); handler.scan( Scan.newBuilder() .namespace(ANY_NAMESPACE_NAME) @@ -3062,7 +2995,8 @@ public void scan_WithConjunctions_ShouldConvertConjunctions() .partitionKey(Key.ofText(ANY_NAME_1, ANY_TEXT_1)) .where(column(ANY_NAME_3).isEqualToText(ANY_TEXT_3)) .or(column(ANY_NAME_4).isEqualToInt(20)) - .build()); + .build(), + context); handler.scan( Scan.newBuilder() .namespace(ANY_NAMESPACE_NAME) @@ -3076,7 +3010,8 @@ public void scan_WithConjunctions_ShouldConvertConjunctions() condition(column(ANY_NAME_4).isGreaterThanInt(30)) .and(column(ANY_NAME_4).isLessThanOrEqualToInt(40)) .build()) - .build()); + .build(), + context); handler.scan( Scan.newBuilder() .namespace(ANY_NAMESPACE_NAME) @@ -3090,7 +3025,8 @@ public void scan_WithConjunctions_ShouldConvertConjunctions() condition(column(ANY_NAME_4).isLessThanOrEqualToInt(50)) .or(column(ANY_NAME_4).isGreaterThanInt(60)) .build()) - .build()); + .build(), + context); handler.scan( Scan.newBuilder() .namespace(ANY_NAMESPACE_NAME) @@ -3098,7 +3034,8 @@ public void scan_WithConjunctions_ShouldConvertConjunctions() .partitionKey(Key.ofText(ANY_NAME_1, ANY_TEXT_1)) .where(column(ANY_NAME_3).isLikeText(ANY_TEXT_3)) .or(column(ANY_NAME_3).isLikeText(ANY_TEXT_4)) - .build()); + .build(), + context); handler.scan( Scan.newBuilder() .namespace(ANY_NAMESPACE_NAME) @@ -3106,7 +3043,8 @@ public void scan_WithConjunctions_ShouldConvertConjunctions() .all() .where(column(ANY_NAME_1).isGreaterThanText(ANY_TEXT_3)) .and(column(ANY_NAME_2).isLessThanOrEqualToText(ANY_TEXT_4)) - .build()); + .build(), + context); handler.scan( Scan.newBuilder() .namespace(ANY_NAMESPACE_NAME) @@ -3114,7 +3052,8 @@ public void scan_WithConjunctions_ShouldConvertConjunctions() .all() .where(column(ANY_NAME_1).isGreaterThanText(ANY_TEXT_3)) .and(column(ANY_NAME_3).isEqualToText(ANY_TEXT_4)) - .build()); + .build(), + context); // Assert verify(storage) @@ -3271,12 +3210,13 @@ public void scan_WithConjunctions_ShouldConvertConjunctions() .build()); } - private List scanOrGetScanner(Scan scan, ScanType scanType) throws CrudException { + private List scanOrGetScanner(Scan scan, ScanType scanType, TransactionContext context) + throws CrudException { if (scanType == ScanType.SCAN) { - return handler.scan(scan); + return handler.scan(scan, context); } - try (TransactionCrudOperable.Scanner scanner = handler.getScanner(scan)) { + try (TransactionCrudOperable.Scanner scanner = handler.getScanner(scan, context)) { switch (scanType) { case SCANNER_ONE: List results = new ArrayList<>(); diff --git a/core/src/test/java/com/scalar/db/transaction/consensuscommit/MutationConditionsValidatorTest.java b/core/src/test/java/com/scalar/db/transaction/consensuscommit/MutationConditionsValidatorTest.java index 8bf8136bf8..673e84f5c2 100644 --- a/core/src/test/java/com/scalar/db/transaction/consensuscommit/MutationConditionsValidatorTest.java +++ b/core/src/test/java/com/scalar/db/transaction/consensuscommit/MutationConditionsValidatorTest.java @@ -33,22 +33,26 @@ public class MutationConditionsValidatorTest { private MutationConditionsValidator validator; - private @Mock TransactionResult existingRecord; - private @Mock Put put; - private @Mock Delete delete; - private @Mock PutIf putIf; - private @Mock PutIfExists putIfExists; - private @Mock PutIfNotExists putIfNotExists; - private @Mock DeleteIf deleteIf; - private @Mock DeleteIfExists deleteIfExists; + @Mock private TransactionResult existingRecord; + @Mock private Put put; + @Mock private Delete delete; + @Mock private PutIf putIf; + @Mock private PutIfExists putIfExists; + @Mock private PutIfNotExists putIfNotExists; + @Mock private DeleteIf deleteIf; + @Mock private DeleteIfExists deleteIfExists; private static final String C1 = "col_1"; private static final String C2 = "col_2"; private static final String C3 = "col_3"; + private TransactionContext context; + @Mock private Snapshot snapshot; + @BeforeEach public void setUp() throws Exception { MockitoAnnotations.openMocks(this).close(); - validator = new MutationConditionsValidator("a_tx_id"); + validator = new MutationConditionsValidator(); + context = new TransactionContext("a_tx_id", snapshot, Isolation.SNAPSHOT, false, false); } @Test @@ -58,7 +62,7 @@ public void setUp() throws Exception { prepareMutationWithCondition(put, putIf); // Act Assert - Assertions.assertThatThrownBy(() -> validator.checkIfConditionIsSatisfied(put, null)) + Assertions.assertThatThrownBy(() -> validator.checkIfConditionIsSatisfied(put, null, context)) .isInstanceOf(UnsatisfiedConditionException.class); } @@ -69,7 +73,7 @@ public void setUp() throws Exception { prepareMutationWithCondition(put, putIfExists); // Act Assert - Assertions.assertThatThrownBy(() -> validator.checkIfConditionIsSatisfied(put, null)) + Assertions.assertThatThrownBy(() -> validator.checkIfConditionIsSatisfied(put, null, context)) .isInstanceOf(UnsatisfiedConditionException.class); } @@ -80,7 +84,8 @@ public void setUp() throws Exception { prepareMutationWithCondition(put, putIfExists); // Act Assert - Assertions.assertThatCode(() -> validator.checkIfConditionIsSatisfied(put, existingRecord)) + Assertions.assertThatCode( + () -> validator.checkIfConditionIsSatisfied(put, existingRecord, context)) .doesNotThrowAnyException(); } @@ -91,7 +96,8 @@ public void setUp() throws Exception { prepareMutationWithCondition(put, putIfNotExists); // Act Assert - Assertions.assertThatThrownBy(() -> validator.checkIfConditionIsSatisfied(put, existingRecord)) + Assertions.assertThatThrownBy( + () -> validator.checkIfConditionIsSatisfied(put, existingRecord, context)) .isInstanceOf(UnsatisfiedConditionException.class); } @@ -102,7 +108,7 @@ public void setUp() throws Exception { prepareMutationWithCondition(put, putIfNotExists); // Act Assert - Assertions.assertThatCode(() -> validator.checkIfConditionIsSatisfied(put, null)) + Assertions.assertThatCode(() -> validator.checkIfConditionIsSatisfied(put, null, context)) .doesNotThrowAnyException(); } @@ -113,7 +119,8 @@ public void setUp() throws Exception { prepareMutationWithCondition(delete, deleteIf); // Act Assert - Assertions.assertThatThrownBy(() -> validator.checkIfConditionIsSatisfied(delete, null)) + Assertions.assertThatThrownBy( + () -> validator.checkIfConditionIsSatisfied(delete, null, context)) .isInstanceOf(UnsatisfiedConditionException.class); } @@ -124,7 +131,8 @@ public void setUp() throws Exception { prepareMutationWithCondition(delete, deleteIfExists); // Act Assert - Assertions.assertThatThrownBy(() -> validator.checkIfConditionIsSatisfied(delete, null)) + Assertions.assertThatThrownBy( + () -> validator.checkIfConditionIsSatisfied(delete, null, context)) .isInstanceOf(UnsatisfiedConditionException.class); } @@ -135,7 +143,8 @@ public void setUp() throws Exception { prepareMutationWithCondition(delete, deleteIfExists); // Act Assert - Assertions.assertThatCode(() -> validator.checkIfConditionIsSatisfied(delete, existingRecord)) + Assertions.assertThatCode( + () -> validator.checkIfConditionIsSatisfied(delete, existingRecord, context)) .doesNotThrowAnyException(); } @@ -310,9 +319,9 @@ private List prepareMutationOperations( private void validateConditionIsSatisfied(Mutation mutation, TransactionResult existingRecord) throws UnsatisfiedConditionException { if (mutation instanceof Put) { - validator.checkIfConditionIsSatisfied((Put) mutation, existingRecord); + validator.checkIfConditionIsSatisfied((Put) mutation, existingRecord, context); } else { - validator.checkIfConditionIsSatisfied((Delete) mutation, existingRecord); + validator.checkIfConditionIsSatisfied((Delete) mutation, existingRecord, context); } } } diff --git a/core/src/test/java/com/scalar/db/transaction/consensuscommit/SnapshotTest.java b/core/src/test/java/com/scalar/db/transaction/consensuscommit/SnapshotTest.java index b0fc3896e9..0970977ce9 100644 --- a/core/src/test/java/com/scalar/db/transaction/consensuscommit/SnapshotTest.java +++ b/core/src/test/java/com/scalar/db/transaction/consensuscommit/SnapshotTest.java @@ -39,7 +39,6 @@ import com.scalar.db.io.TextColumn; import com.scalar.db.io.TextValue; import com.scalar.db.io.Value; -import com.scalar.db.transaction.consensuscommit.Snapshot.ReadWriteSets; import com.scalar.db.util.ScalarDbUtils; import java.util.ArrayList; import java.util.Arrays; @@ -541,7 +540,7 @@ public void putIntoScanSet_ScanGiven_ShouldHoldWhatsGivenInScanSet() { snapshot = prepareSnapshot(Isolation.SNAPSHOT); Scan scan = prepareScan(); TransactionResult result = prepareResult(ANY_ID); - Snapshot.Key key = new Snapshot.Key(scan, result); + Snapshot.Key key = new Snapshot.Key(scan, result, TABLE_METADATA); LinkedHashMap expected = Maps.newLinkedHashMap(Collections.singletonMap(key, result)); @@ -1128,7 +1127,7 @@ public void toSerializable_ScanSetNotChanged_ShouldProcessWithoutExceptions() snapshot = prepareSnapshot(Isolation.SERIALIZABLE); Scan scan = prepareScan(); TransactionResult txResult = prepareResult(ANY_ID + "x"); - Snapshot.Key key = new Snapshot.Key(scan, txResult); + Snapshot.Key key = new Snapshot.Key(scan, txResult, TABLE_METADATA); snapshot.putIntoScanSet(scan, Maps.newLinkedHashMap(Collections.singletonMap(key, txResult))); DistributedStorage storage = mock(DistributedStorage.class); Scanner scanner = mock(Scanner.class); @@ -1151,7 +1150,7 @@ public void toSerializable_ScanSetUpdated_ShouldThrowValidationConflictException snapshot = prepareSnapshot(Isolation.SERIALIZABLE); Scan scan = prepareScan(); TransactionResult txResult = prepareResult(ANY_ID); - Snapshot.Key key = new Snapshot.Key(scan, txResult); + Snapshot.Key key = new Snapshot.Key(scan, txResult, TABLE_METADATA); snapshot.putIntoScanSet(scan, Maps.newLinkedHashMap(Collections.singletonMap(key, txResult))); DistributedStorage storage = mock(DistributedStorage.class); TransactionResult changedTxResult = prepareResult(ANY_ID + "x"); @@ -1176,7 +1175,7 @@ public void toSerializable_ScanSetUpdatedByMyself_ShouldProcessWithoutExceptions snapshot = prepareSnapshot(Isolation.SERIALIZABLE); Scan scan = prepareScan(); TransactionResult txResult = prepareResult(ANY_ID); - Snapshot.Key key = new Snapshot.Key(scan, txResult); + Snapshot.Key key = new Snapshot.Key(scan, txResult, TABLE_METADATA); snapshot.putIntoScanSet(scan, Maps.newLinkedHashMap(Collections.singletonMap(key, txResult))); DistributedStorage storage = mock(DistributedStorage.class); TransactionResult changedTxResult = prepareResult(ANY_ID); @@ -1226,7 +1225,7 @@ public void toSerializable_ScanSetExtended_ShouldThrowValidationConflictExceptio Scan scan = prepareScan(); TransactionResult result1 = prepareResult(ANY_ID + "xx", ANY_TEXT_1, ANY_TEXT_2); TransactionResult result2 = prepareResult(ANY_ID + "x", ANY_TEXT_1, ANY_TEXT_3); - Snapshot.Key key2 = new Snapshot.Key(scan, result2); + Snapshot.Key key2 = new Snapshot.Key(scan, result2, TABLE_METADATA); snapshot.putIntoScanSet(scan, Maps.newLinkedHashMap(ImmutableMap.of(key2, result2))); DistributedStorage storage = mock(DistributedStorage.class); Scanner scanner = mock(Scanner.class); @@ -1278,7 +1277,7 @@ public void toSerializable_ScanSetExtendedByMyself_ShouldProcessWithoutException Scan scan = prepareScan(); TransactionResult result1 = prepareResult(ANY_ID, ANY_TEXT_1, ANY_TEXT_2); TransactionResult result2 = prepareResult(ANY_ID + "x", ANY_TEXT_1, ANY_TEXT_3); - Snapshot.Key key2 = new Snapshot.Key(scan, result2); + Snapshot.Key key2 = new Snapshot.Key(scan, result2, TABLE_METADATA); snapshot.putIntoScanSet(scan, Maps.newLinkedHashMap(ImmutableMap.of(key2, result2))); DistributedStorage storage = mock(DistributedStorage.class); Scanner scanner = mock(Scanner.class); @@ -1304,7 +1303,7 @@ public void toSerializable_ScanSetDeleted_ShouldThrowValidationConflictException snapshot = prepareSnapshot(Isolation.SERIALIZABLE); Scan scan = prepareScan(); TransactionResult txResult = prepareResult(ANY_ID); - Snapshot.Key key = new Snapshot.Key(scan, txResult); + Snapshot.Key key = new Snapshot.Key(scan, txResult, TABLE_METADATA); snapshot.putIntoScanSet(scan, Maps.newLinkedHashMap(Collections.singletonMap(key, txResult))); DistributedStorage storage = mock(DistributedStorage.class); Scanner scanner = mock(Scanner.class); @@ -1330,8 +1329,8 @@ public void toSerializable_ScanSetDeleted_ShouldThrowValidationConflictException Scan scan = prepareScan(); TransactionResult result1 = prepareResult(ANY_ID + "xx", ANY_TEXT_1, ANY_TEXT_2); TransactionResult result2 = prepareResult(ANY_ID + "x", ANY_TEXT_1, ANY_TEXT_3); - Snapshot.Key key1 = new Snapshot.Key(scan, result1); - Snapshot.Key key2 = new Snapshot.Key(scan, result2); + Snapshot.Key key1 = new Snapshot.Key(scan, result1, TABLE_METADATA); + Snapshot.Key key2 = new Snapshot.Key(scan, result2, TABLE_METADATA); snapshot.putIntoScanSet( scan, Maps.newLinkedHashMap(ImmutableMap.of(key1, result1, key2, result2))); @@ -1393,8 +1392,8 @@ public void toSerializable_MultipleScansInScanSetExist_ShouldProcessWithoutExcep ScalarDbUtils.toColumn(Attribute.toIdValue("id2"))), TABLE_METADATA)); - Snapshot.Key key1 = new Snapshot.Key(scan1, result1); - Snapshot.Key key2 = new Snapshot.Key(scan2, result2); + Snapshot.Key key1 = new Snapshot.Key(scan1, result1, TABLE_METADATA); + Snapshot.Key key2 = new Snapshot.Key(scan2, result2, TABLE_METADATA); snapshot.putIntoScanSet( scan1, @@ -1484,7 +1483,7 @@ public void toSerializable_ScanWithLimitInScanSet_ShouldProcessWithoutExceptions Scan scan = prepareScanWithLimit(1); TransactionResult result1 = prepareResult(ANY_ID + "x"); TransactionResult result2 = prepareResult(ANY_ID + "x"); - Snapshot.Key key1 = new Snapshot.Key(scan, result1); + Snapshot.Key key1 = new Snapshot.Key(scan, result1, TABLE_METADATA); snapshot.putIntoScanSet(scan, Maps.newLinkedHashMap(Collections.singletonMap(key1, result1))); DistributedStorage storage = mock(DistributedStorage.class); Scan scanWithProjectionsWithoutLimit = @@ -1513,7 +1512,7 @@ public void toSerializable_ScanWithLimitInScanSet_ShouldProcessWithoutExceptions TransactionResult result1 = prepareResult(ANY_ID + "x", ANY_TEXT_1, ANY_TEXT_3); TransactionResult result2 = prepareResult(ANY_ID + "x", ANY_TEXT_1, ANY_TEXT_4); TransactionResult insertedResult = prepareResult(ANY_ID + "xx", ANY_TEXT_1, ANY_TEXT_2); - Snapshot.Key key1 = new Snapshot.Key(scan, result1); + Snapshot.Key key1 = new Snapshot.Key(scan, result1, TABLE_METADATA); snapshot.putIntoScanSet(scan, Maps.newLinkedHashMap(ImmutableMap.of(key1, result1))); DistributedStorage storage = mock(DistributedStorage.class); Scan scanWithProjectionsWithoutLimit = @@ -1544,7 +1543,7 @@ public void toSerializable_ScanWithLimitInScanSet_ShouldProcessWithoutExceptions TransactionResult result1 = prepareResult(ANY_ID + "x", ANY_TEXT_1, ANY_TEXT_3); TransactionResult result2 = prepareResult(ANY_ID + "x", ANY_TEXT_1, ANY_TEXT_4); TransactionResult insertedResult = prepareResult(ANY_ID, ANY_TEXT_1, ANY_TEXT_2); - Snapshot.Key key1 = new Snapshot.Key(scan, result1); + Snapshot.Key key1 = new Snapshot.Key(scan, result1, TABLE_METADATA); snapshot.putIntoScanSet(scan, Maps.newLinkedHashMap(ImmutableMap.of(key1, result1))); DistributedStorage storage = mock(DistributedStorage.class); Scan scanWithProjectionsWithoutLimit = @@ -1574,8 +1573,8 @@ public void toSerializable_ScanWithLimitInScanSet_ShouldProcessWithoutExceptions TransactionResult result1 = prepareResult(ANY_ID + "x", ANY_TEXT_1, ANY_TEXT_2); TransactionResult result2 = prepareResult(ANY_ID + "x", ANY_TEXT_1, ANY_TEXT_3); TransactionResult insertedResult = prepareResult(ANY_ID + "xx", ANY_TEXT_1, ANY_TEXT_4); - Snapshot.Key key1 = new Snapshot.Key(scan, result1); - Snapshot.Key key2 = new Snapshot.Key(scan, result2); + Snapshot.Key key1 = new Snapshot.Key(scan, result1, TABLE_METADATA); + Snapshot.Key key2 = new Snapshot.Key(scan, result2, TABLE_METADATA); snapshot.putIntoScanSet( scan, Maps.newLinkedHashMap(ImmutableMap.of(key1, result1, key2, result2))); DistributedStorage storage = mock(DistributedStorage.class); @@ -1607,8 +1606,8 @@ public void toSerializable_ScanWithLimitInScanSet_ShouldProcessWithoutExceptions TransactionResult result1 = prepareResult(ANY_ID + "x", ANY_TEXT_1, ANY_TEXT_2); TransactionResult result2 = prepareResult(ANY_ID + "x", ANY_TEXT_1, ANY_TEXT_3); TransactionResult insertedResult = prepareResult(ANY_ID, ANY_TEXT_1, ANY_TEXT_4); - Snapshot.Key key1 = new Snapshot.Key(scan, result1); - Snapshot.Key key2 = new Snapshot.Key(scan, result2); + Snapshot.Key key1 = new Snapshot.Key(scan, result1, TABLE_METADATA); + Snapshot.Key key2 = new Snapshot.Key(scan, result2, TABLE_METADATA); snapshot.putIntoScanSet( scan, Maps.newLinkedHashMap(ImmutableMap.of(key1, result1, key2, result2))); DistributedStorage storage = mock(DistributedStorage.class); @@ -1639,9 +1638,9 @@ public void toSerializable_ScanWithLimitInScanSet_ShouldProcessWithoutExceptions TransactionResult result1 = prepareResult(ANY_ID + "x", ANY_TEXT_1, ANY_TEXT_1); TransactionResult result2 = prepareResult(ANY_ID + "x", ANY_TEXT_2, ANY_TEXT_1); TransactionResult result3 = prepareResult(ANY_ID + "x", ANY_TEXT_3, ANY_TEXT_1); - Snapshot.Key key1 = new Snapshot.Key(scan, result1); - Snapshot.Key key2 = new Snapshot.Key(scan, result2); - Snapshot.Key key3 = new Snapshot.Key(scan, result3); + Snapshot.Key key1 = new Snapshot.Key(scan, result1, TABLE_METADATA); + Snapshot.Key key2 = new Snapshot.Key(scan, result2, TABLE_METADATA); + Snapshot.Key key3 = new Snapshot.Key(scan, result3, TABLE_METADATA); snapshot.putIntoScanSet( scan, Maps.newLinkedHashMap(ImmutableMap.of(key1, result1, key2, result2, key3, result3))); @@ -1672,9 +1671,9 @@ public void toSerializable_ScanWithLimitInScanSet_ShouldProcessWithoutExceptions TransactionResult result1 = prepareResult(ANY_ID + "x", ANY_TEXT_1, ANY_TEXT_1); TransactionResult result2 = prepareResult(ANY_ID + "x", ANY_TEXT_2, ANY_TEXT_1); TransactionResult result3 = prepareResult(ANY_ID + "x", ANY_TEXT_3, ANY_TEXT_1); - Snapshot.Key key1 = new Snapshot.Key(scan, result1); - Snapshot.Key key2 = new Snapshot.Key(scan, result2); - Snapshot.Key key3 = new Snapshot.Key(scan, result3); + Snapshot.Key key1 = new Snapshot.Key(scan, result1, TABLE_METADATA); + Snapshot.Key key2 = new Snapshot.Key(scan, result2, TABLE_METADATA); + Snapshot.Key key3 = new Snapshot.Key(scan, result3, TABLE_METADATA); snapshot.putIntoScanSet( scan, Maps.newLinkedHashMap(ImmutableMap.of(key1, result1, key2, result2, key3, result3))); @@ -1706,9 +1705,9 @@ public void toSerializable_ScanWithLimitInScanSet_ShouldProcessWithoutExceptions TransactionResult result1 = prepareResult(ANY_ID + "x", ANY_TEXT_1, ANY_TEXT_1); TransactionResult result2 = prepareResult(ANY_ID + "x", ANY_TEXT_2, ANY_TEXT_1); TransactionResult result3 = prepareResult(ANY_ID + "x", ANY_TEXT_3, ANY_TEXT_1); - Snapshot.Key key1 = new Snapshot.Key(scan, result1); - Snapshot.Key key2 = new Snapshot.Key(scan, result2); - Snapshot.Key key3 = new Snapshot.Key(scan, result3); + Snapshot.Key key1 = new Snapshot.Key(scan, result1, TABLE_METADATA); + Snapshot.Key key2 = new Snapshot.Key(scan, result2, TABLE_METADATA); + Snapshot.Key key3 = new Snapshot.Key(scan, result3, TABLE_METADATA); snapshot.putIntoScanSet( scan, Maps.newLinkedHashMap(ImmutableMap.of(key1, result1, key2, result2, key3, result3))); @@ -1738,7 +1737,7 @@ public void toSerializable_ScannerSetNotChanged_ShouldProcessWithoutExceptions() Scan scan = prepareScan(); TransactionResult result1 = prepareResult(ANY_ID + "x", ANY_TEXT_1, ANY_TEXT_2); TransactionResult result2 = prepareResult(ANY_ID + "x", ANY_TEXT_1, ANY_TEXT_3); - Snapshot.Key key1 = new Snapshot.Key(scan, result1); + Snapshot.Key key1 = new Snapshot.Key(scan, result1, TABLE_METADATA); snapshot.putIntoScannerSet(scan, Maps.newLinkedHashMap(ImmutableMap.of(key1, result1))); DistributedStorage storage = mock(DistributedStorage.class); Scan scanWithProjections = @@ -1767,7 +1766,7 @@ public void toSerializable_ScannerSetNotChanged_ShouldProcessWithoutExceptions() snapshot.putIntoDeleteSet(deleteKey, delete); Scan scan = prepareScan(); TransactionResult result = prepareResult(ANY_ID); - Snapshot.Key key = new Snapshot.Key(scan, result); + Snapshot.Key key = new Snapshot.Key(scan, result, TABLE_METADATA); // Act Assert Throwable thrown = @@ -1787,7 +1786,7 @@ public void toSerializable_ScannerSetNotChanged_ShouldProcessWithoutExceptions() snapshot.putIntoWriteSet(putKey, put); Scan scan = prepareScan(); TransactionResult result = prepareResult(ANY_ID); - Snapshot.Key key = new Snapshot.Key(scan, result); + Snapshot.Key key = new Snapshot.Key(scan, result, TABLE_METADATA); // Act Assert Throwable thrown = @@ -2020,7 +2019,7 @@ public void verifyNoOverlap_ScanWithIndexGivenAndPutInWriteSetInSameTable_Should .indexKey(Key.ofText(ANY_NAME_4, ANY_TEXT_4)) .build(); TransactionResult result = prepareResult(ANY_ID); - Snapshot.Key key = new Snapshot.Key(scan, result); + Snapshot.Key key = new Snapshot.Key(scan, result, TABLE_METADATA); // Act Throwable thrown = @@ -2051,7 +2050,7 @@ public void verifyNoOverlap_ScanWithIndexGivenAndPutInWriteSetInSameTable_Should .indexKey(Key.ofText(ANY_NAME_4, ANY_TEXT_4)) .build(); TransactionResult result = prepareResult(ANY_ID); - Snapshot.Key key = new Snapshot.Key(scan, result); + Snapshot.Key key = new Snapshot.Key(scan, result, TABLE_METADATA); // Act Assert Throwable thrown = @@ -2092,7 +2091,7 @@ public void verifyNoOverlap_ScanWithIndexAndPutWithSameIndexKeyGiven_ShouldThrow .indexKey(Key.ofText(ANY_NAME_4, ANY_TEXT_4)) .build(); TransactionResult result = prepareResult(ANY_ID); - Snapshot.Key key = new Snapshot.Key(scan, result); + Snapshot.Key key = new Snapshot.Key(scan, result, TABLE_METADATA); // Act Throwable thrown = @@ -2136,7 +2135,7 @@ public void verifyNoOverlap_ScanWithIndexAndPutWithSameIndexKeyGiven_ShouldThrow .where(ConditionBuilder.column(ANY_NAME_3).isEqualToText(ANY_TEXT_3)) .build(); TransactionResult result = prepareResult(ANY_ID); - Snapshot.Key key = new Snapshot.Key(scan, result); + Snapshot.Key key = new Snapshot.Key(scan, result, TABLE_METADATA); // Act Throwable thrown = @@ -2160,7 +2159,7 @@ public void verifyNoOverlap_ScanAllGivenAndPutInWriteSetInSameTable_ShouldThrowE .forNamespace(ANY_NAMESPACE_NAME) .forTable(ANY_TABLE_NAME); TransactionResult result = prepareResult(ANY_ID); - Snapshot.Key key = new Snapshot.Key(scanAll, result); + Snapshot.Key key = new Snapshot.Key(scanAll, result, TABLE_METADATA); // Act Assert Throwable thrown = @@ -2186,7 +2185,7 @@ public void verifyNoOverlap_ScanAllGivenAndPutInWriteSetInSameTable_ShouldThrowE .forNamespace(ANY_NAMESPACE_NAME_2) .forTable(ANY_TABLE_NAME_2); TransactionResult result = prepareResult(ANY_ID); - Snapshot.Key key = new Snapshot.Key(scanAll, result); + Snapshot.Key key = new Snapshot.Key(scanAll, result, TABLE_METADATA); // Act Assert Throwable thrown = @@ -2206,7 +2205,7 @@ public void verifyNoOverlap_CrossPartitionScanGivenAndPutInSameTable_ShouldThrow snapshot.putIntoWriteSet(putKey, put); Scan scan = prepareCrossPartitionScan(); TransactionResult result = prepareResult(ANY_ID); - Snapshot.Key key = new Snapshot.Key(scan, result); + Snapshot.Key key = new Snapshot.Key(scan, result, TABLE_METADATA); // Act Throwable thrown = @@ -2226,7 +2225,7 @@ public void verifyNoOverlap_CrossPartitionScanGivenAndPutInSameTable_ShouldThrow snapshot.putIntoWriteSet(putKey, put); Scan scan = prepareCrossPartitionScan(ANY_NAMESPACE_NAME_2, ANY_TABLE_NAME); TransactionResult result = prepareResult(ANY_ID); - Snapshot.Key key = new Snapshot.Key(scan, result); + Snapshot.Key key = new Snapshot.Key(scan, result, TABLE_METADATA); // Act Throwable thrown = @@ -2246,7 +2245,7 @@ public void verifyNoOverlap_CrossPartitionScanGivenAndPutInSameTable_ShouldThrow snapshot.putIntoWriteSet(putKey, put); Scan scan = prepareCrossPartitionScan(ANY_NAMESPACE_NAME, ANY_TABLE_NAME_2); TransactionResult result = prepareResult(ANY_ID); - Snapshot.Key key = new Snapshot.Key(scan, result); + Snapshot.Key key = new Snapshot.Key(scan, result, TABLE_METADATA); // Act Throwable thrown = @@ -2371,137 +2370,4 @@ public void verifyNoOverlap_CrossPartitionScanGivenAndPutInSameTable_ShouldThrow // Assert assertThat(thrown).isInstanceOf(IllegalArgumentException.class); } - - @Test - void getReadWriteSet_ReadSetAndWriteSetGiven_ShouldReturnProperValue() { - // Arrange - snapshot = prepareSnapshot(Isolation.SNAPSHOT); - - Get get1 = prepareGet(); - TransactionResult result1 = prepareResult("t1"); - Snapshot.Key readKey1 = new Snapshot.Key(get1); - snapshot.putIntoReadSet(readKey1, Optional.of(result1)); - Get get2 = prepareAnotherGet(); - TransactionResult result2 = prepareResult("t2"); - Snapshot.Key readKey2 = new Snapshot.Key(get2); - snapshot.putIntoReadSet(readKey2, Optional.of(result2)); - - Put put1 = preparePut(); - Snapshot.Key putKey1 = new Snapshot.Key(put1); - snapshot.putIntoWriteSet(putKey1, put1); - Put put2 = prepareAnotherPut(); - Snapshot.Key putKey2 = new Snapshot.Key(put2); - snapshot.putIntoWriteSet(putKey2, put2); - - // Act - ReadWriteSets readWriteSets = snapshot.getReadWriteSets(); - { - // The method returns an immutable value, so the following update shouldn't be included. - Get delayedGet = - new Get(new Key(ANY_NAME_1, ANY_TEXT_2), new Key(ANY_NAME_2, ANY_TEXT_1)) - .withConsistency(Consistency.LINEARIZABLE) - .forNamespace(ANY_NAMESPACE_NAME) - .forTable(ANY_TABLE_NAME); - TransactionResult delayedResult = prepareResult("t3"); - snapshot.putIntoReadSet(new Snapshot.Key(delayedGet), Optional.of(delayedResult)); - - Put delayedPut = - new Put(new Key(ANY_NAME_1, ANY_TEXT_2), new Key(ANY_NAME_2, ANY_TEXT_1)) - .withConsistency(Consistency.LINEARIZABLE) - .forNamespace(ANY_NAMESPACE_NAME) - .forTable(ANY_TABLE_NAME) - .withValue(ANY_NAME_3, ANY_TEXT_3); - snapshot.putIntoWriteSet(new Snapshot.Key(delayedPut), delayedPut); - } - - // Assert - assertThat(readWriteSets.readSetMap).size().isEqualTo(2); - for (Map.Entry> entry : - readWriteSets.readSetMap.entrySet()) { - if (entry.getKey().equals(readKey1)) { - assertThat(entry.getValue()).isPresent().get().isEqualTo(result1); - } else if (entry.getKey().equals(readKey2)) { - assertThat(entry.getValue()).isPresent().get().isEqualTo(result2); - } else { - throw new AssertionError("Unexpected key: " + entry.getKey()); - } - } - - assertThat(readWriteSets.writeSet).size().isEqualTo(2); - for (Map.Entry entry : readWriteSets.writeSet) { - if (entry.getKey().equals(putKey1)) { - assertThat(entry.getValue()).isEqualTo(put1); - } else if (entry.getKey().equals(putKey2)) { - assertThat(entry.getValue()).isEqualTo(put2); - } else { - throw new AssertionError("Unexpected key: " + entry.getKey()); - } - } - } - - @Test - void getReadWriteSet_ReadSetAndDeleteSetGiven_ShouldReturnProperValue() { - // Arrange - snapshot = prepareSnapshot(Isolation.SNAPSHOT); - - Get get1 = prepareGet(); - TransactionResult result1 = prepareResult("t1"); - Snapshot.Key readKey1 = new Snapshot.Key(get1); - snapshot.putIntoReadSet(readKey1, Optional.of(result1)); - Get get2 = prepareAnotherGet(); - TransactionResult result2 = prepareResult("t2"); - Snapshot.Key readKey2 = new Snapshot.Key(get2); - snapshot.putIntoReadSet(readKey2, Optional.of(result2)); - - Delete delete1 = prepareDelete(); - Snapshot.Key deleteKey1 = new Snapshot.Key(delete1); - snapshot.putIntoDeleteSet(deleteKey1, delete1); - Delete delete2 = prepareAnotherDelete(); - Snapshot.Key deleteKey2 = new Snapshot.Key(delete2); - snapshot.putIntoDeleteSet(deleteKey2, delete2); - - // Act - ReadWriteSets readWriteSets = snapshot.getReadWriteSets(); - { - // The method returns an immutable value, so the following update shouldn't be included. - Get delayedGet = - new Get(new Key(ANY_NAME_1, ANY_TEXT_2), new Key(ANY_NAME_2, ANY_TEXT_1)) - .withConsistency(Consistency.LINEARIZABLE) - .forNamespace(ANY_NAMESPACE_NAME) - .forTable(ANY_TABLE_NAME); - TransactionResult delayedResult = prepareResult("t3"); - snapshot.putIntoReadSet(new Snapshot.Key(delayedGet), Optional.of(delayedResult)); - - Delete delayedDelete = - new Delete(new Key(ANY_NAME_1, ANY_TEXT_2), new Key(ANY_NAME_2, ANY_TEXT_1)) - .withConsistency(Consistency.LINEARIZABLE) - .forNamespace(ANY_NAMESPACE_NAME) - .forTable(ANY_TABLE_NAME); - snapshot.putIntoDeleteSet(new Snapshot.Key(delayedDelete), delayedDelete); - } - - // Assert - assertThat(readWriteSets.readSetMap).size().isEqualTo(2); - for (Map.Entry> entry : - readWriteSets.readSetMap.entrySet()) { - if (entry.getKey().equals(readKey1)) { - assertThat(entry.getValue()).isPresent().get().isEqualTo(result1); - } else if (entry.getKey().equals(readKey2)) { - assertThat(entry.getValue()).isPresent().get().isEqualTo(result2); - } else { - throw new AssertionError("Unexpected key: " + entry.getKey()); - } - } - - assertThat(readWriteSets.deleteSet).size().isEqualTo(2); - for (Map.Entry entry : readWriteSets.deleteSet) { - if (entry.getKey().equals(deleteKey1)) { - assertThat(entry.getValue()).isEqualTo(delete1); - } else if (entry.getKey().equals(deleteKey2)) { - assertThat(entry.getValue()).isEqualTo(delete2); - } else { - throw new AssertionError("Unexpected key: " + entry.getKey()); - } - } - } } diff --git a/core/src/test/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitManagerTest.java b/core/src/test/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitManagerTest.java index f5528bfd76..19e878e747 100644 --- a/core/src/test/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitManagerTest.java +++ b/core/src/test/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitManagerTest.java @@ -63,6 +63,7 @@ public class TwoPhaseConsensusCommitManagerTest { @Mock private Coordinator coordinator; @Mock private ParallelExecutor parallelExecutor; @Mock private RecoveryExecutor recoveryExecutor; + @Mock private CrudHandler crud; @Mock private CommitHandler commit; private TwoPhaseConsensusCommitManager manager; @@ -83,6 +84,7 @@ public void setUp() throws Exception { coordinator, parallelExecutor, recoveryExecutor, + crud, commit); } @@ -94,9 +96,11 @@ public void begin_NoArgumentGiven_ReturnWithSomeTxIdAndSnapshotIsolation() { TwoPhaseConsensusCommit transaction = (TwoPhaseConsensusCommit) manager.begin(); // Assert - assertThat(transaction.getCrudHandler().getSnapshot().getId()).isNotNull(); - assertThat(transaction.getCrudHandler().getSnapshot().getIsolation()) - .isEqualTo(Isolation.SNAPSHOT); + assertThat(transaction.getTransactionContext().transactionId).isNotNull(); + assertThat(transaction.getTransactionContext().isolation).isEqualTo(Isolation.SNAPSHOT); + assertThat(transaction.getTransactionContext().readOnly).isFalse(); + assertThat(transaction.getCrudHandler()).isEqualTo(crud); + assertThat(transaction.getCommitHandler()).isEqualTo(commit); } @Test @@ -107,13 +111,15 @@ public void begin_TxIdGiven_ReturnWithSpecifiedTxIdAndSnapshotIsolation() { TwoPhaseConsensusCommit transaction = (TwoPhaseConsensusCommit) manager.begin(ANY_TX_ID); // Assert - assertThat(transaction.getCrudHandler().getSnapshot().getId()).isEqualTo(ANY_TX_ID); - assertThat(transaction.getCrudHandler().getSnapshot().getIsolation()) - .isEqualTo(Isolation.SNAPSHOT); + assertThat(transaction.getTransactionContext().transactionId).isEqualTo(ANY_TX_ID); + assertThat(transaction.getTransactionContext().isolation).isEqualTo(Isolation.SNAPSHOT); + assertThat(transaction.getTransactionContext().readOnly).isFalse(); + assertThat(transaction.getCrudHandler()).isEqualTo(crud); + assertThat(transaction.getCommitHandler()).isEqualTo(commit); } @Test - public void begin_CalledTwice_ReturnRespectiveConsensusCommitWithSharedObjects() { + public void begin_CalledTwice_ShouldReturnTransactionsWithSharedHandlers() { // Arrange // Act @@ -121,12 +127,11 @@ public void begin_CalledTwice_ReturnRespectiveConsensusCommitWithSharedObjects() TwoPhaseConsensusCommit transaction2 = (TwoPhaseConsensusCommit) manager.begin(); // Assert - assertThat(transaction1.getCrudHandler()).isNotEqualTo(transaction2.getCrudHandler()); - assertThat(transaction1.getCrudHandler().getSnapshot().getId()) - .isNotEqualTo(transaction2.getCrudHandler().getSnapshot().getId()); - assertThat(transaction1.getCommitHandler()) - .isEqualTo(transaction2.getCommitHandler()) - .isEqualTo(commit); + assertThat(transaction1.getCrudHandler()).isSameAs(transaction2.getCrudHandler()); + assertThat(transaction1.getCommitHandler()).isSameAs(transaction2.getCommitHandler()); + assertThat(transaction1.getTransactionContext()) + .isNotSameAs(transaction2.getTransactionContext()); + assertThat(transaction1.getId()).isNotEqualTo(transaction2.getId()); } @Test @@ -168,9 +173,11 @@ public void start_NoArgumentGiven_ReturnWithSomeTxIdAndSnapshotIsolation() TwoPhaseConsensusCommit transaction = (TwoPhaseConsensusCommit) manager.start(); // Assert - assertThat(transaction.getCrudHandler().getSnapshot().getId()).isNotNull(); - assertThat(transaction.getCrudHandler().getSnapshot().getIsolation()) - .isEqualTo(Isolation.SNAPSHOT); + assertThat(transaction.getTransactionContext().transactionId).isNotNull(); + assertThat(transaction.getTransactionContext().isolation).isEqualTo(Isolation.SNAPSHOT); + assertThat(transaction.getTransactionContext().readOnly).isFalse(); + assertThat(transaction.getCrudHandler()).isEqualTo(crud); + assertThat(transaction.getCommitHandler()).isEqualTo(commit); } @Test @@ -182,13 +189,15 @@ public void start_TxIdGiven_ReturnWithSpecifiedTxIdAndSnapshotIsolation() TwoPhaseConsensusCommit transaction = (TwoPhaseConsensusCommit) manager.start(ANY_TX_ID); // Assert - assertThat(transaction.getCrudHandler().getSnapshot().getId()).isEqualTo(ANY_TX_ID); - assertThat(transaction.getCrudHandler().getSnapshot().getIsolation()) - .isEqualTo(Isolation.SNAPSHOT); + assertThat(transaction.getTransactionContext().transactionId).isEqualTo(ANY_TX_ID); + assertThat(transaction.getTransactionContext().isolation).isEqualTo(Isolation.SNAPSHOT); + assertThat(transaction.getTransactionContext().readOnly).isFalse(); + assertThat(transaction.getCrudHandler()).isEqualTo(crud); + assertThat(transaction.getCommitHandler()).isEqualTo(commit); } @Test - public void start_CalledTwice_ReturnRespectiveConsensusCommitWithSharedObjects() + public void start_CalledTwice_ShouldReturnTransactionsWithSharedHandlers() throws TransactionException { // Arrange @@ -197,12 +206,11 @@ public void start_CalledTwice_ReturnRespectiveConsensusCommitWithSharedObjects() TwoPhaseConsensusCommit transaction2 = (TwoPhaseConsensusCommit) manager.start(); // Assert - assertThat(transaction1.getCrudHandler()).isNotEqualTo(transaction2.getCrudHandler()); - assertThat(transaction1.getCrudHandler().getSnapshot().getId()) - .isNotEqualTo(transaction2.getCrudHandler().getSnapshot().getId()); - assertThat(transaction1.getCommitHandler()) - .isEqualTo(transaction2.getCommitHandler()) - .isEqualTo(commit); + assertThat(transaction1.getCrudHandler()).isSameAs(transaction2.getCrudHandler()); + assertThat(transaction1.getCommitHandler()).isSameAs(transaction2.getCommitHandler()); + assertThat(transaction1.getTransactionContext()) + .isNotSameAs(transaction2.getTransactionContext()); + assertThat(transaction1.getId()).isNotEqualTo(transaction2.getId()); } @Test @@ -248,9 +256,11 @@ public void join_TxIdGiven_ReturnWithSpecifiedTxIdAndSnapshotIsolation() ((DecoratedTwoPhaseCommitTransaction) manager.join(ANY_TX_ID)).getOriginalTransaction(); // Assert - assertThat(transaction.getCrudHandler().getSnapshot().getId()).isEqualTo(ANY_TX_ID); - assertThat(transaction.getCrudHandler().getSnapshot().getIsolation()) - .isEqualTo(Isolation.SNAPSHOT); + assertThat(transaction.getTransactionContext().transactionId).isEqualTo(ANY_TX_ID); + assertThat(transaction.getTransactionContext().isolation).isEqualTo(Isolation.SNAPSHOT); + assertThat(transaction.getTransactionContext().readOnly).isFalse(); + assertThat(transaction.getCrudHandler()).isEqualTo(crud); + assertThat(transaction.getCommitHandler()).isEqualTo(commit); } @Test diff --git a/core/src/test/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitTest.java b/core/src/test/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitTest.java index 0703acfeac..e9a46e0b2a 100644 --- a/core/src/test/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitTest.java +++ b/core/src/test/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitTest.java @@ -6,6 +6,7 @@ import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -56,10 +57,12 @@ public class TwoPhaseConsensusCommitTest { private static final String ANY_TEXT_4 = "text4"; private static final String ANY_TX_ID = "any_id"; + private TransactionContext context; @Mock private Snapshot snapshot; @Mock private CrudHandler crud; @Mock private CommitHandler commit; @Mock private ConsensusCommitMutationOperationChecker mutationOperationChecker; + private TwoPhaseConsensusCommit transaction; @BeforeEach @@ -67,9 +70,8 @@ public void setUp() throws Exception { MockitoAnnotations.openMocks(this).close(); // Arrange - transaction = new TwoPhaseConsensusCommit(crud, commit, mutationOperationChecker); - - when(crud.areAllScannersClosed()).thenReturn(true); + context = spy(new TransactionContext(ANY_TX_ID, snapshot, Isolation.SNAPSHOT, false, false)); + transaction = new TwoPhaseConsensusCommit(context, crud, commit, mutationOperationChecker); } private Get prepareGet() { @@ -107,15 +109,14 @@ public void get_GetGiven_ShouldCallCrudHandlerGet() throws CrudException { // Arrange Get get = prepareGet(); TransactionResult result = mock(TransactionResult.class); - when(crud.get(get)).thenReturn(Optional.of(result)); - when(crud.getSnapshot()).thenReturn(snapshot); + when(crud.get(get, context)).thenReturn(Optional.of(result)); // Act Optional actual = transaction.get(get); // Assert assertThat(actual).isPresent(); - verify(crud).get(get); + verify(crud).get(get, context); } @Test @@ -124,15 +125,14 @@ public void scan_ScanGiven_ShouldCallCrudHandlerScan() throws CrudException { Scan scan = prepareScan(); TransactionResult result = mock(TransactionResult.class); List results = Collections.singletonList(result); - when(crud.scan(scan)).thenReturn(results); - when(crud.getSnapshot()).thenReturn(snapshot); + when(crud.scan(scan, context)).thenReturn(results); // Act List actual = transaction.scan(scan); // Assert assertThat(actual.size()).isEqualTo(1); - verify(crud).scan(scan); + verify(crud).scan(scan, context); } @Test @@ -143,7 +143,7 @@ public void getScannerAndScannerOne_ShouldCallCrudHandlerGetScannerAndScannerOne TransactionCrudOperable.Scanner scanner = mock(TransactionCrudOperable.Scanner.class); Result result = mock(Result.class); when(scanner.one()).thenReturn(Optional.of(result)); - when(crud.getScanner(scan)).thenReturn(scanner); + when(crud.getScanner(scan, context)).thenReturn(scanner); // Act TransactionCrudOperable.Scanner actualScanner = transaction.getScanner(scan); @@ -151,7 +151,7 @@ public void getScannerAndScannerOne_ShouldCallCrudHandlerGetScannerAndScannerOne // Assert assertThat(actualResult).hasValue(result); - verify(crud).getScanner(scan); + verify(crud).getScanner(scan, context); verify(scanner).one(); } @@ -164,7 +164,7 @@ public void getScannerAndScannerAll_ShouldCallCrudHandlerGetScannerAndScannerAll Result result1 = mock(Result.class); Result result2 = mock(Result.class); when(scanner.all()).thenReturn(Arrays.asList(result1, result2)); - when(crud.getScanner(scan)).thenReturn(scanner); + when(crud.getScanner(scan, context)).thenReturn(scanner); // Act TransactionCrudOperable.Scanner actualScanner = transaction.getScanner(scan); @@ -172,7 +172,7 @@ public void getScannerAndScannerAll_ShouldCallCrudHandlerGetScannerAndScannerAll // Assert assertThat(actualResults).containsExactly(result1, result2); - verify(crud).getScanner(scan); + verify(crud).getScanner(scan, context); verify(scanner).all(); } @@ -180,13 +180,12 @@ public void getScannerAndScannerAll_ShouldCallCrudHandlerGetScannerAndScannerAll public void put_PutGiven_ShouldCallCrudHandlerPut() throws ExecutionException, CrudException { // Arrange Put put = preparePut(); - when(crud.getSnapshot()).thenReturn(snapshot); // Act transaction.put(put); // Assert - verify(crud).put(put); + verify(crud).put(put, context); verify(mutationOperationChecker).check(put); } @@ -195,13 +194,12 @@ public void put_TwoPutsGiven_ShouldCallCrudHandlerPutTwice() throws ExecutionException, CrudException { // Arrange Put put = preparePut(); - when(crud.getSnapshot()).thenReturn(snapshot); // Act transaction.put(Arrays.asList(put, put)); // Assert - verify(crud, times(2)).put(put); + verify(crud, times(2)).put(put, context); verify(mutationOperationChecker, times(2)).check(put); } @@ -210,13 +208,12 @@ public void delete_DeleteGiven_ShouldCallCrudHandlerDelete() throws CrudException, ExecutionException { // Arrange Delete delete = prepareDelete(); - when(crud.getSnapshot()).thenReturn(snapshot); // Act transaction.delete(delete); // Assert - verify(crud).delete(delete); + verify(crud).delete(delete, context); verify(mutationOperationChecker).check(delete); } @@ -225,13 +222,12 @@ public void delete_TwoDeletesGiven_ShouldCallCrudHandlerDeleteTwice() throws CrudException, ExecutionException { // Arrange Delete delete = prepareDelete(); - when(crud.getSnapshot()).thenReturn(snapshot); // Act transaction.delete(Arrays.asList(delete, delete)); // Assert - verify(crud, times(2)).delete(delete); + verify(crud, times(2)).delete(delete, context); verify(mutationOperationChecker, times(2)).check(delete); } @@ -261,7 +257,7 @@ public void insert_InsertGiven_ShouldCallCrudHandlerPut() .textValue(ANY_NAME_3, ANY_TEXT_3) .enableInsertMode() .build(); - verify(crud).put(expectedPut); + verify(crud).put(expectedPut, context); verify(mutationOperationChecker).check(expectedPut); } @@ -291,7 +287,7 @@ public void upsert_UpsertGiven_ShouldCallCrudHandlerPut() .textValue(ANY_NAME_3, ANY_TEXT_3) .enableImplicitPreRead() .build(); - verify(crud).put(expectedPut); + verify(crud).put(expectedPut, context); verify(mutationOperationChecker).check(expectedPut); } @@ -322,7 +318,7 @@ public void update_UpdateWithoutConditionGiven_ShouldCallCrudHandlerPut() .condition(ConditionBuilder.putIfExists()) .enableImplicitPreRead() .build(); - verify(crud).put(expectedPut); + verify(crud).put(expectedPut, context); verify(mutationOperationChecker).check(expectedPut); } @@ -360,7 +356,7 @@ public void update_UpdateWithConditionGiven_ShouldCallCrudHandlerPut() .build()) .enableImplicitPreRead() .build(); - verify(crud).put(expectedPut); + verify(crud).put(expectedPut, context); verify(mutationOperationChecker).check(expectedPut); } @@ -388,10 +384,7 @@ public void update_UpdateWithConditionGiven_ShouldCallCrudHandlerPut() .enableImplicitPreRead() .build(); - when(crud.getSnapshot()).thenReturn(snapshot); - when(snapshot.getId()).thenReturn("id"); - - doThrow(UnsatisfiedConditionException.class).when(crud).put(put); + doThrow(UnsatisfiedConditionException.class).when(crud).put(put, context); // Act Assert assertThatCode(() -> transaction.update(update)).doesNotThrowAnyException(); @@ -428,13 +421,10 @@ public void update_UpdateWithConditionGiven_ShouldCallCrudHandlerPut() .enableImplicitPreRead() .build(); - when(crud.getSnapshot()).thenReturn(snapshot); - when(snapshot.getId()).thenReturn("id"); - UnsatisfiedConditionException unsatisfiedConditionException = mock(UnsatisfiedConditionException.class); when(unsatisfiedConditionException.getMessage()).thenReturn("PutIf"); - doThrow(unsatisfiedConditionException).when(crud).put(put); + doThrow(unsatisfiedConditionException).when(crud).put(put, context); // Act Assert assertThatThrownBy(() -> transaction.update(update)) @@ -468,13 +458,10 @@ public void update_UpdateWithConditionGiven_ShouldCallCrudHandlerPut() .enableImplicitPreRead() .build(); - when(crud.getSnapshot()).thenReturn(snapshot); - when(snapshot.getId()).thenReturn("id"); - UnsatisfiedConditionException unsatisfiedConditionException = mock(UnsatisfiedConditionException.class); when(unsatisfiedConditionException.getMessage()).thenReturn("PutIfExists"); - doThrow(unsatisfiedConditionException).when(crud).put(put); + doThrow(unsatisfiedConditionException).when(crud).put(put, context); // Act Assert assertThatThrownBy(() -> transaction.update(update)) @@ -489,14 +476,13 @@ public void mutate_PutAndDeleteGiven_ShouldCallCrudHandlerPutAndDelete() // Arrange Put put = preparePut(); Delete delete = prepareDelete(); - when(crud.getSnapshot()).thenReturn(snapshot); // Act transaction.mutate(Arrays.asList(put, delete)); // Assert - verify(crud).put(put); - verify(crud).delete(delete); + verify(crud).put(put, context); + verify(crud).delete(delete, context); verify(mutationOperationChecker).check(put); verify(mutationOperationChecker).check(delete); } @@ -505,16 +491,15 @@ public void mutate_PutAndDeleteGiven_ShouldCallCrudHandlerPutAndDelete() public void prepare_ProcessedCrudGiven_ShouldPrepareRecordsWithSnapshot() throws PreparationException, CrudException { // Arrange - when(crud.getSnapshot()).thenReturn(snapshot); // Act transaction.prepare(); // Assert - verify(crud).areAllScannersClosed(); - verify(crud).readIfImplicitPreReadEnabled(); - verify(crud).waitForRecoveryCompletionIfNecessary(); - verify(commit).prepareRecords(snapshot); + verify(context).areAllScannersClosed(); + verify(crud).readIfImplicitPreReadEnabled(context); + verify(crud).waitForRecoveryCompletionIfNecessary(context); + verify(commit).prepareRecords(context); } @Test @@ -522,8 +507,7 @@ public void prepare_ProcessedCrudGiven_ShouldPrepareRecordsWithSnapshot() prepare_ProcessedCrudGiven_CrudConflictExceptionThrownWhileImplicitPreRead_ShouldThrowPreparationConflictException() throws CrudException { // Arrange - when(crud.getSnapshot()).thenReturn(snapshot); - doThrow(CrudConflictException.class).when(crud).readIfImplicitPreReadEnabled(); + doThrow(CrudConflictException.class).when(crud).readIfImplicitPreReadEnabled(context); // Act Assert assertThatThrownBy(transaction::prepare).isInstanceOf(PreparationConflictException.class); @@ -534,8 +518,7 @@ public void prepare_ProcessedCrudGiven_ShouldPrepareRecordsWithSnapshot() prepare_ProcessedCrudGiven_CrudExceptionThrownWhileImplicitPreRead_ShouldThrowPreparationException() throws CrudException { // Arrange - when(crud.getSnapshot()).thenReturn(snapshot); - doThrow(CrudException.class).when(crud).readIfImplicitPreReadEnabled(); + doThrow(CrudException.class).when(crud).readIfImplicitPreReadEnabled(context); // Act Assert assertThatThrownBy(transaction::prepare).isInstanceOf(PreparationException.class); @@ -544,7 +527,7 @@ public void prepare_ProcessedCrudGiven_ShouldPrepareRecordsWithSnapshot() @Test public void prepare_ScannerNotClosed_ShouldThrowIllegalStateException() { // Arrange - when(crud.areAllScannersClosed()).thenReturn(false); + when(context.areAllScannersClosed()).thenReturn(false); // Act Assert assertThatThrownBy(() -> transaction.prepare()).isInstanceOf(IllegalStateException.class); @@ -555,8 +538,9 @@ public void prepare_ScannerNotClosed_ShouldThrowIllegalStateException() { prepare_CrudConflictExceptionThrownByCrudHandlerWaitForRecoveryCompletionIfNecessary_ShouldThrowPreparationConflictException() throws CrudException { // Arrange - when(crud.getSnapshot()).thenReturn(snapshot); - doThrow(CrudConflictException.class).when(crud).waitForRecoveryCompletionIfNecessary(); + CrudConflictException crudConflictException = mock(CrudConflictException.class); + when(crudConflictException.getMessage()).thenReturn("error"); + doThrow(crudConflictException).when(crud).waitForRecoveryCompletionIfNecessary(context); // Act Assert assertThatThrownBy(() -> transaction.prepare()) @@ -568,8 +552,9 @@ public void prepare_ScannerNotClosed_ShouldThrowIllegalStateException() { prepare_CrudExceptionThrownByCrudHandlerWaitForRecoveryCompletionIfNecessary_ShouldThrowPreparationException() throws CrudException { // Arrange - when(crud.getSnapshot()).thenReturn(snapshot); - doThrow(CrudException.class).when(crud).waitForRecoveryCompletionIfNecessary(); + CrudException crudException = mock(CrudException.class); + when(crudException.getMessage()).thenReturn("error"); + doThrow(crudException).when(crud).waitForRecoveryCompletionIfNecessary(context); // Act Assert assertThatThrownBy(() -> transaction.prepare()).isInstanceOf(PreparationException.class); @@ -580,13 +565,12 @@ public void validate_ProcessedCrudGiven_ShouldValidateRecordsWithSnapshot() throws ValidationException, PreparationException { // Arrange transaction.prepare(); - when(crud.getSnapshot()).thenReturn(snapshot); // Act transaction.validate(); // Assert - verify(commit).validateRecords(snapshot); + verify(commit).validateRecords(context); } @Test @@ -594,14 +578,13 @@ public void commit_ShouldCommitStateAndRecords() throws CommitException, UnknownTransactionStatusException, PreparationException { // Arrange transaction.prepare(); - when(crud.getSnapshot()).thenReturn(snapshot); // Act transaction.commit(); // Assert - verify(commit).commitState(snapshot); - verify(commit).commitRecords(snapshot); + verify(commit).commitState(context); + verify(commit).commitRecords(context); } @Test @@ -611,16 +594,14 @@ public void commit_SerializableUsedAndValidatedState_ShouldCommitProperly() // Arrange transaction.prepare(); transaction.validate(); - when(crud.getSnapshot()).thenReturn(snapshot); - when(snapshot.getId()).thenReturn(ANY_TX_ID); - when(snapshot.isValidationRequired()).thenReturn(true); + when(context.isValidationRequired()).thenReturn(true); // Act transaction.commit(); // Assert - verify(commit).commitState(snapshot); - verify(commit).commitRecords(snapshot); + verify(commit).commitState(context); + verify(commit).commitRecords(context); } @Test @@ -628,9 +609,7 @@ public void commit_SerializableUsedAndPreparedState_ShouldThrowIllegalStateExcep throws PreparationException { // Arrange transaction.prepare(); - when(crud.getSnapshot()).thenReturn(snapshot); - when(snapshot.getId()).thenReturn(ANY_TX_ID); - when(snapshot.isValidationRequired()).thenReturn(true); + when(context.isValidationRequired()).thenReturn(true); // Act Assert assertThatThrownBy(transaction::commit).isInstanceOf(IllegalStateException.class); @@ -640,32 +619,30 @@ public void commit_SerializableUsedAndPreparedState_ShouldThrowIllegalStateExcep public void rollback_ShouldAbortStateAndRollbackRecords() throws TransactionException { // Arrange transaction.prepare(); - when(crud.getSnapshot()).thenReturn(snapshot); // Act transaction.rollback(); // Assert - verify(crud).closeScanners(); - verify(commit).abortState(snapshot.getId()); - verify(commit).rollbackRecords(snapshot); + verify(context).closeScanners(); + verify(commit).abortState(ANY_TX_ID); + verify(commit).rollbackRecords(context); } @Test public void rollback_CalledAfterPrepareFails_ShouldAbortStateAndRollbackRecords() throws TransactionException { // Arrange - when(crud.getSnapshot()).thenReturn(snapshot); - doThrow(PreparationException.class).when(commit).prepareRecords(snapshot); + doThrow(PreparationException.class).when(commit).prepareRecords(context); // Act assertThatThrownBy(transaction::prepare).isInstanceOf(PreparationException.class); transaction.rollback(); // Assert - verify(crud).closeScanners(); - verify(commit).abortState(snapshot.getId()); - verify(commit).rollbackRecords(snapshot); + verify(context).closeScanners(); + verify(commit).abortState(ANY_TX_ID); + verify(commit).rollbackRecords(context); } @Test @@ -673,17 +650,16 @@ public void rollback_CalledAfterCommitFails_ShouldNeverAbortStateAndRollbackReco throws TransactionException { // Arrange transaction.prepare(); - when(crud.getSnapshot()).thenReturn(snapshot); - doThrow(CommitConflictException.class).when(commit).commitState(snapshot); + doThrow(CommitConflictException.class).when(commit).commitState(context); // Act assertThatThrownBy(transaction::commit).isInstanceOf(CommitException.class); transaction.rollback(); // Assert - verify(crud).closeScanners(); - verify(commit, never()).abortState(snapshot.getId()); - verify(commit, never()).rollbackRecords(snapshot); + verify(context).closeScanners(); + verify(commit, never()).abortState(ANY_TX_ID); + verify(commit, never()).rollbackRecords(context); } @Test @@ -692,14 +668,13 @@ public void rollback_CalledAfterCommitFails_ShouldNeverAbortStateAndRollbackReco throws TransactionException { // Arrange transaction.prepare(); - when(crud.getSnapshot()).thenReturn(snapshot); - when(commit.abortState(snapshot.getId())).thenThrow(UnknownTransactionStatusException.class); + when(commit.abortState(ANY_TX_ID)).thenThrow(UnknownTransactionStatusException.class); // Act Assert assertThatThrownBy(transaction::rollback).isInstanceOf(RollbackException.class); - verify(crud).closeScanners(); - verify(commit, never()).rollbackRecords(snapshot); + verify(context).closeScanners(); + verify(commit, never()).rollbackRecords(context); } @Test @@ -707,13 +682,12 @@ public void rollback_CommittedStateReturnedByAbortState_ShouldThrowRollbackExcep throws TransactionException { // Arrange transaction.prepare(); - when(crud.getSnapshot()).thenReturn(snapshot); - when(commit.abortState(snapshot.getId())).thenReturn(TransactionState.COMMITTED); + when(commit.abortState(ANY_TX_ID)).thenReturn(TransactionState.COMMITTED); // Act Assert assertThatThrownBy(transaction::rollback).isInstanceOf(RollbackException.class); - verify(crud).closeScanners(); - verify(commit, never()).rollbackRecords(snapshot); + verify(context).closeScanners(); + verify(commit, never()).rollbackRecords(context); } } diff --git a/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitNullMetadataIntegrationTestBase.java b/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitNullMetadataIntegrationTestBase.java index ba2f8b5638..cc4b815dda 100644 --- a/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitNullMetadataIntegrationTestBase.java +++ b/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitNullMetadataIntegrationTestBase.java @@ -141,6 +141,13 @@ public void setUp() throws Exception { recovery = spy(new RecoveryHandler(storage, coordinator, tableMetadataManager)); recoveryExecutor = new RecoveryExecutor(coordinator, recovery, tableMetadataManager); groupCommitter = CoordinatorGroupCommitter.from(consensusCommitConfig).orElse(null); + CrudHandler crud = + new CrudHandler( + storage, + recoveryExecutor, + tableMetadataManager, + consensusCommitConfig.isIncludeMetadataEnabled(), + parallelExecutor); CommitHandler commit = spy(createCommitHandler(tableMetadataManager, groupCommitter)); manager = new ConsensusCommitManager( @@ -150,9 +157,9 @@ public void setUp() throws Exception { coordinator, parallelExecutor, recoveryExecutor, + crud, commit, consensusCommitConfig.getIsolation(), - false, groupCommitter); } @@ -527,7 +534,7 @@ private void selection_SelectionGivenForPreparedWhenCoordinatorStateCommitted_Sh transaction.commit(); // Wait for the recovery to complete - ((ConsensusCommit) transaction).getCrudHandler().waitForRecoveryCompletion(); + ((ConsensusCommit) transaction).waitForRecoveryCompletion(); // Assert verify(recovery).recover(any(Selection.class), any(TransactionResult.class), any()); @@ -576,7 +583,7 @@ private void selection_SelectionGivenForPreparedWhenCoordinatorStateAborted_Shou transaction.commit(); // Wait for the recovery to complete - ((ConsensusCommit) transaction).getCrudHandler().waitForRecoveryCompletion(); + ((ConsensusCommit) transaction).waitForRecoveryCompletion(); // Assert verify(recovery).recover(any(Selection.class), any(TransactionResult.class), any()); @@ -672,7 +679,7 @@ public void scan_ScanGivenForPreparedWhenCoordinatorStateAborted_ShouldRollback( transaction.commit(); // Wait for the recovery to complete - ((ConsensusCommit) transaction).getCrudHandler().waitForRecoveryCompletion(); + ((ConsensusCommit) transaction).waitForRecoveryCompletion(); // Assert verify(recovery).recover(any(Selection.class), any(TransactionResult.class), any()); @@ -721,7 +728,7 @@ private void selection_SelectionGivenForDeletedWhenCoordinatorStateCommitted_Sho transaction.commit(); // Wait for the recovery to complete - ((ConsensusCommit) transaction).getCrudHandler().waitForRecoveryCompletion(); + ((ConsensusCommit) transaction).waitForRecoveryCompletion(); // Assert verify(recovery).recover(any(Selection.class), any(TransactionResult.class), any()); @@ -770,7 +777,7 @@ private void selection_SelectionGivenForDeletedWhenCoordinatorStateAborted_Shoul transaction.commit(); // Wait for the recovery to complete - ((ConsensusCommit) transaction).getCrudHandler().waitForRecoveryCompletion(); + ((ConsensusCommit) transaction).waitForRecoveryCompletion(); // Assert verify(recovery).recover(any(Selection.class), any(TransactionResult.class), any()); @@ -866,7 +873,7 @@ public void scan_ScanGivenForDeletedWhenCoordinatorStateAborted_ShouldRollback() transaction.commit(); // Wait for the recovery to complete - ((ConsensusCommit) transaction).getCrudHandler().waitForRecoveryCompletion(); + ((ConsensusCommit) transaction).waitForRecoveryCompletion(); // Assert verify(recovery).recover(any(Selection.class), any(TransactionResult.class), any()); diff --git a/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitSpecificIntegrationTestBase.java b/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitSpecificIntegrationTestBase.java index 42e1748dab..01565763bf 100644 --- a/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitSpecificIntegrationTestBase.java +++ b/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitSpecificIntegrationTestBase.java @@ -3280,7 +3280,7 @@ private void commit_ConflictingPutsGivenForNonExisting_ShouldCommitOneAndAbortTh assertThatThrownBy(transaction1::commit).isInstanceOf(CommitException.class); // Assert - verify(commit).rollbackRecords(any(Snapshot.class)); + verify(commit).rollbackRecords(any(TransactionContext.class)); DistributedTransaction another = manager.beginReadOnly(); assertThat(another.get(gets1.get(from)).isPresent()).isFalse(); @@ -3355,7 +3355,7 @@ private void commit_ConflictingPutAndDeleteGivenForExisting_ShouldCommitPutAndAb assertThatThrownBy(transaction::commit).isInstanceOf(CommitException.class); // Assert - verify(commit).rollbackRecords(any(Snapshot.class)); + verify(commit).rollbackRecords(any(TransactionContext.class)); DistributedTransaction another = manager.beginReadOnly(); Optional fromResult = another.get(gets1.get(from)); assertThat(fromResult).isPresent(); @@ -3426,7 +3426,7 @@ private void commit_ConflictingPutsGivenForExisting_ShouldCommitOneAndAbortTheOt assertThatThrownBy(transaction::commit).isInstanceOf(CommitException.class); // Assert - verify(commit).rollbackRecords(any(Snapshot.class)); + verify(commit).rollbackRecords(any(TransactionContext.class)); List gets1 = prepareGets(namespace1, table1); List gets2 = prepareGets(namespace2, table2); @@ -3618,7 +3618,7 @@ private void commit_ConflictingDeletesGivenForExisting_ShouldCommitOneAndAbortTh assertThatThrownBy(transaction::commit).isInstanceOf(CommitException.class); // Assert - verify(commit).rollbackRecords(any(Snapshot.class)); + verify(commit).rollbackRecords(any(TransactionContext.class)); List gets1 = prepareGets(namespace1, table1); List gets2 = differentTables ? prepareGets(namespace2, table2) : gets1; @@ -8654,6 +8654,13 @@ private ConsensusCommitManager createConsensusCommitManager( recovery = spy(new RecoveryHandler(storage, coordinator, tableMetadataManager)); recoveryExecutor = new RecoveryExecutor(coordinator, recovery, tableMetadataManager); groupCommitter = CoordinatorGroupCommitter.from(consensusCommitConfig).orElse(null); + CrudHandler crud = + new CrudHandler( + storage, + recoveryExecutor, + tableMetadataManager, + consensusCommitConfig.isIncludeMetadataEnabled(), + parallelExecutor); commit = spy(createCommitHandler(tableMetadataManager, groupCommitter, onePhaseCommitEnabled)); return new ConsensusCommitManager( storage, @@ -8662,9 +8669,9 @@ private ConsensusCommitManager createConsensusCommitManager( coordinator, parallelExecutor, recoveryExecutor, + crud, commit, isolation, - false, groupCommitter); } @@ -8709,7 +8716,7 @@ private void waitForRecoveryCompletion(DistributedTransaction transaction) throw } assert transaction instanceof ConsensusCommit; - ((ConsensusCommit) transaction).getCrudHandler().waitForRecoveryCompletion(); + ((ConsensusCommit) transaction).waitForRecoveryCompletion(); } private boolean isGroupCommitEnabled() { diff --git a/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitSpecificIntegrationTestBase.java b/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitSpecificIntegrationTestBase.java index bb416ef610..5a86a881d4 100644 --- a/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitSpecificIntegrationTestBase.java +++ b/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/TwoPhaseConsensusCommitSpecificIntegrationTestBase.java @@ -425,7 +425,7 @@ public void scan_ScanGivenForPreparedWhenCoordinatorStateAborted_ShouldRollback( assertThat(getBalance(result.get())).isEqualTo(0); // a rolled back value // Wait for the recovery to complete - ((TwoPhaseConsensusCommit) transaction).getCrudHandler().waitForRecoveryCompletion(); + ((TwoPhaseConsensusCommit) transaction).waitForRecoveryCompletion(); assertThat(coordinatorForStorage1.getState(ANY_ID_2).isPresent()).isTrue(); assertThat(coordinatorForStorage1.getState(ANY_ID_2).get().getState()) @@ -613,7 +613,7 @@ public void scan_ScanGivenForDeletedWhenCoordinatorStateAborted_ShouldRollback() assertThat(getBalance(result.get())).isEqualTo(0); // a rolled back value // Wait for the recovery to complete - ((TwoPhaseConsensusCommit) transaction).getCrudHandler().waitForRecoveryCompletion(); + ((TwoPhaseConsensusCommit) transaction).waitForRecoveryCompletion(); assertThat(coordinatorForStorage1.getState(ANY_ID_2).isPresent()).isTrue(); assertThat(coordinatorForStorage1.getState(ANY_ID_2).get().getState())