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

DROID opens multiple connection pools to the same database. #362

Closed
nishihatapalmer opened this issue Jan 27, 2020 · 9 comments
Closed

DROID opens multiple connection pools to the same database. #362

nishihatapalmer opened this issue Jan 27, 2020 · 9 comments
Assignees
Labels
Milestone

Comments

@nishihatapalmer
Copy link
Contributor

nishihatapalmer commented Jan 27, 2020

When DROID starts, it opens multiple connection pools to the same database. This can be seen in the logs when it starts, the HikariPool is initialised multiple times. This is slow and unnecessary.

I've traced why this happens in the git history.

  1. DROID used to have a single data source for the embedded derby database (which is all that is needed).
  2. A connection pool was added to improve performance. It's unlikely that this improved performance, since DROID opens a few connections to the database and holds on to them as long as it needs to. It isn't a server application trying to share scarce database connections among thousands of concurrent connections.
  3. A factory for the connection pool was added later. This seems to be because of strange initialisation / dependency issues in spring initialisation. The factory also reloads the driver class every time it creates a new pool.
  4. All the things that used to get the connection pool now get a new connection pool from the factory.

The net result is that all the things that just need an occasional database connection now get a connection pool to themselves (and try to reload the database driver).

@nishihatapalmer
Copy link
Contributor Author

I'm working on how to fix this. Everything still works, so there's no massive issue here, but it would be nice to clean this up.

@nishihatapalmer
Copy link
Contributor Author

Main classes are DerbyPooledDataSource and DerbyPooledDataSourceFactory.

@adamretter
Copy link
Contributor

@nishihatapalmer It's probably a Spring scope thing. If we set the Spring scope to singleton for the DerbyPooledDataSourceFactory we should only get one per JVM instance

@nishihatapalmer
Copy link
Contributor Author

@adamretter I think there's only one factory, but it creates a new DerbyPooledDataSource for each object that wants a data source.

@jcharlet
Copy link
Contributor

brilliant, thanks Matt for looking into this!

@nishihatapalmer
Copy link
Contributor Author

I've created a PR that fixes this issue (in that we only create one connection pool per profile).

See the PR for some additional comments.

@rhubner
Copy link
Contributor

rhubner commented Jan 29, 2020

Hi all my friends,

I think it can be fixed here :
https://github.com/digital-preservation/droid/blob/master/droid-results/src/main/java/uk/gov/nationalarchives/droid/profile/datasource/DerbyPooledDataSourceFactory.java#L141

Mark it as singleton. It should create only one DB pool per application context. Not as @adamretter mentioned per JVM. I'm not sure at the moment how this work when you have hierarchy of application contexts. Each should be separate application context, but they should inherit from master application context.

@nishihatapalmer
Copy link
Contributor Author

Yep, that works :)

Many thanks Radek!

@nishihatapalmer
Copy link
Contributor Author

I also misunderstood @adamretter 's suggestion to mark it as a singleton. I thought we were talking about a single factory, rather than a single output from the factory. Thanks Adam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants