ref(grouping): Use varying grouphash cache retention (#104460)

When we began to talk about caching grouphashes during ingest, one of the considerations was how much space in the cache we'd end up using. To give ourselves a ceiling, we took the worst-case scenario and made sure it wasn't going to be a problem. According to ops it wasn't, so we proceeded. 

In that worst-case scenario, we never get a cache hit, every grouphash is eligible to be cached (has a group already assigned), and every event has two different grouphashes associated with it. In that case, every event we see would lead to two new hashes being added to the cache. We knew that wasn't actually going to be true - we definitely _will_ get hits, we definitely _will_ encounter new grouphashes (which we won't put in the cache because they aren't yet assigned to a group), and not every event has two grouphashes. That said, until we started doing the actual caching, we didn't know just how far from the truth that worst-case scenario was going to be. 

Now that we have rolled it out, we can see that in fact hashes are being added to the cache only about 12% of the time. This means we can safely play around with longer cache retention times to see if it will meaningfully improve our hit rate. To make such experimentation easier, this PR introduces the ability to have the retention time be variable, so that we can compare different values against each other. Instead of a fixed number, we now will be choosing from a list of possible times. So that we can accurately report the expiry time which was used, we're also for now going to cache the time alongside the value itself. (I tried to cache them together as a tuple, but that ended up not working, so instead the time is cached separately with `_expiry` added onto the end of the cache key.)

Once we find the happy medium between hit rate and storage space, we can back out these changes and go back to using a fixed number. I've therefore left TODOs explaining how to do that so it will be easy to switch back.

Author: lobsterkatie

src/sentry/grouping/ingest/hashing.py

@@ -3,7 +3,7 @@
 import copy
 import logging
 from collections.abc import Iterable, Sequence
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, Literal
 
 import sentry_sdk
 from django.core.cache import cache
@@ -209,28 +209,41 @@ def find_grouphash_with_group(
     return None
 
 
+# TODO: This can go once we've settled on an expiry time for each cache
+def _get_cache_expiry(cache_key: str, cache_type: Literal["existence", "object"]) -> int:
+    option_name = f"grouping.ingest_grouphash_{cache_type}_cache_expiry.trial_values"
+    possible_cache_expiries = options.get(option_name)
+    return possible_cache_expiries[hash(cache_key) % len(possible_cache_expiries)]
+
+
 def _grouphash_exists_for_hash_value(hash_value: str, project: Project, use_caching: bool) -> bool:
     """
     Check whether a given hash value has a corresponding `GroupHash` record in the database.
 
     If `use_caching` is True, cache the boolean result. Cache retention is controlled by the
     `grouping.ingest_grouphash_existence_cache_expiry` option.
+
+    TODO: That last sentence is temporarily untrue. While we're experimenting with retention
+    periods, cache retention is actually controlled by the helper `_get_cache_expiry`.
     """
     with metrics.timer(
         "grouping.get_or_create_grouphashes.check_secondary_hash_existence"
     ) as metrics_tags:
         if use_caching:
             cache_key = get_grouphash_existence_cache_key(hash_value, project.id)
-            cache_expiry = options.get("grouping.ingest_grouphash_existence_cache_expiry")
 
             grouphash_exists = cache.get(cache_key)
+            cache_expiry_used_when_setting = cache.get(
+                cache_key + "_expiry",
+                default=options.get("grouping.ingest_grouphash_existence_cache_expiry"),
+            )
             got_cache_hit = grouphash_exists is not None
             metrics_tags["cache_result"] = "hit" if got_cache_hit else "miss"
 
             if got_cache_hit:
                 metrics_tags["grouphash_exists"] = grouphash_exists
                 # TODO: Temporary tag to let us compare hit rates across different retention periods
-                metrics_tags["expiry_seconds"] = cache_expiry
+                metrics_tags["expiry_seconds"] = cache_expiry_used_when_setting
 
                 return grouphash_exists
 
@@ -240,7 +253,13 @@ def _grouphash_exists_for_hash_value(hash_value: str, project: Project, use_cach
             metrics_tags["grouphash_exists"] = grouphash_exists
             metrics_tags["cache_set"] = True
 
+            # TODO: This can go back to being just
+            #     cache_expiry = options.get("grouping.ingest_grouphash_existence_cache_expiry")
+            #     cache.set(cache_key, grouphash_exists, cache_expiry)
+            # once we've settled on a good retention period
+            cache_expiry = _get_cache_expiry(cache_key, cache_type="existence")
             cache.set(cache_key, grouphash_exists, cache_expiry)
+            cache.set(cache_key + "_expiry", cache_expiry, cache_expiry)
 
         return grouphash_exists
 
@@ -255,21 +274,27 @@ def _get_or_create_single_grouphash(
     `GroupHash` object. (Grouphashes without a group aren't cached because their data is about to
     change when a group is assigned.) Cache retention is controlled by the
     `grouping.ingest_grouphash_object_cache_expiry` option.
+
+    TODO: That last sentence is temporarily untrue. While we're experimenting with retention
+    periods, cache retention is actually controlled by the helper `_get_cache_expiry`.
     """
     with metrics.timer(
         "grouping.get_or_create_grouphashes.get_or_create_grouphash"
     ) as metrics_tags:
         if use_caching:
             cache_key = get_grouphash_object_cache_key(hash_value, project.id)
-            cache_expiry = options.get("grouping.ingest_grouphash_object_cache_expiry")
 
             grouphash = cache.get(cache_key)
+            cache_expiry_used_when_setting = cache.get(
+                cache_key + "_expiry",
+                default=options.get("grouping.ingest_grouphash_object_cache_expiry"),
+            )
             got_cache_hit = grouphash is not None
             metrics_tags["cache_result"] = "hit" if got_cache_hit else "miss"
 
             if got_cache_hit:
                 # TODO: Temporary tag to let us compare hit rates across different retention periods
-                metrics_tags["expiry_seconds"] = cache_expiry
+                metrics_tags["expiry_seconds"] = cache_expiry_used_when_setting
 
                 return (grouphash, False)
 
@@ -281,7 +306,13 @@ def _get_or_create_single_grouphash(
         if use_caching and grouphash.group_id is not None:
             metrics_tags["cache_set"] = True
 
+            # TODO: This can go back to being just
+            #     cache_expiry = options.get("grouping.ingest_grouphash_object_cache_expiry")
+            #     cache.set(cache_key, grouphash, cache_expiry)
+            # once we've settled on a good retention period
+            cache_expiry = _get_cache_expiry(cache_key, cache_type="object")
             cache.set(cache_key, grouphash, cache_expiry)
+            cache.set(cache_key + "_expiry", cache_expiry, cache_expiry)
 
         return (grouphash, created)
 

src/sentry/options/defaults.py

@@ -2914,6 +2914,22 @@
     flags=FLAG_AUTOMATOR_MODIFIABLE,
 )
 
+# TODO: Temporary options to let us play around with expiry times and see what hit rates they give
+# us. Once we've decided, we can stick our values into the two expiry options above and get rid of
+# these two options.
+register(
+    "grouping.ingest_grouphash_existence_cache_expiry.trial_values",
+    type=Sequence,
+    default=[60, 120, 600],
+    flags=FLAG_AUTOMATOR_MODIFIABLE,
+)
+register(
+    "grouping.ingest_grouphash_object_cache_expiry.trial_values",
+    type=Sequence,
+    default=[60, 120, 600],
+    flags=FLAG_AUTOMATOR_MODIFIABLE,
+)
+
 
 # Sample rate for double writing to experimental dsn
 register(

tests/sentry/event_manager/test_event_manager_grouping.py

@@ -10,7 +10,7 @@
 import pytest
 from django.core.cache import cache
 
-from sentry import audit_log, options
+from sentry import audit_log
 from sentry.conf.server import DEFAULT_GROUPING_CONFIG, SENTRY_GROUPING_CONFIG_TRANSITION_DURATION
 from sentry.event_manager import _get_updated_group_title
 from sentry.eventtypes.base import DefaultEvent
@@ -20,7 +20,7 @@
     get_grouphash_object_cache_key,
 )
 from sentry.grouping.ingest.config import update_or_set_grouping_config_if_needed
-from sentry.grouping.ingest.hashing import get_or_create_grouphashes
+from sentry.grouping.ingest.hashing import _get_cache_expiry, get_or_create_grouphashes
 from sentry.models.auditlogentry import AuditLogEntry
 from sentry.models.group import Group
 from sentry.models.grouphash import GroupHash
@@ -400,13 +400,17 @@ def run_test(
             grouping_config_id = "old_config"
             grouping_config_option = "sentry:secondary_grouping_config"
             cache_key = get_grouphash_existence_cache_key(hash_value, self.project.id)
-            cache_expiry = options.get("grouping.ingest_grouphash_existence_cache_expiry")
+            # TODO: This can go back to being `options.get("grouping.ingest_grouphash_existence_cache_expiry")`
+            # once we've settled on a retention period
+            cache_expiry = _get_cache_expiry(cache_key, cache_type="existence")
             cached_value: Any = grouphash_exists
         else:
             grouping_config_id = "new_config"
             grouping_config_option = "sentry:grouping_config"
             cache_key = get_grouphash_object_cache_key(hash_value, self.project.id)
-            cache_expiry = options.get("grouping.ingest_grouphash_object_cache_expiry")
+            # TODO: This can go back to being `options.get("grouping.ingest_grouphash_object_cache_expiry")`
+            # once we've settled on a retention period
+            cache_expiry = _get_cache_expiry(cache_key, cache_type="object")
             cached_value = grouphash
 
         self.project.update_option(grouping_config_option, grouping_config_id)