From 54339ada82342e04e7e4ebfd985c546b8c6ceb6c Mon Sep 17 00:00:00 2001 From: Marton Nagy Date: Wed, 26 Jan 2022 00:54:17 +0100 Subject: [PATCH] Safeguard Oracle CI tests with lockIdSeed [DPP-802] (#12573) * Fixes OracleAround so it creates unique oracle users * Fixes rouge connection pool in JdbcLedgerDaoTransactionsSpec * Fixes cleanup in OracleAroundAll * Introduces lockIdSeed for test frameworks * Adapts usage changelog_begin changelog_end --- .../src/it/scala/http/OracleIntTest.scala | 2 +- .../scala/http/HttpServiceOracleInt.scala | 6 +- .../main/scala/com/daml/http/perf/Main.scala | 24 +-- .../http/dbbackend/ContractDaoBenchmark.scala | 24 +-- .../on/sql/MainWithEphemeralOracleUser.scala | 22 ++- .../indexer/ha/IndexerStabilitySpec.scala | 3 + .../ha/IndexerStabilityTestFixture.scala | 7 + .../backend/StorageBackendProvider.scala | 5 +- .../backend/StorageBackendTestsDBLock.scala | 97 ++++----- .../dao/JdbcLedgerDaoBackendOracle.scala | 3 - .../dao/JdbcLedgerDaoTransactionsSpec.scala | 28 +-- .../ha/IndexerStabilityOracleSpec.scala | 6 +- .../ha/IndexerStabilityPostgresSpec.scala | 3 + .../ha/TestDBLockStorageBackendSpec.scala | 2 + libs-scala/oracle-testing/BUILD.bazel | 1 + .../scala/testing/oracle/OracleAround.scala | 185 +++++++++++------- .../testing/oracle/OracleAroundAll.scala | 1 + .../testing/oracle/OracleAroundSuite.scala | 21 +- .../postgresql/PostgresAroundSuite.scala | 3 + .../trigger/TriggerServiceFixture.scala | 6 +- 20 files changed, 253 insertions(+), 196 deletions(-) diff --git a/ledger-service/http-json-oracle/src/it/scala/http/OracleIntTest.scala b/ledger-service/http-json-oracle/src/it/scala/http/OracleIntTest.scala index 36d34e988d..db4cd85b92 100644 --- a/ledger-service/http-json-oracle/src/it/scala/http/OracleIntTest.scala +++ b/ledger-service/http-json-oracle/src/it/scala/http/OracleIntTest.scala @@ -15,5 +15,5 @@ class OracleIntTest with Matchers with Inside { override protected def jdbcConfig: JdbcConfig = - defaultJdbcConfig(oracleJdbcUrl, oracleUser, oraclePwd) + defaultJdbcConfig(oracleJdbcUrlWithoutCredentials, oracleUserName, oracleUserPwd) } diff --git a/ledger-service/http-json-oracle/src/itlib/scala/http/HttpServiceOracleInt.scala b/ledger-service/http-json-oracle/src/itlib/scala/http/HttpServiceOracleInt.scala index 992164b418..dd3dbbffd0 100644 --- a/ledger-service/http-json-oracle/src/itlib/scala/http/HttpServiceOracleInt.scala +++ b/ledger-service/http-json-oracle/src/itlib/scala/http/HttpServiceOracleInt.scala @@ -22,9 +22,9 @@ trait HttpServiceOracleInt extends AbstractHttpServiceIntegrationTestFuns with O protected[this] def jdbcConfig_ = HttpServiceOracleInt.defaultJdbcConfig( - oracleJdbcUrl, - oracleUser, - oraclePwd, + oracleJdbcUrlWithoutCredentials, + oracleUserName, + oracleUserPwd, disableContractPayloadIndexing = disableContractPayloadIndexing, ) } diff --git a/ledger-service/http-json-perf/src/main/scala/com/daml/http/perf/Main.scala b/ledger-service/http-json-perf/src/main/scala/com/daml/http/perf/Main.scala index e512d6dda7..7eca7e8fa0 100644 --- a/ledger-service/http-json-perf/src/main/scala/com/daml/http/perf/Main.scala +++ b/ledger-service/http-json-perf/src/main/scala/com/daml/http/perf/Main.scala @@ -198,42 +198,42 @@ object Main extends StrictLogging { ) import com.daml.testing.oracle, oracle.OracleAround - val Oracle: T = QueryStoreBracket[OracleRunner, oracle.User]( + val Oracle: T = QueryStoreBracket[OracleRunner, OracleAround.RichOracleUser]( () => new OracleRunner, _.start(), _ jdbcConfig _, _.stop(_), ) - private[this] final class OracleRunner extends OracleAround { + private[this] final class OracleRunner { private val defaultUser = "ORACLE_USER" private val retainData = sys.env.get("RETAIN_DATA").exists(_ equalsIgnoreCase "true") private val useDefaultUser = sys.env.get("USE_DEFAULT_USER").exists(_ equalsIgnoreCase "true") - type St = oracle.User + type St = OracleAround.RichOracleUser - def start() = Try { - connectToOracle() - if (useDefaultUser) createNewUser(defaultUser) else createNewRandomUser(): St + def start(): Try[St] = Try { + if (useDefaultUser) OracleAround.createOrReuseUser(defaultUser) + else OracleAround.createNewUniqueRandomUser() } - def jdbcConfig(user: St) = { + def jdbcConfig(user: St): JdbcConfig = { import DbStartupMode._ val startupMode: DbStartupMode = if (retainData) CreateIfNeededAndStart else CreateAndStart JdbcConfig( dbutils.JdbcConfig( "oracle.jdbc.OracleDriver", - oracleJdbcUrl, - user.name, - user.pwd, + user.jdbcUrlWithoutCredentials, + user.oracleUser.name, + user.oracleUser.pwd, ConnectionPool.PoolSize.Production, ), startMode = startupMode, ) } - def stop(user: St) = { - if (retainData) Success((): Unit) else Try(dropUser(user.name)) + def stop(user: St): Try[Unit] = { + if (retainData) Success(()) else Try(user.drop()) } } diff --git a/ledger-service/http-json/src/bench/scala/com/daml/http/dbbackend/ContractDaoBenchmark.scala b/ledger-service/http-json/src/bench/scala/com/daml/http/dbbackend/ContractDaoBenchmark.scala index 9ed97ca54f..8d05bf09ee 100644 --- a/ledger-service/http-json/src/bench/scala/com/daml/http/dbbackend/ContractDaoBenchmark.scala +++ b/ledger-service/http-json/src/bench/scala/com/daml/http/dbbackend/ContractDaoBenchmark.scala @@ -15,8 +15,9 @@ import com.daml.http.domain.TemplateId import com.daml.http.util.Logging.instanceUUIDLogCtx import com.daml.metrics.Metrics import com.daml.testing.oracle +import com.daml.testing.oracle.OracleAround.RichOracleUser import com.daml.testing.postgresql.{PostgresAround, PostgresDatabase} -import oracle.{OracleAround, User} +import oracle.OracleAround import org.openjdk.jmh.annotations._ import scala.concurrent.ExecutionContext @@ -25,7 +26,7 @@ import spray.json._ import spray.json.DefaultJsonProtocol._ @State(Scope.Benchmark) -abstract class ContractDaoBenchmark extends OracleAround { +abstract class ContractDaoBenchmark { self: BenchmarkDbConnection => protected var dao: ContractDao = _ @@ -107,23 +108,20 @@ trait BenchmarkDbConnection { def cleanup(): Unit } -trait OracleBenchmarkDbConn extends BenchmarkDbConnection with OracleAround { +trait OracleBenchmarkDbConn extends BenchmarkDbConnection { - private var user: User = _ + private var user: RichOracleUser = _ private val disableContractPayloadIndexing = false - override def connectToDb() = { - connectToOracle() - } + override def connectToDb() = user = OracleAround.createNewUniqueRandomUser() override def createDbJdbcConfig: JdbcConfig = { - user = createNewRandomUser() val cfg = JdbcConfig( dbutils.JdbcConfig( driver = "oracle.jdbc.OracleDriver", - url = oracleJdbcUrl, - user = user.name, - password = user.pwd, + url = user.jdbcUrlWithoutCredentials, + user = user.oracleUser.name, + password = user.oracleUser.pwd, poolSize = ConnectionPool.PoolSize.Integration, ), startMode = DbStartupMode.CreateOnly, @@ -134,9 +132,7 @@ trait OracleBenchmarkDbConn extends BenchmarkDbConnection with OracleAround { cfg } - override def cleanup(): Unit = { - dropUser(user.name) - } + override def cleanup(): Unit = user.drop() } trait PostgresBenchmarkDbConn extends BenchmarkDbConnection with PostgresAround { diff --git a/ledger/ledger-on-sql/src/test/lib/scala/com/daml/ledger/on/sql/MainWithEphemeralOracleUser.scala b/ledger/ledger-on-sql/src/test/lib/scala/com/daml/ledger/on/sql/MainWithEphemeralOracleUser.scala index 571946f59d..1e097f23d1 100644 --- a/ledger/ledger-on-sql/src/test/lib/scala/com/daml/ledger/on/sql/MainWithEphemeralOracleUser.scala +++ b/ledger/ledger-on-sql/src/test/lib/scala/com/daml/ledger/on/sql/MainWithEphemeralOracleUser.scala @@ -8,21 +8,27 @@ import com.daml.ledger.resources.ResourceContext import com.daml.resources.ProgramResource import com.daml.testing.oracle.OracleAround -// TODO for Brian: please verify usage of OracleAround here -object MainWithEphemeralOracleUser extends OracleAround { +object MainWithEphemeralOracleUser { def main(args: Array[String]): Unit = { val originalConfig = Config .parse[Unit]("SQL Ledger", _ => (), (), args) .getOrElse(sys.exit(1)) - connectToOracle() - val user = createNewRandomUser() - sys.addShutdownHook(dropUser(user.name)) - val oracleJdbcUrl = - s"jdbc:oracle:thin:${user.name}/${user.pwd}@$oracleHost:$oraclePort/ORCLPDB1" + val user = OracleAround.createNewUniqueRandomUser() + sys.addShutdownHook(user.drop()) val config = originalConfig.copy( - participants = originalConfig.participants.map(_.copy(serverJdbcUrl = oracleJdbcUrl)), + participants = originalConfig.participants.map(participantConfig => + participantConfig.copy( + serverJdbcUrl = user.jdbcUrl, + indexerConfig = participantConfig.indexerConfig.copy( + haConfig = participantConfig.indexerConfig.haConfig.copy( + indexerLockId = user.lockIdSeed, + indexerWorkerLockId = user.lockIdSeed + 1, + ) + ), + ) + ), extra = ExtraConfig( // Oracle is only used as persistence for the participant; we use in-memory ledger persistence here. jdbcUrl = Some("jdbc:sqlite:file:test?mode=memory&cache=shared") diff --git a/ledger/participant-integration-api/src/test/lib/scala/platform/indexer/ha/IndexerStabilitySpec.scala b/ledger/participant-integration-api/src/test/lib/scala/platform/indexer/ha/IndexerStabilitySpec.scala index 2dc4c9b1f6..97952c05c8 100644 --- a/ledger/participant-integration-api/src/test/lib/scala/platform/indexer/ha/IndexerStabilitySpec.scala +++ b/ledger/participant-integration-api/src/test/lib/scala/platform/indexer/ha/IndexerStabilitySpec.scala @@ -27,6 +27,8 @@ trait IndexerStabilitySpec // To be overriden by the spec implementation def jdbcUrl: String + // This will be used to pick lock IDs for DB locking + def lockIdSeed: Int // The default EC is coming from AsyncTestSuite and is serial, do not use it implicit val ec: ExecutionContext = system.dispatcher @@ -47,6 +49,7 @@ trait IndexerStabilitySpec updatesPerSecond, indexerCount, jdbcUrl, + lockIdSeed, materializer, ) .use[Unit] { indexers => diff --git a/ledger/participant-integration-api/src/test/lib/scala/platform/indexer/ha/IndexerStabilityTestFixture.scala b/ledger/participant-integration-api/src/test/lib/scala/platform/indexer/ha/IndexerStabilityTestFixture.scala index 9a5322aa7f..9bd6c98234 100644 --- a/ledger/participant-integration-api/src/test/lib/scala/platform/indexer/ha/IndexerStabilityTestFixture.scala +++ b/ledger/participant-integration-api/src/test/lib/scala/platform/indexer/ha/IndexerStabilityTestFixture.scala @@ -39,6 +39,7 @@ object IndexerStabilityTestFixture { updatesPerSecond: Int, indexerCount: Int, jdbcUrl: String, + lockIdSeed: Int, materializer: Materializer, ): ResourceOwner[Indexers] = new ResourceOwner[Indexers] { override def acquire()(implicit context: ResourceContext): Resource[Indexers] = { @@ -46,6 +47,7 @@ object IndexerStabilityTestFixture { updatesPerSecond = updatesPerSecond, indexerCount = indexerCount, jdbcUrl = jdbcUrl, + lockIdSeed = lockIdSeed, )(context, materializer) } } @@ -54,11 +56,16 @@ object IndexerStabilityTestFixture { updatesPerSecond: Int, indexerCount: Int, jdbcUrl: String, + lockIdSeed: Int, )(implicit resourceContext: ResourceContext, materializer: Materializer): Resource[Indexers] = { val indexerConfig = IndexerConfig( participantId = EndlessReadService.participantId, jdbcUrl = jdbcUrl, startupMode = IndexerStartupMode.MigrateAndStart, + haConfig = HaConfig( + indexerLockId = lockIdSeed, + indexerWorkerLockId = lockIdSeed + 1, + ), ) newLoggingContext { implicit loggingContext => diff --git a/ledger/participant-integration-api/src/test/lib/scala/platform/store/backend/StorageBackendProvider.scala b/ledger/participant-integration-api/src/test/lib/scala/platform/store/backend/StorageBackendProvider.scala index fedb77dfbc..f07ae026ca 100644 --- a/ledger/participant-integration-api/src/test/lib/scala/platform/store/backend/StorageBackendProvider.scala +++ b/ledger/participant-integration-api/src/test/lib/scala/platform/store/backend/StorageBackendProvider.scala @@ -21,6 +21,7 @@ import org.scalatest.Suite */ private[backend] trait StorageBackendProvider { protected def jdbcUrl: String + protected def lockIdSeed: Int protected def backend: TestBackend protected final def ingest(dbDtos: Vector[DbDto], connection: Connection): Unit = { @@ -61,14 +62,14 @@ trait StorageBackendProviderPostgres extends StorageBackendProvider with Postgre private[backend] trait StorageBackendProviderH2 extends StorageBackendProvider { this: Suite => override protected def jdbcUrl: String = "jdbc:h2:mem:storage_backend_provider;db_close_delay=-1" + override protected def lockIdSeed: Int = + throw new UnsupportedOperationException // DB Locking is not supported for H2 override protected val backend: TestBackend = TestBackend(H2StorageBackendFactory) } private[backend] trait StorageBackendProviderOracle extends StorageBackendProvider with OracleAroundAll { this: Suite => - override protected def jdbcUrl: String = - s"jdbc:oracle:thin:$oracleUser/$oraclePwd@$oracleHost:$oraclePort/ORCLPDB1" override protected val backend: TestBackend = TestBackend(OracleStorageBackendFactory) } diff --git a/ledger/participant-integration-api/src/test/lib/scala/platform/store/backend/StorageBackendTestsDBLock.scala b/ledger/participant-integration-api/src/test/lib/scala/platform/store/backend/StorageBackendTestsDBLock.scala index 28f8578101..7cc73f630f 100644 --- a/ledger/participant-integration-api/src/test/lib/scala/platform/store/backend/StorageBackendTestsDBLock.scala +++ b/ledger/participant-integration-api/src/test/lib/scala/platform/store/backend/StorageBackendTestsDBLock.scala @@ -21,113 +21,114 @@ private[platform] trait StorageBackendTestsDBLock extends Matchers with Eventual protected def dbLock: DBLockStorageBackend protected def getConnection: Connection + protected def lockIdSeed: Int behavior of "DBLockStorageBackend" it should "allow to acquire the same shared lock many times" in dbLockTestCase(1) { c => - dbLock.tryAcquire(dbLock.lock(1), LockMode.Shared)(c(1)) should not be empty - dbLock.tryAcquire(dbLock.lock(1), LockMode.Shared)(c(1)) should not be empty - dbLock.tryAcquire(dbLock.lock(1), LockMode.Shared)(c(1)) should not be empty - dbLock.tryAcquire(dbLock.lock(1), LockMode.Shared)(c(1)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Shared)(c(1)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Shared)(c(1)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Shared)(c(1)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Shared)(c(1)) should not be empty } it should "allow to acquire the same exclusive lock many times" in dbLockTestCase(1) { c => - dbLock.tryAcquire(dbLock.lock(1), LockMode.Exclusive)(c(1)) should not be empty - dbLock.tryAcquire(dbLock.lock(1), LockMode.Exclusive)(c(1)) should not be empty - dbLock.tryAcquire(dbLock.lock(1), LockMode.Exclusive)(c(1)) should not be empty - dbLock.tryAcquire(dbLock.lock(1), LockMode.Exclusive)(c(1)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Exclusive)(c(1)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Exclusive)(c(1)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Exclusive)(c(1)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Exclusive)(c(1)) should not be empty } it should "allow shared locking" in dbLockTestCase(2) { c => - dbLock.tryAcquire(dbLock.lock(1), LockMode.Shared)(c(1)) should not be empty - dbLock.tryAcquire(dbLock.lock(1), LockMode.Shared)(c(2)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Shared)(c(1)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Shared)(c(2)) should not be empty } it should "allow shared locking many times" in dbLockTestCase(5) { c => - dbLock.tryAcquire(dbLock.lock(1), LockMode.Shared)(c(1)) should not be empty - dbLock.tryAcquire(dbLock.lock(1), LockMode.Shared)(c(2)) should not be empty - dbLock.tryAcquire(dbLock.lock(1), LockMode.Shared)(c(3)) should not be empty - dbLock.tryAcquire(dbLock.lock(1), LockMode.Shared)(c(4)) should not be empty - dbLock.tryAcquire(dbLock.lock(1), LockMode.Shared)(c(5)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Shared)(c(1)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Shared)(c(2)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Shared)(c(3)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Shared)(c(4)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Shared)(c(5)) should not be empty } it should "not allow exclusive when locked by shared" in dbLockTestCase(2) { c => - dbLock.tryAcquire(dbLock.lock(1), LockMode.Shared)(c(1)) should not be empty - dbLock.tryAcquire(dbLock.lock(1), LockMode.Exclusive)(c(2)) shouldBe empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Shared)(c(1)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Exclusive)(c(2)) shouldBe empty } it should "not allow exclusive when locked by exclusive" in dbLockTestCase(2) { c => - dbLock.tryAcquire(dbLock.lock(1), LockMode.Exclusive)(c(1)) should not be empty - dbLock.tryAcquire(dbLock.lock(1), LockMode.Exclusive)(c(2)) shouldBe empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Exclusive)(c(1)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Exclusive)(c(2)) shouldBe empty } it should "not allow shared when locked by exclusive" in dbLockTestCase(2) { c => - dbLock.tryAcquire(dbLock.lock(1), LockMode.Exclusive)(c(1)) should not be empty - dbLock.tryAcquire(dbLock.lock(1), LockMode.Shared)(c(2)) shouldBe empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Exclusive)(c(1)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Shared)(c(2)) shouldBe empty } it should "unlock successfully a shared lock" in dbLockTestCase(2) { c => - val lock = dbLock.tryAcquire(dbLock.lock(1), LockMode.Shared)(c(1)).get - dbLock.tryAcquire(dbLock.lock(1), LockMode.Exclusive)(c(2)) shouldBe empty + val lock = dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Shared)(c(1)).get + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Exclusive)(c(2)) shouldBe empty dbLock.release(lock)(c(1)) shouldBe true - dbLock.tryAcquire(dbLock.lock(1), LockMode.Exclusive)(c(2)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Exclusive)(c(2)) should not be empty } it should "release successfully a shared lock if connection closed" in dbLockTestCase(2) { c => - dbLock.tryAcquire(dbLock.lock(1), LockMode.Shared)(c(1)).get - dbLock.tryAcquire(dbLock.lock(1), LockMode.Exclusive)(c(2)) shouldBe empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Shared)(c(1)).get + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Exclusive)(c(2)) shouldBe empty c(1).close() eventually(timeout)( - dbLock.tryAcquire(dbLock.lock(1), LockMode.Exclusive)(c(2)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Exclusive)(c(2)) should not be empty ) } it should "unlock successfully an exclusive lock" in dbLockTestCase(2) { c => - val lock = dbLock.tryAcquire(dbLock.lock(1), LockMode.Exclusive)(c(1)).get - dbLock.tryAcquire(dbLock.lock(1), LockMode.Exclusive)(c(2)) shouldBe empty + val lock = dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Exclusive)(c(1)).get + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Exclusive)(c(2)) shouldBe empty dbLock.release(lock)(c(1)) shouldBe true - dbLock.tryAcquire(dbLock.lock(1), LockMode.Exclusive)(c(2)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Exclusive)(c(2)) should not be empty } it should "release successfully an exclusive lock if connection closed" in dbLockTestCase(2) { c => - dbLock.tryAcquire(dbLock.lock(1), LockMode.Exclusive)(c(1)).get - dbLock.tryAcquire(dbLock.lock(1), LockMode.Exclusive)(c(2)) shouldBe empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Exclusive)(c(1)).get + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Exclusive)(c(2)) shouldBe empty c(1).close() eventually(timeout)( - dbLock.tryAcquire(dbLock.lock(1), LockMode.Exclusive)(c(2)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Exclusive)(c(2)) should not be empty ) } it should "be able to lock exclusive, if all shared locks are released" in dbLockTestCase(4) { c => - val shared1 = dbLock.tryAcquire(dbLock.lock(1), LockMode.Shared)(c(1)).get - val shared2 = dbLock.tryAcquire(dbLock.lock(1), LockMode.Shared)(c(2)).get - val shared3 = dbLock.tryAcquire(dbLock.lock(1), LockMode.Shared)(c(3)).get - dbLock.tryAcquire(dbLock.lock(1), LockMode.Exclusive)(c(4)) shouldBe empty + val shared1 = dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Shared)(c(1)).get + val shared2 = dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Shared)(c(2)).get + val shared3 = dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Shared)(c(3)).get + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Exclusive)(c(4)) shouldBe empty dbLock.release(shared1)(c(1)) - dbLock.tryAcquire(dbLock.lock(1), LockMode.Exclusive)(c(4)) shouldBe empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Exclusive)(c(4)) shouldBe empty dbLock.release(shared2)(c(2)) - dbLock.tryAcquire(dbLock.lock(1), LockMode.Exclusive)(c(4)) shouldBe empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Exclusive)(c(4)) shouldBe empty dbLock.release(shared3)(c(3)) - dbLock.tryAcquire(dbLock.lock(1), LockMode.Exclusive)(c(4)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Exclusive)(c(4)) should not be empty } it should "lock immediately, or fail immediately" in dbLockTestCase(2) { c => - dbLock.tryAcquire(dbLock.lock(1), LockMode.Shared)(c(1)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Shared)(c(1)) should not be empty val start = System.currentTimeMillis() - dbLock.tryAcquire(dbLock.lock(1), LockMode.Exclusive)(c(2)) shouldBe empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Exclusive)(c(2)) shouldBe empty (System.currentTimeMillis() - start) should be < 500L } it should "fail to unlock lock held by others" in dbLockTestCase(2) { c => - val lock = dbLock.tryAcquire(dbLock.lock(1), LockMode.Shared)(c(1)).get + val lock = dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Shared)(c(1)).get dbLock.release(lock)(c(2)) shouldBe false } it should "fail to unlock lock which is not held by anyone" in dbLockTestCase(1) { c => - dbLock.release(Lock(dbLock.lock(1), LockMode.Shared))(c(1)) shouldBe false + dbLock.release(Lock(dbLock.lock(lockIdSeed), LockMode.Shared))(c(1)) shouldBe false } it should "fail if attempt to use backend-foreign lock-id for locking" in dbLockTestCase(1) { c => @@ -140,10 +141,10 @@ private[platform] trait StorageBackendTestsDBLock extends Matchers with Eventual } it should "lock successfully exclusive different locks" in dbLockTestCase(1) { c => - dbLock.tryAcquire(dbLock.lock(1), LockMode.Exclusive)(c(1)) should not be empty - dbLock.tryAcquire(dbLock.lock(2), LockMode.Exclusive)(c(1)) should not be empty - dbLock.tryAcquire(dbLock.lock(3), LockMode.Exclusive)(c(1)) should not be empty - dbLock.tryAcquire(dbLock.lock(4), LockMode.Exclusive)(c(1)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed), LockMode.Exclusive)(c(1)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed + 1), LockMode.Exclusive)(c(1)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed + 2), LockMode.Exclusive)(c(1)) should not be empty + dbLock.tryAcquire(dbLock.lock(lockIdSeed + 3), LockMode.Exclusive)(c(1)) should not be empty } private val timeout = Timeout(Span(10, Seconds)) diff --git a/ledger/participant-integration-api/src/test/lib/scala/platform/store/dao/JdbcLedgerDaoBackendOracle.scala b/ledger/participant-integration-api/src/test/lib/scala/platform/store/dao/JdbcLedgerDaoBackendOracle.scala index 6574b11c5f..a8813666af 100644 --- a/ledger/participant-integration-api/src/test/lib/scala/platform/store/dao/JdbcLedgerDaoBackendOracle.scala +++ b/ledger/participant-integration-api/src/test/lib/scala/platform/store/dao/JdbcLedgerDaoBackendOracle.scala @@ -11,7 +11,4 @@ private[dao] trait JdbcLedgerDaoBackendOracle extends JdbcLedgerDaoBackend with this: AsyncTestSuite => override protected val dbType: DbType = DbType.Oracle - - override protected def jdbcUrl: String = - s"jdbc:oracle:thin:$oracleUser/$oraclePwd@$oracleHost:$oraclePort/ORCLPDB1" } diff --git a/ledger/participant-integration-api/src/test/lib/scala/platform/store/dao/JdbcLedgerDaoTransactionsSpec.scala b/ledger/participant-integration-api/src/test/lib/scala/platform/store/dao/JdbcLedgerDaoTransactionsSpec.scala index acf0a4f301..a7e2a30956 100644 --- a/ledger/participant-integration-api/src/test/lib/scala/platform/store/dao/JdbcLedgerDaoTransactionsSpec.scala +++ b/ledger/participant-integration-api/src/test/lib/scala/platform/store/dao/JdbcLedgerDaoTransactionsSpec.scala @@ -503,7 +503,7 @@ private[dao] trait JdbcLedgerDaoTransactionsSpec extends OptionValues with Insid // `pageSize = 2` and the offset gaps in the `commandWithOffsetGaps` above are to make sure // that streaming works with event pages separated by offsets that don't have events in the store - ledgerDao <- createLedgerDao( + response <- createLedgerDaoResourceOwner( pageSize = 2, eventsProcessingParallelism = 8, acsIdPageSize = 2, @@ -511,16 +511,16 @@ private[dao] trait JdbcLedgerDaoTransactionsSpec extends OptionValues with Insid acsContractFetchingParallelism = 2, acsGlobalParallelism = 10, acsIdQueueLimit = 1000000, - ) - - response <- ledgerDao.transactionsReader - .getFlatTransactions( - beginOffset, - endOffset, - Map(alice -> Set.empty[Identifier]), - verbose = true, - ) - .runWith(Sink.seq) + ).use( + _.transactionsReader + .getFlatTransactions( + beginOffset, + endOffset, + Map(alice -> Set.empty[Identifier]), + verbose = true, + ) + .runWith(Sink.seq) + )(ResourceContext(executionContext)) readTxs = extractAllTransactions(response) } yield { @@ -641,7 +641,7 @@ private[dao] trait JdbcLedgerDaoTransactionsSpec extends OptionValues with Insid ): Vector[Transaction] = responses.foldLeft(Vector.empty[Transaction])((b, a) => b ++ a._2.transactions.toVector) - private def createLedgerDao( + private def createLedgerDaoResourceOwner( pageSize: Int, eventsProcessingParallelism: Int, acsIdPageSize: Int, @@ -660,8 +660,8 @@ private[dao] trait JdbcLedgerDaoTransactionsSpec extends OptionValues with Insid acsGlobalParallelism = acsGlobalParallelism, acsIdQueueLimit = acsIdQueueLimit, MockitoSugar.mock[ErrorFactories], - ).acquire()(ResourceContext(executionContext)) - }.asFuture + ) + } // XXX SC much of this is repeated because we're more concerned here // with whether each query is tested than whether the specifics of the diff --git a/ledger/participant-integration-api/src/test/suite/scala/platform/indexer/ha/IndexerStabilityOracleSpec.scala b/ledger/participant-integration-api/src/test/suite/scala/platform/indexer/ha/IndexerStabilityOracleSpec.scala index 20bc415bed..98c410c7fd 100644 --- a/ledger/participant-integration-api/src/test/suite/scala/platform/indexer/ha/IndexerStabilityOracleSpec.scala +++ b/ledger/participant-integration-api/src/test/suite/scala/platform/indexer/ha/IndexerStabilityOracleSpec.scala @@ -5,8 +5,4 @@ package com.daml.platform.indexer.ha import com.daml.testing.oracle.OracleAroundAll -final class IndexerStabilityOracleSpec extends IndexerStabilitySpec with OracleAroundAll { - - override def jdbcUrl: String = - s"jdbc:oracle:thin:$oracleUser/$oraclePwd@$oracleHost:$oraclePort/ORCLPDB1" -} +final class IndexerStabilityOracleSpec extends IndexerStabilitySpec with OracleAroundAll diff --git a/ledger/participant-integration-api/src/test/suite/scala/platform/indexer/ha/IndexerStabilityPostgresSpec.scala b/ledger/participant-integration-api/src/test/suite/scala/platform/indexer/ha/IndexerStabilityPostgresSpec.scala index be6bebd75b..ea111b5888 100644 --- a/ledger/participant-integration-api/src/test/suite/scala/platform/indexer/ha/IndexerStabilityPostgresSpec.scala +++ b/ledger/participant-integration-api/src/test/suite/scala/platform/indexer/ha/IndexerStabilityPostgresSpec.scala @@ -8,4 +8,7 @@ import com.daml.testing.postgresql.PostgresAroundEach final class IndexerStabilityPostgresSpec extends IndexerStabilitySpec with PostgresAroundEach { override def jdbcUrl: String = postgresDatabase.url + + override def lockIdSeed: Int = + 1000 // it does not matter in case of Postgres: it is always a different instance } diff --git a/ledger/participant-integration-api/src/test/suite/scala/platform/indexer/ha/TestDBLockStorageBackendSpec.scala b/ledger/participant-integration-api/src/test/suite/scala/platform/indexer/ha/TestDBLockStorageBackendSpec.scala index 8d80f832ae..1dbef41a5c 100644 --- a/ledger/participant-integration-api/src/test/suite/scala/platform/indexer/ha/TestDBLockStorageBackendSpec.scala +++ b/ledger/participant-integration-api/src/test/suite/scala/platform/indexer/ha/TestDBLockStorageBackendSpec.scala @@ -22,5 +22,7 @@ class TestDBLockStorageBackendSpec override def dbLock: DBLockStorageBackend = _dbLock + override def lockIdSeed: Int = 1000 // Seeding not needed for this test + override def getConnection: Connection = new TestConnection } diff --git a/libs-scala/oracle-testing/BUILD.bazel b/libs-scala/oracle-testing/BUILD.bazel index 8dcfa08629..ecb2b5cf48 100644 --- a/libs-scala/oracle-testing/BUILD.bazel +++ b/libs-scala/oracle-testing/BUILD.bazel @@ -23,5 +23,6 @@ da_scala_library( deps = [ "//libs-scala/ports", "@maven//:org_scalatest_scalatest_compatible", + "@maven//:org_slf4j_slf4j_api", ], ) diff --git a/libs-scala/oracle-testing/src/main/scala/testing/oracle/OracleAround.scala b/libs-scala/oracle-testing/src/main/scala/testing/oracle/OracleAround.scala index 7260b9ee0b..001eb8e20a 100644 --- a/libs-scala/oracle-testing/src/main/scala/testing/oracle/OracleAround.scala +++ b/libs-scala/oracle-testing/src/main/scala/testing/oracle/OracleAround.scala @@ -5,91 +5,132 @@ package com.daml.testing.oracle import com.daml.ports._ import java.sql._ -import scala.util.{Random, Using} -private[daml] final case class User(name: String, pwd: String) +import org.slf4j.LoggerFactory -trait OracleAround { - @volatile - private var systemUser: String = _ - @volatile - private var systemPwd: String = _ - @volatile - private var port: Port = _ - @volatile - private var host: String = _ +import scala.annotation.tailrec +import scala.util.{Failure, Random, Success, Try, Using} - def oraclePort: Port = port +object OracleAround { - def oracleHost: String = host + case class RichOracleUser( + oracleUser: OracleUser, + jdbcUrlWithoutCredentials: String, + jdbcUrl: String, + drop: () => Unit, + ) { - def oracleJdbcUrl: String = s"jdbc:oracle:thin:@$oracleHost:$oraclePort/ORCLPDB1" - - protected def connectToOracle(): Unit = { - systemUser = sys.env("ORACLE_USERNAME") - systemPwd = sys.env("ORACLE_PWD") - port = Port(sys.env.get("ORACLE_PORT").map(_.toInt).getOrElse(1521)) - host = sys.env.getOrElse("ORACLE_HOST", "localhost") + /** In CI we re-use the same Oracle instance for testing, so non-colliding DB-lock-ids need to be assigned + * + * @return A positive integer, which defines a unique / mutually exclusive range of usable lock ids: [seed, seed + 10) + */ + def lockIdSeed: Int = { + assert(oracleUser.id > 0, "Lock ID seeding is not supported, cannot ensure unique lock-ids") + assert(oracleUser.id < 10 * 1000 * 1000) + oracleUser.id * 10 + } } - protected def createNewRandomUser(): User = { - // See https://docs.oracle.com/cd/B19306_01/server.102/b14200/sql_elements008.htm#i27570 - // for name restrictions. - val u = "u" + Random.alphanumeric.take(29).mkString("") - createNewUser(u.toUpperCase) + case class OracleUser(name: String, id: Int) { + val pwd: String = "hunter2" } - protected def checkUserExists(con: Connection, name: String): Boolean = { - Using(con.createStatement()) { stmt => - val res = stmt.executeQuery( - s"""SELECT count(*) AS user_count FROM all_users WHERE username='${name.toUpperCase}'""" - ) - val count = if (res.next()) { - res.getInt("user_count") - } else 0 - count > 0 - }.get + private val logger = LoggerFactory.getLogger(getClass) + + def createNewUniqueRandomUser(): RichOracleUser = createRichOracleUser { stmt => + val id = Random.nextInt(1000 * 1000) + 1 + val user = OracleUser(s"U$id", id) + createUser(stmt, user.name, user.pwd) + logger.info(s"New unique random Oracle user created $user") + user } - protected def createNewUser(name: String, pwd: String = "hunter2"): User = { - Using.Manager { use => - val con = use( - DriverManager.getConnection( - s"jdbc:oracle:thin:@$oracleHost:$oraclePort/ORCLPDB1", - "sys as sysdba", // TODO this is needed for being able to grant the execute access for the sys.dbms_lock below. Consider making this configurable - systemPwd, - ) - ) - if (!checkUserExists(con, name)) { - val stmt = con.createStatement() - stmt.execute(s"""create user $name identified by $pwd""") - stmt.execute(s"""grant connect, resource to $name""") - stmt.execute( - s"""grant create table, create materialized view, create view, create procedure, create sequence, create type to $name""" - ) - stmt.execute(s"""alter user $name quota unlimited on users""") - - // for DBMS_LOCK access - stmt.execute(s"""GRANT EXECUTE ON SYS.DBMS_LOCK TO $name""") - stmt.execute(s"""GRANT SELECT ON V_$$MYSTAT TO $name""") - stmt.execute(s"""GRANT SELECT ON V_$$LOCK TO $name""") - } else false - }.get - User(name, pwd) + def createOrReuseUser(name: String): RichOracleUser = createRichOracleUser { stmt => + val user = OracleUser(name.toUpperCase, -1) + if (!userExists(stmt, user.name)) { + createUser(stmt, user.name, user.pwd) + logger.info(s"User $name not found: new Oracle user created $user") + } else { + logger.info(s"User $name already created: re-using existing Oracle user $user") + } + user } - protected def dropUser(name: String): Unit = { - Using.Manager { use => - val con = use( - DriverManager.getConnection( - s"jdbc:oracle:thin:@$oracleHost:$oraclePort/ORCLPDB1", - systemUser, - systemPwd, - ) - ) - val stmt = use(con.createStatement()) - stmt.execute(s"""drop user $name cascade""") - }.get + private def userExists(stmt: Statement, name: String): Boolean = { + val res = stmt.executeQuery( + s"""SELECT count(*) AS user_count FROM all_users WHERE username='$name'""" + ) + res.next() + res.getInt("user_count") > 0 + } + + private def createUser(stmt: Statement, name: String, pwd: String): Unit = { + stmt.execute(s"""create user $name identified by $pwd""") + stmt.execute(s"""grant connect, resource to $name""") + stmt.execute( + s"""grant create table, create materialized view, create view, create procedure, create sequence, create type to $name""" + ) + stmt.execute(s"""alter user $name quota unlimited on users""") + + // for DBMS_LOCK access + stmt.execute(s"""GRANT EXECUTE ON SYS.DBMS_LOCK TO $name""") + stmt.execute(s"""GRANT SELECT ON V_$$MYSTAT TO $name""") + stmt.execute(s"""GRANT SELECT ON V_$$LOCK TO $name""") () } + + private def createRichOracleUser(createBody: Statement => OracleUser): RichOracleUser = { + val systemUser = sys.env("ORACLE_USERNAME") + val systemPwd = sys.env("ORACLE_PWD") + val host = sys.env.getOrElse("ORACLE_HOST", "localhost") + val port = Port(sys.env("ORACLE_PORT").toInt) + val jdbcUrlWithoutCredentials = s"jdbc:oracle:thin:@$host:$port/ORCLPDB1" + + def withStmt[T](connectingUserName: String)(body: Statement => T): T = + Using.resource( + DriverManager.getConnection( + jdbcUrlWithoutCredentials, + connectingUserName, + systemPwd, + ) + ) { connection => + connection.setAutoCommit(false) + val result = Using.resource(connection.createStatement())(body) + connection.commit() + result + } + + @tailrec + def retry[T](times: Int, sleepMillisBeforeReTry: Long = 0)(body: => T): T = Try(body) match { + case Success(t) => t + case Failure(_) if times > 0 => + if (sleepMillisBeforeReTry > 0) Thread.sleep(sleepMillisBeforeReTry) + retry(times - 1)(body) + case Failure(t) => throw t + } + + retry(20, 100) { + withStmt( + "sys as sysdba" // TODO this is needed for being able to grant the execute access for the sys.dbms_lock below. Consider making this configurable + ) { stmt => + logger.info("Trying to create Oracle user") + val oracleUser = createBody(stmt) + logger.info(s"Oracle user ready $oracleUser") + RichOracleUser( + oracleUser = oracleUser, + jdbcUrlWithoutCredentials = jdbcUrlWithoutCredentials, + jdbcUrl = s"jdbc:oracle:thin:${oracleUser.name}/${oracleUser.pwd}@$host:$port/ORCLPDB1", + drop = () => { + retry(10, 1000) { + logger.info(s"Trying to remove Oracle user ${oracleUser.name}") + withStmt(systemUser)(_.execute(s"""drop user ${oracleUser.name} cascade""")) + } + logger.info(s"Oracle user removed successfully ${oracleUser.name}") + () + }, + ) + } + } + } + } diff --git a/libs-scala/oracle-testing/src/main/scala/testing/oracle/OracleAroundAll.scala b/libs-scala/oracle-testing/src/main/scala/testing/oracle/OracleAroundAll.scala index 04d00b6bd2..57cf83360a 100644 --- a/libs-scala/oracle-testing/src/main/scala/testing/oracle/OracleAroundAll.scala +++ b/libs-scala/oracle-testing/src/main/scala/testing/oracle/OracleAroundAll.scala @@ -16,5 +16,6 @@ trait OracleAroundAll extends OracleAroundSuite with BeforeAndAfterAll { override protected def afterAll(): Unit = { super.afterAll() + dropUser() } } diff --git a/libs-scala/oracle-testing/src/main/scala/testing/oracle/OracleAroundSuite.scala b/libs-scala/oracle-testing/src/main/scala/testing/oracle/OracleAroundSuite.scala index 499d970ff8..51e3f6c56c 100644 --- a/libs-scala/oracle-testing/src/main/scala/testing/oracle/OracleAroundSuite.scala +++ b/libs-scala/oracle-testing/src/main/scala/testing/oracle/OracleAroundSuite.scala @@ -3,23 +3,22 @@ package com.daml.testing.oracle +import com.daml.testing.oracle.OracleAround.RichOracleUser import org.scalatest.Suite -trait OracleAroundSuite extends OracleAround { +trait OracleAroundSuite { self: Suite => @volatile - private var user: User = _ + private var user: RichOracleUser = _ - protected def oracleUser: String = user.name - protected def oraclePwd: String = user.pwd + def jdbcUrl: String = user.jdbcUrl + def lockIdSeed: Int = user.lockIdSeed + def oracleUserName: String = user.oracleUser.name + def oracleUserPwd: String = user.oracleUser.pwd + def oracleJdbcUrlWithoutCredentials: String = user.jdbcUrlWithoutCredentials - protected def createNewUser(): Unit = { - connectToOracle() - user = createNewRandomUser() - } + protected def createNewUser(): Unit = user = OracleAround.createNewUniqueRandomUser() - protected def dropUser(): Unit = { - dropUser(oracleUser) - } + protected def dropUser(): Unit = user.drop() } diff --git a/libs-scala/postgresql-testing/src/main/scala/com/digitalasset/testing/postgresql/PostgresAroundSuite.scala b/libs-scala/postgresql-testing/src/main/scala/com/digitalasset/testing/postgresql/PostgresAroundSuite.scala index d9a3cdc716..4a4ce582fa 100644 --- a/libs-scala/postgresql-testing/src/main/scala/com/digitalasset/testing/postgresql/PostgresAroundSuite.scala +++ b/libs-scala/postgresql-testing/src/main/scala/com/digitalasset/testing/postgresql/PostgresAroundSuite.scala @@ -13,6 +13,9 @@ trait PostgresAroundSuite extends PostgresAround { protected def postgresDatabase: PostgresDatabase = database.get + protected def lockIdSeed: Int = + 1000 // For postgres each test-suite uses different DB, so no unique lock-ids needed + protected def createNewDatabase(): PostgresDatabase = { database = Some(createNewRandomDatabase()) postgresDatabase diff --git a/triggers/service/src/test/scala/com/digitalasset/daml/lf/engine/trigger/TriggerServiceFixture.scala b/triggers/service/src/test/scala/com/digitalasset/daml/lf/engine/trigger/TriggerServiceFixture.scala index 656de7ba03..5101e7f854 100644 --- a/triggers/service/src/test/scala/com/digitalasset/daml/lf/engine/trigger/TriggerServiceFixture.scala +++ b/triggers/service/src/test/scala/com/digitalasset/daml/lf/engine/trigger/TriggerServiceFixture.scala @@ -458,9 +458,9 @@ trait TriggerDaoOracleFixture private lazy val jdbcConfig_ = JdbcConfig( "oracle.jdbc.OracleDriver", - oracleJdbcUrl, - oracleUser, - oraclePwd, + oracleJdbcUrlWithoutCredentials, + oracleUserName, + oracleUserPwd, ConnectionPool.PoolSize.Production, ) // TODO For whatever reason we need a larger pool here, otherwise