diff --git a/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/sweep/queue/TargetedSweepTest.java b/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/sweep/queue/TargetedSweepTest.java index 42bdfa5c1fd..4fddf2cbd31 100644 --- a/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/sweep/queue/TargetedSweepTest.java +++ b/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/sweep/queue/TargetedSweepTest.java @@ -39,9 +39,11 @@ import com.palantir.atlasdb.transaction.api.TransactionConflictException; import java.nio.charset.StandardCharsets; import java.util.List; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; import java.util.stream.Collectors; import java.util.stream.Stream; +import org.awaitility.Awaitility; import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; @@ -174,6 +176,7 @@ public void sweepDeletesValuesWrittenOnPreCommitConditionFailure() { long startTs = putWriteAndFailOnPreCommitConditionReturningStartTimestamp(SINGLE_WRITE); serializableTxManager.setUnreadableTimestamp(startTs + 1); + waitForImmutableTimestampToBeStrictlyGreaterThan(startTs); sweepNextBatch(ShardAndStrategy.conservative(0)); assertNoEntryForCellInKvs(TABLE_CONS, TEST_CELL); } @@ -313,4 +316,30 @@ public void cleanup() {} private void sweepNextBatch(ShardAndStrategy shardStrategy) { sweepQueue.sweepNextBatch(shardStrategy, Sweeper.of(shardStrategy).getSweepTimestamp(sweepTimestampSupplier)); } + + /** + * After this method returns successfully, we know that there has existed some point in time when the immutable + * timestamp from the {@link #serializableTxManager} was strictly greater than the timestamp parameter that was + * passed. Note that in the general case, this does not guarantee that the immutable timestamp is currently + * greater than the timestamp parameter in the presence of concurrent transaction starts. This is because it may + * go backwards under certain (non-deterministic) scheduling conditions. Writers of sweep tests must ensure that + * this is the case when writing their tests. + * + * In the absence of concurrent transaction starts, however, this does guarantee that the immutable timestamp + * will be strictly greater than the timestamp parameter going forward. Test writers must ensure this is the case + * if they wish to meaningfully use this method. + * + * Some tests assert that Sweep has deleted values written by a given transaction. This requires the immutable + * timestamp to have progressed past the start timestamp of said transaction, which in turn requires that that + * transaction has released its immutable timestamp lock. Releasing this lock happens asynchronously. + * + * We thus perform an explicit wait before any iterations of Sweep which change state in a way that assertions + * depend on. + */ + private void waitForImmutableTimestampToBeStrictlyGreaterThan(long timestampToBePassed) { + Awaitility.await() + .atMost(5, TimeUnit.SECONDS) + .pollInterval(10, TimeUnit.MILLISECONDS) + .until(() -> timestampToBePassed < serializableTxManager.getImmutableTimestamp()); + } } diff --git a/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/SnapshotTransactionTest.java b/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/SnapshotTransactionTest.java index 2849190ebb4..69edf57bcee 100644 --- a/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/SnapshotTransactionTest.java +++ b/atlasdb-tests-shared/src/test/java/com/palantir/atlasdb/transaction/impl/SnapshotTransactionTest.java @@ -147,6 +147,7 @@ import org.apache.commons.lang3.mutable.MutableLong; import org.apache.commons.lang3.tuple.Pair; import org.assertj.core.api.Assertions; +import org.awaitility.Awaitility; import org.jmock.Expectations; import org.jmock.Mockery; import org.jmock.lib.concurrent.DeterministicScheduler; @@ -377,7 +378,12 @@ public void testImmutableTs() throws Exception { return t.getTimestamp(); }); assertThat(firstTs).isLessThan(txManager.getImmutableTimestamp()); - assertThat(startTs).isLessThan(txManager.getImmutableTimestamp()); + + // Immutable timestamp may not be cleared immediately. + Awaitility.await("immutable timestamp should advance past our transaction's start timestamp") + .atMost(5, TimeUnit.SECONDS) + .pollInterval(10, TimeUnit.MILLISECONDS) + .until(() -> startTs < txManager.getImmutableTimestamp()); } // If lock happens concurrent with get, we aren't sure that we can rollback the transaction