Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 4cf094f

Browse files
Fix ConnectionPool to raise MaxConnectionsError instead of Connection... (#3698)
* Fix ConnectionPool to raise MaxConnectionsError instead of ConnectionError - 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 #3684 * Update async cluster to handle MaxConnectionsError separately Add an explanatory comment to the exception handling for MaxConnectionsError in the async cluster implementation, matching the synchronous version. This ensures consistent behavior between sync and async implementations when connection pool is exhausted, preventing unnecessary cluster reinitialization. ---------
1 parent a001416 commit 4cf094f

File tree

7 files changed

+142
-6
lines changed

7 files changed

+142
-6
lines changed

‎redis/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
DataError,
2121
InvalidPipelineStack,
2222
InvalidResponse,
23+
MaxConnectionsError,
2324
OutOfMemoryError,
2425
PubSubError,
2526
ReadOnlyError,
@@ -65,6 +66,7 @@ def int_or_str(value):
6566
"default_backoff",
6667
"InvalidPipelineStack",
6768
"InvalidResponse",
69+
"MaxConnectionsError",
6870
"OutOfMemoryError",
6971
"PubSubError",
7072
"ReadOnlyError",

‎redis/asyncio/cluster.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -814,7 +814,13 @@ async def _execute_command(
814814
moved = False
815815

816816
return await target_node.execute_command(*args, **kwargs)
817-
except (BusyLoadingError, MaxConnectionsError):
817+
except BusyLoadingError:
818+
raise
819+
except MaxConnectionsError:
820+
# MaxConnectionsError indicates client-side resource exhaustion
821+
# (too many connections in the pool), not a node failure.
822+
# Don't treat this as a node failure - just re-raise the error
823+
# without reinitializing the cluster.
818824
raise
819825
except (ConnectionError, TimeoutError):
820826
# Connection retries are being handled in the node's

‎redis/cluster.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
DataError,
4040
ExecAbortError,
4141
InvalidPipelineStack,
42+
MaxConnectionsError,
4243
MovedError,
4344
RedisClusterException,
4445
RedisError,
@@ -1235,6 +1236,12 @@ def _execute_command(self, target_node, *args, **kwargs):
12351236
return response
12361237
except AuthenticationError:
12371238
raise
1239+
except MaxConnectionsError:
1240+
# MaxConnectionsError indicates client-side resource exhaustion
1241+
# (too many connections in the pool), not a node failure.
1242+
# Don't treat this as a node failure - just re-raise the error
1243+
# without reinitializing the cluster.
1244+
raise
12381245
except (ConnectionError, TimeoutError) as e:
12391246
# ConnectionError can also be raised if we couldn't get a
12401247
# connection from the pool before timing out, so check that

‎redis/connection.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
ChildDeadlockedError,
3232
ConnectionError,
3333
DataError,
34+
MaxConnectionsError,
3435
RedisError,
3536
ResponseError,
3637
TimeoutError,
@@ -1556,7 +1557,7 @@ def get_encoder(self) -> Encoder:
15561557
def make_connection(self) -> "ConnectionInterface":
15571558
"Create a new connection"
15581559
if self._created_connections >= self.max_connections:
1559-
raise ConnectionError("Too many connections")
1560+
raise MaxConnectionsError("Too many connections")
15601561
self._created_connections += 1
15611562

15621563
if self.cache is not None:

‎redis/exceptions.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,13 @@ class SlotNotCoveredError(RedisClusterException):
220220
pass
221221

222222

223-
class MaxConnectionsError(ConnectionError): ...
223+
class MaxConnectionsError(ConnectionError):
224+
"""
225+
Raised when a connection pool has reached its max_connections limit.
226+
This indicates pool exhaustion rather than an actual connection failure.
227+
"""
228+
229+
pass
224230

225231

226232
class CrossSlotTransactionError(RedisClusterException):

‎tests/test_connection_pool.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,13 @@ def test_multiple_connections(self, master_host):
7676
assert c1 != c2
7777

7878
def test_max_connections(self, master_host):
79-
connection_kwargs = {"host": master_host[0], "port": master_host[1]}
80-
pool = self.get_pool(max_connections=2, connection_kwargs=connection_kwargs)
79+
# Use DummyConnection to avoid actual connection to Redis
80+
# This prevents authentication issues and makes the test more reliable
81+
# while still properly testing the MaxConnectionsError behavior
82+
pool = self.get_pool(max_connections=2, connection_class=DummyConnection)
8183
pool.get_connection()
8284
pool.get_connection()
83-
with pytest.raises(redis.ConnectionError):
85+
with pytest.raises(redis.MaxConnectionsError):
8486
pool.get_connection()
8587

8688
def test_reuse_previously_released_connection(self, master_host):

‎tests/test_max_connections_error.py

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
import pytest
2+
import redis
3+
from unittest import mock
4+
from redis.connection import ConnectionInterface
5+
6+
7+
class DummyConnection(ConnectionInterface):
8+
"""A dummy connection class for testing that doesn't actually connect to Redis"""
9+
10+
def __init__(self, *args, **kwargs):
11+
self.connected = False
12+
13+
def connect(self):
14+
self.connected = True
15+
16+
def disconnect(self):
17+
self.connected = False
18+
19+
def register_connect_callback(self, callback):
20+
pass
21+
22+
def deregister_connect_callback(self, callback):
23+
pass
24+
25+
def set_parser(self, parser_class):
26+
pass
27+
28+
def get_protocol(self):
29+
return 2
30+
31+
def on_connect(self):
32+
pass
33+
34+
def check_health(self):
35+
return True
36+
37+
def send_packed_command(self, command, check_health=True):
38+
pass
39+
40+
def send_command(self, *args, **kwargs):
41+
pass
42+
43+
def can_read(self, timeout=0):
44+
return False
45+
46+
def read_response(self, disable_decoding=False, **kwargs):
47+
return "PONG"
48+
49+
50+
@pytest.mark.onlynoncluster
51+
def test_max_connections_error_inheritance():
52+
"""Test that MaxConnectionsError is a subclass of ConnectionError"""
53+
assert issubclass(redis.MaxConnectionsError, redis.ConnectionError)
54+
55+
56+
@pytest.mark.onlynoncluster
57+
def test_connection_pool_raises_max_connections_error():
58+
"""Test that ConnectionPool raises MaxConnectionsError and not ConnectionError"""
59+
# Use a dummy connection class that doesn't try to connect to a real Redis server
60+
pool = redis.ConnectionPool(max_connections=1, connection_class=DummyConnection)
61+
pool.get_connection()
62+
63+
with pytest.raises(redis.MaxConnectionsError):
64+
pool.get_connection()
65+
66+
67+
@pytest.mark.skipif(
68+
not hasattr(redis, "RedisCluster"), reason="RedisCluster not available"
69+
)
70+
def test_cluster_handles_max_connections_error():
71+
"""
72+
Test that RedisCluster doesn't reinitialize when MaxConnectionsError is raised
73+
"""
74+
# Create a more complete mock cluster
75+
cluster = mock.MagicMock(spec=redis.RedisCluster)
76+
cluster.cluster_response_callbacks = {}
77+
cluster.RedisClusterRequestTTL = 3 # Set the TTL to avoid infinite loops
78+
cluster.nodes_manager = mock.MagicMock()
79+
node = mock.MagicMock()
80+
81+
# Mock get_redis_connection to return a mock Redis client
82+
redis_conn = mock.MagicMock()
83+
cluster.get_redis_connection.return_value = redis_conn
84+
85+
# Setup get_connection to be called and return a connection that will raise
86+
connection = mock.MagicMock()
87+
88+
# Patch the get_connection function in the cluster module
89+
with mock.patch("redis.cluster.get_connection", return_value=connection):
90+
# Test MaxConnectionsError
91+
connection.send_command.side_effect = redis.MaxConnectionsError(
92+
"Too many connections"
93+
)
94+
95+
# Call the method and check that the exception is raised
96+
with pytest.raises(redis.MaxConnectionsError):
97+
redis.RedisCluster._execute_command(cluster, node, "GET", "key")
98+
99+
# Verify nodes_manager.initialize was NOT called
100+
cluster.nodes_manager.initialize.assert_not_called()
101+
102+
# Reset the mock for the next test
103+
cluster.nodes_manager.initialize.reset_mock()
104+
105+
# Now test with regular ConnectionError to ensure it DOES reinitialize
106+
connection.send_command.side_effect = redis.ConnectionError("Connection lost")
107+
108+
with pytest.raises(redis.ConnectionError):
109+
redis.RedisCluster._execute_command(cluster, node, "GET", "key")
110+
111+
# Verify nodes_manager.initialize WAS called
112+
cluster.nodes_manager.initialize.assert_called_once()

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /