Skip to content

Commit 4221751

Browse files
authored
feat(replays): Delete videos on replay delete request (#68463)
If we did not delete this the videos would remain in storage until the retention-period expires. Relates to: #63255
1 parent 7fd84e7 commit 4221751

File tree

2 files changed

+29
-4
lines changed

2 files changed

+29
-4
lines changed

src/sentry/replays/tasks.py

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33
import concurrent.futures as cf
44
from typing import Any
55

6+
from google.cloud.exceptions import NotFound
7+
68
from sentry.replays.lib.kafka import initialize_replays_publisher
7-
from sentry.replays.lib.storage import filestore, storage
9+
from sentry.replays.lib.storage import filestore, make_video_filename, storage, storage_kv
810
from sentry.replays.models import ReplayRecordingSegment
911
from sentry.replays.usecases.events import archive_event
1012
from sentry.replays.usecases.reader import fetch_segments_metadata
@@ -54,15 +56,18 @@ def delete_replay_recording(project_id: int, replay_id: str) -> None:
5456
# Filestore and direct storage segments are split into two different delete operations.
5557
direct_storage_segments = []
5658
filestore_segments = []
59+
video_filenames = []
5760
for segment in segments_from_metadata:
61+
video_filenames.append(make_video_filename(segment))
5862
if segment.file_id:
5963
filestore_segments.append(segment)
6064
else:
6165
direct_storage_segments.append(segment)
6266

6367
# Issue concurrent delete requests when interacting with a remote service provider.
64-
if direct_storage_segments:
65-
with cf.ThreadPoolExecutor(max_workers=100) as pool:
68+
with cf.ThreadPoolExecutor(max_workers=100) as pool:
69+
pool.map(_delete_if_exists, video_filenames)
70+
if direct_storage_segments:
6671
pool.map(storage.delete, direct_storage_segments)
6772

6873
# This will only run if "filestore" was used to store the files. This hasn't been the
@@ -81,3 +86,11 @@ def archive_replay(publisher: KafkaPublisher, project_id: int, replay_id: str) -
8186
# We publish manually here because we sometimes provide a managed Kafka
8287
# publisher interface which has its own setup and teardown behavior.
8388
publisher.publish("ingest-replay-events", message)
89+
90+
91+
def _delete_if_exists(filename: str) -> None:
92+
"""Delete the blob if it exists or silence the 404."""
93+
try:
94+
storage_kv.delete(filename)
95+
except NotFound:
96+
pass

tests/sentry/replays/test_project_replay_details.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,12 @@
77

88
from sentry.models.files.file import File
99
from sentry.replays.lib import kafka
10-
from sentry.replays.lib.storage import RecordingSegmentStorageMeta, storage
10+
from sentry.replays.lib.storage import (
11+
RecordingSegmentStorageMeta,
12+
make_video_filename,
13+
storage,
14+
storage_kv,
15+
)
1116
from sentry.replays.models import ReplayRecordingSegment
1217
from sentry.replays.testutils import assert_expected_response, mock_expected_response, mock_replay
1318
from sentry.testutils.cases import APITestCase, ReplaysSnubaTestCase
@@ -210,6 +215,7 @@ def test_delete_replay_from_clickhouse_data(self):
210215
file_id=None,
211216
)
212217
storage.set(metadata1, b"hello, world!")
218+
storage_kv.set(make_video_filename(metadata1), b"hello, world!")
213219

214220
metadata2 = RecordingSegmentStorageMeta(
215221
project_id=self.project.id,
@@ -219,6 +225,8 @@ def test_delete_replay_from_clickhouse_data(self):
219225
file_id=None,
220226
)
221227
storage.set(metadata2, b"hello, world!")
228+
# Intentionally not written.
229+
# storage_kv.set(make_video_filename(metadata2), b"hello, world!")
222230

223231
metadata3 = RecordingSegmentStorageMeta(
224232
project_id=self.project.id,
@@ -228,6 +236,7 @@ def test_delete_replay_from_clickhouse_data(self):
228236
file_id=None,
229237
)
230238
storage.set(metadata3, b"hello, world!")
239+
storage_kv.set(make_video_filename(metadata3), b"hello, world!")
231240

232241
with self.feature(REPLAYS_FEATURES):
233242
with TaskRunner():
@@ -237,3 +246,6 @@ def test_delete_replay_from_clickhouse_data(self):
237246
assert storage.get(metadata1) is None
238247
assert storage.get(metadata2) is None
239248
assert storage.get(metadata3) is not None
249+
assert storage_kv.get(make_video_filename(metadata1)) is None
250+
assert storage_kv.get(make_video_filename(metadata2)) is None
251+
assert storage_kv.get(make_video_filename(metadata3)) is not None

0 commit comments

Comments
 (0)