Hi there,
we are currently working on some security enhancements and noticed a shortcoming in HikariCP and were kinda wondered if this is by design?
So, the issue: you can't change (driver)properties once a pool is created?
From what we see, there is only one option: close/shutdown the old one and create a new one in his place.
The problem here would be Hibernate and others who would wrap the DataSource
Some options are to monitor the HikariConfig changes, wrap Hikari in another DataSource(questionable) or add a ConnectionManager.
If we have overseen something, please let us know.
We've just never had this request. I don't know of any technical barrier to implementing this.
@riba2101 Pull requests welcome. You would need to create a new DataSource instance, initialize it, atomically swap it in; I suggest changing the DataSource member to an AtomicReference to facilitate that.
Possibly add reinitialize() method to the HikariPoolMXBean and/or add a setDataSourceProperties(Properties properties) method to the HikariConfigMXBean that causes a re-initialization.
Would this allow a user to change MySQL password at runtime ? We're looking for a way to use vault for leased db credentials and don't want to have to restart service to enact updated credentials
This is the exact same problem we had. What we ended up doing was create a datasource that knows how to communicate with vault and supplied it to Hikari.
However I will supply a PR that would ease up vault integration
I just ran across this. Ideally, changing the configuration should affect subsequent calls to getConnection(). Baring that, changing the configuration after the pool is already created should throw an exception. Currently, the code fails (to behave as expected) silently.
I have fixed this problem by replacing the property value. The getConnection(String username, String password) is kinda dummy method in hikari since it will be eventually called getConnection() method. This means that even though we change username/password in hikariconfig it will still call the property file that was created at initializeDataSource() method.
This property is private, however, it doesn't mean that we cannot change the value at runtime. By using java reflection, I was able to change the property value, which allowed us to have dynamic passwords for the db. This is the code I used:
//Get HikariDataSource from the bean
HikariDataSource hikariDataSource = (HikariDataSource) apiContext.getBean("opsapids");
HikariPool hikariPool = (HikariPool) FieldUtils.readField(hikariDataSource, "fastPathPool", Boolean.TRUE);
DriverDataSource driverDataSource = (DriverDataSource) FieldUtils.readField(hikariPool, "dataSource", Boolean.TRUE);
Properties driverProperties = (Properties) FieldUtils.readField(driverDataSource, "driverProperties", Boolean.TRUE);
/*
* Screw it here :-)
*/
if (rdsToken != null)
{
driverProperties.put("password", rdsToken);
hikariDataSource.setPassword(rdsToken);
}
@brettwooldridge I would love to see this feature as well... Being able to play with the pool configurations at runtime for performance tuning and being able to chance the password without down time would be huge time saver!
The FAQ indicates that it _is_ possible to change some of these properties at runtime: https://github.com/brettwooldridge/HikariCP/wiki/FAQ#q-can-i-change-the-usernamepassword-or-other-pool-properties-at-runtime
That looks like it would cover my use case of periodically fetching new creds for RDS. Does that existing MXBean address the overall problem discussed in this issue?
@jklukas Yes, if the property getter/setter is available on the HikariConfigMXBean then it is capable of changing the runtime behavior of the pool.
A similar question was recently asked in the discussion group, with respect to RDS.
I just want to add my 2 cents here... Log4j2 updates the logger configurations on the fly on runtime when you change the configurations file, and those are very complex configurations to change, which can include ongoing database connections and network connections to be substituted... I may risk sounding silly, but maybe you could analyze their code and get inspired...I know, easier said than done, but I just wanted to share another way of maybe solving this.
@feinstein It may or may not be obvious, but logging and connection pools are quite different. 😄 The properties that can be altered are limited, and simply re-loading the properties file would provide less clarity about which properties are applicable at runtime in comparison to the specific API provided by the MXBean. I understand its simplicity, but at the same time it would foster misunderstanding and generate greater support load when the behavior was not as expected.
@brettwooldridge hahahah I do imagine they are quite different, but since Log4j2 also deals with database loggers, maybe their strategy could be useful, I don't know, I am not familiar with their code base, just the behavior.
I understand some properties can't be altered, but what about swapping the pools behind the scenes?
I also understand swapping the pools won't be trivial, but am I too wrong on saying it's possible? As far as I can see, you could provide new connections from the new pool, and when a user returns a connection from the old pool, the connection is closed. Once the old pool have no more acting connections, the old pool is removed entirely. This way, the new pool will take over the old pool gradually (or maybe even forced-shutdown if it takes too long, this timeout could be a new configuration option as well).
This swapping could be a little tricky, if the user changes the file multiple times, there will be a list of old pools waiting to die. If the user forgets to return a connection and the connection doesn't have a timeout, there will be a zombie pool on the old pools lists as well. Also, if the user is changing database connection configurations, like the password, it will have to keep two active database instances to satisfy both pools. Maybe there could be a boolean property on the MXBean that informs the user if there's an old pool being swapped, so the user has to be careful not to shutdown the old database yet? Or maybe you just force close all the connections, as if the database connection got interrupted and swap the new connection afterwards? This would be more aggressive, but all database operations should be ready to react to this scenario, could also be a configuration option for the user to select?
Sorry for dumping you with all those ideas, I just want to contribute the way I can to make this library even more awesome :D
@feinstein First, thanks for your interest and feedback.
Here’s the thing. It’s just software, so nearly anything is possible. It comes down to the philosophy of the project. Minimalism.
There is a reason I quote Edsger Dijkstra on the project page:
“Simplicity is prerequisite for reliability."
I could have just as easily quoted Steve Jobs:
People think focus means saying yes to the thing you've got to focus on. But that's not what it means at all. It means saying no to the hundred other good ideas that there are. You have to pick carefully. I'm actually as proud of the things we haven't done as the things I have done. Innovation is saying no to 1,000 things.
Your own two statements sum it up well, ”I also understand swapping the pools won't be trivial...”, and ”This swapping could be a little tricky...”. Supporting this kind of live swapping would certainly substantially complicate the code, and likely introduce bugs and race conditions into a currently battle tested library. And to what end? In order to support a “nice to have” use-case that is used on rare occasions? One that is already covered by the existing API.
The older I get, I’ve been at this for 30 years, the more beauty I see in products, software and solutions that do one thing extremely well, without a lot of bells and whistles.
I understand your point and your philosophy and I can't say that you are wrong.
Thank you for the attention and support!
Just testing please ignore
How does Hikari deal with multiple connection pool instances (for example, creating a connection pool per user with unique credentials)? Are the connections thrown around between them? What I'm thinking of is creating a new pool for the requesting user and destroying it afterwards (assume at the end of HTTP request or even later).
@Dragas That seems like a fairly unconventional way to handle it. Each user in your app will have their own database user / password? What is the use case?
If you want to start and stop a pool at the HTTP request level it kind of defeats the purpose of a connection pool and could possibly perform worse than just opening a new connection to the DB every time without a pool and sharing that connection across the lifetime of the request. If you have a limited number of users you could consider having some kind of cache of thread pools that expire over time. If you have a lot of users having a different database user / pass doesn't make a lot of sense.
@billoneil The usecase is permitting access to parts of database per particular user. I agree that using database connection pools for this is a bad idea, so I might as well stay with regular connections instead. Cheers.
@dragas one solution is to store each all user data together and have queries return a subset based on a column or schema that differentiates between the users. In other words, move authentication up to the application layer.
So, is there a way to add a new db connection through the new db password??
Most helpful comment
Would this allow a user to change MySQL password at runtime ? We're looking for a way to use vault for leased db credentials and don't want to have to restart service to enact updated credentials