chore(issues): post GA cleanup for GroupSnooze caches (#69975)

Cleaning up #69556 and
#69939

Also changing cached `value` to more meaningful names.

Author: Bartek Ogryczak

src/sentry/conf/server.py

@@ -1577,10 +1577,6 @@ def custom_parameter_sort(parameter: dict) -> tuple[str, int]:
     "organizations:grouping-title-ui": False,
     # Enable experimental new version of Merged Issues where sub-hashes are shown
     "organizations:grouping-tree-ui": False,
-    # Enable caching group counts in GroupSnooze
-    "organizations:groupsnooze-cached-counts": False,
-    # Enable caching group frequency rates in GroupSnooze
-    "organizations:groupsnooze-cached-rates": False,
     # Allows an org to have a larger set of project ownership rules per project
     "organizations:higher-ownership-limit": False,
     # Enable incidents feature

src/sentry/features/temporary.py

@@ -77,8 +77,6 @@ def register_temporary_features(manager: FeatureManager):
     manager.add("organizations:grouping-suppress-unnecessary-secondary-hash", OrganizationFeature, FeatureHandlerStrategy.INTERNAL)
     manager.add("organizations:grouping-title-ui", OrganizationFeature, FeatureHandlerStrategy.REMOTE)
     manager.add("organizations:grouping-tree-ui", OrganizationFeature, FeatureHandlerStrategy.REMOTE)
-    manager.add("organizations:groupsnooze-cached-counts", OrganizationFeature, FeatureHandlerStrategy.OPTIONS)
-    manager.add("organizations:groupsnooze-cached-rates", OrganizationFeature, FeatureHandlerStrategy.OPTIONS)
     manager.add("organizations:higher-ownership-limit", OrganizationFeature, FeatureHandlerStrategy.INTERNAL)
     manager.add("organizations:increased-issue-owners-rate-limit", OrganizationFeature, FeatureHandlerStrategy.INTERNAL)
     manager.add("organizations:integrations-custom-alert-priorities", OrganizationFeature, FeatureHandlerStrategy.REMOTE)

src/sentry/models/groupsnooze.py

@@ -7,7 +7,7 @@
 from django.db.models.signals import post_delete, post_save
 from django.utils import timezone
 
-from sentry import features, tsdb
+from sentry import tsdb
 from sentry.backup.scopes import RelocationScope
 from sentry.db.models import (
     BaseManager,
@@ -97,28 +97,20 @@ def is_valid(
         return True
 
     def test_frequency_rates(self) -> bool:
-        if features.has(
-            "organizations:groupsnooze-cached-rates", organization=self.group.project.organization
-        ):
-            return self.test_frequency_rates_w_cache()
-        else:
-            return self.test_frequency_rates_no_cache()
-
-    def test_frequency_rates_w_cache(self) -> bool:
         cache_key = f"groupsnooze:v1:{self.id}:test_frequency_rate:events_seen_counter"
 
         cache_ttl = self.window * 60  # Redis TTL in seconds (window is in minutes)
 
-        value: int | float = float("inf")  # using +inf as a sentinel value
+        cached_event_count: int | float = float("inf")  # using +inf as a sentinel value
 
         try:
-            value = cache.incr(cache_key)
+            cached_event_count = cache.incr(cache_key)
             cache.touch(cache_key, cache_ttl)
         except ValueError:
             # key doesn't exist, fall back on sentinel value
             pass
 
-        if value < self.count:
+        if cached_event_count < self.count:
             metrics.incr("groupsnooze.test_frequency_rates", tags={"cached": "true", "hit": "true"})
             return True
 
@@ -145,27 +137,6 @@ def test_frequency_rates_w_cache(self) -> bool:
 
         return True
 
-    def test_frequency_rates_no_cache(self) -> bool:
-        metrics.incr("groupsnooze.test_frequency_rates", tags={"cached": "false"})
-        metrics.incr("groupsnooze.test_frequency_rates.snuba_call")
-
-        end = timezone.now()
-        start = end - timedelta(minutes=self.window)
-
-        rate = tsdb.backend.get_sums(
-            model=get_issue_tsdb_group_model(self.group.issue_category),
-            keys=[self.group_id],
-            start=start,
-            end=end,
-            tenant_ids={"organization_id": self.group.project.organization_id},
-            referrer_suffix="frequency_snoozes",
-        )[self.group_id]
-
-        if rate >= self.count:
-            return False
-
-        return True
-
     def test_user_rates_or_counts(self, group: Group) -> bool:
         """
         Test if the number of unique users or rate of users seen by the group is below the snooze threshold.
@@ -178,59 +149,28 @@ def test_user_rates_or_counts(self, group: Group) -> bool:
           to query Snuba. This functionality relies on the fact that this is called in
           post-processing for every event, so we can assume that the call-count == event count.
         """
-        if features.has(
-            "organizations:groupsnooze-cached-counts", organization=self.group.project.organization
-        ):
-            if self.user_window:
-                if not self.test_user_rates_w_cache():
-                    return False
-            elif not self.test_user_counts_w_cache(group):
-                return False
-            return True
-        else:
-            if self.user_window:
-                if not self.test_user_rates_no_cache():
-                    return False
-            elif not self.test_user_counts_no_cache(group):
+        if self.user_window:
+            if not self.test_user_rates():
                 return False
-            return True
-
-    def test_user_rates_no_cache(self) -> bool:
-        metrics.incr("groupsnooze.test_user_rates", tags={"cached": "false"})
-        metrics.incr("groupsnooze.test_user_rates.snuba_call")
-
-        end = timezone.now()
-        start = end - timedelta(minutes=self.user_window)
-
-        rate = tsdb.backend.get_distinct_counts_totals(
-            model=get_issue_tsdb_user_group_model(self.group.issue_category),
-            keys=[self.group_id],
-            start=start,
-            end=end,
-            tenant_ids={"organization_id": self.group.project.organization_id},
-            referrer_suffix="user_count_snoozes",
-        )[self.group_id]
-
-        if rate >= self.user_count:
+        elif not self.test_user_counts(group):
             return False
-
         return True
 
-    def test_user_rates_w_cache(self) -> bool:
+    def test_user_rates(self) -> bool:
         cache_key = f"groupsnooze:v1:{self.id}:test_user_rate:events_seen_counter"
 
         cache_ttl = self.user_window * 60  # Redis TTL in seconds (window is in minutes)
 
-        value: int | float = float("inf")  # using +inf as a sentinel value
+        cached_event_count: int | float = float("inf")  # using +inf as a sentinel value
 
         try:
-            value = cache.incr(cache_key)
+            cached_event_count = cache.incr(cache_key)
             cache.touch(cache_key, cache_ttl)
         except ValueError:
             # key doesn't exist, fall back on sentinel value
             pass
 
-        if value < self.user_count:
+        if cached_event_count < self.user_count:
             # if number of hits within the window is less than the threshold, we can't have reached enough users
             metrics.incr("groupsnooze.test_user_rates", tags={"cached": "true", "hit": "true"})
             return True
@@ -258,17 +198,7 @@ def test_user_rates_w_cache(self) -> bool:
 
         return True
 
-    def test_user_counts_no_cache(self, group: Group) -> bool:
-        metrics.incr("groupsnooze.test_user_counts", tags={"cached": "false"})
-        metrics.incr("groupsnooze.test_user_counts.snuba_call")
-
-        threshold = self.user_count + self.state["users_seen"]
-        real_count = group.count_users_seen(
-            referrer=Referrer.TAGSTORE_GET_GROUPS_USER_COUNTS_GROUP_SNOOZE.value
-        )
-        return real_count < threshold
-
-    def test_user_counts_w_cache(self, group: Group) -> bool:
+    def test_user_counts(self, group: Group) -> bool:
         cache_key = f"groupsnooze:v1:{self.id}:test_user_counts:events_seen_counter"
         try:
             users_seen = self.state["users_seen"]
@@ -279,14 +209,14 @@ def test_user_counts_w_cache(self, group: Group) -> bool:
 
         CACHE_TTL = 3600  # Redis TTL in seconds
 
-        value: int | float = float("inf")  # using +inf as a sentinel value
+        cached_event_count: int | float = float("inf")  # using +inf as a sentinel value
         try:
-            value = cache.incr(cache_key)
+            cached_event_count = cache.incr(cache_key)
         except ValueError:
             # key doesn't exist, fall back on sentinel value
             pass
 
-        if value < threshold:
+        if cached_event_count < threshold:
             # if we've seen less than that many events, we can't possibly have seen enough users
             metrics.incr("groupsnooze.test_user_counts", tags={"cached": "true", "hit": "true"})
             return True

tests/sentry/models/test_groupsnooze.py

@@ -10,7 +10,6 @@
 from sentry.models.groupsnooze import GroupSnooze
 from sentry.testutils.cases import PerformanceIssueTestCase, SnubaTestCase, TestCase
 from sentry.testutils.helpers.datetime import before_now, freeze_time, iso_format
-from sentry.testutils.helpers.features import apply_feature_flag_on_cls
 from sentry.utils.samples import load_data
 from tests.sentry.issues.test_utils import SearchIssueTestMixin
 
@@ -188,16 +187,6 @@ def test_rate_reached_generic_issue(self):
         snooze = GroupSnooze.objects.create(group=generic_group, count=10, window=24 * 60)
         assert not snooze.is_valid(test_rates=True)
 
-
-@apply_feature_flag_on_cls("organizations:groupsnooze-cached-counts")
-@apply_feature_flag_on_cls("organizations:groupsnooze-cached-rates")
-class GroupSnoozeWCacheTest(GroupSnoozeTest):
-    """
-    Test the cached version of the snooze.
-    Runs all the test defined in GroupSnoozeTest with the cached version of the snooze.
-    Plus the tests defined below.
-    """
-
     def test_test_user_rates_w_cache(self):
         snooze = GroupSnooze.objects.create(group=self.group, user_count=100, user_window=60)