Revert "build: update redis and remove redis-py-cluster (#64711)"

getsentry-bot · anonrig · getsentry-bot · commit c699781d7e0b · 2024-02-15T16:57:26.000Z
This reverts commit 58c2f2c. Co-authored-by: anonrig <1935246+anonrig@users.noreply.github.com>
diff --git a/fixtures/stubs-for-mypy/rediscluster/__init__.pyi b/fixtures/stubs-for-mypy/rediscluster/__init__.pyi
@@ -0,0 +1,3 @@
+from .client import RedisCluster
+
+__all__ = ('RedisCluster',)
diff --git a/fixtures/stubs-for-mypy/rediscluster/client.pyi b/fixtures/stubs-for-mypy/rediscluster/client.pyi
@@ -0,0 +1,8 @@
+from typing import TypeVar
+
+from redis import Redis
+
+T = TypeVar("T", str, bytes)
+
+class RedisCluster(Redis[T]):
+    ...
diff --git a/fixtures/stubs-for-mypy/rediscluster/exceptions.pyi b/fixtures/stubs-for-mypy/rediscluster/exceptions.pyi
@@ -0,0 +1,3 @@
+from redis.exceptions import RedisError
+
+class ClusterError(RedisError): ...
diff --git a/requirements-base.txt b/requirements-base.txt
@@ -52,7 +52,8 @@ fido2>=0.9.2
 python3-saml>=1.15.0
 PyYAML>=6.0.1
 rb>=1.9.0
-redis>=4.1.0
+redis-py-cluster>=2.1.0
+redis>=3.4.1
 requests-oauthlib>=1.2.0
 requests>=2.25.1
 # [start] jsonschema format validators
@@ -62,7 +63,7 @@ rfc3986-validator>=0.1.1
 sentry-arroyo>=2.16.0
 sentry-kafka-schemas>=0.1.50
 sentry-ophio==0.1.5
-sentry-redis-tools>=0.2.0
+sentry-redis-tools>=0.1.7
 sentry-relay>=0.8.45
 sentry-sdk>=1.39.2
 snuba-sdk>=2.0.25
diff --git a/requirements-dev-frozen.txt b/requirements-dev-frozen.txt
@@ -10,7 +10,6 @@ amqp==5.2.0
 anyio==3.7.1
 asgiref==3.7.2
 async-generator==1.10
-async-timeout==4.0.3
 attrs==23.1.0
 avalara==20.9.0
 beautifulsoup4==4.7.1
@@ -159,7 +158,8 @@ pyuwsgi==2.0.23
 pyvat==1.3.15
 pyyaml==6.0.1
 rb==1.10.0
-redis==4.5.4
+redis==3.4.1
+redis-py-cluster==2.1.0
 referencing==0.30.2
 regex==2022.9.13
 reportlab==4.0.7
@@ -179,7 +179,7 @@ sentry-forked-django-stubs==4.2.7.post3
 sentry-forked-djangorestframework-stubs==3.14.5.post1
 sentry-kafka-schemas==0.1.50
 sentry-ophio==0.1.5
-sentry-redis-tools==0.2.0
+sentry-redis-tools==0.1.7
 sentry-relay==0.8.45
 sentry-sdk==1.39.2
 sentry-usage-accountant==0.0.10
@@ -211,11 +211,10 @@ types-pillow==9.5.0.4
 types-protobuf==4.23.0.1
 types-psutil==5.9.5.16
 types-psycopg2==2.9.21
-types-pyopenssl==23.2.0.2
 types-python-dateutil==2.8.19
 types-pytz==2022.1.2
 types-pyyaml==6.0.11
-types-redis==4.6.0.3
+types-redis==3.5.18
 types-requests==2.31.0.20231231
 types-setuptools==65.3.0
 types-simplejson==3.17.7.2
diff --git a/requirements-dev.txt b/requirements-dev.txt
@@ -53,7 +53,7 @@ types-python-dateutil
 types-pytz
 types-pyyaml
 # make sure to match close-enough to redis==
-types-redis>=4.1
+types-redis<4
 types-requests>=2.31.0.20231231
 types-setuptools
 types-simplejson>=3.17.7.2
diff --git a/requirements-frozen.txt b/requirements-frozen.txt
@@ -9,7 +9,6 @@
 amqp==5.2.0
 anyio==3.7.1
 asgiref==3.7.2
-async-timeout==4.0.3
 attrs==23.1.0
 avalara==20.9.0
 beautifulsoup4==4.7.1
@@ -106,7 +105,8 @@ pyuwsgi==2.0.23
 pyvat==1.3.15
 pyyaml==6.0.1
 rb==1.10.0
-redis==4.5.4
+redis==3.4.1
+redis-py-cluster==2.1.0
 referencing==0.30.2
 regex==2022.9.13
 reportlab==4.0.7
@@ -120,7 +120,7 @@ s3transfer==0.10.0
 sentry-arroyo==2.16.0
 sentry-kafka-schemas==0.1.50
 sentry-ophio==0.1.5
-sentry-redis-tools==0.2.0
+sentry-redis-tools==0.1.7
 sentry-relay==0.8.45
 sentry-sdk==1.39.2
 sentry-usage-accountant==0.0.10
diff --git a/src/sentry/ingest/transaction_clusterer/rules.py b/src/sentry/ingest/transaction_clusterer/rules.py
@@ -60,11 +60,10 @@ def write(self, project: Project, rules: RuleSet) -> None:
         key = self._get_rules_key(project)
 
         with client.pipeline() as p:
+            # to be consistent with other stores, clear previous hash entries:
+            p.delete(key)
             if len(rules) > 0:
-                p.hset(key, mapping=rules)
-            else:
-                # to be consistent with other stores, clear previous hash entries:
-                p.delete(key)
+                p.hmset(key, rules)
             p.execute()
 
     def update_rule(self, project: Project, rule: str, last_used: int) -> None:
diff --git a/src/sentry/processing/backpressure/memory.py b/src/sentry/processing/backpressure/memory.py
@@ -4,7 +4,7 @@
 
 import rb
 import requests
-from redis.cluster import RedisCluster
+from rediscluster import RedisCluster
 
 
 @dataclass
diff --git a/src/sentry/ratelimits/concurrent.py b/src/sentry/ratelimits/concurrent.py
@@ -8,7 +8,6 @@
 
 from sentry.exceptions import InvalidConfiguration
 from sentry.utils import redis
-from sentry.utils.redis import disconnect_redis_connection_pools
 
 logger = logging.getLogger(__name__)
 
@@ -35,7 +34,7 @@ def __init__(self, max_tll_seconds: int = DEFAULT_MAX_TTL_SECONDS) -> None:
     def validate(self) -> None:
         try:
             self.client.ping()
-            disconnect_redis_connection_pools(self.client)
+            self.client.connection_pool.disconnect()
         except Exception as e:
             raise InvalidConfiguration(str(e))
 
diff --git a/src/sentry/ratelimits/redis.py b/src/sentry/ratelimits/redis.py
@@ -11,7 +11,6 @@
 from sentry.ratelimits.base import RateLimiter
 from sentry.utils import redis
 from sentry.utils.hashlib import md5_text
-from sentry.utils.redis import disconnect_redis_connection_pools
 
 if TYPE_CHECKING:
     from sentry.models.project import Project
@@ -66,7 +65,7 @@ def _construct_redis_key(
     def validate(self) -> None:
         try:
             self.client.ping()
-            disconnect_redis_connection_pools(self.client)
+            self.client.connection_pool.disconnect()
         except Exception as e:
             raise InvalidConfiguration(str(e))
 
diff --git a/src/sentry/ratelimits/sliding_windows.py b/src/sentry/ratelimits/sliding_windows.py
@@ -12,7 +12,6 @@
 
 from sentry.exceptions import InvalidConfiguration
 from sentry.utils import redis
-from sentry.utils.redis import disconnect_redis_connection_pools
 from sentry.utils.services import Service
 
 __all__ = ["Quota", "GrantedQuota", "RequestedQuota", "Timestamp"]
@@ -143,7 +142,7 @@ def impl(self):
     def validate(self) -> None:
         try:
             self.client.ping()
-            disconnect_redis_connection_pools(self.client)
+            self.client.connection_pool.disconnect()
         except Exception as e:
             raise InvalidConfiguration(str(e))
 
diff --git a/src/sentry/statistical_detectors/redis.py b/src/sentry/statistical_detectors/redis.py
@@ -55,7 +55,7 @@ def bulk_write_states(
                 if state is None:
                     continue
                 key = self.make_key(payload)
-                pipeline.hset(key, mapping=state)
+                pipeline.hmset(key, state)
                 pipeline.expire(key, self.ttl)
 
             pipeline.execute()
diff --git a/src/sentry/tsdb/redis.py b/src/sentry/tsdb/redis.py
@@ -11,7 +11,7 @@
 
 from django.utils import timezone
 from django.utils.encoding import force_bytes
-from redis.commands.core import Script
+from redis.client import Script
 
 from sentry.tsdb.base import BaseTSDB
 from sentry.utils.compat import crc32
diff --git a/src/sentry/utils/kvstore/redis.py b/src/sentry/utils/kvstore/redis.py
@@ -3,8 +3,8 @@
 from datetime import timedelta
 from typing import TypeVar
 
-from redis.client import StrictRedis
-from redis.cluster import RedisCluster
+from redis import StrictRedis
+from rediscluster import RedisCluster
 
 from sentry.utils.kvstore.abstract import KVStorage
 
diff --git a/src/sentry/utils/redis.py b/src/sentry/utils/redis.py
@@ -9,11 +9,11 @@
 
 import rb
 from django.utils.functional import SimpleLazyObject
-from redis.client import StrictRedis
-from redis.cluster import RedisCluster
-from redis.commands.core import Script
+from redis.client import Script
 from redis.connection import ConnectionPool
-from redis.exceptions import BusyLoadingError, ClusterError, ConnectionError
+from redis.exceptions import BusyLoadingError, ConnectionError
+from rediscluster import RedisCluster
+from rediscluster.exceptions import ClusterError
 from sentry_redis_tools.failover_redis import FailoverRedis
 
 from sentry import options
@@ -157,7 +157,7 @@ def __str__(self):
         return "Redis Cluster"
 
 
-TCluster = TypeVar("TCluster", rb.Cluster, RedisCluster | StrictRedis)
+TCluster = TypeVar("TCluster", rb.Cluster, RedisCluster)
 
 
 class ClusterManager(Generic[TCluster]):
@@ -167,7 +167,7 @@ def __init__(self: ClusterManager[rb.Cluster], options_manager) -> None:
 
     @overload
     def __init__(
-        self: ClusterManager[RedisCluster | StrictRedis], options_manager, cluster_type: type[Any]
+        self: ClusterManager[RedisCluster], options_manager, cluster_type: type[Any]
     ) -> None:
         ...
 
@@ -205,7 +205,7 @@ def get(self, key: str, *, decode_responses: bool = True) -> TCluster:
 # completed, remove the rb ``clusters`` module variable and rename
 # redis_clusters to clusters.
 clusters: ClusterManager[rb.Cluster] = ClusterManager(options.default_manager)
-redis_clusters: ClusterManager[RedisCluster | StrictRedis] = ClusterManager(
+redis_clusters: ClusterManager[RedisCluster] = ClusterManager(
     options.default_manager, _RedisCluster
 )
 
@@ -320,13 +320,3 @@ def call_script(client, keys, args):
         return script[0](keys, args, client)
 
     return call_script
-
-
-# Since the implementation to disconnect connection pools differ between
-# RedisCluster and StrictRedis after v4, we need this function.
-def disconnect_redis_connection_pools(client: StrictRedis | RedisCluster) -> None:
-    if isinstance(client, RedisCluster):
-        client.disconnect_connection_pools()
-    else:
-        assert isinstance(client, StrictRedis)
-        client.connection_pool.disconnect()
diff --git a/tests/sentry/buffer/test_redis.py b/tests/sentry/buffer/test_redis.py
@@ -62,9 +62,9 @@ def test_process_pending_multiple_batches(self, process_incr):
     def test_process_does_bubble_up_json(self, process):
         client = self.buf.get_routing_client()
 
-        client.hset(
+        client.hmset(
             "foo",
-            mapping={
+            {
                 "e+foo": '["s","bar"]',
                 "e+datetime": '["dt","1493791566.000000"]',
                 "f": '{"pk": ["i","1"]}',
@@ -87,9 +87,9 @@ def test_process_does_bubble_up_json(self, process):
     def test_process_does_bubble_up_pickle(self, process):
         client = self.buf.get_routing_client()
 
-        client.hset(
+        client.hmset(
             "foo",
-            mapping={
+            {
                 "e+foo": "S'bar'\np1\n.",
                 "f": "(dp1\nS'pk'\np2\nI1\ns.",
                 "i+times_seen": "2",
@@ -246,9 +246,9 @@ def test_process_pending_partitions_none(self, process_pending, process_incr):
     def test_process_uses_signal_only(self, process):
         client = self.buf.get_routing_client()
 
-        client.hset(
+        client.hmset(
             "foo",
-            mapping={
+            {
                 "f": '{"pk": ["i","1"]}',
                 "i+times_seen": "1",
                 "m": "unittest.mock.Mock",
diff --git a/tests/sentry/utils/test_redis.py b/tests/sentry/utils/test_redis.py
@@ -53,9 +53,9 @@ def test_specific_cluster(self, RetryingRedisCluster):
         # object to verify it's correct.
 
         # cluster foo is fine since it's a single node, without specific client_class
-        assert isinstance(manager.get("foo")._setupfunc(), FailoverRedis)  # type: ignore[union-attr]
-        # baz works because it's explicitly is_redis_cluster
-        assert manager.get("baz")._setupfunc() is RetryingRedisCluster.return_value  # type: ignore[union-attr]
+        assert isinstance(manager.get("foo")._setupfunc(), FailoverRedis)  # type: ignore[attr-defined]
+        # baz works becasue it's explicitly is_redis_cluster
+        assert manager.get("baz")._setupfunc() is RetryingRedisCluster.return_value  # type: ignore[attr-defined]
 
         # bar is not a valid redis or redis cluster definition
         # becasue it is two hosts, without explicitly saying is_redis_cluster

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+from .client import RedisCluster`
	`2`	`+`
	`3`	`+__all__ = ('RedisCluster',)`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+from redis.exceptions import RedisError`
	`2`	`+`
	`3`	`+class ClusterError(RedisError): ...`