-
Notifications
You must be signed in to change notification settings - Fork 11
Use Connection Pool in PG Queries #248
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
Conversation
document-store/src/main/java/org/hypertrace/core/documentstore/postgres/PostgresCollection.java
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #248 +/- ##
============================================
+ Coverage 80.44% 80.48% +0.03%
Complexity 1151 1151
============================================
Files 215 215
Lines 5502 5533 +31
Branches 487 489 +2
============================================
+ Hits 4426 4453 +27
- Misses 750 753 +3
- Partials 326 327 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@suresh-prakash Shall we switch to using connection pooling for update/write queries as well? (We are only using it for READ queries in this PR). |
|
@suresh-prakash One issue with was seeing with pooling is connections getting stuck as can be seen in this table: From what I found online, this is because the queries on these connections haven't committed (even |
| resultSet.next(); | ||
| long count = resultSet.getLong(1); | ||
| // Reset autoCommit before returning connection to pool | ||
| connection.setAutoCommit(false); |
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.
Any reason why this is necessary?
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.
Methods like PostgresCollection#update, we commit manually as:
if (documentOptional.isEmpty()) {
connection.commit();
return empty();
}
Setting auto-commit for such queries can be risky (haven't validate what the exact behaviour is in this case).
So, we turn this to off whenever we return a connection back to the pool. So when update() gets a pooled connection, its auto-commit is set to false.
|
@suresh-prakash Why it works today with a single connection is because when we do
|
Ideally, we should be using connection pooling everywhere. |
Do we really have any queries that use transactions? Can you point to those queries? |
|
@suresh-prakash Added a separate pool for manual transactions as discussed. |
| } | ||
|
|
||
| @Test | ||
| public void testGetConnection() throws SQLException { |
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.
Thanks for doing this. 🙂

Description
Currently, the library uses a single connection and serialises all queries on it. This results in poor perf. This PR configures it to use the connection pool by default.
Testing
Setup: See the
Queriessection for all the queries that were used in the load test. An artificial delay of 0.1s was introduced usingpg_sleep()to simulate some load on the system.Overall Performance Improvement
Detailed Query Comparison
Concurrency: 50
Concurrency: 75
Concurrency: 100
Tail Latency Analysis
P99 Latency Improvements
P90 Latency Improvements
Queries
Checklist: