-
Notifications
You must be signed in to change notification settings - Fork 14
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
Make connection pool configurable #158
Conversation
Benchmark Result
Current status
|
e7a7872
to
61b99e3
Compare
61b99e3
to
0d4d7ed
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just minor nits/questions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work on making this more configurable!
I see that this affects performance quite a bit, especially for leases. Can we try to understand why? I just want to make sure that a MaxLifetime
does not impact with the correctness of the queries (i.e. the connection expires and we miss some updates while reconnecting).
Also, couple of nitpicks I noticed while reading.
478dcc8
to
1c6af34
Compare
f43a6d1
to
bf66a62
Compare
@marco6 Regarding the benchmark: The lease test calls |
Seems like the test was not on the database after all. Removing the test seems like way to go, but maybe in another PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! 🎉 There are still a couple of nitpicks, but I'm pre-approving this so you can merge as soon as they are fixed!
Make Connection Pool configurable
We use a connection pool to talk with the database. This connection pool can be configured.
Existing connections are not actively closed by us, we lazily close them via the connection pool configuration.
Through this PR we can set the connection pool configurations via flags.
This PR attempts to mitigate an issue where the leader node is killed and connections are "hanging" on the old connection of the killed node.
Configuration
Sql allows us to configure these parameters:
Current Configuration
We currently use:
Optimal default values
From a discussion with the dqlite team:
MaxConnections
Max ConnLifetime