Update how db connections are retrieved and released #802
Conversation
This file contains 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
matthewmcgarvey
force-pushed
the
query-connection-release
branch
from
19cda6f to
f51cb39
Compare
matthewmcgarvey
commented
Jan 26, 2022
src/avram/database.cr
Outdated
| @@ -153,7 +153,9 @@ abstract class Avram::Database | |||
|
|
|||
| # :nodoc: | |||
| def run | |||
| yield current_connection || db | |||
| yield current_connection | |||
There was a problem hiding this comment.
The || db was unnecessary as current_connection checks out a connection if there isn't already one
| @@ -62,6 +62,12 @@ describe Avram::Queryable do | |||
| query.statement.should eq "SELECT #{User::COLUMN_SQL} FROM users WHERE users.age >= $1 AND users.name = $2" | |||
| end | |||
|
|
|||
| it "releases connection if no open transaction", tags: Avram::SpecHelper::TRUNCATE do | |||
There was a problem hiding this comment.
Exactly like #800 we use truncation to avoid having a transaction on the connection by default
grepsedawk
approved these changes
Jan 26, 2022
jwoertink
approved these changes
Jan 26, 2022
matthewmcgarvey
changed the title
jwoertink
approved these changes
Jan 26, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #799
In #800 we already fixed one place where we weren't releasing connections correctly. We found another place where queries were not releasing connections since they don't use transactions. Instead of add the connection release code in two places, I updated
current_connectionto bewith_connectionthat takes a block. When the block finishes (error or not) it will always attempt to release the connection back to the pool. If there is an open transaction, it won't do it.