From 6126fc22308afbaf97283b4a6600b7722f68aa21 Mon Sep 17 00:00:00 2001 From: nicu-da Date: Thu, 28 Oct 2021 02:24:42 -0700 Subject: [PATCH] Remove append only prefix from conformance tests [KVL-1152] (#11424) CHANGELOG_BEGIN [api-test-tool] - remove AppendOnly prefix from test suites as the append-only schema is the only one left CHANGELOG_END --- compatibility/bazel_tools/testing.bzl | 15 +- ledger/daml-on-sql/BUILD.bazel | 6 +- .../BUILD.bazel | 12 +- .../CommandDeduplicationBase.scala | 27 +- .../KVCommandDeduplicationBase.scala | 237 ------------------ .../AppendOnlyKVCommandDeduplicationIT.scala | 30 --- .../suites/CommandDeduplicationIT.scala | 3 +- ...a => CommandDeduplicationParallelIT.scala} | 7 +- ...la => CompletionDeduplicationInfoIT.scala} | 6 +- .../suites/KVCommandDeduplicationIT.scala | 162 +++++++++++- .../ledger/api/testtool/tests/Tests.scala | 13 +- ledger/ledger-on-memory/BUILD.bazel | 20 +- ledger/ledger-on-sql/BUILD.bazel | 40 +-- ledger/sandbox-classic/BUILD.bazel | 18 +- ledger/sandbox/BUILD.bazel | 16 +- 15 files changed, 242 insertions(+), 370 deletions(-) delete mode 100644 ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/infrastructure/deduplication/KVCommandDeduplicationBase.scala delete mode 100644 ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/AppendOnlyKVCommandDeduplicationIT.scala rename ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/{AppendOnlyCommandDeduplicationParallelIT.scala => CommandDeduplicationParallelIT.scala} (96%) rename ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/{AppendOnlyCompletionDeduplicationInfoIT.scala => CompletionDeduplicationInfoIT.scala} (97%) diff --git a/compatibility/bazel_tools/testing.bzl b/compatibility/bazel_tools/testing.bzl index 30a81271cb63..c2560ab3e3ca 100644 --- a/compatibility/bazel_tools/testing.bzl +++ b/compatibility/bazel_tools/testing.bzl @@ -477,7 +477,6 @@ excluded_test_tool_tests = [ "exclusions": [ "KVCommandDeduplicationIT:KVCommandDeduplicationDeduplicateSubmitterBasic", "KVCommandDeduplicationIT:KVCommandDeduplicationSimpleDeduplicationBasic", - "KVCommandDeduplicationIT:KVCommandDeduplicationCommitterDeduplication", ], }, ], @@ -497,7 +496,7 @@ excluded_test_tool_tests = [ }, { "start": "1.17.0-snapshot.20210915.7841.0.b4328b3d", # The first version this test appeared - "end": "1.18.0-snapshot.20210928.7948.0.b4d00317", + "end": "1.18.0-snapshot.20211026.8179.0.e474b2d1", # The version when this test was removed "platform_ranges": [ { "start": "1.18.0-snapshot.20210928.7948.1", @@ -531,6 +530,18 @@ excluded_test_tool_tests = [ }, ], }, + { + "start": "1.18.0-snapshot.20211026.8179.0.e474b2d1", + "platform_ranges": [ + { + "end": "1.18.0-snapshot.20211026.8179.0.e474b2d1", + "exclusions": [ + "KVCommandDeduplicationIT:KVCommandDeduplicationSimpleDeduplicationMixedClients", + "CommandDeduplicationIT", # Latest version of the test is dependent on having the submission id populated + ], + }, + ], + }, ] def in_range(version, range): diff --git a/ledger/daml-on-sql/BUILD.bazel b/ledger/daml-on-sql/BUILD.bazel index 3d0579f14396..4f3e29b73eee 100644 --- a/ledger/daml-on-sql/BUILD.bazel +++ b/ledger/daml-on-sql/BUILD.bazel @@ -113,9 +113,9 @@ conformance_test( test_tool_args = [ "--verbose", "--additional=MultiPartySubmissionIT", - "--additional=AppendOnlyCommandDeduplicationParallelIT", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandService", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandSubmissionService", + "--additional=CommandDeduplicationParallelIT", + "--additional=CompletionDeduplicationInfoITCommandService", + "--additional=CompletionDeduplicationInfoITCommandSubmissionService", "--additional=ContractIdIT:Accept", ], ) diff --git a/ledger/ledger-api-test-tool-on-canton/BUILD.bazel b/ledger/ledger-api-test-tool-on-canton/BUILD.bazel index b2eb5cee0f6e..ec3bd237cfd2 100644 --- a/ledger/ledger-api-test-tool-on-canton/BUILD.bazel +++ b/ledger/ledger-api-test-tool-on-canton/BUILD.bazel @@ -88,11 +88,11 @@ conformance_test( ",ExceptionsIT,ExceptionRaceConditionIT" + # need UCK mode - added below ",DeeplyNestedValueIT" + # FIXME: Too deeply nested values flake with a time out (half of the time) ",CommandServiceIT:CSReturnStackTrace" + # FIXME: Ensure canton returns stack trace - ",CommandDeduplicationIT:ParticipantCommandDeduplicationSimpleDeduplicationBasic,CommandDeduplicationIT:ParticipantCommandDeduplicationDeduplicateSubmitterBasic" + # sync vs async error (part of canton #6301) + ",CommandDeduplicationIT:ParticipantCommandDeduplicationSimpleDeduplicationBasic,CommandDeduplicationIT:ParticipantCommandDeduplicationDeduplicateSubmitterBasic,CommandDeduplicationIT:ParticipantCommandDeduplicationSimpleDeduplicationMixedClients" + # sync vs async error (part of canton #6301) # Also exclude "optional tests" - which are run separately below - ",AppendOnlyCompletionDeduplicationInfoITCommandService" + - ",AppendOnlyCompletionDeduplicationInfoITCommandSubmissionService" + - ",AppendOnlyCommandDeduplicationParallelIT" + + ",CompletionDeduplicationInfoITCommandService" + + ",CompletionDeduplicationInfoITCommandSubmissionService" + + ",CommandDeduplicationParallelIT" + ",ContractIdIT" + ",MultiPartySubmissionIT" + ",ParticipantPruningIT" + @@ -161,8 +161,8 @@ conformance_test( "--verbose", "--include=ContractIdIT:RejectV0,ContractIdIT:RejectNonSuffixedV1Cid,ContractIdIT:AcceptSuffixedV1Cid" + ",MultiPartySubmissionIT" + - # ",AppendOnlyCommandDeduplicationParallelIT" + # FIXME: Deduplication test not yet passing on canton - canton-#6301 - ",AppendOnlyCompletionDeduplicationInfoITCommandService,AppendOnlyCompletionDeduplicationInfoITCommandSubmissionService", + # ",CommandDeduplicationParallelIT" + # FIXME: Deduplication test not yet passing on canton - canton-#6301 + ",CompletionDeduplicationInfoITCommandService,CompletionDeduplicationInfoITCommandSubmissionService", "--exclude=MultiPartySubmissionIT:MPSLookupOtherByKeyInvisible" + # Enable once error parsing more permissive ",MultiPartySubmissionIT:MPSCreateInsufficientAuthorization" + # canton returns INTERNAL instead of INVALID_ARGUMENT ",ContractIdIT:AcceptSuffixedV1CidExerciseTarget", # Racy with: ABORTED: CONTRACT_NOT_FOUND(14,0): Contract could not be found with id diff --git a/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/infrastructure/deduplication/CommandDeduplicationBase.scala b/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/infrastructure/deduplication/CommandDeduplicationBase.scala index 17b48485f8a8..ca6101adebd9 100644 --- a/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/infrastructure/deduplication/CommandDeduplicationBase.scala +++ b/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/infrastructure/deduplication/CommandDeduplicationBase.scala @@ -191,10 +191,9 @@ private[testtool] abstract class CommandDeduplicationBase( } ) - // appendOnlySchema - without the submission id we cannot assert the received completions for parallel submissions // staticTime - we run calls in parallel and with static time we would need to advance the time, // therefore this cannot be run in static time - if (deduplicationFeatures.appendOnlySchema && !staticTime) + if (!staticTime) testGivenAllParticipants( s"${testNamingPrefix}SimpleDeduplicationMixedClients", "Deduplicate commands within the deduplication time window using the command client and the command submission client", @@ -221,7 +220,10 @@ private[testtool] abstract class CommandDeduplicationBase( ) val submitRequest = ledger .submitRequest(party, Dummy(party).create.command) - .update(_.commands.commandId := submitAndWaitRequest.getCommands.commandId) + .update( + _.commands.commandId := submitAndWaitRequest.getCommands.commandId, + _.commands.deduplicationTime := deduplicationDuration.asProtobuf, + ) def submitAndAssertAccepted(submitAndWait: Boolean) = { if (submitAndWait) ledger.submitAndWait(submitAndWaitRequest) @@ -414,15 +416,11 @@ private[testtool] abstract class CommandDeduplicationBase( val submissionId = UUID.randomUUID().toString submitRequest(ledger)(request.update(_.commands.submissionId := submissionId)) .flatMap(ledgerEnd => { - if (deduplicationFeatures.appendOnlySchema) - // The [[Completion.submissionId]] is set only for append-only ledgers - ledger - .findCompletion(ledger.completionStreamRequest(ledgerEnd)(parties: _*))( - _.submissionId == submissionId - ) - .map[Seq[Completion]](_.toList) - else - ledger.firstCompletions(ledger.completionStreamRequest(ledgerEnd)(parties: _*)) + ledger + .findCompletion(ledger.completionStreamRequest(ledgerEnd)(parties: _*))( + _.submissionId == submissionId + ) + .map[Seq[Completion]](_.toList) }) .map { completions => assertSingleton("Expected only one completion", completions) @@ -455,11 +453,8 @@ object CommandDeduplicationBase { } /** @param participantDeduplication If participant deduplication is enabled then we will receive synchronous rejections - * @param appendOnlySchema For [[Completion]], the submission id and deduplication period are filled only for append only schemas - * Therefore, we need to assert on those fields only if it's an append only schema */ case class DeduplicationFeatures( - participantDeduplication: Boolean, - appendOnlySchema: Boolean, + participantDeduplication: Boolean ) } diff --git a/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/infrastructure/deduplication/KVCommandDeduplicationBase.scala b/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/infrastructure/deduplication/KVCommandDeduplicationBase.scala deleted file mode 100644 index 8d248c00907b..000000000000 --- a/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/infrastructure/deduplication/KVCommandDeduplicationBase.scala +++ /dev/null @@ -1,237 +0,0 @@ -// Copyright (c) 2021 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -package com.daml.ledger.api.testtool.infrastructure.deduplication - -import com.daml.ledger.api.testtool.infrastructure.Allocation.{ - Participant, - Participants, - SingleParty, - allocate, -} -import com.daml.ledger.api.testtool.infrastructure.ProtobufConverters._ -import com.daml.ledger.api.testtool.infrastructure.deduplication.CommandDeduplicationBase.DelayMechanism -import com.daml.ledger.api.testtool.infrastructure.participant.ParticipantTestContext -import com.daml.ledger.api.v1.admin.config_management_service.TimeModel -import com.daml.ledger.api.v1.commands.Commands.DeduplicationPeriod -import com.daml.ledger.api.v1.completion.Completion -import com.daml.ledger.test.model.Test.DummyWithAnnotation -import com.daml.timer.Delayed -import org.slf4j.LoggerFactory - -import scala.concurrent.duration.{Duration, DurationInt, FiniteDuration} -import scala.concurrent.{ExecutionContext, Future} -import scala.util.control.NonFatal - -/** Command deduplication tests for KV ledgers - * KV ledgers have committer side deduplication. - * The committer side deduplication period adds `minSkew` to the participant-side one, so we have to account for that as well. - * If updating the time model fails then the tests will assume a `minSkew` of 1 second. - */ -abstract class KVCommandDeduplicationBase( - timeoutScaleFactor: Double, - ledgerTimeInterval: FiniteDuration, - staticTime: Boolean, -) extends CommandDeduplicationBase(timeoutScaleFactor, ledgerTimeInterval, staticTime) { - private[this] val logger = LoggerFactory.getLogger(getClass.getName) - - testGivenAllParticipants( - s"${testNamingPrefix}CommitterDeduplication", - "Deduplicate commands in the committer using max deduplication duration as deduplication period", - allocate(SingleParty), - runConcurrently = false, - )(implicit ec => - configuredParticipants => { case Participants(Participant(ledger, party)) => - lazy val request = ledger - .submitRequest(party, DummyWithAnnotation(party, "submission").create.command) - .update( - _.commands.deduplicationPeriod := DeduplicationPeriod.DeduplicationDuration( - deduplicationDuration.asProtobuf - ) - ) - runWithConfig(configuredParticipants) { (maxDeduplicationDuration, minSkew) => - for { - completion1 <- submitRequestAndAssertCompletionAccepted(ledger)(request, party) - // Validate committer deduplication - duplicateCompletion <- submitRequestAndAssertAsyncDeduplication(ledger)(request, party) - // Wait for the end of committer deduplication - _ <- delay(ledger, maxDeduplicationDuration, minSkew) - // Deduplication has finished - completion2 <- submitRequestAndAssertCompletionAccepted(ledger)(request, party) - // Inspect created contracts - _ <- assertPartyHasActiveContracts(ledger)( - party = party, - noOfActiveContracts = 2, - ) - } yield { - assert( - completion1.commandId == request.commands.get.commandId, - "The command ID of the first completion does not match the command ID of the submission", - ) - assert( - completion2.commandId == request.commands.get.commandId, - "The command ID of the second completion does not match the command ID of the submission", - ) - assert( - duplicateCompletion.commandId == request.commands.get.commandId, - "The command ID of the duplicate completion does not match the command ID of the submission", - ) - // The [[Completion.deduplicationPeriod]] is set only for append-only ledgers - if (deduplicationFeatures.appendOnlySchema) { - val expectedCompletionDeduplicationPeriod = - Completion.DeduplicationPeriod.DeduplicationDuration( - maxDeduplicationDuration.asProtobuf - ) - assert( - completion1.deduplicationPeriod == expectedCompletionDeduplicationPeriod, - s"First completion deduplication period [${completion1.deduplicationPeriod}] is not the max deduplication", - ) - assert( - duplicateCompletion.deduplicationPeriod == expectedCompletionDeduplicationPeriod, - s"Duplicate completion deduplication period [${completion1.deduplicationPeriod}] is not the max deduplication", - ) - assert( - completion2.deduplicationPeriod == expectedCompletionDeduplicationPeriod, - s"Second completion deduplication period [${completion1.deduplicationPeriod}] is not the max deduplication", - ) - } - } - } - } - ) - - protected override def runWithDeduplicationDelay( - participants: Seq[ParticipantTestContext] - )( - testWithDelayMechanism: DelayMechanism => Future[Unit] - )(implicit ec: ExecutionContext): Future[Unit] = { - runWithConfig(participants) { case (maxDeduplicationDuration, minSkew) => - val anyParticipant = participants.head - testWithDelayMechanism(() => delay(anyParticipant, maxDeduplicationDuration, minSkew)) - } - } - - private def delay( - ledger: ParticipantTestContext, - maxDeduplicationDuration: Duration, - minSkew: Duration, - )(implicit ec: ExecutionContext) = { - val committerDeduplicationWindow = - maxDeduplicationDuration.plus(minSkew).plus(ledgerWaitInterval) - if (staticTime) { - forwardTimeWithDuration(ledger, committerDeduplicationWindow) - } else { - Delayed.by(committerDeduplicationWindow)(()) - } - } - - private def forwardTimeWithDuration( - ledger: ParticipantTestContext, - duration: Duration, - )(implicit ec: ExecutionContext): Future[Unit] = - ledger - .time() - .flatMap(currentTime => { - ledger.setTime(currentTime, currentTime.plusMillis(duration.toMillis)) - }) - - private def runWithConfig( - participants: Seq[ParticipantTestContext] - )( - test: (FiniteDuration, FiniteDuration) => Future[Unit] - )(implicit ec: ExecutionContext): Future[Unit] = { - // deduplication duration is increased by minSkew in the committer so we set the skew to a low value for testing - val minSkew = scaledDuration(1.second).asProtobuf - val anyParticipant = participants.head - anyParticipant - .configuration() - .flatMap(ledgerConfiguration => { - val maxDeduplicationTime = ledgerConfiguration.maxDeduplicationTime - .getOrElse( - throw new IllegalStateException( - "Max deduplication time was not set and our deduplication period depends on it" - ) - ) - .asScala - // max deduplication should be set to 5 seconds through the --max-deduplication-duration flag - assert( - maxDeduplicationTime <= 5.seconds, - s"Max deduplication time [$maxDeduplicationTime] is too high for the test.", - ) - runWithUpdatedTimeModel( - participants, - _.update(_.minSkew := minSkew), - )(timeModel => - test( - asFiniteDuration(maxDeduplicationTime), - asFiniteDuration(timeModel.getMinSkew.asScala), - ) - ) - }) - } - - private def runWithUpdatedTimeModel( - participants: Seq[ParticipantTestContext], - timeModelUpdate: TimeModel => TimeModel, - )(test: TimeModel => Future[Unit])(implicit ec: ExecutionContext): Future[Unit] = { - val anyParticipant = participants.head - anyParticipant - .getTimeModel() - .flatMap(timeModel => { - def restoreTimeModel(participant: ParticipantTestContext) = { - val ledgerTimeModelRestoreResult = for { - time <- participant.time() - _ <- participant - .setTimeModel( - time.plusSeconds(30), - timeModel.configurationGeneration + 1, - timeModel.getTimeModel, - ) - } yield {} - ledgerTimeModelRestoreResult.recover { case NonFatal(exception) => - logger.warn("Failed to restore time model for ledger", exception) - () - } - } - - for { - time <- anyParticipant.time() - updatedModel = timeModelUpdate(timeModel.getTimeModel) - (timeModelForTest, participantThatDidTheUpdate) <- tryTimeModelUpdateOnAllParticipants( - participants, - _.setTimeModel( - time.plusSeconds(30), - timeModel.configurationGeneration, - updatedModel, - ) - .map(_ => updatedModel), - ) - _ <- test(timeModelForTest) - .transformWith(testResult => - restoreTimeModel(participantThatDidTheUpdate).transform(_ => testResult) - ) - } yield {} - }) - } - - /** Try to run the update sequentially on all the participants. - * The function returns the first success or the last failure of the update operation. - * Useful for updating the configuration when we don't know which participant can update the config, - * as only the first one that submitted the initial configuration has the permissions to do so. - */ - private def tryTimeModelUpdateOnAllParticipants( - participants: Seq[ParticipantTestContext], - timeModelUpdate: ParticipantTestContext => Future[TimeModel], - )(implicit ec: ExecutionContext): Future[(TimeModel, ParticipantTestContext)] = { - participants.foldLeft( - Future.failed[(TimeModel, ParticipantTestContext)]( - new IllegalStateException("No participant") - ) - ) { (result, participant) => - result.recoverWith { case NonFatal(_) => - timeModelUpdate(participant).map(_ -> participant) - } - } - } - -} diff --git a/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/AppendOnlyKVCommandDeduplicationIT.scala b/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/AppendOnlyKVCommandDeduplicationIT.scala deleted file mode 100644 index b409175408ba..000000000000 --- a/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/AppendOnlyKVCommandDeduplicationIT.scala +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright (c) 2021 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -package com.daml.ledger.api.testtool.suites - -import com.daml.ledger.api.testtool.infrastructure.deduplication.CommandDeduplicationBase.DeduplicationFeatures -import com.daml.ledger.api.testtool.infrastructure.deduplication.{ - CommandDeduplicationBase, - KVCommandDeduplicationBase, -} - -import scala.concurrent.duration.FiniteDuration - -/** For append-only schemas we can run extra assertions based on the [[com.daml.ledger.api.v1.completion.Completion.submissionId]] and the [[com.daml.ledger.api.v1.completion.Completion.deduplicationPeriod]] - * Therefore this test suite is more comprehensive compared to [[KVCommandDeduplicationIT]] - */ -class AppendOnlyKVCommandDeduplicationIT( - timeoutScaleFactor: Double, - ledgerTimeInterval: FiniteDuration, - staticTime: Boolean, -) extends KVCommandDeduplicationBase(timeoutScaleFactor, ledgerTimeInterval, staticTime) { - - override protected def testNamingPrefix: String = "AppendOnlyKVCommandDeduplication" - - override def deduplicationFeatures: CommandDeduplicationBase.DeduplicationFeatures = - DeduplicationFeatures( - participantDeduplication = false, - appendOnlySchema = true, - ) -} diff --git a/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/CommandDeduplicationIT.scala b/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/CommandDeduplicationIT.scala index 8426b2d3556e..632cb47ae8db 100644 --- a/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/CommandDeduplicationIT.scala +++ b/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/CommandDeduplicationIT.scala @@ -34,7 +34,6 @@ final class CommandDeduplicationIT( override def deduplicationFeatures: CommandDeduplicationBase.DeduplicationFeatures = DeduplicationFeatures( - participantDeduplication = true, - appendOnlySchema = false, + participantDeduplication = true ) } diff --git a/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/AppendOnlyCommandDeduplicationParallelIT.scala b/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/CommandDeduplicationParallelIT.scala similarity index 96% rename from ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/AppendOnlyCommandDeduplicationParallelIT.scala rename to ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/CommandDeduplicationParallelIT.scala index 27deb0b7da5e..85a60db2ff65 100644 --- a/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/AppendOnlyCommandDeduplicationParallelIT.scala +++ b/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/CommandDeduplicationParallelIT.scala @@ -30,13 +30,10 @@ import io.grpc.Status.Code import scala.collection.compat._ import scala.concurrent.duration._ import scala.concurrent.{ExecutionContext, Future} -import scala.util.{Failure, Random, Success} import scala.util.control.NonFatal +import scala.util.{Failure, Random, Success} -/** Should be enabled for ledgers that fill the submission ID in the completions, - * as we need to use the submission id to find completions for parallel submissions - */ -class AppendOnlyCommandDeduplicationParallelIT extends LedgerTestSuite { +class CommandDeduplicationParallelIT extends LedgerTestSuite { private val deduplicationDuration = 3.seconds private val numberOfParallelRequests = 10 diff --git a/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/AppendOnlyCompletionDeduplicationInfoIT.scala b/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/CompletionDeduplicationInfoIT.scala similarity index 97% rename from ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/AppendOnlyCompletionDeduplicationInfoIT.scala rename to ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/CompletionDeduplicationInfoIT.scala index 1fa926e5d468..a46b07f5e3dd 100644 --- a/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/AppendOnlyCompletionDeduplicationInfoIT.scala +++ b/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/CompletionDeduplicationInfoIT.scala @@ -7,7 +7,7 @@ import com.daml.ledger.api.SubmissionIdGenerator import com.daml.ledger.api.testtool.infrastructure.Allocation._ import com.daml.ledger.api.testtool.infrastructure.LedgerTestSuite import com.daml.ledger.api.testtool.infrastructure.participant.ParticipantTestContext -import com.daml.ledger.api.testtool.suites.AppendOnlyCompletionDeduplicationInfoIT._ +import com.daml.ledger.api.testtool.suites.CompletionDeduplicationInfoIT._ import com.daml.ledger.api.v1.command_service.SubmitAndWaitRequest import com.daml.ledger.api.v1.command_submission_service.SubmitRequest import com.daml.ledger.api.v1.commands.Command @@ -24,7 +24,7 @@ import io.grpc.Status import scala.concurrent.duration.DurationInt import scala.concurrent.{ExecutionContext, Future} -final class AppendOnlyCompletionDeduplicationInfoIT[ServiceRequest]( +final class CompletionDeduplicationInfoIT[ServiceRequest]( service: Service[ServiceRequest] ) extends LedgerTestSuite { @@ -56,7 +56,7 @@ final class AppendOnlyCompletionDeduplicationInfoIT[ServiceRequest]( }) } -private[testtool] object AppendOnlyCompletionDeduplicationInfoIT { +private[testtool] object CompletionDeduplicationInfoIT { private[testtool] sealed trait Service[ProtoRequestType] extends Serializable with Product { def buildRequest( diff --git a/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/KVCommandDeduplicationIT.scala b/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/KVCommandDeduplicationIT.scala index 48cd1fa26376..da66c8cefb8d 100644 --- a/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/KVCommandDeduplicationIT.scala +++ b/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/KVCommandDeduplicationIT.scala @@ -3,30 +3,172 @@ package com.daml.ledger.api.testtool.suites -import com.daml.ledger.api.testtool.infrastructure.deduplication.CommandDeduplicationBase.DeduplicationFeatures -import com.daml.ledger.api.testtool.infrastructure.deduplication.{ - CommandDeduplicationBase, - KVCommandDeduplicationBase, +import com.daml.ledger.api.testtool.infrastructure.ProtobufConverters._ +import com.daml.ledger.api.testtool.infrastructure.deduplication.CommandDeduplicationBase +import com.daml.ledger.api.testtool.infrastructure.deduplication.CommandDeduplicationBase.{ + DeduplicationFeatures, + DelayMechanism, } +import com.daml.ledger.api.testtool.infrastructure.participant.ParticipantTestContext +import com.daml.ledger.api.v1.admin.config_management_service.TimeModel +import com.daml.timer.Delayed +import org.slf4j.LoggerFactory -import scala.concurrent.duration.FiniteDuration +import scala.concurrent.duration._ +import scala.concurrent.{ExecutionContext, Future} +import scala.util.control.NonFatal /** Command deduplication tests for KV ledgers - * KV ledgers have both participant side deduplication and committer side deduplication. - * The committer side deduplication period adds `minSkew` to the participant-side one, so we have to account for that as well. + * KV ledgers have committer side deduplication. + * The committer side deduplication period adds `minSkew` to the deduplication period, so we have to account for that as well. * If updating the time model fails then the tests will assume a `minSkew` of 1 second. */ class KVCommandDeduplicationIT( timeoutScaleFactor: Double, ledgerTimeInterval: FiniteDuration, staticTime: Boolean, -) extends KVCommandDeduplicationBase(timeoutScaleFactor, ledgerTimeInterval, staticTime) { +) extends CommandDeduplicationBase(timeoutScaleFactor, ledgerTimeInterval, staticTime) { + + private[this] val logger = LoggerFactory.getLogger(getClass.getName) override def testNamingPrefix: String = "KVCommandDeduplication" override def deduplicationFeatures: CommandDeduplicationBase.DeduplicationFeatures = DeduplicationFeatures( - participantDeduplication = false, - appendOnlySchema = false, + participantDeduplication = false ) + + protected override def runWithDeduplicationDelay( + participants: Seq[ParticipantTestContext] + )( + testWithDelayMechanism: DelayMechanism => Future[Unit] + )(implicit ec: ExecutionContext): Future[Unit] = { + runWithConfig(participants) { case (maxDeduplicationDuration, minSkew) => + val anyParticipant = participants.head + testWithDelayMechanism(() => delay(anyParticipant, maxDeduplicationDuration, minSkew)) + } + } + + private def delay( + ledger: ParticipantTestContext, + maxDeduplicationDuration: Duration, + minSkew: Duration, + )(implicit ec: ExecutionContext) = { + val committerDeduplicationWindow = + maxDeduplicationDuration.plus(minSkew).plus(ledgerWaitInterval) + if (staticTime) { + forwardTimeWithDuration(ledger, committerDeduplicationWindow) + } else { + Delayed.by(committerDeduplicationWindow)(()) + } + } + + private def forwardTimeWithDuration( + ledger: ParticipantTestContext, + duration: Duration, + )(implicit ec: ExecutionContext): Future[Unit] = + ledger + .time() + .flatMap(currentTime => { + ledger.setTime(currentTime, currentTime.plusMillis(duration.toMillis)) + }) + + private def runWithConfig( + participants: Seq[ParticipantTestContext] + )( + test: (FiniteDuration, FiniteDuration) => Future[Unit] + )(implicit ec: ExecutionContext): Future[Unit] = { + // deduplication duration is increased by minSkew in the committer so we set the skew to a low value for testing + val minSkew = scaledDuration(1.second).asProtobuf + val anyParticipant = participants.head + anyParticipant + .configuration() + .flatMap(ledgerConfiguration => { + val maxDeduplicationTime = ledgerConfiguration.maxDeduplicationTime + .getOrElse( + throw new IllegalStateException( + "Max deduplication time was not set and our deduplication period depends on it" + ) + ) + .asScala + // max deduplication should be set to 5 seconds through the --max-deduplication-duration flag + assert( + maxDeduplicationTime <= 5.seconds, + s"Max deduplication time [$maxDeduplicationTime] is too high for the test.", + ) + runWithUpdatedTimeModel( + participants, + _.update(_.minSkew := minSkew), + )(timeModel => + test( + asFiniteDuration(maxDeduplicationTime), + asFiniteDuration(timeModel.getMinSkew.asScala), + ) + ) + }) + } + + private def runWithUpdatedTimeModel( + participants: Seq[ParticipantTestContext], + timeModelUpdate: TimeModel => TimeModel, + )(test: TimeModel => Future[Unit])(implicit ec: ExecutionContext): Future[Unit] = { + val anyParticipant = participants.head + anyParticipant + .getTimeModel() + .flatMap(timeModel => { + def restoreTimeModel(participant: ParticipantTestContext) = { + val ledgerTimeModelRestoreResult = for { + time <- participant.time() + _ <- participant + .setTimeModel( + time.plusSeconds(30), + timeModel.configurationGeneration + 1, + timeModel.getTimeModel, + ) + } yield {} + ledgerTimeModelRestoreResult.recover { case NonFatal(exception) => + logger.warn("Failed to restore time model for ledger", exception) + () + } + } + + for { + time <- anyParticipant.time() + updatedModel = timeModelUpdate(timeModel.getTimeModel) + (timeModelForTest, participantThatDidTheUpdate) <- tryTimeModelUpdateOnAllParticipants( + participants, + _.setTimeModel( + time.plusSeconds(30), + timeModel.configurationGeneration, + updatedModel, + ) + .map(_ => updatedModel), + ) + _ <- test(timeModelForTest) + .transformWith(testResult => + restoreTimeModel(participantThatDidTheUpdate).transform(_ => testResult) + ) + } yield {} + }) + } + + /** Try to run the update sequentially on all the participants. + * The function returns the first success or the last failure of the update operation. + * Useful for updating the configuration when we don't know which participant can update the config, + * as only the first one that submitted the initial configuration has the permissions to do so. + */ + private def tryTimeModelUpdateOnAllParticipants( + participants: Seq[ParticipantTestContext], + timeModelUpdate: ParticipantTestContext => Future[TimeModel], + )(implicit ec: ExecutionContext): Future[(TimeModel, ParticipantTestContext)] = { + participants.foldLeft( + Future.failed[(TimeModel, ParticipantTestContext)]( + new IllegalStateException("No participant") + ) + ) { (result, participant) => + result.recoverWith { case NonFatal(_) => + timeModelUpdate(participant).map(_ -> participant) + } + } + } } diff --git a/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/tests/Tests.scala b/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/tests/Tests.scala index c5f780301177..076340a732c8 100644 --- a/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/tests/Tests.scala +++ b/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/tests/Tests.scala @@ -4,7 +4,7 @@ package com.daml.ledger.api.testtool.tests import com.daml.ledger.api.testtool.infrastructure.{BenchmarkReporter, Envelope, LedgerTestSuite} -import com.daml.ledger.api.testtool.suites.AppendOnlyCompletionDeduplicationInfoIT.{ +import com.daml.ledger.api.testtool.suites.CompletionDeduplicationInfoIT.{ CommandService, CommandSubmissionService, } @@ -68,14 +68,9 @@ object Tests { staticTime: Boolean = Defaults.StaticTime, ): Vector[LedgerTestSuite] = Vector( - new AppendOnlyCompletionDeduplicationInfoIT(CommandService), - new AppendOnlyCompletionDeduplicationInfoIT(CommandSubmissionService), - new AppendOnlyKVCommandDeduplicationIT( - timeoutScaleFactor, - ledgerClockGranularity, - staticTime, - ), - new AppendOnlyCommandDeduplicationParallelIT, + new CompletionDeduplicationInfoIT(CommandService), + new CompletionDeduplicationInfoIT(CommandSubmissionService), + new CommandDeduplicationParallelIT, new ContractIdIT, new KVCommandDeduplicationIT(timeoutScaleFactor, ledgerClockGranularity, staticTime), new MultiPartySubmissionIT, diff --git a/ledger/ledger-on-memory/BUILD.bazel b/ledger/ledger-on-memory/BUILD.bazel index c7683fe19d67..ea63c7993bf9 100644 --- a/ledger/ledger-on-memory/BUILD.bazel +++ b/ledger/ledger-on-memory/BUILD.bazel @@ -152,9 +152,9 @@ conformance_test( "--additional=MultiPartySubmissionIT", "--additional=ContractIdIT:RejectV0,ContractIdIT:AcceptSuffixedV1,ContractIdIT:AcceptNonSuffixedV1", "--additional=ParticipantPruningIT", - "--additional=AppendOnlyCommandDeduplicationParallelIT", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandService", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandSubmissionService", + "--additional=CommandDeduplicationParallelIT", + "--additional=CompletionDeduplicationInfoITCommandService", + "--additional=CompletionDeduplicationInfoITCommandSubmissionService", "--additional=KVCommandDeduplicationIT", "--exclude=CommandDeduplicationIT", # It's a KV ledger so it needs the KV variant # Disable tests targeting only multi-participant setups @@ -199,11 +199,11 @@ conformance_test( test_tool_args = [ "--verbose", "--additional=ParticipantPruningIT", - "--additional=AppendOnlyKVCommandDeduplicationIT", + "--additional=KVCommandDeduplicationIT", # The following two tests don't actually care about multi-participant but they do need append-only. - "--additional=AppendOnlyCommandDeduplicationParallelIT", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandService", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandSubmissionService", + "--additional=CommandDeduplicationParallelIT", + "--additional=CompletionDeduplicationInfoITCommandService", + "--additional=CompletionDeduplicationInfoITCommandSubmissionService", "--exclude=CommandDeduplicationIT", # It's a KV ledger so it needs the KV variant "--exclude=ConfigManagementServiceIT", ], @@ -226,9 +226,9 @@ conformance_test( test_tool_args = [ "--verbose", "--additional=ParticipantPruningIT", - "--additional=AppendOnlyCommandDeduplicationParallelIT", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandService", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandSubmissionService", + "--additional=CommandDeduplicationParallelIT", + "--additional=CompletionDeduplicationInfoITCommandService", + "--additional=CompletionDeduplicationInfoITCommandSubmissionService", "--additional=KVCommandDeduplicationIT", "--exclude=CommandDeduplicationIT", # It's a KV ledger so it needs the KV variant "--exclude=ConfigManagementServiceIT", diff --git a/ledger/ledger-on-sql/BUILD.bazel b/ledger/ledger-on-sql/BUILD.bazel index c565478aae3d..118e171cfd0a 100644 --- a/ledger/ledger-on-sql/BUILD.bazel +++ b/ledger/ledger-on-sql/BUILD.bazel @@ -319,10 +319,10 @@ conformance_test( tags = [], test_tool_args = [ "--verbose", - "--additional=AppendOnlyCommandDeduplicationParallelIT", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandService", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandSubmissionService", - "--additional=AppendOnlyKVCommandDeduplicationIT", + "--additional=CommandDeduplicationParallelIT", + "--additional=CompletionDeduplicationInfoITCommandService", + "--additional=CompletionDeduplicationInfoITCommandSubmissionService", + "--additional=KVCommandDeduplicationIT", "--additional=ParticipantPruningIT", "--additional=MultiPartySubmissionIT", "--exclude=CommandDeduplicationIT", # It's a KV ledger so it needs the KV variant @@ -346,10 +346,10 @@ conformance_test( tags = [], test_tool_args = [ "--verbose", - "--additional=AppendOnlyCommandDeduplicationParallelIT", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandService", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandSubmissionService", - "--additional=AppendOnlyKVCommandDeduplicationIT", + "--additional=CommandDeduplicationParallelIT", + "--additional=CompletionDeduplicationInfoITCommandService", + "--additional=CompletionDeduplicationInfoITCommandSubmissionService", + "--additional=KVCommandDeduplicationIT", "--additional=ParticipantPruningIT", "--additional=MultiPartySubmissionIT", "--exclude=CommandDeduplicationIT", # It's a KV ledger so it needs the KV variant @@ -372,10 +372,10 @@ conformance_test( tags = [] if oracle_testing else ["manual"], test_tool_args = [ "--verbose", - "--additional=AppendOnlyCommandDeduplicationParallelIT", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandService", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandSubmissionService", - "--additional=AppendOnlyKVCommandDeduplicationIT", + "--additional=CommandDeduplicationParallelIT", + "--additional=CompletionDeduplicationInfoITCommandService", + "--additional=CompletionDeduplicationInfoITCommandSubmissionService", + "--additional=KVCommandDeduplicationIT", "--additional=ParticipantPruningIT", "--additional=MultiPartySubmissionIT", "--exclude=CommandDeduplicationIT", # It's a KV ledger so it needs the KV variant @@ -399,10 +399,10 @@ conformance_test( tags = [], test_tool_args = [ "--verbose", - "--additional=AppendOnlyCommandDeduplicationParallelIT", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandService", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandSubmissionService", - "--additional=AppendOnlyKVCommandDeduplicationIT", + "--additional=CommandDeduplicationParallelIT", + "--additional=CompletionDeduplicationInfoITCommandService", + "--additional=CompletionDeduplicationInfoITCommandSubmissionService", + "--additional=KVCommandDeduplicationIT", "--additional=ParticipantPruningIT", "--additional=MultiPartySubmissionIT", "--exclude=CommandDeduplicationIT", # It's a KV ledger so it needs the KV variant @@ -426,10 +426,10 @@ conformance_test( tags = [] if oracle_testing else ["manual"], test_tool_args = [ "--verbose", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandService", - "--additional=AppendOnlyCommandDeduplicationParallelIT", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandSubmissionService", - "--additional=AppendOnlyKVCommandDeduplicationIT", + "--additional=CompletionDeduplicationInfoITCommandService", + "--additional=CommandDeduplicationParallelIT", + "--additional=CompletionDeduplicationInfoITCommandSubmissionService", + "--additional=KVCommandDeduplicationIT", "--additional=ParticipantPruningIT", "--additional=MultiPartySubmissionIT", "--exclude=CommandDeduplicationIT", # It's a KV ledger so it needs the KV variant diff --git a/ledger/sandbox-classic/BUILD.bazel b/ledger/sandbox-classic/BUILD.bazel index 7228954afa79..8b5ff801c4e7 100644 --- a/ledger/sandbox-classic/BUILD.bazel +++ b/ledger/sandbox-classic/BUILD.bazel @@ -350,9 +350,9 @@ server_conformance_test( "--concurrent-test-runs=1", # sandbox classic doesn't scale well with concurrent tests (almost no effect on overall run time) "--timeout-scale-factor=2", # sandbox classic is slow in general "--open-world", - "--additional=AppendOnlyCommandDeduplicationParallelIT", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandService", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandSubmissionService", + "--additional=CommandDeduplicationParallelIT", + "--additional=CompletionDeduplicationInfoITCommandService", + "--additional=CompletionDeduplicationInfoITCommandSubmissionService", "--additional=ContractIdIT:Accept", "--additional=ParticipantPruningIT", # Excluding tests that require using pruneAllDivulgedContracts option that is not supported by sandbox-classic @@ -373,9 +373,9 @@ server_conformance_test( "--concurrent-test-runs=1", # sandbox classic doesn't scale well with concurrent tests (almost no effect on overall run time) "--timeout-scale-factor=2", # sandbox classic is slow in general "--open-world", - "--additional=AppendOnlyCommandDeduplicationParallelIT", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandService", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandSubmissionService", + "--additional=CommandDeduplicationParallelIT", + "--additional=CompletionDeduplicationInfoITCommandService", + "--additional=CompletionDeduplicationInfoITCommandSubmissionService", "--exclude=ClosedWorldIT", ], ) @@ -392,9 +392,9 @@ server_conformance_test( "--concurrent-test-runs=1", # sandbox classic doesn't scale well with concurrent tests (almost no effect on overall run time) "--timeout-scale-factor=2", # sandbox classic is slow in general "--open-world", - "--additional=AppendOnlyCommandDeduplicationParallelIT", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandService", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandSubmissionService", + "--additional=CommandDeduplicationParallelIT", + "--additional=CompletionDeduplicationInfoITCommandService", + "--additional=CompletionDeduplicationInfoITCommandSubmissionService", "--exclude=ClosedWorldIT", ], ) diff --git a/ledger/sandbox/BUILD.bazel b/ledger/sandbox/BUILD.bazel index 9fffa518c5b9..2fa6e9479302 100644 --- a/ledger/sandbox/BUILD.bazel +++ b/ledger/sandbox/BUILD.bazel @@ -288,10 +288,10 @@ server_conformance_test( servers = NEXT_SERVERS, test_tool_args = [ "--open-world", - "--additional=AppendOnlyCommandDeduplicationParallelIT", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandService", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandSubmissionService", - "--additional=AppendOnlyKVCommandDeduplicationIT", + "--additional=CommandDeduplicationParallelIT", + "--additional=CompletionDeduplicationInfoITCommandService", + "--additional=CompletionDeduplicationInfoITCommandSubmissionService", + "--additional=KVCommandDeduplicationIT", "--exclude=CommandDeduplicationIT", # It's a KV ledger so it needs the KV variant "--additional=ContractIdIT:RejectV0,ContractIdIT:AcceptSuffixedV1,ContractIdIT:AcceptNonSuffixedV1", "--exclude=ClosedWorldIT", @@ -322,10 +322,10 @@ server_conformance_test( test_tool_args = [ "--static-time", "--open-world", - "--additional=AppendOnlyCommandDeduplicationParallelIT", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandService", - "--additional=AppendOnlyCompletionDeduplicationInfoITCommandSubmissionService", - "--additional=AppendOnlyKVCommandDeduplicationIT", + "--additional=CommandDeduplicationParallelIT", + "--additional=CompletionDeduplicationInfoITCommandService", + "--additional=CompletionDeduplicationInfoITCommandSubmissionService", + "--additional=KVCommandDeduplicationIT", "--exclude=CommandDeduplicationIT", # It's a KV ledger so it needs the KV variant "--exclude=ClosedWorldIT", ],