fix(releases): Prevent row-lock contention on last_seen bump

src/sentry/models/releaseenvironment.py

@@ -1,5 +1,3 @@
-from datetime import timedelta
-
 from django.db import models
 from django.utils import timezone
 
@@ -13,6 +11,7 @@
 )
 from sentry.utils import metrics
 from sentry.utils.cache import cache
+from sentry.utils.last_seen import try_bump_last_seen
 
 
 @cell_silo_model
@@ -63,16 +62,15 @@ def _get_or_create_impl(cls, project, release, environment, datetime, metric_tag
 
         metric_tags["created"] = "true" if created else "false"
 
-        # TODO(dcramer): this would be good to buffer, but until then we minimize
-        # updates to once a minute, and allow Postgres to optimistically skip
-        # it even if we can't
-        if not created and instance.last_seen < datetime - timedelta(seconds=60):
-            metric_tags["bumped"] = "true"
-            cls.objects.filter(
-                id=instance.id, last_seen__lt=datetime - timedelta(seconds=60)
-            ).update(last_seen=datetime)
-            instance.last_seen = datetime
-            cache.set(cache_key, instance, 3600)
+        if not created:
+            try_bump_last_seen(
+                model_class=cls,
+                instance=instance,
+                datetime=datetime,
+                bump_key=f"releaseenv_bump:{instance.id}",
+                cache_key=cache_key,
+                metrics_tags=metric_tags,
+            )
         else:
             metric_tags["bumped"] = "false"
 

src/sentry/models/releaseprojectenvironment.py

@@ -1,6 +1,6 @@
 from __future__ import annotations
 
-from datetime import datetime, timedelta
+from datetime import datetime
 from enum import Enum
 from typing import TypedDict
 
@@ -17,6 +17,7 @@
 )
 from sentry.utils import metrics
 from sentry.utils.cache import cache
+from sentry.utils.last_seen import try_bump_last_seen
 
 
 class ReleaseStages(str, Enum):
@@ -82,14 +83,15 @@ def _get_or_create_impl(cls, release, project, environment, datetime, metrics_ta
 
         metrics_tags["created"] = "true" if created else "false"
 
-        # Same as releaseenvironment model. Minimizes last_seen updates to once a minute
-        if not created and instance.last_seen < datetime - timedelta(seconds=60):
-            cls.objects.filter(
-                id=instance.id, last_seen__lt=datetime - timedelta(seconds=60)
-            ).update(last_seen=datetime)
-            instance.last_seen = datetime
-            cache.set(cache_key, instance, 3600)
-            metrics_tags["bumped"] = "true"
+        if not created:
+            try_bump_last_seen(
+                model_class=cls,
+                instance=instance,
+                datetime=datetime,
+                bump_key=f"releaseprojectenv_bump:{instance.id}",
+                cache_key=cache_key,
+                metrics_tags=metrics_tags,
+            )
         else:
             metrics_tags["bumped"] = "false"
 

src/sentry/utils/last_seen.py

@@ -0,0 +1,45 @@
+from __future__ import annotations
+
+from datetime import datetime as dt
+from datetime import timedelta
+from typing import Any, Protocol
+
+from django.core.cache import cache
+from django.db.utils import OperationalError
+
+
+class HasLastSeen(Protocol):
+    id: int
+    last_seen: dt
+
+
+def try_bump_last_seen(
+    *,
+    model_class: Any,
+    instance: HasLastSeen,
+    datetime: dt,
+    bump_key: str,
+    cache_key: str,
+    metrics_tags: dict[str, str],
+) -> bool:
+    """Throttled last_seen bump — at most once per 60s per row via a cache-based lock."""
+    if instance.last_seen >= datetime - timedelta(seconds=60):
+        metrics_tags["bumped"] = "false"
+        return False
+
+    if not cache.add(bump_key, "1", timeout=60):
+        metrics_tags["bumped"] = "skipped"
+        return False
+
+    try:
+        model_class.objects.filter(id=instance.id, last_seen__lt=datetime).update(
+            last_seen=datetime
+        )
+    except OperationalError:
+        metrics_tags["bumped"] = "error"
+        return False
+
+    instance.last_seen = datetime
+    cache.set(cache_key, instance, 3600)
+    metrics_tags["bumped"] = "true"
+    return True

tests/sentry/models/test_releaseenvironment.py

@@ -1,11 +1,14 @@
 from datetime import timedelta
+from unittest.mock import patch
 
+from django.db.utils import OperationalError
 from django.utils import timezone
 
 from sentry.models.environment import Environment
 from sentry.models.release import Release
 from sentry.models.releaseenvironment import ReleaseEnvironment
 from sentry.testutils.cases import TestCase
+from sentry.utils.cache import cache
 
 
 class GetOrCreateTest(TestCase):
@@ -52,3 +55,68 @@ def test_simple(self) -> None:
         )
         assert relenv.id == relenv2.id
         assert ReleaseEnvironment.objects.get(id=relenv.id).last_seen == relenv2.last_seen
+
+    def test_bump_skipped_when_cache_lock_held(self) -> None:
+        """Second worker skips the DB update when another worker already holds the bump lock."""
+        project = self.create_project(name="foo")
+        datetime_now = timezone.now()
+
+        release = Release.objects.create(organization_id=project.organization_id, version="abcdef")
+        release.add_project(project)
+        env = Environment.objects.create(organization_id=project.organization_id, name="prod")
+
+        ReleaseEnvironment.get_or_create(
+            project=project, release=release, environment=env, datetime=datetime_now
+        )
+
+        datetime_next = datetime_now + timedelta(days=1)
+
+        re = ReleaseEnvironment.objects.get(
+            organization_id=project.organization_id, release=release, environment=env
+        )
+        bump_key = f"releaseenv_bump:{re.id}"
+        cache.set(bump_key, "1", timeout=60)
+
+        with patch.object(
+            ReleaseEnvironment.objects,
+            "filter",
+            wraps=ReleaseEnvironment.objects.filter,
+        ) as mock_filter:
+            result = ReleaseEnvironment.get_or_create(
+                project=project, release=release, environment=env, datetime=datetime_next
+            )
+            for call in mock_filter.call_args_list:
+                assert "last_seen__lt" not in (call.kwargs or {})
+
+        re.refresh_from_db()
+        assert re.last_seen == datetime_now
+        assert result is not None
+
+    def test_bump_survives_operational_error(self) -> None:
+        """OperationalError on the UPDATE doesn't prevent the instance from being returned."""
+        project = self.create_project(name="foo")
+        datetime_now = timezone.now()
+
+        release = Release.objects.create(organization_id=project.organization_id, version="abcdef")
+        release.add_project(project)
+        env = Environment.objects.create(organization_id=project.organization_id, name="prod")
+
+        ReleaseEnvironment.get_or_create(
+            project=project, release=release, environment=env, datetime=datetime_now
+        )
+
+        datetime_next = datetime_now + timedelta(days=1)
+
+        with patch("sentry.models.releaseenvironment.ReleaseEnvironment.objects") as mock_manager:
+            mock_qs = mock_manager.filter.return_value
+            mock_qs.update.side_effect = OperationalError("canceling statement due to user request")
+
+            result = ReleaseEnvironment.get_or_create(
+                project=project, release=release, environment=env, datetime=datetime_next
+            )
+
+        assert result is not None
+        re = ReleaseEnvironment.objects.get(
+            organization_id=project.organization_id, release=release, environment=env
+        )
+        assert re.last_seen == datetime_now

tests/sentry/models/test_releaseprojectenvironment.py

@@ -1,11 +1,14 @@
 from datetime import timedelta
+from unittest.mock import patch
 
+from django.db.utils import OperationalError
 from django.utils import timezone
 
 from sentry.models.environment import Environment
 from sentry.models.release import Release
 from sentry.models.releaseprojectenvironment import ReleaseProjectEnvironment
 from sentry.testutils.cases import TestCase
+from sentry.utils.cache import cache
 
 
 class GetOrCreateTest(TestCase):
@@ -83,3 +86,68 @@ def test_no_update_too_close(self) -> None:
         )
         assert release_project_env.first_seen == self.datetime_now
         assert release_project_env.last_seen == self.datetime_now
+
+    def test_bump_skipped_when_cache_lock_held(self) -> None:
+        """Second worker skips the DB update when another worker already holds the bump lock."""
+        ReleaseProjectEnvironment.get_or_create(
+            project=self.project,
+            release=self.release,
+            environment=self.environment,
+            datetime=self.datetime_now,
+        )
+
+        datetime_next = self.datetime_now + timedelta(days=1)
+
+        rpe = ReleaseProjectEnvironment.objects.get(
+            project=self.project, release=self.release, environment=self.environment
+        )
+        bump_key = f"releaseprojectenv_bump:{rpe.id}"
+        cache.set(bump_key, "1", timeout=60)
+
+        with patch.object(
+            ReleaseProjectEnvironment.objects,
+            "filter",
+            wraps=ReleaseProjectEnvironment.objects.filter,
+        ) as mock_filter:
+            result = ReleaseProjectEnvironment.get_or_create(
+                project=self.project,
+                release=self.release,
+                environment=self.environment,
+                datetime=datetime_next,
+            )
+            for call in mock_filter.call_args_list:
+                assert "last_seen__lt" not in (call.kwargs or {})
+
+        rpe.refresh_from_db()
+        assert rpe.last_seen == self.datetime_now
+        assert result is not None
+
+    def test_bump_survives_operational_error(self) -> None:
+        """OperationalError on the UPDATE doesn't prevent the instance from being returned."""
+        ReleaseProjectEnvironment.get_or_create(
+            project=self.project,
+            release=self.release,
+            environment=self.environment,
+            datetime=self.datetime_now,
+        )
+
+        datetime_next = self.datetime_now + timedelta(days=1)
+
+        with patch(
+            "sentry.models.releaseprojectenvironment.ReleaseProjectEnvironment.objects"
+        ) as mock_manager:
+            mock_qs = mock_manager.filter.return_value
+            mock_qs.update.side_effect = OperationalError("canceling statement due to user request")
+
+            result = ReleaseProjectEnvironment.get_or_create(
+                project=self.project,
+                release=self.release,
+                environment=self.environment,
+                datetime=datetime_next,
+            )
+
+        assert result is not None
+        rpe = ReleaseProjectEnvironment.objects.get(
+            project=self.project, release=self.release, environment=self.environment
+        )
+        assert rpe.last_seen == self.datetime_now