Skip to content

[collector]bugfix: close jdbc connection on statement creation failure#4184

Open
hutiefang76 wants to merge 3 commits into
apache:masterfrom
hutiefang76:codex/hertzbeat-4181-close-jdbc-connection
Open

[collector]bugfix: close jdbc connection on statement creation failure#4184
hutiefang76 wants to merge 3 commits into
apache:masterfrom
hutiefang76:codex/hertzbeat-4181-close-jdbc-connection

Conversation

@hutiefang76

Copy link
Copy Markdown
Contributor

What changed

This closes the new JDBC connection when statement creation or statement setup fails in JdbcCommonCollect.

The existing code closed cached JDBC connections on failure, but a freshly opened connection could remain open if setReadOnly(true), createStatement(), setQueryTimeout(...), or setMaxRows(...) threw before the method returned the statement. That path can leave PostgreSQL PgConnection instances waiting for finalization.

Validation

  • Reproduced the leak with a regression test: before the fix, JdbcCommonCollectTest#testCloseConnectionWhenCreateStatementFails failed because connection.close() was not called.
  • JAVA_HOME=$(/usr/libexec/java_home -v 26) ./mvnw -s <central-only-settings> -pl hertzbeat-collector/hertzbeat-collector-basic -am -Dtest=JdbcCommonCollectTest#testCloseConnectionWhenCreateStatementFails -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false test
  • JAVA_HOME=$(/usr/libexec/java_home -v 26) ./mvnw -s <central-only-settings> -pl hertzbeat-collector/hertzbeat-collector-basic -am -Dtest=JdbcCommonCollectTest -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false test
  • JAVA_HOME=$(/usr/libexec/java_home -v 26) ./mvnw -s <central-only-settings> -pl hertzbeat-collector/hertzbeat-collector-basic -am -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false test
  • git diff --check

Fixes #4181

statement.setMaxRows(1000);
if (reuseConnection) {
JdbcConnect jdbcConnect = new JdbcConnect(connection);
connectionCommonCache.addCache(identifier, jdbcConnect);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There is another issue here: when two threads access the same cache identifier simultaneously, addCache—because it is a non-atomic operation—may still cause a memory leak. For example: multiple monitoring tasks use the same URL. The connection being replaced may be silently overwritten without being closed.

Could you please add to or edit this section?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] PgConnection连接未释放,内存泄露问题

2 participants