Skip to content

Fix communication failures with aws-secretsmanager-jdbc #1932

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.

Sign up for GitHub

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

Jump to bottom
Open
gontard wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Fix communication failures with aws-secretsmanager-jdbc #1932

gontard wants to merge 1 commit into from
+4 −3

Conversation

@gontard
Copy link

@gontard gontard commented May 2, 2022

See commit message.

@gontard
Fix communication failures with aws-secretsmanager-jdbc
334a93c 
Using aws-secretsmanager-jdbc with HikariCP triggers sporadic
communication link failures with MySQL.

aws-secretsmanager-jdbc library provides JDBC drivers to retrieve
DB credentials, host and password from the Amazon Secrets Manager.
See:

- https://docs.aws.amazon.com/secretsmanager/latest/userguide/retrieving-secrets_jdbc.html
- https://github.com/aws/aws-secretsmanager-jdbc

By specifying the secret id as a JDBC an URL, and
com.amazonaws.secretsmanager.sql.AWSSecretsManagerMySQLDriver as
JDBC driver, HikariCP can connect to the MySQL endpoint.

But in this stituation, communication link failures occur:
com.mysql.cj.jdbc.exceptions.CommunicationsException: Communications link failure
The last packet successfully received from the server was 5,006 milliseconds ago.
The last packet sent successfully to the server was 5,007 milliseconds ago.

These failures are due a MySQL bug: http://bugs.mysql.com/bug.php?id=75615 which is
bypassed by HikariCP when the MySQL driver is used. See:

 - brettwooldridge#236
 - 8af2bc5 (Fix brettwooldridge#236 via workaround for MySQL issue http://bugs.mysql.com/bug.php?id=75615, 2015-01-24)

This commit applies the same hack for AWSSecretsManagerMySQLDriver.
@gontard gontard force-pushed the mysql-75615-workaround branch from b027d98 to 334a93c Compare May 2, 2022 11:59
@gontard gontard marked this pull request as ready for review May 2, 2022 12:29
lfbayer
lfbayer reviewed Sep 27, 2022
View reviewed changes
src/main/java/com/zaxxer/hikari/pool/PoolBase.java
(jdbcUrl != null && jdbcUrl.contains("mysql")) ||
(dataSource != null && dataSource.getClass().getName().contains("Mysql"))) {
(dataSource != null && dataSource.getClass().getName().contains("Mysql")) ||
"com.amazonaws.secretsmanager.sql.AWSSecretsManagerMySQLDriver".equals(driverClassName)) {
Copy link
Collaborator

@lfbayer lfbayer Sep 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In your case, what is the dataSource classname? Does it contain the name "MySQL", if so the existing contains("Mysql") maybe should just be changed to a case insensitive check.

@brettwooldridge brettwooldridge force-pushed the dev branch 2 times, most recently from 6d319ab to 7cacd0f Compare September 23, 2024 20:03
@gontard
Copy link
Author

gontard commented Jul 3, 2025

@CodiumAI-Agent /improve

@QodoAI-Agent
Copy link

QodoAI-Agent commented Jul 3, 2025

PR Code Suggestions ✨

CategorySuggestion                                                                                                                                    Impact
General
Broaden driver class match

Broaden the driver class match to support shading or relocated packages by checking
for a key substring instead of a full class name. This ensures future compatibility
if the AWS driver package changes.

src/main/java/com/zaxxer/hikari/pool/PoolBase.java [598]

-"com.amazonaws.secretsmanager.sql.AWSSecretsManagerMySQLDriver".equals(driverClassName)
+driverClassName != null && driverClassName.contains("AWSSecretsManagerMySQLDriver")
Suggestion importance[1-10]: 6

__

Why: Using contains with a null check broadens compatibility for relocated AWS driver classes, improving future-proofing with minimal risk.

Low
Use case-insensitive matching

Perform case-insensitive checks for both dsClassName and jdbcUrl by converting them
to lower case before calling contains. This avoids missing matches due to different
capitalizations.

src/main/java/com/zaxxer/hikari/pool/PoolBase.java [595-596]

-(dsClassName != null && dsClassName.contains("Mysql")) ||
-(jdbcUrl != null && jdbcUrl.contains("mysql"))
+(dsClassName != null && dsClassName.toLowerCase().contains("mysql")) ||
+(jdbcUrl != null && jdbcUrl.toLowerCase().contains("mysql"))
Suggestion importance[1-10]: 5

__

Why: Converting to lowercase before contains ensures detection of all capitalization variants in dsClassName and jdbcUrl, enhancing reliability.

Low
Remove redundant null check

Remove the redundant null check on dataSource since the method is only called when
dataSource is guaranteed non-null, simplifying the condition.

src/main/java/com/zaxxer/hikari/pool/PoolBase.java [597]

-(dataSource != null && dataSource.getClass().getName().contains("Mysql"))
+dataSource.getClass().getName().contains("Mysql")
Suggestion importance[1-10]: 3

__

Why: The private method is only invoked with a non-null dataSource, so omitting the null check simplifies the code without affecting correctness.

Low

@brettwooldridge brettwooldridge force-pushed the dev branch 2 times, most recently from d7a9b2f to e56fa46 Compare July 20, 2025 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lfbayer lfbayer lfbayer left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@gontard @QodoAI-Agent @lfbayer