feat(dlq): Extend Count policy to save to/load from Redis #2537
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2537 +/- ##
==========================================
+ Coverage 92.68% 92.70% +0.01%
==========================================
Files 609 611 +2
Lines 28123 28200 +77
==========================================
+ Hits 26065 26142 +77
Misses 2058 2058
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Using a Redis Hash instead of a Sorted Set since the values (# of hits in time bucket) will not be unique. There is a better way of doing this using the new Redis TimeSeries type but that is only available in Redis >= 4.0.0.
I'm not sure what directory the policy should live in, left it in /snuba/state for now but open to suggestions.
snuba/state/stateful_count.py
Outdated
| """ | ||
|
|
||
| def __init__(self, redis_hash_name: str, limit: int, seconds: int = 60) -> None: | ||
| self.__name = redis_hash_name |
There was a problem hiding this comment.
just curious is the redis_hash_name going to be set by the consumer? like will different consumers have different hash names?
There was a problem hiding this comment.
Yeah each consumer would have its own DLQ, which means a policy would be instantiated per consumer. The name of the hash storing the counts would be set wherever the consumer is instantiated with a DLQ.
There was a problem hiding this comment.
What's the reason for putting StatefulCountInvalidMessagePolicy in Snuba and not arroyo? I see a valid use case for this in arroyo. Sentry can now start using arroyo. The Sentry consumers like indexer and subscriptions would be more resilient if they also had DLQ for them. Having it in arroyo would make it possible for those consumers to use it in the future.
Co-authored-by: Lyn Nagara <lyn.nagara@gmail.com>
Redis is only accessible in Snuba and not Arroyo, the base class does this counting anyway but doesn't add anything to Redis like this policy does. Sentry could simply use that policy as is or extend it like this one if the state of hits should be saved/loaded. |
Added a new DLQ Policy which extends the Count policy and adds functionality to save the hits in Redis and to be able to load the state upon instantiation.