From 947e3375d7d19d6258323a3d9610f153af17ee29 Mon Sep 17 00:00:00 2001 From: oshai Date: Thu, 21 Feb 2019 23:29:43 +0200 Subject: [PATCH 1/2] max ttl should not cause query exception if passed, only not given as new connection on take --- .../jasync/sql/db/ConcreteConnectionBase.kt | 4 +- .../sql/db/pool/ActorBasedObjectPool.kt | 45 +++++++++++-------- .../sql/db/pool/MaxTtlPassedException.kt | 4 +- .../db/pool/AbstractAsyncObjectPoolSpec.kt | 11 ++--- .../sql/db/pool/ActorBasedObjectPoolTest.kt | 10 +++-- 5 files changed, 40 insertions(+), 34 deletions(-) diff --git a/db-async-common/src/main/java/com/github/jasync/sql/db/ConcreteConnectionBase.kt b/db-async-common/src/main/java/com/github/jasync/sql/db/ConcreteConnectionBase.kt index 2f52d934..68f17e79 100644 --- a/db-async-common/src/main/java/com/github/jasync/sql/db/ConcreteConnectionBase.kt +++ b/db-async-common/src/main/java/com/github/jasync/sql/db/ConcreteConnectionBase.kt @@ -14,9 +14,11 @@ import com.github.jasync.sql.db.util.onCompleteAsync import java.util.concurrent.CompletableFuture abstract class ConcreteConnectionBase( - val configuration: Configuration, override val creationTime: Long = System.currentTimeMillis() + val configuration: Configuration ) : ConcreteConnection { + override val creationTime: Long = System.currentTimeMillis() + override fun inTransaction(f: (Connection) -> CompletableFuture): CompletableFuture { return this.sendQuery("BEGIN").flatMapAsync(configuration.executionContext) { val p = CompletableFuture() diff --git a/db-async-common/src/main/java/com/github/jasync/sql/db/pool/ActorBasedObjectPool.kt b/db-async-common/src/main/java/com/github/jasync/sql/db/pool/ActorBasedObjectPool.kt index 980d1e78..771885d8 100644 --- a/db-async-common/src/main/java/com/github/jasync/sql/db/pool/ActorBasedObjectPool.kt +++ b/db-async-common/src/main/java/com/github/jasync/sql/db/pool/ActorBasedObjectPool.kt @@ -8,7 +8,6 @@ import com.github.jasync.sql.db.util.failed import com.github.jasync.sql.db.util.map import com.github.jasync.sql.db.util.mapTry import com.github.jasync.sql.db.util.onComplete -import jdk.nashorn.internal.runtime.regexp.joni.Config.log import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CoroutineStart import kotlinx.coroutines.SupervisorJob @@ -51,7 +50,7 @@ internal constructor( configuration: PoolConfiguration, testItemsPeriodically: Boolean, extraTimeForTimeoutCompletion: Long = TimeUnit.SECONDS.toMillis(30) - ) : AsyncObjectPool, CoroutineScope { +) : AsyncObjectPool, CoroutineScope { @Suppress("unused", "RedundantVisibilityModifier") public constructor( @@ -329,17 +328,21 @@ private class ObjectPoolActor( availableItems.forEach { val item = it.item logger.trace { "test: ${item.id} available ${it.timeElapsed} ms" } - if (it.timeElapsed > configuration.maxIdle) { - logger.trace { "releasing idle item ${item.id}" } - item.destroy() - } else if (configuration.maxObjectTtl !=null && System.currentTimeMillis() - item.creationTime > configuration.maxObjectTtl) { - logger.trace { "releasing item past ttl ${item.id}" } - item.destroy() - } else { - val test = objectFactory.test(item) - inUseItems[item] = ItemInUseHolder(item.id, isInTest = true, testFuture = test) - test.mapTry { _, t -> - offerOrLog(GiveBack(item, CompletableFuture(), t, originalTime = it.time)) { "test item" } + when { + it.timeElapsed > configuration.maxIdle -> { + logger.trace { "releasing idle item ${item.id}" } + item.destroy() + } + configuration.maxObjectTtl != null && System.currentTimeMillis() - item.creationTime > configuration.maxObjectTtl -> { + logger.trace { "releasing item past ttl ${item.id}" } + item.destroy() + } + else -> { + val test = objectFactory.test(item) + inUseItems[item] = ItemInUseHolder(item.id, isInTest = true, testFuture = test) + test.mapTry { _, t -> + offerOrLog(GiveBack(item, CompletableFuture(), t, originalTime = it.time)) { "test item" } + } } } } @@ -452,6 +455,7 @@ private class ObjectPoolActor( private fun borrowFirstAvailableItem(future: CompletableFuture): Boolean { val itemHolder = availableItems.remove() try { + validateTtl(itemHolder.item) itemHolder.item.borrowTo(future) return true } catch (e: Exception) { @@ -461,6 +465,13 @@ private class ObjectPoolActor( return false } + private fun validateTtl(item: T) { + val age = System.currentTimeMillis() - item.creationTime + if (configuration.maxObjectTtl != null && age > configuration.maxObjectTtl) { + throw MaxTtlPassedException(item.id, age, configuration.maxObjectTtl) + } + } + private val totalItems: Int get() = inUseItems.size + inCreateItems.size + availableItems.size private fun createNewItemPutInWaitQueue(message: Take) { @@ -494,15 +505,11 @@ private class ObjectPoolActor( } } - private fun validate(a: T) { - val tried = objectFactory.validate(a) + private fun validate(item: T) { + val tried = objectFactory.validate(item) when (tried) { is Failure -> throw tried.exception } - val age = System.currentTimeMillis() - a.creationTime - if (configuration.maxObjectTtl!=null && age > configuration.maxObjectTtl) { - throw MaxTtlPassedException(a, age, configuration.maxObjectTtl) - } } } diff --git a/db-async-common/src/main/java/com/github/jasync/sql/db/pool/MaxTtlPassedException.kt b/db-async-common/src/main/java/com/github/jasync/sql/db/pool/MaxTtlPassedException.kt index cf70d94d..2c7876f1 100644 --- a/db-async-common/src/main/java/com/github/jasync/sql/db/pool/MaxTtlPassedException.kt +++ b/db-async-common/src/main/java/com/github/jasync/sql/db/pool/MaxTtlPassedException.kt @@ -1,4 +1,4 @@ package com.github.jasync.sql.db.pool -class MaxTtlPassedException(obj: PooledObject, age: Long, maxTtl: Long) : - RuntimeException("Object ${obj.id} aged out of pool with age $age over maxTtl $maxTtl") {} \ No newline at end of file +class MaxTtlPassedException(id: String, age: Long, maxTtl: Long) : + RuntimeException("Object $id passed max ttl with age $age over maxTtl $maxTtl") {} \ No newline at end of file diff --git a/db-async-common/src/test/java/com/github/jasync/sql/db/pool/AbstractAsyncObjectPoolSpec.kt b/db-async-common/src/test/java/com/github/jasync/sql/db/pool/AbstractAsyncObjectPoolSpec.kt index 1991d8bf..ea6babb8 100644 --- a/db-async-common/src/test/java/com/github/jasync/sql/db/pool/AbstractAsyncObjectPoolSpec.kt +++ b/db-async-common/src/test/java/com/github/jasync/sql/db/pool/AbstractAsyncObjectPoolSpec.kt @@ -78,11 +78,11 @@ abstract class AbstractAsyncObjectPoolSpec> { //reset(factory) // Considered bad form, but necessary as we depend on previous state in these tests //"takes maxObjects back" - val returns = taken.subList(0,taken.size-1).map { + val returns = taken.map { p.giveBack(it).get() } - assertEquals(4, returns.size) - (0..3).forEach { + assertEquals(5, returns.size) + (0..4).forEach { assertThat(returns[it]).isEqualTo(p) } @@ -93,11 +93,6 @@ abstract class AbstractAsyncObjectPoolSpec> { //"destroy down to maxIdle widgets" Thread.sleep(3000) - verify(exactly = 4) { factory.destroy(any()) } - // aged out widget should be destroyed on giveback - verifyExceptionInHierarchy(MaxTtlPassedException::class.java) { - p.giveBack(taken.last()).get() - } verify(exactly = 5) { factory.destroy(any()) } } diff --git a/db-async-common/src/test/java/com/github/jasync/sql/db/pool/ActorBasedObjectPoolTest.kt b/db-async-common/src/test/java/com/github/jasync/sql/db/pool/ActorBasedObjectPoolTest.kt index 52da2468..53347ef7 100644 --- a/db-async-common/src/test/java/com/github/jasync/sql/db/pool/ActorBasedObjectPoolTest.kt +++ b/db-async-common/src/test/java/com/github/jasync/sql/db/pool/ActorBasedObjectPoolTest.kt @@ -4,7 +4,6 @@ import com.github.jasync.sql.db.util.FP import com.github.jasync.sql.db.util.Try import com.github.jasync.sql.db.verifyException import org.assertj.core.api.Assertions.assertThat -import org.assertj.core.api.Assertions.assertThatExceptionOfType import org.awaitility.kotlin.await import org.awaitility.kotlin.matches import org.awaitility.kotlin.untilCallTo @@ -168,12 +167,15 @@ class ActorBasedObjectPoolTest { } @Test - fun `on giveback items pool should reclaim aged-out items`() { + fun `on take items pool should reclaim items pass ttl`() { tested = ActorBasedObjectPool(factory, configuration.copy(maxObjectTtl = 50), false) val widget = tested.take().get() Thread.sleep(70) - assertThatExceptionOfType(ExecutionException::class.java).isThrownBy { tested.giveBack(widget).get() }.withCauseInstanceOf(MaxTtlPassedException::class.java) - assertThat(tested.availableItems).isEmpty() + tested.giveBack(widget).get() + val widget2 = tested.take().get() + assertThat(widget).isNotEqualTo(widget2) + assertThat(factory.created.size).isEqualTo(2) + assertThat(factory.destroyed[0]).isEqualTo(widget) } @Test From d2f8131c2987f6f381f657e088309c6adb20af66 Mon Sep 17 00:00:00 2001 From: oshai Date: Thu, 21 Feb 2019 23:35:33 +0200 Subject: [PATCH 2/2] moved max ttl to be last parameter to preserve compatibility --- .../github/jasync/sql/db/ConnectionPoolConfiguration.kt | 8 ++++---- .../com/github/jasync/sql/db/pool/PoolConfiguration.kt | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/db-async-common/src/main/java/com/github/jasync/sql/db/ConnectionPoolConfiguration.kt b/db-async-common/src/main/java/com/github/jasync/sql/db/ConnectionPoolConfiguration.kt index 5f081ff7..906c0f75 100644 --- a/db-async-common/src/main/java/com/github/jasync/sql/db/ConnectionPoolConfiguration.kt +++ b/db-async-common/src/main/java/com/github/jasync/sql/db/ConnectionPoolConfiguration.kt @@ -57,7 +57,6 @@ data class ConnectionPoolConfiguration @JvmOverloads constructor( val username: String = "dbuser", val password: String? = null, val maxActiveConnections: Int = 1, - val maxConnectionTtl: Long? = null, val maxIdleTime: Long = TimeUnit.MINUTES.toMillis(1), val maxPendingQueries: Int = Int.MAX_VALUE, val connectionValidationInterval: Long = 5000, @@ -72,7 +71,8 @@ data class ConnectionPoolConfiguration @JvmOverloads constructor( val maximumMessageSize: Int = 16777216, val allocator: ByteBufAllocator = PooledByteBufAllocator.DEFAULT, val applicationName: String? = null, - val interceptors: List> = emptyList() + val interceptors: List> = emptyList(), + val maxConnectionTtl: Long? = null ) { init { @@ -134,7 +134,6 @@ data class ConnectionPoolConfigurationBuilder @JvmOverloads constructor( var password: String? = null, var maxActiveConnections: Int = 1, var maxIdleTime: Long = TimeUnit.MINUTES.toMillis(1), - var maxConnectionTtl: Long? = null, var maxPendingQueries: Int = Int.MAX_VALUE, var connectionValidationInterval: Long = 5000, var connectionCreateTimeout: Long = 5000, @@ -148,7 +147,8 @@ data class ConnectionPoolConfigurationBuilder @JvmOverloads constructor( var maximumMessageSize: Int = 16777216, var allocator: ByteBufAllocator = PooledByteBufAllocator.DEFAULT, var applicationName: String? = null, - var interceptors: MutableList> = mutableListOf>() + var interceptors: MutableList> = mutableListOf>(), + var maxConnectionTtl: Long? = null ) { fun build(): ConnectionPoolConfiguration = ConnectionPoolConfiguration( host = host, diff --git a/db-async-common/src/main/java/com/github/jasync/sql/db/pool/PoolConfiguration.kt b/db-async-common/src/main/java/com/github/jasync/sql/db/pool/PoolConfiguration.kt index e8589ac6..412eccdb 100644 --- a/db-async-common/src/main/java/com/github/jasync/sql/db/pool/PoolConfiguration.kt +++ b/db-async-common/src/main/java/com/github/jasync/sql/db/pool/PoolConfiguration.kt @@ -23,12 +23,12 @@ data class PoolConfiguration @JvmOverloads constructor( val maxObjects: Int, val maxIdle: Long, val maxQueueSize: Int, - val maxObjectTtl: Long? = null, val validationInterval: Long = 5000, val createTimeout: Long = 5000, val testTimeout: Long = 5000, val queryTimeout: Long? = null, - val coroutineDispatcher: CoroutineDispatcher = Dispatchers.Default + val coroutineDispatcher: CoroutineDispatcher = Dispatchers.Default, + val maxObjectTtl: Long? = null ) { companion object { @Suppress("unused")