wip

Author: lcian

src/sentry/api/endpoints/debug_files.py

@@ -274,8 +274,15 @@ def download(self, debug_file_id, project: Project):
 
         try:
             fp = debug_file.getfile()
+
+            def stream_debug_file():
+                try:
+                    yield from iter(lambda: fp.read(4096), b"")
+                finally:
+                    fp.close()
+
             response = StreamingHttpResponse(
-                iter(lambda: fp.read(4096), b""), content_type="application/octet-stream"
+                stream_debug_file(), content_type="application/octet-stream"
             )
             response["Content-Length"] = debug_file.get_file_size()
             response["Content-Disposition"] = (

src/sentry/debug_files/debug_files.py

@@ -23,25 +23,36 @@ def maybe_renew_debug_files(debug_files: Sequence[ProjectDebugFile]) -> None:
         days=options.get("system.debug-files-renewal-age-threshold-days")
     )
 
-    # We first check if any file needs renewal, before going to the database.
-    needs_bump = any(dif.date_accessed <= threshold_date for dif in debug_files)
-    if not needs_bump:
-        return
-
-    # For Objectstore-backed files, issue a HEAD request to bump the TTI.
+    ids_to_renew = []
     for dif in debug_files:
-        if dif.storage_path is not None and dif.date_accessed <= threshold_date:
+        if dif.date_accessed > threshold_date:
+            continue
+
+        # For Objectstore-backed files, issue a HEAD request to bump the TTI.
+        if dif.storage_path is not None:
             try:
                 dif._get_objectstore_session().head(dif.storage_path)
             except Exception:
-                logger.exception("Failed to bump TTI for Debug File")
+                logger.exception(
+                    "debugfile.objectstore_tti_renewal_failed",
+                    extra={
+                        "project_debug_file_id": dif.id,
+                        "project_id": dif.project_id,
+                        "storage_path": dif.storage_path,
+                    },
+                )
+                continue
+
+        ids_to_renew.append(dif.id)
+
+    if not ids_to_renew:
+        return
 
     # Update `date_accessed` in the db.
-    ids = [dif.id for dif in debug_files]
     with metrics.timer("debug_files_renewal"):
         with atomic_transaction(using=(router.db_for_write(ProjectDebugFile),)):
             updated_rows_count = ProjectDebugFile.objects.filter(
-                id__in=ids, date_accessed__lte=threshold_date
+                id__in=ids_to_renew, date_accessed__lte=threshold_date
             ).update(date_accessed=now)
             if updated_rows_count > 0:
                 metrics.incr("debug_files_renewal.were_renewed", updated_rows_count)

src/sentry/demo_mode/tasks.py

@@ -18,6 +18,7 @@
 from sentry.models.files import FileBlobOwner
 from sentry.models.organization import Organization
 from sentry.models.project import Project
+from sentry.objectstore import get_debug_files_session
 from sentry.tasks.base import instrumented_task
 from sentry.taskworker.namespaces import demomode_tasks
 from sentry.utils.db import atomic_transaction
@@ -224,6 +225,8 @@ def _sync_release_artifact_bundle(
 def _sync_project_debug_file(
     source_project_debug_file: ProjectDebugFile, target_org: Organization
 ) -> ProjectDebugFile | None:
+    target_project = None
+    target_storage_path = None
     try:
         with atomic_transaction(using=(router.db_for_write(ProjectDebugFile))):
             target_project = _find_matching_project(
@@ -234,9 +237,30 @@ def _sync_project_debug_file(
             if not target_project:
                 return None
 
+            if source_project_debug_file.storage_path is not None:
+                source_fileobj = source_project_debug_file.getfile()
+                try:
+                    target_storage_path = get_debug_files_session(
+                        target_org.id, target_project.id
+                    ).put(
+                        source_fileobj,
+                        compression="none",
+                        content_type=source_project_debug_file.get_content_type(),
+                    )
+                finally:
+                    source_fileobj.close()
+
             return ProjectDebugFile.objects.create(
                 project_id=target_project.id,
-                file=source_project_debug_file.file,
+                file=(
+                    None
+                    if source_project_debug_file.storage_path is not None
+                    else source_project_debug_file.file
+                ),
+                storage_path=target_storage_path,
+                content_type=source_project_debug_file.content_type,
+                file_size=source_project_debug_file.file_size,
+                date_created=source_project_debug_file.date_created,
                 checksum=source_project_debug_file.checksum,
                 object_name=source_project_debug_file.object_name,
                 cpu_name=source_project_debug_file.cpu_name,
@@ -246,6 +270,8 @@ def _sync_project_debug_file(
                 date_accessed=source_project_debug_file.date_accessed,
             )
     except IntegrityError as e:
+        if target_project is not None and target_storage_path is not None:
+            get_debug_files_session(target_org.id, target_project.id).delete(target_storage_path)
         sentry_sdk.capture_exception(e)
         return None
 

src/sentry/models/debugfile.py

@@ -323,6 +323,15 @@ def delete(self, *args: Any, **kwargs: Any) -> tuple[int, dict[str, int]]:
                 self._get_objectstore_session().delete(self.storage_path)
             except Project.DoesNotExist:
                 logger.info("Project already deleted, object will be cleaned up by TTI")
+            except (RequestError, HTTPError):
+                logger.exception(
+                    "debugfile.objectstore_delete_failed",
+                    extra={
+                        "project_debug_file_id": self.id,
+                        "project_id": self.project_id,
+                        "storage_path": self.storage_path,
+                    },
+                )
         elif self.file is not None:
             # If another debug file row still references this File, keep the File.
             # Concurrent last-reference deletes can still leave an unreferenced File

tests/sentry/api/endpoints/test_debug_files.py

@@ -1,9 +1,11 @@
 import zipfile
 from io import BytesIO
+from unittest.mock import patch
 from uuid import uuid4
 
 from django.core.files.uploadedfile import SimpleUploadedFile
 from django.urls import reverse
+from django.utils import timezone
 
 from sentry.models.debugfile import ProjectDebugFile
 from sentry.models.files.file import File
@@ -155,6 +157,36 @@ def test_access_control(self) -> None:
         response = self.client.get(f"{url}?id={download_id}")
         assert response.status_code == 404
 
+    def test_download_closes_streaming_file(self) -> None:
+        class CloseTrackingBytesIO(BytesIO):
+            was_closed = False
+
+            def close(self) -> None:
+                self.was_closed = True
+                super().close()
+
+        debug_file = ProjectDebugFile.objects.create(
+            project_id=self.project.id,
+            file=None,
+            storage_path="debug-files/key",
+            content_type="text/x-proguard+plain",
+            file_size=len(PROGUARD_SOURCE),
+            date_created=timezone.now(),
+            checksum="e6d3c5185dac63eddfdc1a5edfffa32d46103b44",
+            object_name="proguard-mapping",
+            cpu_name="any",
+            debug_id=PROGUARD_UUID,
+            data={"features": ["mapping"]},
+        )
+        payload = CloseTrackingBytesIO(PROGUARD_SOURCE)
+
+        with patch.object(ProjectDebugFile, "getfile", return_value=payload):
+            response = self.client.get(f"{self.url}?id={debug_file.id}")
+            assert response.status_code == 200, response.content
+            assert close_streaming_response(response) == PROGUARD_SOURCE
+
+        assert payload.was_closed
+
     def test_dsyms_requests(self) -> None:
         response = self._upload_proguard(self.url, PROGUARD_UUID)
         assert response.status_code == 201, response.content

tests/sentry/demo_mode/test_tasks.py

@@ -2,6 +2,7 @@
 from unittest import mock
 from uuid import uuid1
 
+import pytest
 from django.db.utils import IntegrityError
 from django.utils import timezone
 
@@ -18,7 +19,9 @@
 from sentry.models.debugfile import ProguardArtifactRelease, ProjectDebugFile
 from sentry.models.organization import Organization
 from sentry.models.project import Project
+from sentry.objectstore import get_debug_files_session
 from sentry.testutils.cases import TestCase
+from sentry.testutils.skips import requires_objectstore
 
 
 class SyncArtifactBundlesTest(TestCase):
@@ -243,6 +246,55 @@ def test_sync_project_debug_files(self) -> None:
         assert target_project_debug_file.code_id == source_project_debug_file.code_id
         assert target_project_debug_file.cpu_name == source_project_debug_file.cpu_name
 
+    @requires_objectstore
+    def test_sync_objectstore_project_debug_files(self) -> None:
+        content = b"objectstore-backed-debug-file"
+        content_type = "application/x-mach-binary"
+        date_created = timezone.now()
+        source_storage_path = get_debug_files_session(
+            self.source_org.id, self.source_proj_foo.id
+        ).put(content, compression="none", content_type=content_type)
+        source_project_debug_file = ProjectDebugFile.objects.create(
+            project_id=self.source_proj_foo.id,
+            file=None,
+            storage_path=source_storage_path,
+            content_type=content_type,
+            file_size=len(content),
+            date_created=date_created,
+            checksum="a" * 40,
+            object_name="test.dSYM",
+            cpu_name="x86_64",
+            debug_id="67e9247c-814e-392b-a027-dbde6748fcbf",
+            code_id="code-id",
+            data={"features": ["debug"]},
+        )
+
+        _sync_project_debug_files(
+            source_org=self.source_org,
+            target_org=self.target_org,
+            cutoff_date=self.last_three_days(),
+        )
+
+        target_project_debug_file = ProjectDebugFile.objects.get(
+            project_id=self.target_proj_foo.id,
+            debug_id=source_project_debug_file.debug_id,
+        )
+
+        assert target_project_debug_file.file_id is None
+        assert target_project_debug_file.storage_path is not None
+        assert target_project_debug_file.storage_path != source_project_debug_file.storage_path
+        assert target_project_debug_file.content_type == content_type
+        assert target_project_debug_file.file_size == len(content)
+        assert target_project_debug_file.date_created == date_created
+        assert target_project_debug_file.getfile().read() == content
+
+        target_project_debug_file.delete()
+        source_project_debug_file.refresh_from_db()
+        assert source_project_debug_file.getfile().read() == content
+
+        with pytest.raises(ProjectDebugFile.DoesNotExist):
+            target_project_debug_file.refresh_from_db()
+
     def test_sync_project_debug_files_with_old_uploads(self) -> None:
         source_project_debug_file = self.create_dif_file(
             self.source_proj_foo,