Implement internal connection pools for the read-write plugin #359
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sergiyvamz
reviewed
Mar 6, 2023
wrapper/src/main/java/software/amazon/jdbc/ConnectionProvider.java
Outdated
Show resolved
Hide resolved
sergiyvamz
reviewed
Mar 6, 2023
wrapper/src/main/java/software/amazon/jdbc/ConnectionProviderManager.java
Outdated
Show resolved
Hide resolved
sergiyvamz
reviewed
Mar 6, 2023
wrapper/src/main/java/software/amazon/jdbc/ConnectionProviderManager.java
Show resolved
Hide resolved
sergiyvamz
reviewed
Mar 6, 2023
wrapper/src/main/java/software/amazon/jdbc/HikariPoolConfigurator.java
Outdated
Show resolved
Hide resolved
sergiyvamz
reviewed
Mar 6, 2023
wrapper/src/main/java/software/amazon/jdbc/HikariPooledConnectionProvider.java
Outdated
Show resolved
Hide resolved
sergiyvamz
reviewed
Mar 6, 2023
wrapper/src/main/java/software/amazon/jdbc/HikariPooledConnectionProvider.java
Outdated
Show resolved
Hide resolved
sergiyvamz
reviewed
Mar 6, 2023
| @Override | ||
| public Connection connect( | ||
| @NonNull String url, @NonNull Properties props) throws SQLException { | ||
| // TODO: Is this method ever called? It seems to only be referenced by tests/benchmarks. If |
Contributor
There was a problem hiding this comment.
Agree, it don't recall it's ever used. Might consider to deprecate it.
sergiyvamz
reviewed
Mar 6, 2023
wrapper/src/main/java/software/amazon/jdbc/HikariPooledConnectionProvider.java
Outdated
Show resolved
Hide resolved
sergiyvamz
reviewed
Mar 6, 2023
| JdbcCallable<Connection, SQLException> connectFunc) throws SQLException { | ||
| final Connection conn = connectFunc.call(); | ||
|
|
||
| if (conn != null) { |
Contributor
There was a problem hiding this comment.
That may need additional checks to avoid generating keys for connections that already have keys and come from connection pool. It's not part of POC certainly.
…ols (#27) - updated HikariPooledConnectionProvider to use jdbcUrl instead of dataSourceClassName - fixed issue in StaleDnsHelper where the URL was included in the comparison between cluster and instance DNS. Only the IP address should be compared - fixed some incorrect test code in ReadWriteSplittingTests - disabled flaky test in ReadWriteSplittingTests. The HostListProvider should be improved to verify the initial host spec role before re-enabling it
- added test for failover failure (no instances available) - added test for failover while in a transaction
davecramer
reviewed
Apr 19, 2023
|
|
||
| ### Internal connection pooling | ||
|
|
||
| Whenever `setReadOnly(true)` is first called on a `Connection` object, the read-write plugin will internally open a new physical connection to a reader. After this first call, the physical reader connection will be cached for the given `Connection`. Future calls to `setReadOnly `on the same `Connection` object will not require opening a new physical connection. However, calling `setReadOnly(true)` for the first time on a new `Connection` object will require the plugin to establish another new physical connection to a reader. If your application frequently establishes new read-only connections, you can enable internal connection pooling to improve performance. When enabled, the wrapper driver will maintain an internal connection pool for each instance in the cluster. This allows the read-write plugin to reuse reader connections that were established by previous `Connection` objects. |
Contributor
There was a problem hiding this comment.
However, calling setReadOnly(true) for the first time on a new Connection object will require the plugin to establish another new physical connection to a reader.
Seems to just repeat the information above.
fix: hanging pooled connection test
congoamz
changed the title
karenc-bq
approved these changes
Apr 27, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add functionality to use internal connection pools to establish connections. The motivation for using pools comes from the read-write plugin's need to regularly establish new connections. Using pools will increase performance and decrease demands on the database by allowing connections to be re-used.
Additional Reviewers
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.