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

Add facility to plug-in custom DB connection pools #1030

Merged
merged 1 commit into from Jun 4, 2015

Conversation

Projects
None yet
6 participants
@arteam
Member

arteam commented May 3, 2015

Dropwizard by default is shipped with the Tomcat JDBC Connection pool. It's mostly fine, but has some disadvantages (performance, limited metrics, handling network failures). It would be great if we had facility to plug-in other DB pools into Dropwizard. For example, Brett Wooldridge's
wonderful HikariCP or BoneCP.

Of cource, nothing stops us from implementing a factory, that creates a ManagedDataSource. The problem is that dropwizard-jdbi, dropwizard-hibernate anddropwizard-migrations modules require DataSourceFactory for the configuration. DataSourceFactory is closely tied with the Tomcat pool. So if we use an another pool, we are forced to write own factories for JDBI, Hibernate, etc...

A solution is to provide an interface that abstracts a configuration of a connection pool. Database manipulation modules will be linked with the interface, but not with DataSourceFactory. A custom pool factory just need to implement this interface, and it could be plug-in as a Tomcat pool
replacement.

If we writed Dropwizard from the scratch we would move the Tomcat pool to a separate module from dropwizard-db. But now we have many users and it would be a nightmare for them to migrate to a new implementation.

So we are forced to retain the Tomcat JDBC pool in dropwizard-db. New pool users should be advised to exclude it manually in the Maven configuration.

@joschi

View changes

Show outdated Hide outdated dropwizard-db/src/main/java/io/dropwizard/db/AbstractDataSourceFactory.java
@joschi

View changes

Show outdated Hide outdated dropwizard-db/src/main/java/io/dropwizard/db/AbstractDataSourceFactory.java
@joschi

View changes

Show outdated Hide outdated dropwizard-db/src/main/java/io/dropwizard/db/AbstractDataSourceFactory.java

@jplock jplock added this to the 0.9.0 milestone May 18, 2015

@arteam

This comment has been minimized.

Show comment
Hide comment
@arteam

arteam Jun 1, 2015

Member

Ping. Is any additional work needed in this PR?

Member

arteam commented Jun 1, 2015

Ping. Is any additional work needed in this PR?

@carlo-rtr

View changes

Show outdated Hide outdated dropwizard-db/src/main/java/io/dropwizard/db/DataSourceFactory.java
@carlo-rtr

This comment has been minimized.

Show comment
Hide comment
@carlo-rtr

carlo-rtr Jun 3, 2015

Member

One small nitpick, 'HelloWorldConfigurationin the example should declare the variables asPooledDataSourceFactory`.

Otherwise this PR looks good to me. Thanks for the hard work!

Member

carlo-rtr commented Jun 3, 2015

One small nitpick, 'HelloWorldConfigurationin the example should declare the variables asPooledDataSourceFactory`.

Otherwise this PR looks good to me. Thanks for the hard work!

@arteam

This comment has been minimized.

Show comment
Hide comment
@arteam

arteam Jun 4, 2015

Member

One small nitpick, 'HelloWorldConfigurationin the example should declare the variables
asPooledDataSourceFactory`.

I don't think it's farsightedly, because in this case we will need to enable polymorphic-configuration for PooledDataSourceFactory. And this looks like overkill, because nobody probably will want to change pool implementations in the configuration.

Member

arteam commented Jun 4, 2015

One small nitpick, 'HelloWorldConfigurationin the example should declare the variables
asPooledDataSourceFactory`.

I don't think it's farsightedly, because in this case we will need to enable polymorphic-configuration for PooledDataSourceFactory. And this looks like overkill, because nobody probably will want to change pool implementations in the configuration.

Add facility to plug-in custom DB connection pools
Dropwizard by default is shipped with the Tomcat JDBC Connection pool.
It's mostly fine, but has some disadvantages (performance, limited metrics,
handling network failures). It would be great if we had facility to
plug-in other DB pools into Dropwizard. For example, Brett Wooldridge's
wonderful HikariCP or BoneCP.

Of cource, nothing stops from implementing a factory, that creates a
ManagedDataSource. The problem is that dropwizard-jdbi, dropwizard-hibernate
and dropwizard-migrates modules require DataSourceFactory for the configuration.

DataSourceFactory is closely tied with the Tomcat pool. So if we use another
pool, we are forced to write own factories for JDBI, Hibernate, etc...

A solution is to provide an interface that abstracts a configuration of
a connection pool. Database manipulation modules will be linked with the
interface, but not with DataSourceFactory. A custom pool factory just need to
implement this interface, and it could be plug-in as a Tomcat pool
replacement.

If we writed Dropwizard from the scratch we would move the Tomcat pool
to a separate module from dropwizard-db. But now we have many users and it
would be a nightmare for them to migrate to a new implementation.

So we are forced to retain the Tomcat JDBC pool in dropwizard-db. New pool
users should be advised to exclude it manually in the Maven configuration.
@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Jun 4, 2015

Coverage Status

Coverage decreased (-0.09%) to 72.18% when pulling 1033cc6 on arteam:abstract_ds into b63feeb on dropwizard:master.

coveralls commented Jun 4, 2015

Coverage Status

Coverage decreased (-0.09%) to 72.18% when pulling 1033cc6 on arteam:abstract_ds into b63feeb on dropwizard:master.

carlo-rtr added a commit that referenced this pull request Jun 4, 2015

Merge pull request #1030 from arteam/abstract_ds
Add facility to plug-in custom DB connection pools

@carlo-rtr carlo-rtr merged commit bdfde90 into dropwizard:master Jun 4, 2015

@arteam arteam deleted the arteam:abstract_ds branch Jun 12, 2015

joschi added a commit that referenced this pull request Nov 3, 2015

Deprecate PooledDataSourceFactory#getHealthCheckValidation{Query,Time…
…out}

The PooledDataSourceFactory#getHealthCheckValidation{Query,Timeout} methods were introduced while
refactoring DataSourceFactory to allow custom DB connection pools (PR #1030) but broke the serialization
of the default configuration for DataSourceFactory. By deprecating these methods and pulling the
original DataSourceFactory#getValidation{Query,Timeout} methods into PooledDataSourceFactory, the
original behaviour was restored.

Fixes #1321, refs #1030

(cherry picked from commit 84c7463)

nickbabcock added a commit to nickbabcock/dropwizard that referenced this pull request Nov 13, 2015

Deprecate PooledDataSourceFactory#getHealthCheckValidation{Query,Time…
…out}

The PooledDataSourceFactory#getHealthCheckValidation{Query,Timeout} methods were introduced while
refactoring DataSourceFactory to allow custom DB connection pools (PR #1030) but broke the serialization
of the default configuration for DataSourceFactory. By deprecating these methods and pulling the
original DataSourceFactory#getValidation{Query,Timeout} methods into PooledDataSourceFactory, the
original behaviour was restored.

Fixes #1321, refs #1030

(cherry picked from commit 84c7463)
@stantonk

This comment has been minimized.

Show comment
Hide comment
@stantonk

stantonk Aug 24, 2016

@arteam is there documentation on how to use this? I don't quite see how:

Of cource, nothing stops us from implementing a factory, that creates a ManagedDataSource. The problem is that dropwizard-jdbi, dropwizard-hibernate anddropwizard-migrations modules require DataSourceFactory for the configuration. DataSourceFactory is closely tied with the Tomcat pool. So if we use an another pool, we are forced to write own factories for JDBI, Hibernate, etc...

has been sidestepped.

stantonk commented Aug 24, 2016

@arteam is there documentation on how to use this? I don't quite see how:

Of cource, nothing stops us from implementing a factory, that creates a ManagedDataSource. The problem is that dropwizard-jdbi, dropwizard-hibernate anddropwizard-migrations modules require DataSourceFactory for the configuration. DataSourceFactory is closely tied with the Tomcat pool. So if we use an another pool, we are forced to write own factories for JDBI, Hibernate, etc...

has been sidestepped.

@stantonk

This comment has been minimized.

Show comment
Hide comment
@stantonk

stantonk Aug 24, 2016

Seems like one is still left needing something like https://github.com/DeloitteDigitalUK/dropwizard-hikaricp, which as far as I can tell, still misses out on the nice features that DBIFactory provides.

stantonk commented Aug 24, 2016

Seems like one is still left needing something like https://github.com/DeloitteDigitalUK/dropwizard-hikaricp, which as far as I can tell, still misses out on the nice features that DBIFactory provides.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment