You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@shardingsphere.apache.org by "gxgmy521 (via GitHub)" <gi...@apache.org> on 2024/11/27 09:47:45 UTC

[I] Connection leak when DriverDatabaseConnectionManager#createConnections throw exception [shardingsphere]

gxgmy521 opened a new issue, #33831:
URL: https://github.com/apache/shardingsphere/issues/33831

## Bug Report

**For English only**, other languages will not accept.

Before report a bug, make sure you have:

- Searched open and closed [GitHub issues](https://github.com/apache/shardingsphere/issues).
- Read documentation: [ShardingSphere Doc](https://shardingsphere.apache.org/document/current/en/overview).

Please pay attention on issues you submitted, because we maybe need more details.
If no response anymore and we cannot reproduce it on current information, we will **close it**.

Please answer these questions before submitting your issue. Thanks!

### Which version of ShardingSphere did you use?
5.5.0

### Which project did you use? ShardingSphere-JDBC or ShardingSphere-Proxy?
ShardingSphere-JDBC
### Expected behavior

### Actual behavior

### Reason analyze (If you can)
![image](https://github.com/user-attachments/assets/a72e2c8f-3981-40ee-84a8-8ea54b389cf1)
In line 342, step 1, a database connection is obtained from the database connection pool.
In line 343, step 2, the previously recorded method is called for method replay.
However, if an exception was throwed from step2（for example the database server crashed）, there is no opportunity to call the connection’s close method, leading to a connection leak.
### Steps to reproduce the behavior, such as: SQL to execute, sharding rule configuration, when exception occur etc.

### Example codes for reproduce this issue (such as a github link).

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] Connection leak when DriverDatabaseConnectionManager#createConnections throw exception [shardingsphere]

Posted by "gxgmy521 (via GitHub)" <gi...@apache.org>.

gxgmy521 commented on issue #33831:
URL: https://github.com/apache/shardingsphere/issues/33831#issuecomment-2516057199

   > It sounds like what you need is Mockito's `@org.mockito.Spy` to wrap the H2database connection?
   
   I do not know how to make a pr. I write unit test to produce the connection leak issue。 https://github.com/gxgmy521/shardingsphere/commit/95c4a98bfc0ceed54bbc34148b0c1ffd3cee6d3e


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] Connection leak when DriverDatabaseConnectionManager#createConnections throw exception [shardingsphere]

Posted by "linghengqian (via GitHub)" <gi...@apache.org>.

linghengqian commented on issue #33831:
URL: https://github.com/apache/shardingsphere/issues/33831#issuecomment-2507153818

   Are you going to submit a PR? The failing unit tests are clearly on your hands.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] Connection leak when DriverDatabaseConnectionManager#createConnections throw exception [shardingsphere]

Posted by "linghengqian (via GitHub)" <gi...@apache.org>.

linghengqian commented on issue #33831:
URL: https://github.com/apache/shardingsphere/issues/33831#issuecomment-2506365028

   > for example the database server crashed
   
   - This doesn't sound like there is any way to check it. `java.sql.Connection` does not have an interface for health checking. So I'm assuming you want to do this? Are you going to submit a PR with unit tests?
   ```java
   if (1 == connectionSize) {
               Connection connection = createConnection(databaseName, dataSourceName, dataSource, connectionContext.getTransactionContext());
               try {
                   methodInvocationRecorder.replay(connection);
               } catch (final SQLException ex) {
                   connection.close();
                   throw ex;
               }
               return Collections.singletonList(connection);
           }
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] Connection leak when DriverDatabaseConnectionManager#createConnections throw exception [shardingsphere]

Posted by "linghengqian (via GitHub)" <gi...@apache.org>.

linghengqian commented on issue #33831:
URL: https://github.com/apache/shardingsphere/issues/33831#issuecomment-2510617962

   Why do you need a mock connection?🤔 In unit tests, you can always create a connection from an h2database in mysql-compatible mode.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] Connection leak when DriverDatabaseConnectionManager#createConnections throw exception [shardingsphere]

Posted by "linghengqian (via GitHub)" <gi...@apache.org>.

linghengqian commented on issue #33831:
URL: https://github.com/apache/shardingsphere/issues/33831#issuecomment-2516168371

   I'm not sure why you don't know how to open a pull request. If you don't mind, I'd like to add a volunteer tag to the current issue? The unit tests I saw have obvious formatting issues.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] Connection leak when DriverDatabaseConnectionManager#createConnections throw exception [shardingsphere]

Posted by "gxgmy521 (via GitHub)" <gi...@apache.org>.

gxgmy521 commented on issue #33831:
URL: https://github.com/apache/shardingsphere/issues/33831#issuecomment-2516298363

   > I'm not sure why you don't know how to open a pull request. If you don't mind, I'd like to add a volunteer tag to the current issue? The unit tests I saw have obvious formatting issues.
   
   I am no very familiar with the code conduct and style.  I will make a PR and adjust the unit test based on the https://shardingsphere.apache.org/community/en/involved/conduct/code/


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] Connection leak when DriverDatabaseConnectionManager#createConnections throw exception [shardingsphere]

Posted by "linghengqian (via GitHub)" <gi...@apache.org>.

linghengqian commented on issue #33831:
URL: https://github.com/apache/shardingsphere/issues/33831#issuecomment-2610208097

   - @gxgmy521 Hi, are you still working on the current issue? 
   
   - I just quickly noticed that some integration tests around seata client under GraalVM Native Image in master branch have connection leaks, but I haven't confirmed whether that todo bug is related to the current issue.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] Connection leak when DriverDatabaseConnectionManager#createConnections throw exception [shardingsphere]

Posted by "linghengqian (via GitHub)" <gi...@apache.org>.

linghengqian commented on issue #33831:
URL: https://github.com/apache/shardingsphere/issues/33831#issuecomment-2510731542

   It sounds like what you need is Mockito's `@org.mockito.Spy` to wrap the H2database connection? 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] Connection leak when DriverDatabaseConnectionManager#createConnections throw exception [shardingsphere]

Posted by "linghengqian (via GitHub)" <gi...@apache.org>.

linghengqian commented on issue #33831:
URL: https://github.com/apache/shardingsphere/issues/33831#issuecomment-2611633567

   - Long story short, I confirmed that the current issue affects the unit tests of the master branch. I have opened https://github.com/apache/shardingsphere/pull/34463 .


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] Connection leak when DriverDatabaseConnectionManager#createConnections throw exception [shardingsphere]

Posted by "gxgmy521 (via GitHub)" <gi...@apache.org>.

gxgmy521 commented on issue #33831:
URL: https://github.com/apache/shardingsphere/issues/33831#issuecomment-2506928463

   #
   
   > > for example the database server crashed
   > 
   > * This doesn't sound like there is any way to check it. `java.sql.Connection` does not have an interface for health checking. So I'm assuming you want to do this? Are you going to submit a PR with unit tests?
   > 
   > ```java
   > if (1 == connectionSize) {
   >             Connection connection = createConnection(databaseName, dataSourceName, dataSource, connectionContext.getTransactionContext());
   >             try {
   >                 methodInvocationRecorder.replay(connection);
   >             } catch (final SQLException ex) {
   >                 connection.close();
   >                 throw ex;
   >             }
   >             return Collections.singletonList(connection);
   >         }
   > ```
   yes 。we should catch the SQLException and close the connection。
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] Connection leak when DriverDatabaseConnectionManager#createConnections throw exception [shardingsphere]

Posted by "linghengqian (via GitHub)" <gi...@apache.org>.

linghengqian commented on issue #33831:
URL: https://github.com/apache/shardingsphere/issues/33831#issuecomment-2507566795

   But then again, the unit tests I wrote didn't have connection leaks, there was always extra logic to close all connections. You need to provide independent unit tests.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] Connection leak when DriverDatabaseConnectionManager#createConnections throw exception [shardingsphere]

Posted by "gxgmy521 (via GitHub)" <gi...@apache.org>.

gxgmy521 commented on issue #33831:
URL: https://github.com/apache/shardingsphere/issues/33831#issuecomment-2510378222

   
   > But then again, the unit tests I wrote didn't have connection leaks, there was always extra logic to close all connections. You need to provide independent unit tests.
   
   Sorry I do not know how to mock the connection leaks in the unit tests 。
   You mention that "there was always extra logic to close all connections."  I guess you are talking about the capabilities of the connection pool (ie, the “removeAbandoned” parameter in Tomcat JDBC pool )？In my opinion ，we should close the connection immediately instead of handing it over to the connection pool。


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] Connection leak when DriverDatabaseConnectionManager#createConnections throw exception [shardingsphere]

Posted by "gxgmy521 (via GitHub)" <gi...@apache.org>.

gxgmy521 commented on issue #33831:
URL: https://github.com/apache/shardingsphere/issues/33831#issuecomment-2510726736

   > Why do you need a mock connection?🤔 In unit tests, you can always create a connection from an h2database in mysql-compatible mode.
   Without a mocked connection， I do not know how to control it throw an SQLException. But if mock a connection, I do not know how to re produce the connection leaks in unit test. I only reproduce the connection leak in the test environment with the help of ChaosBlade Tool. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] Connection leak when DriverDatabaseConnectionManager#createConnections throw exception [shardingsphere]

Posted by "terrymanu (via GitHub)" <gi...@apache.org>.

terrymanu closed issue #33831: Connection leak when DriverDatabaseConnectionManager#createConnections throw exception
URL: https://github.com/apache/shardingsphere/issues/33831


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org