-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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 redis connection leakage #7631
Fix redis connection leakage #7631
Conversation
0474cd4
to
292d937
Compare
292d937
to
8718c50
Compare
Codecov Report
@@ Coverage Diff @@
## master #7631 +/- ##
==========================================
- Coverage 89.75% 89.74% -0.02%
==========================================
Files 138 138
Lines 16996 17000 +4
Branches 2508 2509 +1
==========================================
+ Hits 15255 15256 +1
- Misses 1491 1493 +2
- Partials 250 251 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would you mind adding some test for this change?
redis integration tests are failing |
I take a look, thanks |
@auvipy How did you manage to run tests locally ? The build in |
Well, I managed to get the tests running, but I think this fix introduced another problem with threading. |
yeah if you can fix that please push and let me know |
It seemed to stuck on result retrieval under threaded environments, not sure why. |
I'm starting to think that maybe this approach wasn't the correct fix, I'll close for now until I discover otherwise or found another fix. |
Note: Before submitting this pull request, please review our contributing
guidelines.
Description
This PR is picked from #6985 since it has gone stale. (One of the maintainers asked for a rebase and that's all she wrote.)
This tries to resolve #6819 by ensuring there's only 1 redis connection pool across all the gevent concurrencies.