Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[EXPOSED-46] Add a possibility to set a delay for the repetition attempts #1742

Merged
merged 9 commits into from
Jun 20, 2023
28 changes: 21 additions & 7 deletions exposed-core/api/exposed-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -536,9 +536,11 @@ public final class org/jetbrains/exposed/sql/Database$Companion {

public final class org/jetbrains/exposed/sql/DatabaseConfig {
public static final field Companion Lorg/jetbrains/exposed/sql/DatabaseConfig$Companion;
public synthetic fun <init> (Lorg/jetbrains/exposed/sql/SqlLogger;ZLjava/lang/Integer;IIZLjava/lang/Long;IZLorg/jetbrains/exposed/sql/vendors/DatabaseDialect;Lorg/jetbrains/exposed/sql/Schema;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public synthetic fun <init> (Lorg/jetbrains/exposed/sql/SqlLogger;ZLjava/lang/Integer;IIJJZLjava/lang/Long;IZLorg/jetbrains/exposed/sql/vendors/DatabaseDialect;Lorg/jetbrains/exposed/sql/Schema;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public final fun getDefaultFetchSize ()Ljava/lang/Integer;
public final fun getDefaultIsolationLevel ()I
public final fun getDefaultMaxRepetitionDelay ()J
public final fun getDefaultMinRepetitionDelay ()J
public final fun getDefaultReadOnly ()Z
public final fun getDefaultRepetitionAttempts ()I
public final fun getDefaultSchema ()Lorg/jetbrains/exposed/sql/Schema;
Expand All @@ -553,10 +555,12 @@ public final class org/jetbrains/exposed/sql/DatabaseConfig {

public final class org/jetbrains/exposed/sql/DatabaseConfig$Builder {
public fun <init> ()V
public fun <init> (Lorg/jetbrains/exposed/sql/SqlLogger;ZLjava/lang/Integer;IIZLjava/lang/Long;IZLorg/jetbrains/exposed/sql/vendors/DatabaseDialect;Lorg/jetbrains/exposed/sql/Schema;I)V
public synthetic fun <init> (Lorg/jetbrains/exposed/sql/SqlLogger;ZLjava/lang/Integer;IIZLjava/lang/Long;IZLorg/jetbrains/exposed/sql/vendors/DatabaseDialect;Lorg/jetbrains/exposed/sql/Schema;IILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun <init> (Lorg/jetbrains/exposed/sql/SqlLogger;ZLjava/lang/Integer;IIJJZLjava/lang/Long;IZLorg/jetbrains/exposed/sql/vendors/DatabaseDialect;Lorg/jetbrains/exposed/sql/Schema;I)V
public synthetic fun <init> (Lorg/jetbrains/exposed/sql/SqlLogger;ZLjava/lang/Integer;IIJJZLjava/lang/Long;IZLorg/jetbrains/exposed/sql/vendors/DatabaseDialect;Lorg/jetbrains/exposed/sql/Schema;IILkotlin/jvm/internal/DefaultConstructorMarker;)V
public final fun getDefaultFetchSize ()Ljava/lang/Integer;
public final fun getDefaultIsolationLevel ()I
public final fun getDefaultMaxRepetitionDelay ()J
public final fun getDefaultMinRepetitionDelay ()J
public final fun getDefaultReadOnly ()Z
public final fun getDefaultRepetitionAttempts ()I
public final fun getDefaultSchema ()Lorg/jetbrains/exposed/sql/Schema;
Expand All @@ -569,6 +573,8 @@ public final class org/jetbrains/exposed/sql/DatabaseConfig$Builder {
public final fun getWarnLongQueriesDuration ()Ljava/lang/Long;
public final fun setDefaultFetchSize (Ljava/lang/Integer;)V
public final fun setDefaultIsolationLevel (I)V
public final fun setDefaultMaxRepetitionDelay (J)V
public final fun setDefaultMinRepetitionDelay (J)V
public final fun setDefaultReadOnly (Z)V
public final fun setDefaultRepetitionAttempts (I)V
public final fun setDefaultSchema (Lorg/jetbrains/exposed/sql/Schema;)V
Expand Down Expand Up @@ -2813,21 +2819,25 @@ public final class org/jetbrains/exposed/sql/transactions/ThreadLocalTransaction
public fun bindTransactionToThread (Lorg/jetbrains/exposed/sql/Transaction;)V
public fun currentOrNull ()Lorg/jetbrains/exposed/sql/Transaction;
public fun getDefaultIsolationLevel ()I
public fun getDefaultMaxRepetitionDelay ()J
public fun getDefaultMinRepetitionDelay ()J
public fun getDefaultReadOnly ()Z
public fun getDefaultRepetitionAttempts ()I
public final fun getThreadLocal ()Ljava/lang/ThreadLocal;
public fun newTransaction (IZLorg/jetbrains/exposed/sql/Transaction;)Lorg/jetbrains/exposed/sql/Transaction;
public fun setDefaultIsolationLevel (I)V
public fun setDefaultMaxRepetitionDelay (J)V
public fun setDefaultMinRepetitionDelay (J)V
public fun setDefaultReadOnly (Z)V
public fun setDefaultRepetitionAttempts (I)V
}

public final class org/jetbrains/exposed/sql/transactions/ThreadLocalTransactionManagerKt {
public static final fun inTopLevelTransaction (IIZLorg/jetbrains/exposed/sql/Database;Lorg/jetbrains/exposed/sql/Transaction;Lkotlin/jvm/functions/Function1;)Ljava/lang/Object;
public static synthetic fun inTopLevelTransaction$default (IIZLorg/jetbrains/exposed/sql/Database;Lorg/jetbrains/exposed/sql/Transaction;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)Ljava/lang/Object;
public static final fun transaction (IIZLorg/jetbrains/exposed/sql/Database;Lkotlin/jvm/functions/Function1;)Ljava/lang/Object;
public static final fun inTopLevelTransaction (IIZLorg/jetbrains/exposed/sql/Database;Lorg/jetbrains/exposed/sql/Transaction;JJLkotlin/jvm/functions/Function1;)Ljava/lang/Object;
public static synthetic fun inTopLevelTransaction$default (IIZLorg/jetbrains/exposed/sql/Database;Lorg/jetbrains/exposed/sql/Transaction;JJLkotlin/jvm/functions/Function1;ILjava/lang/Object;)Ljava/lang/Object;
public static final fun transaction (IIZLorg/jetbrains/exposed/sql/Database;JJLkotlin/jvm/functions/Function1;)Ljava/lang/Object;
public static final fun transaction (Lorg/jetbrains/exposed/sql/Database;Lkotlin/jvm/functions/Function1;)Ljava/lang/Object;
public static synthetic fun transaction$default (IIZLorg/jetbrains/exposed/sql/Database;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)Ljava/lang/Object;
public static synthetic fun transaction$default (IIZLorg/jetbrains/exposed/sql/Database;JJLkotlin/jvm/functions/Function1;ILjava/lang/Object;)Ljava/lang/Object;
public static synthetic fun transaction$default (Lorg/jetbrains/exposed/sql/Database;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)Ljava/lang/Object;
}

Expand All @@ -2851,10 +2861,14 @@ public abstract interface class org/jetbrains/exposed/sql/transactions/Transacti
public abstract fun bindTransactionToThread (Lorg/jetbrains/exposed/sql/Transaction;)V
public abstract fun currentOrNull ()Lorg/jetbrains/exposed/sql/Transaction;
public abstract fun getDefaultIsolationLevel ()I
public abstract fun getDefaultMaxRepetitionDelay ()J
public abstract fun getDefaultMinRepetitionDelay ()J
public abstract fun getDefaultReadOnly ()Z
public abstract fun getDefaultRepetitionAttempts ()I
public abstract fun newTransaction (IZLorg/jetbrains/exposed/sql/Transaction;)Lorg/jetbrains/exposed/sql/Transaction;
public abstract fun setDefaultIsolationLevel (I)V
public abstract fun setDefaultMaxRepetitionDelay (J)V
public abstract fun setDefaultMinRepetitionDelay (J)V
public abstract fun setDefaultReadOnly (Z)V
public abstract fun setDefaultRepetitionAttempts (I)V
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ class DatabaseConfig private constructor(
val defaultFetchSize: Int?,
val defaultIsolationLevel: Int,
val defaultRepetitionAttempts: Int,
val defaultMinRepetitionDelay: Long,
val defaultMaxRepetitionDelay: Long,
val defaultReadOnly: Boolean,
val warnLongQueriesDuration: Long?,
val maxEntitiesToStoreInCachePerEntity: Int,
Expand Down Expand Up @@ -43,6 +45,18 @@ class DatabaseConfig private constructor(
* Default attempts are 3
*/
var defaultRepetitionAttempts: Int = 3,
/**
* The minimum number of milliseconds to wait before retrying a transaction if SQLException happens
* Can be overridden on per-transaction level by specifying `minRepetitionDelay` parameter on call
* Default minimum delay is 0
*/
var defaultMinRepetitionDelay: Long = 0,
/**
* The maximum number of milliseconds to wait before retrying a transaction if SQLException happens
* Can be overridden on per-transaction level by specifying `maxRepetitionDelay` parameter on call
* Default maximum delay is 0
*/
var defaultMaxRepetitionDelay: Long = 0,
bog-walk marked this conversation as resolved.
Show resolved Hide resolved

/**
* Should all connections/transactions be executed in read-only mode by default or not
Expand Down Expand Up @@ -96,6 +110,8 @@ class DatabaseConfig private constructor(
defaultFetchSize = builder.defaultFetchSize,
defaultIsolationLevel = builder.defaultIsolationLevel,
defaultRepetitionAttempts = builder.defaultRepetitionAttempts,
defaultMinRepetitionDelay = builder.defaultMinRepetitionDelay,
defaultMaxRepetitionDelay = builder.defaultMaxRepetitionDelay,
defaultReadOnly = builder.defaultReadOnly,
warnLongQueriesDuration = builder.warnLongQueriesDuration,
maxEntitiesToStoreInCachePerEntity = builder.maxEntitiesToStoreInCachePerEntity,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import org.jetbrains.exposed.sql.exposedLogger
import org.jetbrains.exposed.sql.statements.api.ExposedConnection
import org.jetbrains.exposed.sql.statements.api.ExposedSavepoint
import java.sql.SQLException
import java.util.concurrent.ThreadLocalRandom

class ThreadLocalTransactionManager(
private val db: Database,
Expand All @@ -21,6 +22,18 @@ class ThreadLocalTransactionManager(
@TestOnly
set

@Volatile
override var defaultMinRepetitionDelay: Long = db.config.defaultMinRepetitionDelay
@Deprecated("Use DatabaseConfig to define the defaultMinRepetitionDelay")
@TestOnly
set

@Volatile
override var defaultMaxRepetitionDelay: Long = db.config.defaultMaxRepetitionDelay
@Deprecated("Use DatabaseConfig to define the defaultMaxRepetitionDelay")
@TestOnly
set

@Volatile
override var defaultIsolationLevel: Int = db.config.defaultIsolationLevel
get() {
Expand Down Expand Up @@ -147,14 +160,19 @@ fun <T> transaction(db: Database? = null, statement: Transaction.() -> T): T =
db.transactionManager.defaultIsolationLevel,
db.transactionManager.defaultRepetitionAttempts,
db.transactionManager.defaultReadOnly,
db, statement
db,
db.transactionManager.defaultMinRepetitionDelay,
db.transactionManager.defaultMaxRepetitionDelay,
statement
)

fun <T> transaction(
transactionIsolation: Int,
repetitionAttempts: Int,
readOnly: Boolean = false,
db: Database? = null,
minRepetitionDelay: Long = 0,
maxRepetitionDelay: Long = 0,
statement: Transaction.() -> T
): T =
keepAndRestoreTransactionRefAfterRun(db) {
Expand Down Expand Up @@ -187,7 +205,16 @@ fun <T> transaction(
} finally {
TransactionManager.resetCurrent(currentManager)
}
} ?: inTopLevelTransaction(transactionIsolation, repetitionAttempts, readOnly, db, null, statement)
} ?: inTopLevelTransaction(
transactionIsolation,
repetitionAttempts,
readOnly,
db,
null,
minRepetitionDelay,
maxRepetitionDelay,
statement
)
}
}

Expand All @@ -197,6 +224,8 @@ fun <T> inTopLevelTransaction(
readOnly: Boolean = false,
db: Database? = null,
outerTransaction: Transaction? = null,
minRepetitionDelay: Long = 0,
maxRepetitionDelay: Long = 0,
statement: Transaction.() -> T
): T {

Expand All @@ -205,6 +234,11 @@ fun <T> inTopLevelTransaction(

val outerManager = outerTransaction?.db.transactionManager.takeIf { it.currentOrNull() != null }

var intermediateDelay = minRepetitionDelay
var retryInterval = if (repetitionAttempts > 0) {
maxOf((maxRepetitionDelay - minRepetitionDelay) / (repetitionAttempts + 1), 1)
} else 0

while (true) {
db?.let { db.transactionManager.let { m -> TransactionManager.resetCurrent(m) } }
val transaction = db.transactionManager.newTransaction(transactionIsolation, readOnly, outerTransaction)
Expand All @@ -221,6 +255,21 @@ fun <T> inTopLevelTransaction(
if (repetitions >= repetitionAttempts) {
throw e
}
// set delay value with an exponential backoff time period.
val delay = when {
minRepetitionDelay < maxRepetitionDelay -> {
intermediateDelay += retryInterval * repetitions
ThreadLocalRandom.current().nextLong(intermediateDelay , intermediateDelay + retryInterval)
}
minRepetitionDelay == maxRepetitionDelay -> minRepetitionDelay
else -> 0
}
exposedLogger.warn("Wait $delay milliseconds before retrying")
try {
Thread.sleep(delay)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you tell me how it will work with newSuspendTransaction?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to do some tests using newSuspendedTransaction with the repetition delay. Here's what I found :

  • newSuspendedTransaction calls suspendedTransactionAsyncInternal which does not handle transaction repetitions
try {
        transaction.statement().apply {
            if (shouldCommit) transaction.commit()
        }
    } catch (e: SQLException) {
        handleSQLException(e, transaction, 1)
        throw e
  • if newSuspendedTransaction is called within a normal transaction, then the transactions are repeated with the calculated delay.

Is there something else I should check?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a user, I would expect that both transaction and newSuspendTransaction had the same retry policy

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi. Sorry I couldn't find the time to check your response until today.

I don't have any experience working with coroutines but I tried to implement the same retry policy in newSuspendTransaction as the one in transaction.

Unfortunately, I couldn't make it work : When implementing the while loop to handle retries, I got the following error in suspendedTransactionAsyncInternal, Which I don't really understand:

Type mismatch: inferred type is Deferred<Unit> but Deferred<T> was expected

I didn't pursue this further because for me the scope of this pull request is to be able to set a delay whenever I have a repetition attempt. Which is currently only handled in transaction.

I think adding repetition handling in newSuspendTransaction can be added in new separate issue. What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, let's move it to the separate issue

} catch (e: InterruptedException) {
// Do nothing
}
} catch (e: Throwable) {
val currentStatement = transaction.currentStatement
transaction.rollbackLoggingException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ private object NotInitializedManager : TransactionManager {

override var defaultRepetitionAttempts: Int = -1

override var defaultMinRepetitionDelay: Long = 0

override var defaultMaxRepetitionDelay: Long = 0

override fun newTransaction(isolation: Int, readOnly: Boolean, outerTransaction: Transaction?): Transaction =
error("Please call Database.connect() before using this code")

Expand All @@ -51,6 +55,10 @@ interface TransactionManager {

var defaultRepetitionAttempts: Int

var defaultMinRepetitionDelay: Long

var defaultMaxRepetitionDelay: Long

fun newTransaction(
isolation: Int = defaultIsolationLevel,
readOnly: Boolean = defaultReadOnly,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ class EntityTests : DatabaseTestsBase() {
}

private fun <T> newTransaction(statement: Transaction.() -> T) =
inTopLevelTransaction(TransactionManager.manager.defaultIsolationLevel, 1, false, null, null, statement)
inTopLevelTransaction(TransactionManager.manager.defaultIsolationLevel, 1, false, null, null, 0, 0, statement)

@Test fun sharingEntityBetweenTransactions() {
withTables(Humans) {
Expand Down
8 changes: 6 additions & 2 deletions spring-transaction/api/spring-transaction.api
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
public final class org/jetbrains/exposed/spring/SpringTransactionManager : org/springframework/jdbc/datasource/DataSourceTransactionManager, org/jetbrains/exposed/sql/transactions/TransactionManager {
public fun <init> (Ljavax/sql/DataSource;Lorg/jetbrains/exposed/sql/DatabaseConfig;ZZI)V
public synthetic fun <init> (Ljavax/sql/DataSource;Lorg/jetbrains/exposed/sql/DatabaseConfig;ZZIILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun <init> (Ljavax/sql/DataSource;Lorg/jetbrains/exposed/sql/DatabaseConfig;ZZIJJ)V
public synthetic fun <init> (Ljavax/sql/DataSource;Lorg/jetbrains/exposed/sql/DatabaseConfig;ZZIJJILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun bindTransactionToThread (Lorg/jetbrains/exposed/sql/Transaction;)V
public fun currentOrNull ()Lorg/jetbrains/exposed/sql/Transaction;
public fun getDefaultIsolationLevel ()I
public fun getDefaultMaxRepetitionDelay ()J
public fun getDefaultMinRepetitionDelay ()J
public fun getDefaultReadOnly ()Z
public fun getDefaultRepetitionAttempts ()I
public fun newTransaction (IZLorg/jetbrains/exposed/sql/Transaction;)Lorg/jetbrains/exposed/sql/Transaction;
public fun setDefaultIsolationLevel (I)V
public fun setDefaultMaxRepetitionDelay (J)V
public fun setDefaultMinRepetitionDelay (J)V
public fun setDefaultReadOnly (Z)V
public fun setDefaultRepetitionAttempts (I)V
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ class SpringTransactionManager(
databaseConfig: DatabaseConfig = DatabaseConfig { },
private val showSql: Boolean = false,
@Volatile override var defaultReadOnly: Boolean = databaseConfig.defaultReadOnly,
@Volatile override var defaultRepetitionAttempts: Int = databaseConfig.defaultRepetitionAttempts
@Volatile override var defaultRepetitionAttempts: Int = databaseConfig.defaultRepetitionAttempts,
@Volatile override var defaultMinRepetitionDelay: Long = databaseConfig.defaultMinRepetitionDelay,
@Volatile override var defaultMaxRepetitionDelay: Long = databaseConfig.defaultMaxRepetitionDelay
) : DataSourceTransactionManager(dataSource), TransactionManager {

init {
Expand Down