diff --git a/jdbc/src/main/scala/app/softnetwork/persistence/jdbc/db/SlickDatabase.scala b/jdbc/src/main/scala/app/softnetwork/persistence/jdbc/db/SlickDatabase.scala index 93f55f0..a61b0b8 100644 --- a/jdbc/src/main/scala/app/softnetwork/persistence/jdbc/db/SlickDatabase.scala +++ b/jdbc/src/main/scala/app/softnetwork/persistence/jdbc/db/SlickDatabase.scala @@ -4,14 +4,45 @@ import akka.persistence.jdbc.db.SlickExtension import akka.{actor => classic} import app.softnetwork.io._ import app.softnetwork.utils.ClasspathResources -import com.typesafe.config.Config +import com.typesafe.config.{Config, ConfigValueFactory} import org.slf4j.{Logger, LoggerFactory} import slick.jdbc.JdbcBackend.{Database, Session} import java.nio.file.{Files, Paths} import java.sql.Statement +import java.util.concurrent.atomic.AtomicLong import scala.util.{Failure, Success, Try} +object SlickDatabase { + + /** Config path of the HikariCP pool name, relative to the config handed to + * [[akka.persistence.jdbc.db.SlickExtension]]. akka-persistence-jdbc builds every non-shared + * pool via `slick.jdbc.JdbcBackend.Database.forConfig("slick.db", config)`, and Slick's + * `HikariCPJdbcDataSource` reads its `poolName` from this exact path (defaulting to the path + * itself, `"slick.db"`, when absent). + */ + private[db] val PoolNamePath = "slick.db.poolName" + + /** Config path of the `slick.db` block. */ + private[db] val SlickDbPath = "slick.db" + + /** Monotonic counter (per class loader) that guarantees uniqueness of generated pool names even + * when several instances of the same provider class are created in the same class loader. + */ + private val poolNameCounter = new AtomicLong(0L) + + private[db] def nextPoolNameSuffix(): Long = poolNameCounter.incrementAndGet() + + /** Keep only characters that are safe both as a JMX `ObjectName` value (HikariCP registers the + * pool MBean as `com.zaxxer.hikari:type=Pool (<poolName>)` without quoting) and as a + * Prometheus label value. Anything else collapses to `_`. + */ + private[db] def sanitize(s: String): String = { + val cleaned = s.replaceAll("[^A-Za-z0-9._-]", "_") + if (cleaned.isEmpty) "provider" else cleaned + } +} + trait SlickDatabase extends ClasspathResources { override lazy val log: Logger = LoggerFactory getLogger getClass.getName @@ -36,9 +67,48 @@ trait SlickDatabase extends ClasspathResources { lazy val slickProfile: String = config.getString("slick.profile") + /** A unique HikariCP pool name for this provider's connection pool, of the form + * `slick.db-<providerClass>-<n>`. + * + * Each trait that mixes in [[SlickDatabase]] (`JdbcSchema`, `JdbcStateProvider`, + * `ColumnMappedJdbcStateProvider`, `JdbcOffsetProvider`, ...) builds its own non-shared pool. By + * default akka-persistence-jdbc names them all `slick.db`, which collides in a single JVM the + * moment `slick.db.registerMbeans = true` is set (only the first pool registers its MBean, the + * rest fail with `JMX name (slick.db) is already registered`) and likewise breaks HikariCP's + * native Prometheus tracker. Naming each pool distinctly removes the collision and enables + * per-pool connection metrics. + * + * Override to customise the naming scheme. + */ + protected def poolName: String = { + // `Class.getSimpleName` can throw `InternalError` ("Malformed class name") on JDK 8 for some + // synthetic/anonymous Scala class names, so fall back to the (always-safe) full name. + val simpleName = Try(getClass.getSimpleName).toOption.getOrElse("") + val cls = SlickDatabase.sanitize(if (simpleName.nonEmpty) simpleName else getClass.getName) + s"slick.db-$cls-${SlickDatabase.nextPoolNameSuffix()}" + } + + /** The config actually handed to [[akka.persistence.jdbc.db.SlickExtension]] when building + * [[db]]. + * + * When a `slick.db` block is present and no `poolName` has been configured explicitly, inject a + * unique [[poolName]] so the resulting HikariCP pool is distinctly named. An explicitly + * configured `slick.db.poolName` is left untouched, and any config without a `slick.db` block is + * passed through unchanged. + */ + private def dbConfig: Config = { + if (config.hasPath(SlickDatabase.SlickDbPath) && !config.hasPath(SlickDatabase.PoolNamePath)) { + val name = poolName + log.debug(s"Assigning HikariCP pool name '$name' to ${getClass.getName}") + config.withValue(SlickDatabase.PoolNamePath, ConfigValueFactory.fromAnyRef(name)) + } else { + config + } + } + lazy val db: Database = { log.info(slickProfile) - val db = SlickExtension(classicSystem).database(config).database + val db = SlickExtension(classicSystem).database(dbConfig).database classicSystem.registerOnTermination(shutdown()) db } diff --git a/jdbc/testkit/src/test/scala/app/softnetwork/persistence/jdbc/db/SlickDatabasePoolNameSpec.scala b/jdbc/testkit/src/test/scala/app/softnetwork/persistence/jdbc/db/SlickDatabasePoolNameSpec.scala new file mode 100644 index 0000000..24d390b --- /dev/null +++ b/jdbc/testkit/src/test/scala/app/softnetwork/persistence/jdbc/db/SlickDatabasePoolNameSpec.scala @@ -0,0 +1,74 @@ +package app.softnetwork.persistence.jdbc.db + +import akka.actor +import app.softnetwork.persistence.jdbc.scalatest.H2TestKit +import com.typesafe.config.{Config, ConfigValueFactory} +import com.zaxxer.hikari.HikariDataSource +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import java.lang.management.ManagementFactory +import javax.management.ObjectName + +/** Regression test for issue #13: every `SlickDatabase` provider builds its own HikariCP pool, and + * akka-persistence-jdbc names them all `slick.db`. In a single JVM that produces multiple pools + * sharing one name, which collides on JMX registration (and breaks HikariCP's Prometheus tracker) + * as soon as `registerMbeans = true`. The fix gives each provider a distinct pool name. + */ +class SlickDatabasePoolNameSpec extends AnyFlatSpec with Matchers with H2TestKit { + + /** A bare [[SlickDatabase]] provider sharing this suite's actor system and (optionally tweaked) + * config — stands in for the many traits that mix [[SlickDatabase]] in. + */ + private def newProvider(cfg: Config = config): SlickDatabase = { + val sys = classicSystem + new SlickDatabase { + override implicit def classicSystem: actor.ActorSystem = sys + override def config: Config = cfg + } + } + + private def poolNameOf(sd: SlickDatabase): String = + sd.dataSource.asInstanceOf[HikariDataSource].getPoolName + + "SlickDatabase" should "assign a unique HikariCP pool name to each provider" in { + val a = newProvider() + val b = newProvider() + try { + val nameA = poolNameOf(a) + val nameB = poolNameOf(b) + nameA should startWith("slick.db-") + nameB should startWith("slick.db-") + nameA should not be nameB + } finally { + a.shutdown() + b.shutdown() + } + } + + it should "leave an explicitly configured slick.db.poolName untouched" in { + val cfg = + config.withValue("slick.db.poolName", ConfigValueFactory.fromAnyRef("custom-pool")) + val sd = newProvider(cfg) + try poolNameOf(sd) shouldBe "custom-pool" + finally sd.shutdown() + } + + it should "register a distinct JMX MBean per pool when registerMbeans is enabled" in { + val cfg = + config.withValue("slick.db.registerMbeans", ConfigValueFactory.fromAnyRef(true)) + val a = newProvider(cfg) + val b = newProvider(cfg) + try { + val server = ManagementFactory.getPlatformMBeanServer + val mbeanA = new ObjectName(s"com.zaxxer.hikari:type=Pool (${poolNameOf(a)})") + val mbeanB = new ObjectName(s"com.zaxxer.hikari:type=Pool (${poolNameOf(b)})") + mbeanA should not be mbeanB + server.isRegistered(mbeanA) shouldBe true + server.isRegistered(mbeanB) shouldBe true + } finally { + a.shutdown() + b.shutdown() + } + } +}