From edf61102d4ad361feedf4f587ff54a76f2260be1 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Wed, 5 Nov 2025 14:50:10 +0100 Subject: [PATCH] Unconditionally delete payloads from the attachments cache --- src/sentry/attachments/__init__.py | 21 +++++++++++++++------ src/sentry/attachments/base.py | 5 ----- src/sentry/tasks/store.py | 4 ++-- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/sentry/attachments/__init__.py b/src/sentry/attachments/__init__.py index b8ee913336fc33..9eabaa957d77ad 100644 --- a/src/sentry/attachments/__init__.py +++ b/src/sentry/attachments/__init__.py @@ -4,7 +4,7 @@ "attachment_cache", "store_attachments_for_event", "get_attachments_for_event", - "delete_ratelimited_attachments", + "delete_cached_and_ratelimited_attachments", "CachedAttachment", "MissingAttachmentChunks", ] @@ -70,21 +70,30 @@ def get_attachments_for_event(event: Any) -> Generator[CachedAttachment]: return attachment_cache.get(cache_key) -def delete_ratelimited_attachments( +@sentry_sdk.trace +def delete_cached_and_ratelimited_attachments( project: Project, event: Any, attachments: list[CachedAttachment] ): """ - This deletes all the attachments that are `rate_limited` from `objectstore` in case they are stored, - and it will also remove all the attachments from the attachments cache as well. + This deletes all attachment payloads and metadata from the attachment cache + (if those are stored there), as well as delete all the `rate_limited` + attachments from the `objectstore`. + Non-ratelimited attachments which are already stored in `objectstore` will + be retained there for long-term storage. """ client: ObjectstoreClient | None = None for attachment in attachments: + # deletes from objectstore if no long-term storage is desired if attachment.rate_limited and attachment.stored_id: if client is None: client = get_attachments_client().for_project(project.organization_id, project.id) client.delete(attachment.stored_id) - # all other attachments which only exist in the cache but are not stored will - # be cleaned up here: + # unconditionally deletes any payloads from the attachment cache + attachment.delete() + + # this cleans up the metadata from the attachments cache: + # any payloads living in the attachments cache have been cleared by the + # `attachment.delete()` call above. cache_key = cache_key_for_event(event) attachment_cache.delete(cache_key) diff --git a/src/sentry/attachments/base.py b/src/sentry/attachments/base.py index 5b54d5aba4a49a..8e555ec19c2461 100644 --- a/src/sentry/attachments/base.py +++ b/src/sentry/attachments/base.py @@ -1,6 +1,5 @@ from collections.abc import Generator -import sentry_sdk import zstandard from sentry.utils import metrics @@ -185,9 +184,5 @@ def get_data(self, attachment: CachedAttachment) -> bytes: return bytes(data) - @sentry_sdk.trace def delete(self, key: str): - for attachment in self.get(key): - attachment.delete() - self.inner.delete(ATTACHMENT_META_KEY.format(key=key)) diff --git a/src/sentry/tasks/store.py b/src/sentry/tasks/store.py index cf840981f5738a..7db6a84fb490c8 100644 --- a/src/sentry/tasks/store.py +++ b/src/sentry/tasks/store.py @@ -12,7 +12,7 @@ from sentry_relay.processing import StoreNormalizer from sentry import options, reprocessing2 -from sentry.attachments import delete_ratelimited_attachments, get_attachments_for_event +from sentry.attachments import delete_cached_and_ratelimited_attachments, get_attachments_for_event from sentry.constants import DEFAULT_STORE_NORMALIZER_ARGS from sentry.feedback.usecases.ingest.save_event_feedback import ( save_event_feedback as save_event_feedback_impl, @@ -627,7 +627,7 @@ def _do_save_event( reprocessing2.mark_event_reprocessed(data) if all_attachments and project: - delete_ratelimited_attachments(project, data, all_attachments) + delete_cached_and_ratelimited_attachments(project, data, all_attachments) if start_time: metrics.timing(