Skip to content

Fix ConnectionPool to raise MaxConnectionsError instead of Connection… #3698

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ngabhanenetskope
Copy link

Pull Request check-list

Please make sure to review and check all of these items:

  • [✅] Do tests and lints pass with this change?
  • [✅] Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
  • [✅] Is the new or changed code fully tested?
  • [✅] Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
  • Is there an example added to the examples folder (if applicable)? (N/A - This is an error class enhancement)

Description of change

This PR addresses issue #3684 by introducing a more specific MaxConnectionsError exception that is raised when a connection pool reaches its maximum connections limit.

Current behavior:
When a ConnectionPool reaches its maximum connections limit, it raises a generic ConnectionError. This causes RedisCluster to treat the error as a node failure and trigger an unnecessary cluster reinitialization, leading to increased resource usage.

Changes:

  1. Added a new MaxConnectionsError class in redis/exceptions.py as a subclass of ConnectionError
  2. Modified ConnectionPool.make_connection() in redis/connection.py to raise the new MaxConnectionsError instead of a generic ConnectionError
  3. Updated RedisCluster._execute_command() in redis/cluster.py to handle MaxConnectionsError separately, preventing unnecessary cluster reinitialization
  4. Updated tests to verify the new behavior in both standalone and cluster modes
  5. Added exports in __init__.py to make the new exception available

Testing:

  1. Updated test_max_connections in test_connection_pool.py to expect MaxConnectionsError
  2. Added a new test file test_max_connections_error.py with:
    • Inheritance test to verify MaxConnectionsError extends ConnectionError
    • Connection pool test to verify the correct error is raised
    • Mock-based cluster test to verify cluster doesn't reinitialize on MaxConnectionsError

All tests pass locally, including the new tests specifically for this behavior.

Note: The issue was previously assigned but appears to be inactive. I implemented this fix because it was affecting our production environment.

…Error

- Added MaxConnectionsError class as a subclass of ConnectionError
- Updated connection.py to raise the more specific error
- Updated cluster.py to handle this specific error type
- Added tests to verify the behavior

Fixes redis#3684
@ngabhanenetskope ngabhanenetskope force-pushed the fix-max-connections-error branch from 993234b to 229f857 Compare July 7, 2025 07:00
Removed unnecessary methods and attributes from the DummyConnection class
that were introduced during development. This restores the class to its
minimal implementation needed for testing the MaxConnectionsError behavior.
Removed extra whitespace in the DummyConnection class in test_connection_pool.py
to maintain consistent indentation throughout the file. This helps maintain
code style consistency across the codebase.
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.

1 participant