Refactor healthcheck to use PING instead of ECHO (#3811)

Author: vladvildanov

redis/asyncio/multidb/client.py

@@ -14,10 +14,12 @@
 from redis.multidb.circuit import State as CBState
 from redis.multidb.exception import NoValidDatabaseException, UnhealthyDatabaseException
 from redis.typing import ChannelT, EncodableT, KeyT
+from redis.utils import experimental
 
 logger = logging.getLogger(__name__)
 
 
+@experimental
 class MultiDBClient(AsyncRedisModuleCommands, AsyncCoreCommands):
     """
     Client that operates on multiple logical Redis databases.

redis/asyncio/multidb/config.py

@@ -20,9 +20,9 @@
     DEFAULT_HEALTH_CHECK_INTERVAL,
     DEFAULT_HEALTH_CHECK_POLICY,
     DEFAULT_HEALTH_CHECK_PROBES,
-    EchoHealthCheck,
     HealthCheck,
     HealthCheckPolicies,
+    PingHealthCheck,
 )
 from redis.asyncio.retry import Retry
 from redis.backoff import ExponentialWithJitterBackoff, NoBackoff
@@ -203,7 +203,7 @@ def default_failure_detectors(self) -> List[AsyncFailureDetector]:
 
     def default_health_checks(self) -> List[HealthCheck]:
         return [
-            EchoHealthCheck(),
+            PingHealthCheck(),
         ]
 
     def default_failover_strategy(self) -> AsyncFailoverStrategy:

redis/asyncio/multidb/healthcheck.py

@@ -170,26 +170,19 @@ class HealthCheckPolicies(Enum):
 DEFAULT_HEALTH_CHECK_POLICY: HealthCheckPolicies = HealthCheckPolicies.HEALTHY_ALL
 
 
-class EchoHealthCheck(HealthCheck):
+class PingHealthCheck(HealthCheck):
     """
-    Health check based on ECHO command.
+    Health check based on PING command.
     """
 
     async def check_health(self, database) -> bool:
-        expected_message = ["healthcheck", b"healthcheck"]
-
         if isinstance(database.client, Redis):
-            actual_message = await database.client.execute_command(
-                "ECHO", "healthcheck"
-            )
-            return actual_message in expected_message
+            return await database.client.execute_command("PING")
         else:
             # For a cluster checks if all nodes are healthy.
             all_nodes = database.client.get_nodes()
             for node in all_nodes:
-                actual_message = await node.execute_command("ECHO", "healthcheck")
-
-                if actual_message not in expected_message:
+                if not await node.redis_connection.execute_command("PING"):
                     return False
 
             return True

redis/multidb/client.py

@@ -15,10 +15,12 @@
 from redis.multidb.exception import NoValidDatabaseException, UnhealthyDatabaseException
 from redis.multidb.failure_detector import FailureDetector
 from redis.multidb.healthcheck import HealthCheck, HealthCheckPolicy
+from redis.utils import experimental
 
 logger = logging.getLogger(__name__)
 
 
+@experimental
 class MultiDBClient(RedisModuleCommands, CoreCommands):
     """
     Client that operates on multiple logical Redis databases.

redis/multidb/config.py

@@ -32,9 +32,9 @@
     DEFAULT_HEALTH_CHECK_INTERVAL,
     DEFAULT_HEALTH_CHECK_POLICY,
     DEFAULT_HEALTH_CHECK_PROBES,
-    EchoHealthCheck,
     HealthCheck,
     HealthCheckPolicies,
+    PingHealthCheck,
 )
 from redis.retry import Retry
 
@@ -200,7 +200,7 @@ def default_failure_detectors(self) -> List[FailureDetector]:
 
     def default_health_checks(self) -> List[HealthCheck]:
         return [
-            EchoHealthCheck(),
+            PingHealthCheck(),
         ]
 
     def default_failover_strategy(self) -> FailoverStrategy:

redis/multidb/healthcheck.py

@@ -169,26 +169,19 @@ class HealthCheckPolicies(Enum):
 DEFAULT_HEALTH_CHECK_POLICY: HealthCheckPolicies = HealthCheckPolicies.HEALTHY_ALL
 
 
-class EchoHealthCheck(HealthCheck):
+class PingHealthCheck(HealthCheck):
     """
-    Health check based on ECHO command.
+    Health check based on PING command.
     """
 
     def check_health(self, database) -> bool:
-        expected_message = ["healthcheck", b"healthcheck"]
-
         if isinstance(database.client, Redis):
-            actual_message = database.client.execute_command("ECHO", "healthcheck")
-            return actual_message in expected_message
+            return database.client.execute_command("PING")
         else:
             # For a cluster checks if all nodes are healthy.
             all_nodes = database.client.get_nodes()
             for node in all_nodes:
-                actual_message = node.redis_connection.execute_command(
-                    "ECHO", "healthcheck"
-                )
-
-                if actual_message not in expected_message:
+                if not node.redis_connection.execute_command("PING"):
                     return False
 
             return True

redis/utils.py

@@ -1,6 +1,7 @@
 import datetime
 import logging
 import textwrap
+import warnings
 from collections.abc import Callable
 from contextlib import contextmanager
 from functools import wraps
@@ -326,3 +327,22 @@ async def dummy_fail_async():
     Async fake function for a Retry object if you don't need to handle each failure.
     """
     pass
+
+
+def experimental(cls):
+    """
+    Decorator to mark a class as experimental.
+    """
+    original_init = cls.__init__
+
+    @wraps(original_init)
+    def new_init(self, *args, **kwargs):
+        warnings.warn(
+            f"{cls.__name__} is an experimental and may change or be removed in future versions.",
+            category=UserWarning,
+            stacklevel=2,
+        )
+        original_init(self, *args, **kwargs)
+
+    cls.__init__ = new_init
+    return cls

tests/test_asyncio/test_multidb/test_config.py

@@ -19,7 +19,7 @@
     FailureDetectorAsyncWrapper,
     AsyncFailureDetector,
 )
-from redis.asyncio.multidb.healthcheck import EchoHealthCheck, HealthCheck
+from redis.asyncio.multidb.healthcheck import PingHealthCheck, HealthCheck
 from redis.asyncio.retry import Retry
 from redis.multidb.circuit import CircuitBreaker
 
@@ -58,7 +58,7 @@ def test_default_config(self):
             config.default_failure_detectors()[0], FailureDetectorAsyncWrapper
         )
         assert len(config.default_health_checks()) == 1
-        assert isinstance(config.default_health_checks()[0], EchoHealthCheck)
+        assert isinstance(config.default_health_checks()[0], PingHealthCheck)
         assert config.health_check_interval == DEFAULT_HEALTH_CHECK_INTERVAL
         assert isinstance(
             config.default_failover_strategy(), WeightBasedFailoverStrategy

tests/test_asyncio/test_multidb/test_healthcheck.py

@@ -3,7 +3,7 @@
 
 from redis.asyncio.multidb.database import Database
 from redis.asyncio.multidb.healthcheck import (
-    EchoHealthCheck,
+    PingHealthCheck,
     LagAwareHealthCheck,
     HealthCheck,
     HealthyAllPolicy,
@@ -208,15 +208,15 @@ async def test_policy_raise_unhealthy_database_exception_if_exception_occurs_on_
 
 
 @pytest.mark.onlynoncluster
-class TestEchoHealthCheck:
+class TestPingHealthCheck:
     @pytest.mark.asyncio
     async def test_database_is_healthy_on_echo_response(self, mock_client, mock_cb):
         """
         Mocking responses to mix error and actual responses to ensure that health check retry
         according to given configuration.
         """
-        mock_client.execute_command = AsyncMock(side_effect=["healthcheck"])
-        hc = EchoHealthCheck()
+        mock_client.execute_command = AsyncMock(side_effect=["PONG"])
+        hc = PingHealthCheck()
         db = Database(mock_client, mock_cb, 0.9)
 
         assert await hc.check_health(db)
@@ -230,8 +230,8 @@ async def test_database_is_unhealthy_on_incorrect_echo_response(
         Mocking responses to mix error and actual responses to ensure that health check retry
         according to given configuration.
         """
-        mock_client.execute_command = AsyncMock(side_effect=["wrong"])
-        hc = EchoHealthCheck()
+        mock_client.execute_command = AsyncMock(side_effect=[False])
+        hc = PingHealthCheck()
         db = Database(mock_client, mock_cb, 0.9)
 
         assert not await hc.check_health(db)
@@ -241,9 +241,9 @@ async def test_database_is_unhealthy_on_incorrect_echo_response(
     async def test_database_close_circuit_on_successful_healthcheck(
         self, mock_client, mock_cb
     ):
-        mock_client.execute_command = AsyncMock(side_effect=["healthcheck"])
+        mock_client.execute_command = AsyncMock(side_effect=["PONG"])
         mock_cb.state = CBState.HALF_OPEN
-        hc = EchoHealthCheck()
+        hc = PingHealthCheck()
         db = Database(mock_client, mock_cb, 0.9)
 
         assert await hc.check_health(db)

tests/test_multidb/test_config.py

@@ -16,7 +16,7 @@
 )
 from redis.multidb.database import Database
 from redis.multidb.failure_detector import CommandFailureDetector, FailureDetector
-from redis.multidb.healthcheck import EchoHealthCheck, HealthCheck
+from redis.multidb.healthcheck import PingHealthCheck, HealthCheck
 from redis.multidb.failover import WeightBasedFailoverStrategy, FailoverStrategy
 from redis.retry import Retry
 
@@ -53,7 +53,7 @@ def test_default_config(self):
         assert len(config.default_failure_detectors()) == 1
         assert isinstance(config.default_failure_detectors()[0], CommandFailureDetector)
         assert len(config.default_health_checks()) == 1
-        assert isinstance(config.default_health_checks()[0], EchoHealthCheck)
+        assert isinstance(config.default_health_checks()[0], PingHealthCheck)
         assert config.health_check_interval == DEFAULT_HEALTH_CHECK_INTERVAL
         assert isinstance(
             config.default_failover_strategy(), WeightBasedFailoverStrategy