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

Connections are not released to a connection pool, when ClusterPipeline faces an exception on get_connection #2814

Closed
zakaf opened this issue Jun 27, 2023 · 4 comments · Fixed by #3133
Assignees

Comments

@zakaf
Copy link
Contributor

zakaf commented Jun 27, 2023

Version: What redis-py and what redis version is the issue happening on?
redis-py 4.4.2 / redis 6.0.5

Platform: What platform / version? (For example Python 3.5.1 on Windows 7 / Ubuntu 15.10 / Azure)
Python 3.9 on MacOS Ventura 13.4.1

Description: Description of your issue, stack traces from errors and code that reproduces the issue

Recently, I've faced a situation where connections never get returned to a connection pool, when there is a surge of Redis commands. while investigating the incident, i've found out that the below scenario can cause a connection leak

  1. ClusterPipeline is stacked with commands that needs to be directed to at least two nodes
    • let's say node 1 and node 2
  2. ClusterPipeline obtains a connection to node 1 successfully
  3. ClusterPipeline fails to obtain a connection to node 2 due to ConnectionError
  4. ClusterPipeline retries the entire process again, but doesn't release an acquired connection to node 1 before doing that

below code includes a test case to cause a connection leak and a possible solution to the problem

Let me know, if you guys think this is an actual issue :)

@chayim
Copy link
Contributor

chayim commented Jun 28, 2023

Hi @zakaf! First off, thank you for the report, and a potential bug fix.

@dvora-h mind checking if this makes sense in the 4.6, and 5.0 contexts since that's getting much closer to release? Then we can figure out what we may want to do here.

@dvora-h
Copy link
Collaborator

dvora-h commented Jan 21, 2024

@zakaf this is still relevant and the fix makes sense... do you want to open a PR with the fix or wants me to take it?

@zakaf
Copy link
Contributor Author

zakaf commented Feb 4, 2024

@dvora-h sorry for the delayed response. i've created #3133 to fix this issue

@dvora-h
Copy link
Collaborator

dvora-h commented Feb 4, 2024

Thanks @zakaf

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