Skip to content
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

Fix AbstractR2DBCQuery Fetch Method #490

Closed

Conversation

xeounxzxu
Copy link
Contributor

Changed to explicitly cancel R2DBC connection pool

@velo

issue number : #352

background :

This code gets the db connection via getConnection and performs operations within flatMapMany.

as-is:
However, there is no explicit connection termination in the current version. There is no code to return to the connection pool, which can result in a resource leak.

to-be:
Use Flux.usingWhen to acquire a connection, and when done, call the (Connection::close) method to close the connection and return.
Because this method explicitly closes the connection, we've changed the resource management to allow for safe termination.

@xeounxzxu xeounxzxu marked this pull request as draft July 7, 2024 05:29
@xeounxzxu xeounxzxu marked this pull request as ready for review July 7, 2024 05:29
@xeounxzxu xeounxzxu closed this Jul 7, 2024
@xeounxzxu xeounxzxu reopened this Jul 26, 2024
Copy link
Member

@velo velo left a comment

Choose a reason for hiding this comment

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

Seems this change breaks a bunch of tests...

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Some suggestions could not be made:

  • pom.xml
    • lines 494-497
  • querydsl-libraries/querydsl-jpa/pom.xml
    • lines 19-151

@velo
Copy link
Member

velo commented Aug 2, 2024

For now, going to close this as tests are no passing.

@velo velo closed this Aug 2, 2024
@xeounxzxu
Copy link
Contributor Author

@velo Sorry for the late reply. I will upload it again after changing the test case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants