Skip to content

Commit 4b9d5e4

Browse files
authored
ref(rules): Add functionality to RedisBuffer (#68049)
Add methods to `RedisBuffer` to add data about rules that we can later use to process "slow" rules. Closes [#238](getsentry/team-core-product-foundations#238)
1 parent 4566f9c commit 4b9d5e4

File tree

2 files changed

+135
-9
lines changed

2 files changed

+135
-9
lines changed

src/sentry/buffer/redis.py

Lines changed: 55 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import pickle
55
import threading
66
from datetime import date, datetime, timezone
7+
from enum import Enum
78
from time import time
89
from typing import Any
910

@@ -32,6 +33,8 @@
3233
# Debounce our JSON validation a bit in order to not cause too much additional
3334
# load everywhere
3435
_last_validation_log: float | None = None
36+
Pipeline = Any
37+
# TODO type Pipeline instead of using Any here
3538

3639

3740
def _validate_json_roundtrip(value: dict[str, Any], model: type[models.Model]) -> None:
@@ -49,6 +52,13 @@ def _validate_json_roundtrip(value: dict[str, Any], model: type[models.Model]) -
4952
logger.exception("buffer.invalid_value", extra={"value": value, "model": model})
5053

5154

55+
class RedisOperation(Enum):
56+
SET_ADD = "sadd"
57+
SET_GET = "smembers"
58+
HASH_ADD = "hset"
59+
HASH_GET_ALL = "hgetall"
60+
61+
5262
class PendingBuffer:
5363
def __init__(self, size: int):
5464
assert size > 0
@@ -208,6 +218,48 @@ def get(
208218
col: (int(results[i]) if results[i] is not None else 0) for i, col in enumerate(columns)
209219
}
210220

221+
def get_redis_connection(self, key: str) -> Pipeline | None:
222+
if is_instance_redis_cluster(self.cluster, self.is_redis_cluster):
223+
conn = self.cluster
224+
elif is_instance_rb_cluster(self.cluster, self.is_redis_cluster):
225+
conn = self.cluster.get_local_client_for_key(key)
226+
else:
227+
raise AssertionError("unreachable")
228+
229+
pipe = conn.pipeline()
230+
return pipe
231+
232+
def _execute_redis_operation(self, key: str, operation: RedisOperation, *args: Any) -> Any:
233+
pending_key = self._make_pending_key_from_key(key)
234+
pipe = self.get_redis_connection(pending_key)
235+
if pipe:
236+
getattr(pipe, operation.value)(key, *args)
237+
if args:
238+
pipe.expire(key, self.key_expire)
239+
return pipe.execute()
240+
241+
def push_to_set(self, key: str, value: list[int] | int) -> None:
242+
self._execute_redis_operation(key, RedisOperation.SET_ADD, value)
243+
244+
def get_set(self, key: str) -> list[set[int]]:
245+
return self._execute_redis_operation(key, RedisOperation.SET_GET)
246+
247+
def push_to_hash(
248+
self,
249+
model: type[models.Model],
250+
filters: dict[str, models.Model | str | int],
251+
field: str,
252+
value: int,
253+
) -> None:
254+
key = self._make_key(model, filters)
255+
self._execute_redis_operation(key, RedisOperation.HASH_ADD, field, value)
256+
257+
def get_hash(
258+
self, model: type[models.Model], field: dict[str, models.Model | str | int]
259+
) -> dict[str, str]:
260+
key = self._make_key(model, field)
261+
return self._execute_redis_operation(key, RedisOperation.HASH_GET_ALL)
262+
211263
def incr(
212264
self,
213265
model: type[models.Model],
@@ -226,19 +278,13 @@ def incr(
226278
- Perform a set on signal_only (only if True)
227279
- Add hashmap key to pending flushes
228280
"""
229-
230281
key = self._make_key(model, filters)
231282
pending_key = self._make_pending_key_from_key(key)
232283
# We can't use conn.map() due to wanting to support multiple pending
233284
# keys (one per Redis partition)
234-
if is_instance_redis_cluster(self.cluster, self.is_redis_cluster):
235-
conn = self.cluster
236-
elif is_instance_rb_cluster(self.cluster, self.is_redis_cluster):
237-
conn = self.cluster.get_local_client_for_key(key)
238-
else:
239-
raise AssertionError("unreachable")
240-
241-
pipe = conn.pipeline()
285+
pipe = self.get_redis_connection(key)
286+
if not pipe:
287+
return
242288
pipe.hsetnx(key, "m", f"{model.__module__}.{model.__name__}")
243289
_validate_json_roundtrip(filters, model)
244290

tests/sentry/buffer/test_redis.py

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import datetime
22
import pickle
3+
from collections import defaultdict
34
from unittest import mock
45

56
import pytest
@@ -252,6 +253,85 @@ def test_process_pending_partitions_none(self, process_pending, process_incr):
252253
# Make sure we didn't queue up more
253254
assert len(process_pending.apply_async.mock_calls) == 2
254255

256+
def group_rule_data_by_project_id(self, buffer, project_ids):
257+
project_ids_to_rule_data = defaultdict(list)
258+
for proj_id in project_ids[0]:
259+
rule_group_pairs = buffer.get_hash(Project, {"project_id": proj_id})
260+
for pair in rule_group_pairs:
261+
for k, v in pair.items():
262+
if isinstance(k, bytes):
263+
k = k.decode("utf-8")
264+
if isinstance(v, bytes):
265+
v = v.decode("utf-8")
266+
project_ids_to_rule_data[int(proj_id)].append({k: v})
267+
return project_ids_to_rule_data
268+
269+
def test_enqueue(self):
270+
PROJECT_ID_BUFFER_LIST_KEY = "project_id_buffer_list"
271+
project_id = 1
272+
rule_id = 2
273+
group_id = 3
274+
event_id = 4
275+
group2_id = 5
276+
event2_id = 6
277+
278+
project_id2 = 7
279+
rule2_id = 8
280+
group3_id = 9
281+
event3_id = 10
282+
event4_id = 11
283+
284+
# store the project ids
285+
self.buf.push_to_set(key=PROJECT_ID_BUFFER_LIST_KEY, value=project_id)
286+
self.buf.push_to_set(key=PROJECT_ID_BUFFER_LIST_KEY, value=project_id2)
287+
288+
# store the rules and group per project
289+
self.buf.push_to_hash(
290+
model=Project,
291+
filters={"project_id": project_id},
292+
field=f"{rule_id}:{group_id}",
293+
value=event_id,
294+
)
295+
self.buf.push_to_hash(
296+
model=Project,
297+
filters={"project_id": project_id},
298+
field=f"{rule_id}:{group2_id}",
299+
value=event2_id,
300+
)
301+
self.buf.push_to_hash(
302+
model=Project,
303+
filters={"project_id": project_id2},
304+
field=f"{rule2_id}:{group3_id}",
305+
value=event3_id,
306+
)
307+
308+
project_ids = self.buf.get_set(PROJECT_ID_BUFFER_LIST_KEY)
309+
assert project_ids
310+
311+
project_ids_to_rule_data = self.group_rule_data_by_project_id(self.buf, project_ids)
312+
assert project_ids_to_rule_data[project_id][0].get(f"{rule_id}:{group_id}") == str(event_id)
313+
assert project_ids_to_rule_data[project_id][1].get(f"{rule_id}:{group2_id}") == str(
314+
event2_id
315+
)
316+
assert project_ids_to_rule_data[project_id2][0].get(f"{rule2_id}:{group3_id}") == str(
317+
event3_id
318+
)
319+
320+
# overwrite the value to event4_id
321+
self.buf.push_to_hash(
322+
model=Project,
323+
filters={"project_id": project_id2},
324+
field=f"{rule2_id}:{group3_id}",
325+
value=event4_id,
326+
)
327+
328+
project_ids_to_rule_data = project_ids_to_rule_data = self.group_rule_data_by_project_id(
329+
self.buf, project_ids
330+
)
331+
assert project_ids_to_rule_data[project_id2][0].get(f"{rule2_id}:{group3_id}") == str(
332+
event4_id
333+
)
334+
255335
@mock.patch("sentry.buffer.redis.RedisBuffer._make_key", mock.Mock(return_value="foo"))
256336
@mock.patch("sentry.buffer.base.Buffer.process")
257337
def test_process_uses_signal_only(self, process):

0 commit comments

Comments
 (0)