Skip to content

Commit

Permalink
Require maxConnections in DataSourceJdbcDataSource
Browse files Browse the repository at this point in the history
As mentioned in #1678, `Database.forDataSource` did not require or even
allow an easy way to specify the maximum number of connections, which is
essential for the deadlock fix from #1461 to work. This commit adds a
parameter (without a default, intentionally breaking source
compatibility) and additional scaladoc comments.
  • Loading branch information
szeiger committed Jan 24, 2017
1 parent 0686d64 commit 6ed7333
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ abstract class JdbcTestDB(val confName: String) extends SqlTestDB {
profile.backend.Database.forSource(new JdbcDataSource {
def createConnection(): Connection = wrappedConn
def close(): Unit = ()
val maxConnections: Option[Int] = Some(1)
}, executor)
}
final def blockingRunOnSession[R](f: ExecutionContext => DBIOAction[R, NoStream, Nothing])(implicit session: profile.Backend#Session): R = {
Expand Down
31 changes: 23 additions & 8 deletions slick/src/main/scala/slick/jdbc/JdbcBackend.scala
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,28 @@ trait JdbcBackend extends RelationalBackend {
def forSource(source: JdbcDataSource, executor: AsyncExecutor = AsyncExecutor.default()) =
new DatabaseDef(source, executor)

/** Create a Database based on a DataSource. */
def forDataSource(ds: DataSource, executor: AsyncExecutor = AsyncExecutor.default(), keepAliveConnection: Boolean = false): DatabaseDef =
forSource(new DataSourceJdbcDataSource(ds, keepAliveConnection), executor)
/** Create a Database based on a DataSource.
*
* @param ds The DataSource to use.
* @param maxConnection The maximum number of connections that the DataSource can provide. This is necessary to
* prevent deadlocks when scheduling database actions. Use `None` if there is no hard limit.
* @param executor The AsyncExecutor for scheduling database actions.
* @param keepAliveConnection If this is set to true, one extra connection will be opened as soon as the database
* is accessed for the first time, and kept open until `close()` is called. This is
* useful for named in-memory databases in test environments.
*/
def forDataSource(ds: DataSource, maxConnections: Option[Int], executor: AsyncExecutor = AsyncExecutor.default(), keepAliveConnection: Boolean = false): DatabaseDef =
forSource(new DataSourceJdbcDataSource(ds, keepAliveConnection, maxConnections), executor)

/** Create a Database based on the JNDI name of a DataSource. */
def forName(name: String, executor: AsyncExecutor = null) = new InitialContext().lookup(name) match {
case ds: DataSource => forDataSource(ds, executor match {
/** Create a Database based on the JNDI name of a DataSource.
*
* @param ds The name of the DataSource to use.
* @param maxConnection The maximum number of connections that the DataSource can provide. This is necessary to
* prevent deadlocks when scheduling database actions. Use `None` if there is no hard limit.
* @param executor The AsyncExecutor for scheduling database actions.
*/
def forName(name: String, maxConnections: Option[Int], executor: AsyncExecutor = null) = new InitialContext().lookup(name) match {
case ds: DataSource => forDataSource(ds, maxConnections, executor match {
case null => AsyncExecutor.default(name)
case e => e
})
Expand All @@ -100,7 +115,7 @@ trait JdbcBackend extends RelationalBackend {
def forURL(url: String, user: String = null, password: String = null, prop: Properties = null, driver: String = null,
executor: AsyncExecutor = AsyncExecutor.default(), keepAliveConnection: Boolean = false,
classLoader: ClassLoader = ClassLoaderUtil.defaultClassLoader): DatabaseDef =
forDataSource(new DriverDataSource(url, user, password, prop, driver, classLoader = classLoader), executor, keepAliveConnection)
forDataSource(new DriverDataSource(url, user, password, prop, driver, classLoader = classLoader), None, executor, keepAliveConnection)

/** Create a Database that uses the DriverManager to open new connections. */
def forURL(url: String, prop: Map[String, String]): Database = {
Expand All @@ -114,7 +129,7 @@ trait JdbcBackend extends RelationalBackend {
* This is needed to open a JDBC URL with a driver that was not loaded by the system ClassLoader. */
def forDriver(driver: Driver, url: String, user: String = null, password: String = null, prop: Properties = null,
executor: AsyncExecutor = AsyncExecutor.default()): DatabaseDef =
forDataSource(new DriverDataSource(url, user, password, prop, driverObject = driver), executor)
forDataSource(new DriverDataSource(url, user, password, prop, driverObject = driver), None, executor)

/** Load a database configuration through [[https://github.com/typesafehub/config Typesafe Config]].
*
Expand Down
13 changes: 7 additions & 6 deletions slick/src/main/scala/slick/jdbc/JdbcDataSource.scala
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,10 @@ trait JdbcDataSource extends Closeable {
* Otherwise no action is taken. */
def close(): Unit

/** If this object represents a connection pool managed directly by Slick, return the maximum pool size
* Otherwise return None.
* The AsyncExecutor uses this to make sure to not schedule any more work (that requires a connection) when all pooled JDBC connections are in use.
/** If this object represents a connection pool with a limited size, return the maximum pool size.
* Otherwise return None. This is required to prevent deadlocks when scheduling database actions.
*/
val maxConnections: Option[Int] = None

val maxConnections: Option[Int]
}

object JdbcDataSource extends Logging {
Expand Down Expand Up @@ -60,6 +58,7 @@ trait JdbcDataSourceFactory {

/** A JdbcDataSource for a `DataSource` */
class DataSourceJdbcDataSource(val ds: DataSource, val keepAliveConnection: Boolean,
val maxConnections: Option[Int],
val connectionPreparer: ConnectionPreparer = null) extends JdbcDataSource {
private[this] var openedKeepAliveConnection: Connection = null

Expand Down Expand Up @@ -101,7 +100,7 @@ object DataSourceJdbcDataSource extends JdbcDataSourceFactory {
BeanConfigurator.configure(ds, c.toProperties, Set("url", "user", "password", "properties", "driver", "driverClassName"))
ds
}
new DataSourceJdbcDataSource(ds, c.getBooleanOr("keepAliveConnection"), new ConnectionPreparer(c))
new DataSourceJdbcDataSource(ds, c.getBooleanOr("keepAliveConnection"), None, new ConnectionPreparer(c))
}
}

Expand All @@ -123,6 +122,8 @@ trait DriverBasedJdbcDataSource extends JdbcDataSource {
def deregisterDriver(): Boolean =
if(registeredDriver ne null) { DriverManager.deregisterDriver(registeredDriver); true }
else false

val maxConnections: Option[Int] = None
}

/** A JdbcDataSource for lookup via a `Driver` or the `DriverManager` */
Expand Down
8 changes: 5 additions & 3 deletions slick/src/sphinx/code/Connection.scala
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,22 @@ object Connection extends App {
val coffees = TableQuery[Coffees]
if (false){
val dataSource = null.asInstanceOf[javax.sql.DataSource]
val size = 42
//#forDataSource
val db = Database.forDataSource(dataSource: javax.sql.DataSource)
val db = Database.forDataSource(dataSource: javax.sql.DataSource, Some(size: Int))
//#forDataSource
}
if (false){
val dataSource = null.asInstanceOf[slick.jdbc.DatabaseUrlDataSource]
//#forDatabaseURL
val db = Database.forDataSource(dataSource: slick.jdbc.DatabaseUrlDataSource)
val db = Database.forDataSource(dataSource: slick.jdbc.DatabaseUrlDataSource, None)
//#forDatabaseURL
}
if(false) {
val jndiName = ""
val size = 42
//#forName
val db = Database.forName(jndiName: String)
val db = Database.forName(jndiName: String, Some(size: Int))
//#forName
}
;{
Expand Down
12 changes: 7 additions & 5 deletions slick/src/sphinx/database.rst
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ is set. You may also define a custom environment variable with standard
Typesafe Config syntax, such as ``${?MYSQL_DATABASE_URL}``.

Or you may pass a :api:`DatabaseUrlDataSource <slick/jdbc/DatabaseUrlDataSource>` object to
:api:`forDataSource <slick.jdbc.JdbcBackend$DatabaseFactoryDef@forDataSource(DataSource,AsyncExecutor,Boolean):DatabaseDef>`.
:api:`forDataSource <slick.jdbc.JdbcBackend$DatabaseFactoryDef@forDataSource(DataSource,Option[Int],AsyncExecutor,Boolean):DatabaseDef>`.

.. includecode:: code/Connection.scala#forDatabaseURL

Expand All @@ -63,8 +63,9 @@ Using a DataSource
------------------

You can pass a :javaapi:`DataSource <javax/sql/DataSource>` object to
:api:`forDataSource <slick.jdbc.JdbcBackend$DatabaseFactoryDef@forDataSource(DataSource,AsyncExecutor,Boolean):DatabaseDef>`.
If you got it from the connection pool of your application framework, this plugs the pool into Slick.
:api:`forDataSource <slick.jdbc.JdbcBackend$DatabaseFactoryDef@forDataSource(DataSource,Option[Int],AsyncExecutor,Boolean):DatabaseDef>`.
If you got it from the connection pool of your application framework, this plugs the pool into Slick. If the pool has
a size limit, the correct size should always be specified.

.. includecode:: code/Connection.scala#forDataSource

Expand All @@ -74,8 +75,9 @@ Using a JNDI Name
-----------------

If you are using :wikipedia:`JNDI` you can pass a JNDI name to
:api:`forName <slick.jdbc.JdbcBackend$DatabaseFactoryDef@forName(String,AsyncExecutor):DatabaseDef>`
under which a :javaapi:`DataSource <javax/sql/DataSource>` object can be looked up.
:api:`forName <slick.jdbc.JdbcBackend$DatabaseFactoryDef@forName(String,Option[Int],AsyncExecutor):DatabaseDef>`
under which a :javaapi:`DataSource <javax/sql/DataSource>` object can be looked up. If the data source has
a limit in the number of connections it can provide, the correct size should always be specified.

.. includecode:: code/Connection.scala#forName

Expand Down

0 comments on commit 6ed7333

Please sign in to comment.