Skip to content

Commit

Permalink
perf: EXPOSED-204 Performance problem with getConnection()
Browse files Browse the repository at this point in the history
Rename new properties.

Replace const integer value with a private flag that is passed from
ThreadLocalTransactionManager to ThreadLocalTransaction instance.

Edit tests to also check stored value in TransactionManager, in the event
users access this property directly, before the data source value is retrieved
and cached.
  • Loading branch information
bog-walk committed Dec 13, 2023
1 parent 07d6751 commit 27f3a01
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -72,16 +72,20 @@ class Database private constructor(
return this
}

internal var usesDataSource = false
/** Whether [Database.connect] was invoked with a [DataSource] argument. */
internal var connectsViaDataSource = false
private set

internal var dataSourceTransactionIsolation: Int? = null
/**
* The transaction isolation level defined by a [DataSource] connection.
*
* This should only hold a non-null value if [connectsViaDataSource] has been set to `true`.
*/
internal var dataSourceIsolationLevel: Int? = null

companion object {
internal val dialects = ConcurrentHashMap<String, () -> DatabaseDialect>()

internal const val UNINITIALIZED_DATASOURCE_ISOLATION = -2

private val connectionInstanceImpl: DatabaseConnectionAutoRegistration =
ServiceLoader.load(DatabaseConnectionAutoRegistration::class.java, Database::class.java.classLoader).firstOrNull()
?: error("Can't load implementation for ${DatabaseConnectionAutoRegistration::class.simpleName}")
Expand Down Expand Up @@ -154,7 +158,7 @@ class Database private constructor(
setupConnection = setupConnection,
manager = manager
).apply {
usesDataSource = true
connectsViaDataSource = true
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package org.jetbrains.exposed.sql.transactions
import org.jetbrains.annotations.TestOnly
import org.jetbrains.exposed.exceptions.ExposedSQLException
import org.jetbrains.exposed.sql.Database
import org.jetbrains.exposed.sql.Database.Companion.UNINITIALIZED_DATASOURCE_ISOLATION
import org.jetbrains.exposed.sql.SchemaUtils
import org.jetbrains.exposed.sql.SqlLogger
import org.jetbrains.exposed.sql.Transaction
Expand Down Expand Up @@ -44,14 +43,16 @@ class ThreadLocalTransactionManager(
@Volatile
override var defaultIsolationLevel: Int = db.config.defaultIsolationLevel
get() {
when (field) {
-1 -> field = if (db.usesDataSource) {
db.dataSourceTransactionIsolation ?: UNINITIALIZED_DATASOURCE_ISOLATION
} else {
Database.getDefaultIsolationLevel(db)
when {
field == -1 -> {
if (db.connectsViaDataSource) loadDataSourceIsolationLevel = true
field = Database.getDefaultIsolationLevel(db)
}
UNINITIALIZED_DATASOURCE_ISOLATION -> {
field = db.dataSourceTransactionIsolation ?: Database.getDefaultIsolationLevel(db)
db.connectsViaDataSource && loadDataSourceIsolationLevel -> {
db.dataSourceIsolationLevel?.let {
loadDataSourceIsolationLevel = false
field = it
}
}
}
return field
Expand All @@ -61,6 +62,14 @@ class ThreadLocalTransactionManager(
@TestOnly
set

/**
* Whether the transaction isolation level of the underlying DataSource should be retrieved from the database.
*
* This should only be set to `true` if [Database.connectsViaDataSource] has also been set to `true` and if
* an initial connection to the database has not already been made.
*/
private var loadDataSourceIsolationLevel = false

@Volatile
override var defaultReadOnly: Boolean = db.config.defaultReadOnly

Expand All @@ -79,7 +88,8 @@ class ThreadLocalTransactionManager(
transactionIsolation = outerTransaction?.transactionIsolation ?: isolation,
setupTxConnection = setupTxConnection,
threadLocal = threadLocal,
outerTransaction = outerTransaction
outerTransaction = outerTransaction,
loadDataSourceIsolationLevel = loadDataSourceIsolationLevel,
)
)

Expand All @@ -102,7 +112,8 @@ class ThreadLocalTransactionManager(
override val transactionIsolation: Int,
override val readOnly: Boolean,
val threadLocal: ThreadLocal<Transaction>,
override val outerTransaction: Transaction?
override val outerTransaction: Transaction?,
private val loadDataSourceIsolationLevel: Boolean
) : TransactionInterface {

private val connectionLazy = lazy(LazyThreadSafetyMode.NONE) {
Expand All @@ -112,15 +123,14 @@ class ThreadLocalTransactionManager(
// Transaction isolation should go first as readOnly or autoCommit can start transaction with wrong isolation level
// Some drivers start a transaction right after `setAutoCommit(false)`,
// which makes `setReadOnly` throw an exception if it is called after `setAutoCommit`
if (
db.usesDataSource && db.dataSourceTransactionIsolation == null &&
this@ThreadLocalTransaction.transactionIsolation == UNINITIALIZED_DATASOURCE_ISOLATION
) {
db.dataSourceTransactionIsolation = transactionIsolation
if (db.connectsViaDataSource && loadDataSourceIsolationLevel && db.dataSourceIsolationLevel == null) {
// retrieves the setting of the datasource connection & caches it
db.dataSourceIsolationLevel = transactionIsolation
} else if (
!db.usesDataSource ||
db.dataSourceTransactionIsolation?.equals(this@ThreadLocalTransaction.transactionIsolation) != true
!db.connectsViaDataSource ||
db.dataSourceIsolationLevel?.equals(this@ThreadLocalTransaction.transactionIsolation) != true
) {
// only set the level if there is no cached datasource value or if the value differs
transactionIsolation = this@ThreadLocalTransaction.transactionIsolation
}
readOnly = this@ThreadLocalTransaction.readOnly
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,32 @@ class TransactionIsolationTest : DatabaseTestsBase() {
Assume.assumeTrue(setOf(TestDB.MYSQL, TestDB.MARIADB, TestDB.POSTGRESQL, TestDB.SQLSERVER).containsAll(TestDB.enabledDialects()))
val dialect = TestDB.enabledDialects().first()

val db = Database.connect(HikariDataSource(setupHikariConfig(dialect)))
val db = Database.connect(
HikariDataSource(setupHikariConfig(dialect, "TRANSACTION_REPEATABLE_READ"))
)
val manager = TransactionManager.managerFor(db)

transaction(db) {
// level should be set by hikari dataSource
// transaction manager should use database default since no level is provided other than hikari
assertEquals(Database.getDefaultIsolationLevel(db), manager?.defaultIsolationLevel)

// database level should be set by hikari dataSource
assertTransactionIsolationLevel(dialect, Connection.TRANSACTION_REPEATABLE_READ)
// after first connection, transaction manager should use hikari level by default
assertEquals(Connection.TRANSACTION_REPEATABLE_READ, manager?.defaultIsolationLevel)
}

transaction(transactionIsolation = Connection.TRANSACTION_READ_COMMITTED, db = db) {
// level should be set by transaction-specific setting
assertEquals(Connection.TRANSACTION_REPEATABLE_READ, manager?.defaultIsolationLevel)

// database level should be set by transaction-specific setting
assertTransactionIsolationLevel(dialect, Connection.TRANSACTION_READ_COMMITTED)
}

transaction(db) {
// level should be set by hikari dataSource
assertEquals(Connection.TRANSACTION_REPEATABLE_READ, manager?.defaultIsolationLevel)

// database level should be set by hikari dataSource
assertTransactionIsolationLevel(dialect, Connection.TRANSACTION_REPEATABLE_READ)
}

Expand All @@ -57,37 +69,47 @@ class TransactionIsolationTest : DatabaseTestsBase() {
val dialect = TestDB.enabledDialects().first()

val db = Database.connect(
HikariDataSource(setupHikariConfig(dialect)),
HikariDataSource(setupHikariConfig(dialect, "TRANSACTION_REPEATABLE_READ")),
databaseConfig = DatabaseConfig { defaultIsolationLevel = Connection.TRANSACTION_READ_COMMITTED }
)
val manager = TransactionManager.managerFor(db)

transaction(db) {
// level should be set by DatabaseConfig
// transaction manager should default to use DatabaseConfig level
assertEquals(Connection.TRANSACTION_READ_COMMITTED, manager?.defaultIsolationLevel)

// database level should be set by DatabaseConfig
assertTransactionIsolationLevel(dialect, Connection.TRANSACTION_READ_COMMITTED)
// after first connection, transaction manager should retain DatabaseConfig level
assertEquals(Connection.TRANSACTION_READ_COMMITTED, manager?.defaultIsolationLevel)
}

transaction(transactionIsolation = Connection.TRANSACTION_REPEATABLE_READ, db = db) {
// level should be set by transaction-specific setting
assertEquals(Connection.TRANSACTION_READ_COMMITTED, manager?.defaultIsolationLevel)

// database level should be set by transaction-specific setting
assertTransactionIsolationLevel(dialect, Connection.TRANSACTION_REPEATABLE_READ)
}

transaction(db) {
// level should be set by DatabaseConfig
assertEquals(Connection.TRANSACTION_READ_COMMITTED, manager?.defaultIsolationLevel)

// database level should be set by DatabaseConfig
assertTransactionIsolationLevel(dialect, Connection.TRANSACTION_READ_COMMITTED)
}

TransactionManager.closeAndUnregister(db)
}

private fun setupHikariConfig(dialect: TestDB): HikariConfig {
private fun setupHikariConfig(dialect: TestDB, isolation: String): HikariConfig {
return HikariConfig().apply {
jdbcUrl = dialect.connection.invoke()
driverClassName = dialect.driver
username = dialect.user
password = dialect.pass
maximumPoolSize = 6
isAutoCommit = false
transactionIsolation = "TRANSACTION_REPEATABLE_READ"
transactionIsolation = isolation
validate()
}
}
Expand Down

0 comments on commit 27f3a01

Please sign in to comment.