Skip to content

Commit 877d7a2

Browse files
Disabled SCH in MultiDBClient underlying clients (#3938)
* Disabled SCH in MultiDBClient underlying clients * Fixed comment format * Codestyle fixes * Codestyle fixes * Allow user to override maint_notifications_config * Codestyle fixes --------- Co-authored-by: petyaslavova <petya.slavova@redis.com>
1 parent 9ac9ee6 commit 877d7a2

File tree

3 files changed

+80
-3
lines changed

3 files changed

+80
-3
lines changed

redis/multidb/client.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from redis.backoff import NoBackoff
99
from redis.client import PubSubWorkerThread
1010
from redis.commands import CoreCommands, RedisModuleCommands
11+
from redis.maint_notifications import MaintNotificationsConfig
1112
from redis.multidb.circuit import CircuitBreaker
1213
from redis.multidb.circuit import State as CBState
1314
from redis.multidb.command_executor import DefaultCommandExecutor
@@ -149,7 +150,14 @@ def add_database(self, config: DatabaseConfig, skip_unhealthy: bool = True):
149150
"""
150151
# The retry object is not used in the lower level clients, so we can safely remove it.
151152
# We rely on command_retry in terms of global retries.
152-
config.client_kwargs.update({"retry": Retry(retries=0, backoff=NoBackoff())})
153+
config.client_kwargs["retry"] = Retry(retries=0, backoff=NoBackoff())
154+
155+
# Maintenance notifications are disabled by default in underlying clients,
156+
# but user can override this by providing their own config.
157+
if "maint_notifications_config" not in config.client_kwargs:
158+
config.client_kwargs["maint_notifications_config"] = (
159+
MaintNotificationsConfig(enabled=False)
160+
)
153161

154162
if config.from_url:
155163
client = self._config.client_class.from_url(

redis/multidb/config.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from redis.backoff import ExponentialWithJitterBackoff, NoBackoff
1010
from redis.data_structure import WeightedList
1111
from redis.event import EventDispatcher, EventDispatcherInterface
12+
from redis.maint_notifications import MaintNotificationsConfig
1213
from redis.multidb.circuit import (
1314
DEFAULT_GRACE_PERIOD,
1415
CircuitBreaker,
@@ -164,10 +165,17 @@ def databases(self) -> Databases:
164165
for database_config in self.databases_config:
165166
# The retry object is not used in the lower level clients, so we can safely remove it.
166167
# We rely on command_retry in terms of global retries.
167-
database_config.client_kwargs.update(
168-
{"retry": Retry(retries=0, backoff=NoBackoff())}
168+
database_config.client_kwargs["retry"] = Retry(
169+
retries=0, backoff=NoBackoff()
169170
)
170171

172+
# Maintenance notifications are disabled by default in underlying clients,
173+
# but user can override this by providing their own config.
174+
if "maint_notifications_config" not in database_config.client_kwargs:
175+
database_config.client_kwargs["maint_notifications_config"] = (
176+
MaintNotificationsConfig(enabled=False)
177+
)
178+
171179
if database_config.from_url:
172180
client = self.client_class.from_url(
173181
database_config.from_url, **database_config.client_kwargs

tests/test_multidb/test_config.py

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import pytest
44

55
from redis.connection import ConnectionPool
6+
from redis.maint_notifications import MaintNotificationsConfig
67
from redis.multidb.circuit import (
78
PBCircuitBreakerAdapter,
89
CircuitBreaker,
@@ -134,6 +135,66 @@ def test_overridden_config(self):
134135
assert config.failover_strategy == mock_failover_strategy
135136
assert config.auto_fallback_interval == auto_fallback_interval
136137

138+
def test_underlying_clients_have_disabled_retry_and_maint_notifications(self):
139+
"""
140+
Test that underlying clients have retry disabled (0 retries)
141+
and maintenance notifications disabled.
142+
"""
143+
db_configs = [
144+
DatabaseConfig(
145+
client_kwargs={"host": "host1", "port": "port1"},
146+
weight=1.0,
147+
),
148+
DatabaseConfig(
149+
client_kwargs={"host": "host2", "port": "port2"},
150+
weight=0.9,
151+
),
152+
]
153+
154+
config = MultiDbConfig(databases_config=db_configs)
155+
databases = config.databases()
156+
157+
assert len(databases) == 2
158+
159+
for db, weight in databases:
160+
# Verify retry is disabled (0 retries)
161+
retry = db.client.get_retry()
162+
assert retry is not None
163+
assert retry.get_retries() == 0
164+
165+
# Verify maint_notifications_config is disabled
166+
# When maint_notifications_config.enabled is False, the pool handler is None
167+
pool = db.client.connection_pool
168+
assert pool._maint_notifications_pool_handler is None
169+
170+
def test_user_provided_maint_notifications_config_is_respected(self):
171+
"""
172+
Test that user-provided maint_notifications_config is not overwritten.
173+
"""
174+
user_maint_config = MaintNotificationsConfig(enabled=True)
175+
db_configs = [
176+
DatabaseConfig(
177+
client_kwargs={
178+
"host": "host1",
179+
"port": "port1",
180+
"protocol": 3, # Required for maint notifications
181+
"maint_notifications_config": user_maint_config,
182+
},
183+
weight=1.0,
184+
),
185+
]
186+
187+
config = MultiDbConfig(databases_config=db_configs)
188+
databases = config.databases()
189+
190+
assert len(databases) == 1
191+
192+
db, weight = databases[0]
193+
# Verify user-provided maint_notifications_config is respected
194+
pool = db.client.connection_pool
195+
assert pool._maint_notifications_pool_handler is not None
196+
assert pool._maint_notifications_pool_handler.config.enabled is True
197+
137198

138199
@pytest.mark.onlynoncluster
139200
class TestDatabaseConfig:

0 commit comments

Comments
 (0)